diff mbox series

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

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

Commit Message

Nicholas Piggin March 23, 2023, 2:22 a.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

Harsh Prateek Bora May 9, 2023, 9:51 a.m. UTC | #1 
On Thu, Mar 23, 2023 at 12:22:37PM +1000, Nicholas Piggin wrote:
> 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(-)
> 

<snip>

> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 2e0321ab69..d206903562 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1614,13 +1614,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;

Since there is a common code, this could be better written like:
    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;
        }
	/* fall through below common code for EXCP_HVIRT */
    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;

regards,
Harsh
>      case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
>      case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
>      case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
> -- 
> 2.37.2
> 
>
Nicholas Piggin May 15, 2023, 8:26 a.m. UTC | #2 
On Tue May 9, 2023 at 7:51 PM AEST, Harsh Prateek Bora wrote:
> On Thu, Mar 23, 2023 at 12:22:37PM +1000, Nicholas Piggin wrote:
> > 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(-)
> > 
>
> <snip>
>
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 2e0321ab69..d206903562 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -1614,13 +1614,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;
>
> Since there is a common code, this could be better written like:
>     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;
>         }
> 	/* fall through below common code for EXCP_HVIRT */
>     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;

That would be wrong for the other HSRR fallthroughs above it.

Thanks,
Nick
Harsh Prateek Bora May 15, 2023, 8:32 a.m. UTC | #3 
On 5/15/23 13:56, Nicholas Piggin wrote:
> On Tue May 9, 2023 at 7:51 PM AEST, Harsh Prateek Bora wrote:
>> On Thu, Mar 23, 2023 at 12:22:37PM +1000, Nicholas Piggin wrote:
>>> 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(-)
>>>
>>
>> <snip>
>>
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index 2e0321ab69..d206903562 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -1614,13 +1614,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;
>>
>> Since there is a common code, this could be better written like:
>>      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;
>>          }
>> 	/* fall through below common code for EXCP_HVIRT */
>>      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;
> 
> That would be wrong for the other HSRR fallthroughs above it.
> 
Oh yeh, in that case, may be move it to top of the EXCP_HISI, it would 
need duplicating one line of assignment though (relatively better?).

regards,
Harsh

> Thanks,
> Nick
>
Harsh Prateek Bora May 15, 2023, 9:32 a.m. UTC | #4 
On 5/15/23 14:02, Harsh Prateek Bora wrote:
>>
>> That would be wrong for the other HSRR fallthroughs above it.
>>
> Oh yeh, in that case, may be move it to top of the EXCP_HISI, it would 
> need duplicating one line of assignment though (relatively better?).

correcting myself, no duplication needed if keeping above EXCP_HISI.

> 
> regards,
> Harsh
Nicholas Piggin May 15, 2023, 10:45 a.m. UTC | #5 
On Mon May 15, 2023 at 7:32 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 5/15/23 14:02, Harsh Prateek Bora wrote:
> >>
> >> That would be wrong for the other HSRR fallthroughs above it.
> >>
> > Oh yeh, in that case, may be move it to top of the EXCP_HISI, it would 
> > need duplicating one line of assignment though (relatively better?).
>
> correcting myself, no duplication needed if keeping above EXCP_HISI.

No, because HV_EMU interrupts get an error code that can not be put
into HSRR1.

Thanks,
Nick
Harsh Prateek Bora May 15, 2023, 10:54 a.m. UTC | #6 
On 5/15/23 16:15, Nicholas Piggin wrote:
> On Mon May 15, 2023 at 7:32 PM AEST, Harsh Prateek Bora wrote:
>>
>>
>> On 5/15/23 14:02, Harsh Prateek Bora wrote:
>>>>
>>>> That would be wrong for the other HSRR fallthroughs above it.
>>>>
>>> Oh yeh, in that case, may be move it to top of the EXCP_HISI, it would
>>> need duplicating one line of assignment though (relatively better?).
>>
>> correcting myself, no duplication needed if keeping above EXCP_HISI.
> 
> No, because HV_EMU interrupts get an error code that can not be put
> into HSRR1.
> 
Oh ok, thanks for clarifying.

regards,
Harsh

> 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 2e0321ab69..d206903562 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1614,13 +1614,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          */