qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs
@ 2023-03-27 13:12 Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 2/6] target/ppc: Better CTRL SPR implementation Nicholas Piggin
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

Some 32-bit SPRs are incorrectly implemented as 64-bits on 64-bit
targets.

This changes VRSAVE, DSISR, HDSISR, DAWRX0, PIDR, LPIDR, DEXCR,
HDEXCR, CTRL, TSCR, MMCRH, and PMC[1-6] from to be 32-bit registers.

This only goes by the 32/64 classification in the architecture, it
does not try to implement finer details of SPR implementation (e.g.,
not all bits implemented as simple read/write storage).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c    | 18 +++++++++---------
 target/ppc/helper_regs.c |  2 +-
 target/ppc/misc_helper.c |  4 ++--
 target/ppc/power8-pmu.c  |  2 +-
 target/ppc/translate.c   |  2 +-
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 0ce2e3c91d..5aa0b3f0f1 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5085,8 +5085,8 @@ static void register_book3s_altivec_sprs(CPUPPCState *env)
     }
 
     spr_register_kvm(env, SPR_VRSAVE, "VRSAVE",
-                     &spr_read_generic, &spr_write_generic,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_generic32,
+                     &spr_read_generic, &spr_write_generic32,
                      KVM_REG_PPC_VRSAVE, 0x00000000);
 
 }
