diff mbox series

[v2,6/6] target/ppc: Implement HEIR SPR

Message ID 20230327131218.2721044-6-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/6] target/ppc: Fix width of some 32-bit SPRs | expand

Commit Message

Nicholas Piggin March 27, 2023, 1:12 p.m. UTC 
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(-)

Comments

Michael Neuling March 29, 2023, 5:51 a.m. UTC | #1 
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)
Nicholas Piggin April 2, 2023, 2:48 a.m. UTC | #2 
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
diff mbox series

Patch

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          */