1. Patchew
  2. QEMU
  3. View series

[PATCH] target/ppc: Re-name registers to match ISA

Nicholas Piggin posted 1 patch 2 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231201122455.143936-1-npiggin@gmail.com
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>
target/ppc/cpu_init.c    | 20 ++++++++++----------
target/ppc/helper_regs.c |  2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
[PATCH] target/ppc: Re-name registers to match ISA
Posted by Nicholas Piggin 2 years ago 
Several registers have names that don't match the ISA (or convention
with other QEMU PPC registers), making them unintuitive to use with
GDB.

Fortunately most of these registers are obscure and/or have not been
correctly implemented in the gdb server (e.g., DEC, TB, CFAR), so risk
of breaking users should be low.

QEMU should follow the ISA for register name convention (where there is
no established GDB name).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
There is never a great time to change user interface, but I'd like to
make this change for 9.0.

Thanks,
Nick

 target/ppc/cpu_init.c    | 20 ++++++++++----------
 target/ppc/helper_regs.c |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 40fe14a6c2..15c1f2fdc8 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5062,7 +5062,7 @@ static void register_970_hid_sprs(CPUPPCState *env)
 
 static void register_970_hior_sprs(CPUPPCState *env)
 {
-    spr_register(env, SPR_HIOR, "SPR_HIOR",
+    spr_register(env, SPR_HIOR, "HIOR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_hior, &spr_write_hior,
                  0x00000000);
@@ -5070,11 +5070,11 @@ static void register_970_hior_sprs(CPUPPCState *env)
 
 static void register_book3s_ctrl_sprs(CPUPPCState *env)
 {
-    spr_register(env, SPR_CTRL, "SPR_CTRL",
+    spr_register(env, SPR_CTRL, "CTRL",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, &spr_write_CTRL,
                  0x00000000);
-    spr_register(env, SPR_UCTRL, "SPR_UCTRL",
+    spr_register(env, SPR_UCTRL, "UCTRL",
                  &spr_read_ureg, SPR_NOACCESS,
                  &spr_read_ureg, SPR_NOACCESS,
                  0x00000000);
@@ -5465,7 +5465,7 @@ static void register_book3s_purr_sprs(CPUPPCState *env)
 static void register_power6_dbg_sprs(CPUPPCState *env)
 {
 #if !defined(CONFIG_USER_ONLY)
-    spr_register(env, SPR_CFAR, "SPR_CFAR",
+    spr_register(env, SPR_CFAR, "CFAR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_cfar, &spr_write_cfar,
                  0x00000000);
@@ -5483,7 +5483,7 @@ static void register_power5p_common_sprs(CPUPPCState *env)
 static void register_power6_common_sprs(CPUPPCState *env)
 {
 #if !defined(CONFIG_USER_ONLY)
-    spr_register_kvm(env, SPR_DSCR, "SPR_DSCR",
+    spr_register_kvm(env, SPR_DSCR, "DSCR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_DSCR, 0x00000000);
@@ -5695,7 +5695,7 @@ static void register_power8_book4_sprs(CPUPPCState *env)
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_ACOP, 0);
     /* PID is only in BookE in ISA v2.07 */
-    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
+    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_pidr,
                      KVM_REG_PPC_PID, 0);
@@ -5716,7 +5716,7 @@ static void register_power7_book4_sprs(CPUPPCState *env)
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_ACOP, 0);
     /* PID is only in BookE in ISA v2.06 */
-    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
+    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic32,
                      KVM_REG_PPC_PID, 0);
@@ -5750,7 +5750,7 @@ static void register_power9_mmu_sprs(CPUPPCState *env)
                     &spr_read_generic, &spr_write_generic,
                     0x0000000000000000);
     /* PID is part of the BookS ISA from v3.0 */
-    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
+    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_pidr,
                      KVM_REG_PPC_PID, 0);
@@ -5791,7 +5791,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
             &spr_read_generic, &spr_write_generic32,
             0);
 
-    spr_register(env, SPR_UDEXCR, "DEXCR",
+    spr_register(env, SPR_UDEXCR, "UDEXCR",
             &spr_read_dexcr_ureg, SPR_NOACCESS,
             &spr_read_dexcr_ureg, SPR_NOACCESS,
             0);
@@ -5802,7 +5802,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
             &spr_read_generic, &spr_write_generic32,
             0);
 
-    spr_register(env, SPR_UHDEXCR, "HDEXCR",
+    spr_register(env, SPR_UHDEXCR, "UHDEXCR",
             &spr_read_dexcr_ureg, SPR_NOACCESS,
             &spr_read_dexcr_ureg, SPR_NOACCESS,
             0);
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index f1493ddca0..59cf3dfaae 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -509,7 +509,7 @@ void register_non_embedded_sprs(CPUPPCState *env)
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_DAR, 0x00000000);
     /* Timer */
-    spr_register(env, SPR_DECR, "DECR",
+    spr_register(env, SPR_DECR, "DEC",
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_decr, &spr_write_decr,
                  0x00000000);
-- 
2.42.0
[PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes
Posted by Nicholas Piggin 2 years ago 
The chiptod/TFMR/state machine is not really tied to the other
time register fixes, but they touch some of the same code, and
logically same facility.

Changes since v1 of chiptod patches:
- Split hackish ChipTOD<->TFMR/TBST interface into its own patch
- Fix multi-socket addressing on P9 / chip ID mode (P10 works)
- Change chiptod primary/secondary setting to use class properties
- Add more comments to explain TOD overview and timebase state
  machine.
- SMT support for TFMR, some functionality is limited to thread 0.
- FIRMWARE_CONTROL_ERROR bit implemented in TFMR.
- Misc cleanups and bug fixes.

The hacky part, addressing core from chiptod, is still hacky. Is
there strong objection to it?

This successfully runs skiboot chiptod initialisation code with
POWER9 and POWER10 multi-socket, multi-core, SMT. That requires
skiboot 7.1 (not in-tree), otherwise chiptod init is skipped on
QEMU machines.

Thanks,
Nick

Nicholas Piggin (7):
  target/ppc: Rename TBL to TB on 64-bit
  target/ppc: Improve timebase register defines naming
  target/ppc: Fix move-to timebase SPR access permissions
  pnv/chiptod: Add POWER9/10 chiptod model
  pnv/chiptod: Implement the ChipTOD to Core transfer
  target/ppc: Implement core timebase state machine and TFMR
  target/ppc: Add SMT support to time facilities

 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  55 ++++
 include/hw/ppc/pnv_core.h    |   4 +
 include/hw/ppc/pnv_xscom.h   |   9 +
 target/ppc/cpu.h             |  50 +++-
 hw/ppc/pnv.c                 |  63 +++++
 hw/ppc/pnv_chiptod.c         | 509 +++++++++++++++++++++++++++++++++++
 target/ppc/helper_regs.c     |  39 ++-
 target/ppc/ppc-qmp-cmds.c    |   4 +
 target/ppc/timebase_helper.c | 309 ++++++++++++++++++++-
 target/ppc/translate.c       |  42 ++-
 hw/ppc/meson.build           |   1 +
 hw/ppc/trace-events          |   4 +
 13 files changed, 1067 insertions(+), 25 deletions(-)
 create mode 100644 include/hw/ppc/pnv_chiptod.h
 create mode 100644 hw/ppc/pnv_chiptod.c

-- 
2.42.0
Re: [PATCH] target/ppc: Re-name registers to match ISA
Posted by Cédric Le Goater 1 year, 11 months ago 
On 12/1/23 13:24, Nicholas Piggin wrote:
> Several registers have names that don't match the ISA (or convention
> with other QEMU PPC registers), making them unintuitive to use with
> GDB.
> 
> Fortunately most of these registers are obscure and/or have not been
> correctly implemented in the gdb server (e.g., DEC, TB, CFAR), so risk
> of breaking users should be low.
> 
> QEMU should follow the ISA for register name convention (where there is
> no established GDB name).
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
> There is never a great time to change user interface, but I'd like to
> make this change for 9.0.
> 
> Thanks,
> Nick
> 
>   target/ppc/cpu_init.c    | 20 ++++++++++----------
>   target/ppc/helper_regs.c |  2 +-
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 40fe14a6c2..15c1f2fdc8 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5062,7 +5062,7 @@ static void register_970_hid_sprs(CPUPPCState *env)
>   
>   static void register_970_hior_sprs(CPUPPCState *env)
>   {
> -    spr_register(env, SPR_HIOR, "SPR_HIOR",
> +    spr_register(env, SPR_HIOR, "HIOR",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    &spr_read_hior, &spr_write_hior,
>                    0x00000000);
> @@ -5070,11 +5070,11 @@ static void register_970_hior_sprs(CPUPPCState *env)
>   
>   static void register_book3s_ctrl_sprs(CPUPPCState *env)
>   {
> -    spr_register(env, SPR_CTRL, "SPR_CTRL",
> +    spr_register(env, SPR_CTRL, "CTRL",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    SPR_NOACCESS, &spr_write_CTRL,
>                    0x00000000);
> -    spr_register(env, SPR_UCTRL, "SPR_UCTRL",
> +    spr_register(env, SPR_UCTRL, "UCTRL",
>                    &spr_read_ureg, SPR_NOACCESS,
>                    &spr_read_ureg, SPR_NOACCESS,
>                    0x00000000);
> @@ -5465,7 +5465,7 @@ static void register_book3s_purr_sprs(CPUPPCState *env)
>   static void register_power6_dbg_sprs(CPUPPCState *env)
>   {
>   #if !defined(CONFIG_USER_ONLY)
> -    spr_register(env, SPR_CFAR, "SPR_CFAR",
> +    spr_register(env, SPR_CFAR, "CFAR",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    &spr_read_cfar, &spr_write_cfar,
>                    0x00000000);
> @@ -5483,7 +5483,7 @@ static void register_power5p_common_sprs(CPUPPCState *env)
>   static void register_power6_common_sprs(CPUPPCState *env)
>   {
>   #if !defined(CONFIG_USER_ONLY)
> -    spr_register_kvm(env, SPR_DSCR, "SPR_DSCR",
> +    spr_register_kvm(env, SPR_DSCR, "DSCR",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_DSCR, 0x00000000);
> @@ -5695,7 +5695,7 @@ static void register_power8_book4_sprs(CPUPPCState *env)
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_ACOP, 0);
>       /* PID is only in BookE in ISA v2.07 */
> -    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
> +    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_pidr,
>                        KVM_REG_PPC_PID, 0);
> @@ -5716,7 +5716,7 @@ static void register_power7_book4_sprs(CPUPPCState *env)
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_ACOP, 0);
>       /* PID is only in BookE in ISA v2.06 */
> -    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
> +    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_generic32,
>                        KVM_REG_PPC_PID, 0);
> @@ -5750,7 +5750,7 @@ static void register_power9_mmu_sprs(CPUPPCState *env)
>                       &spr_read_generic, &spr_write_generic,
>                       0x0000000000000000);
>       /* PID is part of the BookS ISA from v3.0 */
> -    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
> +    spr_register_kvm(env, SPR_BOOKS_PID, "PIDR",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_pidr,
>                        KVM_REG_PPC_PID, 0);
> @@ -5791,7 +5791,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
>               &spr_read_generic, &spr_write_generic32,
>               0);
>   
> -    spr_register(env, SPR_UDEXCR, "DEXCR",
> +    spr_register(env, SPR_UDEXCR, "UDEXCR",
>               &spr_read_dexcr_ureg, SPR_NOACCESS,
>               &spr_read_dexcr_ureg, SPR_NOACCESS,
>               0);
> @@ -5802,7 +5802,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
>               &spr_read_generic, &spr_write_generic32,
>               0);
>   
> -    spr_register(env, SPR_UHDEXCR, "HDEXCR",
> +    spr_register(env, SPR_UHDEXCR, "UHDEXCR",
>               &spr_read_dexcr_ureg, SPR_NOACCESS,
>               &spr_read_dexcr_ureg, SPR_NOACCESS,
>               0);
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index f1493ddca0..59cf3dfaae 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -509,7 +509,7 @@ void register_non_embedded_sprs(CPUPPCState *env)
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_DAR, 0x00000000);
>       /* Timer */
> -    spr_register(env, SPR_DECR, "DECR",
> +    spr_register(env, SPR_DECR, "DEC",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    &spr_read_decr, &spr_write_decr,
>                    0x00000000);

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.