@@ -5120,7 +5120,7 @@ static void register_book3s_207_dbg_sprs(CPUPPCState *env)
     spr_register_kvm_hv(env, SPR_DAWRX0, "DAWRX0",
                         SPR_NOACCESS, SPR_NOACCESS,
                         SPR_NOACCESS, SPR_NOACCESS,
-                        &spr_read_generic, &spr_write_generic,
+                        &spr_read_generic, &spr_write_generic32,
                         KVM_REG_PPC_DAWRX, 0x00000000);
     spr_register_kvm_hv(env, SPR_CIABR, "CIABR",
                         SPR_NOACCESS, SPR_NOACCESS,
@@ -5376,7 +5376,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_TSCR, "TSCR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                  0x00000000);
     spr_register_hv(env, SPR_HMER, "HMER",
                  SPR_NOACCESS, SPR_NOACCESS,
@@ -5406,7 +5406,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_MMCRC, "MMCRC",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                  0x00000000);
     spr_register_hv(env, SPR_MMCRH, "MMCRH",
                  SPR_NOACCESS, SPR_NOACCESS,
@@ -5441,7 +5441,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_HDSISR, "HDSISR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                  0x00000000);
     spr_register_hv(env, SPR_HRMOR, "HRMOR",
                  SPR_NOACCESS, SPR_NOACCESS,
@@ -5665,7 +5665,7 @@ static void register_power7_book4_sprs(CPUPPCState *env)
                      KVM_REG_PPC_ACOP, 0);
     spr_register_kvm(env, SPR_BOOKS_PID, "PID",
                      SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_generic32,
                      KVM_REG_PPC_PID, 0);
 #endif
 }
@@ -5730,7 +5730,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
 {
     spr_register(env, SPR_DEXCR, "DEXCR",
             SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic,
+            &spr_read_generic, &spr_write_generic32,
             0);
 
     spr_register(env, SPR_UDEXCR, "DEXCR",
@@ -5741,7 +5741,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
             SPR_NOACCESS, SPR_NOACCESS,
             SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic,
+            &spr_read_generic, &spr_write_generic32,
             0);
 
     spr_register(env, SPR_UHDEXCR, "HDEXCR",
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 779e7db513..fb351c303f 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -448,7 +448,7 @@ void register_non_embedded_sprs(CPUPPCState *env)
     /* Exception processing */
     spr_register_kvm(env, SPR_DSISR, "DSISR",
                      SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_generic32,
                      KVM_REG_PPC_DSISR, 0x00000000);
     spr_register_kvm(env, SPR_DAR, "DAR",
                      SPR_NOACCESS, SPR_NOACCESS,
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index a9bc1522e2..40ddc5c08c 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -190,13 +190,13 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val)
 
 void helper_store_pidr(CPUPPCState *env, target_ulong val)
 {
-    env->spr[SPR_BOOKS_PID] = val;
+    env->spr[SPR_BOOKS_PID] = (uint32_t)val;
     tlb_flush(env_cpu(env));
 }
 
 void helper_store_lpidr(CPUPPCState *env, target_ulong val)
 {
-    env->spr[SPR_LPIDR] = val;
+    env->spr[SPR_LPIDR] = (uint32_t)val;
 
     /*
      * We need to flush the TLB on LPID changes as we only tag HV vs
diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index 1381072b9e..64a64865d7 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -272,7 +272,7 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value)
 {
     pmu_update_cycles(env);
 
-    env->spr[sprn] = value;
+    env->spr[sprn] = (uint32_t)value;
 
     pmc_update_overflow_timer(env, sprn);
 }
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index df324fc7ff..58fa509057 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -413,7 +413,7 @@ void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
 
 void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
 {
-    spr_write_generic(ctx, sprn, gprn);
+    spr_write_generic32(ctx, sprn, gprn);
 
     /*
      * SPR_CTRL writes must force a new translation block,
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/6] target/ppc: Better CTRL SPR implementation
  2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
@ 2023-03-27 13:12 ` Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

The CTRL register is able to write bit zero, and that is reflected in a
bit field in the register that reflects the state of all threads in the
core.

TCG does not implement SMT, so this just requires mirroring that bit into
the first bit of the thread state field.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/translate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 58fa509057..d699acb3d0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -413,7 +413,14 @@ void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
 
 void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
 {
-    spr_write_generic32(ctx, sprn, gprn);
+    /* This does not implement >1 thread */
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+    tcg_gen_extract_tl(t0, cpu_gpr[gprn], 0, 1); /* Extract RUN field */
+    tcg_gen_shli_tl(t1, t0, 8); /* Duplicate the bit in TS */
+    tcg_gen_or_tl(t1, t1, t0);
+    gen_store_spr(sprn, t1);
+    spr_store_dump_spr(sprn);
 
     /*
      * SPR_CTRL writes must force a new translation block,
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt
  2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 2/6] target/ppc: Better CTRL SPR implementation Nicholas Piggin
@ 2023-03-27 13:12 ` Nicholas Piggin
  2023-03-31 21:27   ` Fabiano Rosas
  2023-05-23 18:11   ` Anushree Mathur
  2023-03-27 13:12 ` [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward Nicholas Piggin
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

powerpc ifetch endianness depends on MSR[LE] so it has to byteswap
after cpu_ldl_code(). This corrects DSISR bits in alignment
interrupts when running in little endian mode.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v1:
- Removed big endian ifdef [Fabiano review]
- Acaually use need_byswap helper.

 target/ppc/excp_helper.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 287659c74d..07729967b5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -133,6 +133,24 @@ static void dump_hcall(CPUPPCState *env)
                   env->nip);
 }
 
+/* Return true iff byteswap is needed in a scalar memop */
+static inline bool need_byteswap(CPUArchState *env)
+{
+    /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */
+    return !!(env->msr & ((target_ulong)1 << MSR_LE));
+}
+
+static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr)
+{
+    uint32_t insn = cpu_ldl_code(env, addr);
+
+    if (need_byteswap(env)) {
+        insn = bswap32(insn);
+    }
+
+    return insn;
+}
+
 static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
 {
     const char *es;
@@ -3097,7 +3115,7 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
 
     /* Restore state and reload the insn we executed, for filling in DSISR.  */
     cpu_restore_state(cs, retaddr);
-    insn = cpu_ldl_code(env, env->nip);
+    insn = ppc_ldl_code(env, env->nip);
 
     switch (env->mmu_model) {
     case POWERPC_MMU_SOFT_4xx:
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward
  2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 2/6] target/ppc: Better CTRL SPR implementation Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
@ 2023-03-27 13:12 ` Nicholas Piggin
  2023-03-31 21:25   ` Fabiano Rosas
  2023-03-27 13:12 ` [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers Nicholas Piggin
  2023-03-27 13:12 ` [PATCH v2 6/6] target/ppc: Implement HEIR SPR Nicholas Piggin
  4 siblings, 1 reply; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

This optional behavior was removed from the ISA in v3.0, see
Summary of Changes preface:

  Data Storage Interrupt Status Register for Alignment Interrupt:
  Simplifies the Alignment interrupt by remov- ing the Data Storage
  Interrupt Status Register (DSISR) from the set of registers modified
  by the Alignment interrupt.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v1:
- Use insns_flags instead of excp_model [Fabiano review]

 target/ppc/excp_helper.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 07729967b5..6ac003bcd5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1449,13 +1449,16 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
         break;
     }
     case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
-        /* Get rS/rD and rA from faulting opcode */
-        /*
-         * Note: the opcode fields will not be set properly for a
-         * direct store load/store, but nobody cares as nobody
-         * actually uses direct store segments.
-         */
-        env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
+        /* Optional DSISR update was removed from ISA v3.0 */
+        if (!(env->insns_flags2 & PPC2_ISA300)) {
+            /* Get rS/rD and rA from faulting opcode */
+            /*
+             * Note: the opcode fields will not be set properly for a
+             * direct store load/store, but nobody cares as nobody
+             * actually uses direct store segments.
+             */
+            env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
+        }
         break;
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers
  2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
                   ` (2 preceding siblings ...)
  2023-03-27 13:12 ` [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward Nicholas Piggin
@ 2023-03-27 13:12 ` Nicholas Piggin
  2023-03-31 21:26   ` Fabiano Rosas
  2023-03-27 13:12 ` [PATCH v2 6/6] target/ppc: Implement HEIR SPR Nicholas Piggin
  4 siblings, 1 reply; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

ISA v3.1 introduced prefix instructions. Among the changes, various
synchronous interrupts report whether they were caused by a prefix
instruction in (H)SRR1.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v1:
- Use insns_flags instead of excp_model [Fabiano review]

 target/ppc/excp_helper.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 6ac003bcd5..4e119c4dfc 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1346,12 +1346,21 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
     return false;
 }
 
+static bool is_prefix_excp(CPUPPCState *env, uint32_t insn)
+{
+    if (!(env->insns_flags2 & PPC2_ISA310)) {
+        return false;
+    }
+    return ((insn & 0xfc000000) == 0x04000000);
+}
+
 static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
     int srr0, srr1, lev = -1;
+    uint32_t insn = 0;
 
     /* new srr1 value excluding must-be-zero bits */
     msr = env->msr & ~0x783f0000ULL;
@@ -1390,6 +1399,29 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 
     vector |= env->excp_prefix;
 
+    switch (excp) {
+    case POWERPC_EXCP_MCHECK:
+    case POWERPC_EXCP_DSI:
+    case POWERPC_EXCP_DSEG:
+    case POWERPC_EXCP_ALIGN:
+    case POWERPC_EXCP_PROGRAM:
+    case POWERPC_EXCP_FPU:
+    case POWERPC_EXCP_TRACE:
+    case POWERPC_EXCP_HDSI:
+    case POWERPC_EXCP_HV_EMU:
+    case POWERPC_EXCP_VPU:
+    case POWERPC_EXCP_VSXU:
+    case POWERPC_EXCP_FU:
+    case POWERPC_EXCP_HV_FU:
+        insn = ppc_ldl_code(env, env->nip);
+        if (is_prefix_excp(env, insn)) {
+            msr |= PPC_BIT(34);
+        }
+        break;
+    default:
+        break;
+    }
+
     switch (excp) {
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
         if (!FIELD_EX64(env->msr, MSR, ME)) {
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/6] target/ppc: Implement HEIR SPR
  2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
                   ` (3 preceding siblings ...)
  2023-03-27 13:12 ` [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers Nicholas Piggin
@ 2023-03-27 13:12 ` Nicholas Piggin
  2023-03-29  5:51   ` Michael Neuling
  4 siblings, 1 reply; 12+ messages in thread
From: Nicholas Piggin @ 2023-03-27 13:12 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

The hypervisor emulation assistance interrupt modifies HEIR to
contain the value of the instruction which caused the exception.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h         |  1 +
 target/ppc/cpu_init.c    | 23 +++++++++++++++++++++++
 target/ppc/excp_helper.c | 12 +++++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 557d736dab..8c4a203ecb 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1653,6 +1653,7 @@ void ppc_compat_add_property(Object *obj, const char *name,
 #define SPR_HMER              (0x150)
 #define SPR_HMEER             (0x151)
 #define SPR_PCR               (0x152)
+#define SPR_HEIR              (0x153)
 #define SPR_BOOKE_LPIDR       (0x152)
 #define SPR_BOOKE_TCR         (0x154)
 #define SPR_BOOKE_TLB0PS      (0x158)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 5aa0b3f0f1..ff73be1812 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1629,6 +1629,7 @@ static void register_8xx_sprs(CPUPPCState *env)
  * HSRR0   => SPR 314 (Power 2.04 hypv)
  * HSRR1   => SPR 315 (Power 2.04 hypv)
  * LPIDR   => SPR 317 (970)
+ * HEIR    => SPR 339 (Power 2.05 hypv) (64-bit reg from 3.1)
  * EPR     => SPR 702 (Power 2.04 emb)
  * perf    => 768-783 (Power 2.04)
  * perf    => 784-799 (Power 2.04)
@@ -5522,6 +5523,24 @@ static void register_power6_common_sprs(CPUPPCState *env)
                  0x00000000);
 }
 
+static void register_HEIR32_spr(CPUPPCState *env)
+{
+    spr_register_hv(env, SPR_HEIR, "HEIR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic32,
+                 0x00000000);
+}
+
+static void register_HEIR64_spr(CPUPPCState *env)
+{
+    spr_register_hv(env, SPR_HEIR, "HEIR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+}
+
 static void register_power8_tce_address_control_sprs(CPUPPCState *env)
 {
     spr_register_kvm(env, SPR_TAR, "TAR",
@@ -5950,6 +5969,7 @@ static void init_proc_POWER7(CPUPPCState *env)
     register_power5p_ear_sprs(env);
     register_power5p_tb_sprs(env);
     register_power6_common_sprs(env);
+    register_HEIR32_spr(env);
     register_power6_dbg_sprs(env);
     register_power7_book4_sprs(env);
 
@@ -6072,6 +6092,7 @@ static void init_proc_POWER8(CPUPPCState *env)
     register_power5p_ear_sprs(env);
     register_power5p_tb_sprs(env);
     register_power6_common_sprs(env);
+    register_HEIR32_spr(env);
     register_power6_dbg_sprs(env);
     register_power8_tce_address_control_sprs(env);
     register_power8_ids_sprs(env);
@@ -6234,6 +6255,7 @@ static void init_proc_POWER9(CPUPPCState *env)
     register_power5p_ear_sprs(env);
     register_power5p_tb_sprs(env);
     register_power6_common_sprs(env);
+    register_HEIR32_spr(env);
     register_power6_dbg_sprs(env);
     register_power8_tce_address_control_sprs(env);
     register_power8_ids_sprs(env);
@@ -6409,6 +6431,7 @@ static void init_proc_POWER10(CPUPPCState *env)
     register_power5p_ear_sprs(env);
     register_power5p_tb_sprs(env);
     register_power6_common_sprs(env);
+    register_HEIR64_spr(env);
     register_power6_dbg_sprs(env);
     register_power8_tce_address_control_sprs(env);
     register_power8_ids_sprs(env);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 4e119c4dfc..84f222ba1d 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1596,13 +1596,23 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_HDECR:     /* Hypervisor decrementer exception         */
     case POWERPC_EXCP_HDSI:      /* Hypervisor data storage exception        */
     case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
-    case POWERPC_EXCP_HV_EMU:
     case POWERPC_EXCP_HVIRT:     /* Hypervisor virtualization                */
         srr0 = SPR_HSRR0;
         srr1 = SPR_HSRR1;
         new_msr |= (target_ulong)MSR_HVB;
         new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
         break;
+    case POWERPC_EXCP_HV_EMU:
+        env->spr[SPR_HEIR] = insn;
+        if (is_prefix_excp(env, insn)) {
+            uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
+            env->spr[SPR_HEIR] |= (uint64_t)insn2 << 32;
+        }
+        srr0 = SPR_HSRR0;
+        srr1 = SPR_HSRR1;
+        new_msr |= (target_ulong)MSR_HVB;
+        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+        break;
     case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
     case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
     case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/6] target/ppc: Implement HEIR SPR
  2023-03-27 13:12 ` [PATCH v2 6/6] target/ppc: Implement HEIR SPR Nicholas Piggin
@ 2023-03-29  5:51   ` Michael Neuling
  2023-04-02  2:48     ` Nicholas Piggin
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Neuling @ 2023-03-29  5:51 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

Nick,

> +    case POWERPC_EXCP_HV_EMU:
> +        env->spr[SPR_HEIR] = insn;
> +        if (is_prefix_excp(env, insn)) {
> +            uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
> +            env->spr[SPR_HEIR] |= (uint64_t)insn2 << 32;

Are inst and inst2 in the right locations here? I think you might need
insn in the top half and insn2 in the bottom.

I wrote the little test case below. I'd expect GPR0 and GPR1 to end up
with the same value, but they don't with this code

qemu correctly sees the bad prefix instruction as HSRR1[34] is set.

Mikey

% cat heir.S
#define SPR_HEIR        (0x153)
#define SPR_HSRR0	(0x13a)

start:
        . = 0x10
        .long (1<<26) | 0
        .long 0x0

	. = 0xe40
illegal:
        mfspr 0, SPR_HEIR
        mfspr 2, SPR_HSRR0
        ld    1, 0(2)
	b .

% powerpc64-linux-gnu-gcc -o heir.o -c heir.S
% powerpc64-linux-gnu-objcopy -O binary heir.o
heir.stripped
% qemu-system-ppc64 -nographic-machine powernv10 -cpu POWER10 -display none -vga none -m 1g -accel tcg  -serial mon:stdio -bios /home/mikey/devel/test/heir.stripped
QEMU 7.2.91 monitor - type 'help' for more information
(qemu) info registers

CPU#0
NIP 0000000000000e4c   LR 0000000000000000 CTR 0000000000000000 XER
0000000000000000 CPU#0
MSR 9000000000000000 HID0 0000000000000000  HF fc000006 iidx 7 didx 7
TB 00000000 2494783394 DECR 1800184060
GPR00 0000000004000000 0400000000000000 0000000000000010 0000000001000000
GPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
CR 00000000  [ -  -  -  -  -  -  -  -  ]             RES ffffffffffffffff
FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPSCR 0000000000000000
 SRR0 0000000000000000  SRR1 0000000000000000    PVR 0000000000800200 VRSAVE 0000000000000000
SPRG0 0000000000000000 SPRG1 0000000000000000  SPRG2 0000000000000000  SPRG3 0000000000000000
SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
HSRR0 0000000000000010 HSRR1 9000000020000000
 CFAR 0000000000000e4c
 LPCR 000000000000000c
 PTCR 0000000000000000   DAR 0000000000000000  DSISR 0000000000000000
(qemu) 




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward
  2023-03-27 13:12 ` [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward Nicholas Piggin
@ 2023-03-31 21:25   ` Fabiano Rosas
  0 siblings, 0 replies; 12+ messages in thread
From: Fabiano Rosas @ 2023-03-31 21:25 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza

Nicholas Piggin <npiggin@gmail.com> writes:

> This optional behavior was removed from the ISA in v3.0, see
> Summary of Changes preface:
>
>   Data Storage Interrupt Status Register for Alignment Interrupt:
>   Simplifies the Alignment interrupt by remov- ing the Data Storage
>   Interrupt Status Register (DSISR) from the set of registers modified
>   by the Alignment interrupt.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers
  2023-03-27 13:12 ` [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers Nicholas Piggin
@ 2023-03-31 21:26   ` Fabiano Rosas
  0 siblings, 0 replies; 12+ messages in thread
From: Fabiano Rosas @ 2023-03-31 21:26 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza

Nicholas Piggin <npiggin@gmail.com> writes:

> ISA v3.1 introduced prefix instructions. Among the changes, various
> synchronous interrupts report whether they were caused by a prefix
> instruction in (H)SRR1.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt
  2023-03-27 13:12 ` [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
@ 2023-03-31 21:27   ` Fabiano Rosas
  2023-05-23 18:11   ` Anushree Mathur
  1 sibling, 0 replies; 12+ messages in thread
From: Fabiano Rosas @ 2023-03-31 21:27 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza

Nicholas Piggin <npiggin@gmail.com> writes:

> powerpc ifetch endianness depends on MSR[LE] so it has to byteswap
> after cpu_ldl_code(). This corrects DSISR bits in alignment
> interrupts when running in little endian mode.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/6] target/ppc: Implement HEIR SPR
  2023-03-29  5:51   ` Michael Neuling
@ 2023-04-02  2:48     ` Nicholas Piggin
  0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2023-04-02  2:48 UTC (permalink / raw)
  To: Michael Neuling, qemu-ppc
  Cc: qemu-devel, Fabiano Rosas, Daniel Henrique Barboza

On Wed Mar 29, 2023 at 3:51 PM AEST, Michael Neuling wrote:
> Nick,
>
> > +    case POWERPC_EXCP_HV_EMU:
> > +        env->spr[SPR_HEIR] = insn;
> > +        if (is_prefix_excp(env, insn)) {
> > +            uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
> > +            env->spr[SPR_HEIR] |= (uint64_t)insn2 << 32;
>
> Are inst and inst2 in the right locations here? I think you might need
> insn in the top half and insn2 in the bottom.
>
> I wrote the little test case below. I'd expect GPR0 and GPR1 to end up
> with the same value, but they don't with this code

You're right. I was a bit confused becaue the prefix instructions are
treated as two words, so ld (insn) in little endian doesn't match
HEIR, for example.

The ISA uses the term "image", but that's only really defined for 4
byte instructions AFAIKS. You can deduce though,

  There may be circumstances in which the suffix cannot be loaded [...]
  In such circumstances, bits 32:63 are set to 0s.

So prefix word goes in the high bits. Real P10 agrees, so does
systemsim. I'll fix and re-send.

Is there any better semantics in the ISA or should I raise an issue to
clarify instruction image for prefix?

Thanks,
Nick


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt
  2023-03-27 13:12 ` [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
  2023-03-31 21:27   ` Fabiano Rosas
@ 2023-05-23 18:11   ` Anushree Mathur
  1 sibling, 0 replies; 12+ messages in thread
From: Anushree Mathur @ 2023-05-23 18:11 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Fabiano Rosas, Daniel Henrique Barboza


On 3/27/23 18:42, Nicholas Piggin wrote:
> powerpc ifetch endianness depends on MSR[LE] so it has to byteswap
> after cpu_ldl_code(). This corrects DSISR bits in alignment
> interrupts when running in little endian mode.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> ---
> Since v1:
> - Removed big endian ifdef [Fabiano review]
> - Acaually use need_byswap helper.
>
>   target/ppc/excp_helper.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 287659c74d..07729967b5 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -133,6 +133,24 @@ static void dump_hcall(CPUPPCState *env)
>                     env->nip);
>   }
>   
> +/* Return true iff byteswap is needed in a scalar memop */
> +static inline bool need_byteswap(CPUArchState *env)
> +{
> +    /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */
> +    return !!(env->msr & ((target_ulong)1 << MSR_LE));
> +}
> +
> +static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr)

This hunk fails to compile with configure --disable-tcg

FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o
cc -m64 -mlittle-endian -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. 
-Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui 
-Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g 
-fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef 
-Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes 
-Wredundant-decls -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels 
-Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wmissing-format-attribute -Wno-missing-include-dirs 
-Wno-shift-negative-value -Wno-psabi -isystem 
/home/Shreya/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
/home/Shreya/qemu -iquote /home/Shreya/qemu/include -pthread 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers 
-isystemlinux-headers -DNEED_CPU_H 
'-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' 
'-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ 
libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o -MF 
libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o.d -o 
libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o -c 
../target/ppc/excp_helper.c
../target/ppc/excp_helper.c:143:49: error: unknown type name ‘abi_ptr’; 
did you mean ‘si_ptr’?
   143 | static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr)
       |                                                 ^~~~~~~
       |                                                 si_ptr
../target/ppc/excp_helper.c: In function ‘powerpc_excp_books’:
../target/ppc/excp_helper.c:1416:16: error: implicit declaration of 
function ‘ppc_ldl_code’ [-Werror=implicit-function-declaration]
  1416 |         insn = ppc_ldl_code(env, env->nip);
       |                ^~~~~~~~~~~~
../target/ppc/excp_helper.c:1416:16: error: nested extern declaration of 
‘ppc_ldl_code’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

> +{
> +    uint32_t insn = cpu_ldl_code(env, addr);
> +
> +    if (need_byteswap(env)) {
> +        insn = bswap32(insn);
> +    }
> +
> +    return insn;
> +}
> +
>   static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
>   {
>       const char *es;
> @@ -3097,7 +3115,7 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
>   
>       /* Restore state and reload the insn we executed, for filling in DSISR.  */
>       cpu_restore_state(cs, retaddr);
> -    insn = cpu_ldl_code(env, env->nip);
> +    insn = ppc_ldl_code(env, env->nip);
>   
>       switch (env->mmu_model) {
>       case POWERPC_MMU_SOFT_4xx:

Thanks

Anushree Mathur



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-05-23 19:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 13:12 [PATCH v2 1/6] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
2023-03-27 13:12 ` [PATCH v2 2/6] target/ppc: Better CTRL SPR implementation Nicholas Piggin
2023-03-27 13:12 ` [PATCH v2 3/6] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
2023-03-31 21:27   ` Fabiano Rosas
2023-05-23 18:11   ` Anushree Mathur
2023-03-27 13:12 ` [PATCH v2 4/6] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward Nicholas Piggin
2023-03-31 21:25   ` Fabiano Rosas
2023-03-27 13:12 ` [PATCH v2 5/6] target/ppc: Add SRR1 prefix indication to interrupt handlers Nicholas Piggin
2023-03-31 21:26   ` Fabiano Rosas
2023-03-27 13:12 ` [PATCH v2 6/6] target/ppc: Implement HEIR SPR Nicholas Piggin
2023-03-29  5:51   ` Michael Neuling
2023-04-02  2:48     ` Nicholas Piggin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).