qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-ppc] [PATCH v2] ppc: allow certain HV interrupts to be delivered t


From: Nicholas Piggin
Subject: [Qemu-ppc] [PATCH v2] ppc: allow certain HV interrupts to be delivered to guests
Date: Fri, 21 Oct 2016 15:35:43 +1100

On Fri, 21 Oct 2016 12:09:54 +1100
David Gibson <address@hidden> wrote:

> On Fri, Oct 21, 2016 at 12:40:58AM +1100, Nicholas Piggin wrote:
> > On Thu, 20 Oct 2016 15:08:07 +0200
> > Cédric Le Goater <address@hidden> wrote:
> >   
> > > On 10/20/2016 08:59 AM, Nicholas Piggin wrote:  
> > > > Signed-off-by: Nicholas Piggin <address@hidden>
> > > > ---
> > > >  target-ppc/excp_helper.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> > > > index 53c4075..477af10 100644
> > > > --- a/target-ppc/excp_helper.c
> > > > +++ b/target-ppc/excp_helper.c
> > > > @@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, 
> > > > int excp_model, int excp)
> > > >              /* indicate that we resumed from power save mode */
> > > >              msr |= 0x10000;
> > > >              new_msr |= ((target_ulong)1 << MSR_ME);
> > > > +            new_msr |= (target_ulong)MSR_HVB;
> > > > +        } else {
> > > > +           /* The ISA specifies the HV bit is set when the hardware 
> > > > interrupt
> > > > +            * is raised, however when hypervisors deliver the 
> > > > exception to
> > > > +            * guests, it should not be set.
> > > > +            */
> > > >          }
> > > > -
> > > > -        new_msr |= (target_ulong)MSR_HVB;
> > > >          ail = 0;
> > > >          break;
> > > >      case POWERPC_EXCP_DSEG:      /* Data segment exception             
> > > >       */
> > > >     
> > > 
> > > should not that be cleared later on in powerpc_excp() by :
> > > 
> > >     env->msr = new_msr & env->msr_mask;
> > > 
> > > ? but the routine is rather long so I might be missing a branch.  
> > 
> > No you're right, so it can't leak into the guest, phew!
> > 
> > The problem I get is the interrupt code doing some things differently
> > depending on on the HV bit. For example what I noticed is the guest
> > losing its LE bit upon entry.
> > 
> > Perhaps a cleaner way is for the system reset case to set new_msr
> > according to the ISA, and then apply the msr_mask (or at least mask
> > out HV) before calculating the exception model? Any preference?  
> 
> I think the proposed revision makes sense.
> 

What do you think of this version? This fixes up machine check guest
delivery as well. I'm sending this ahead of the new hcall patch, because
it's a bugfix for existing code. I'll get around to the hcall again next
week.

Thanks,
Nick


ppc hypervisors have delivered system reset and machine check exception
interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR,
or NMI injection in QEMU).

These exceptions are architected to set the HV bit in hardware, however
when injected into a guest, the HV bit should be cleared. Current code
masks off the HV bit before setting the new MSR, however this happens after
the interrupt delivery model has calculated delivery mode for the exception.
This can result in the guest's MSR LE bit being lost.

Provide a new flag for HV exceptions to allow delivery to guests. The
exception model masks out the HV bit.

Also add another sanity check to ensure other such exceptions don't try
to set HV in guest without setting guest_hv_excp

Signed-off-by: Nicholas Piggin <address@hidden>
---
 target-ppc/excp_helper.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 53c4075..1b18433 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
-    int srr0, srr1, asrr0, asrr1, lev, ail;
+    int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;
     bool lpes0;
 
     qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
@@ -149,6 +149,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
      *
      * AIL is initialized here but can be cleared by
      * selected exceptions
+     *
+     * guest_hv_excp is a provision to raise HV architected
+     * exceptions in guests by delivering them with HV bit
+     * clear. System reset and machine check use this.
      */
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_POWER7 ||
@@ -165,6 +169,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
         lpes0 = true;
         ail = 0;
     }
+    guest_hv_excp = 0;
 
     /* Hypervisor emulation assistance interrupt only exists on server
      * arch 2.05 server or later. We also don't want to generate it if
@@ -214,6 +219,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
             cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
         }
         new_msr |= (target_ulong)MSR_HVB;
+        guest_hv_excp = 1;
         ail = 0;
 
         /* machine check exceptions don't have ME set */
@@ -391,8 +397,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
             msr |= 0x10000;
             new_msr |= ((target_ulong)1 << MSR_ME);
         }
-
         new_msr |= (target_ulong)MSR_HVB;
+        guest_hv_excp = 1;
         ail = 0;
         break;
     case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
@@ -610,10 +616,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
 
     /* Sanity check */
     if (!(env->msr_mask & MSR_HVB) && (srr0 == SPR_HSRR0)) {
-        cpu_abort(cs, "Trying to deliver HV exception %d with "
+        cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
                   "no HV support\n", excp);
     }
 
+    /* The ISA specifies the HV bit is set when the hardware interrupt
+     * is raised, however in some cases hypervisors deliver the exception
+     * to guests. HV should be cleared in that case.
+     */
+    if (!(env->msr_mask & MSR_HVB) && (new_msr & MSR_HVB)) {
+        if (guest_hv_excp) {
+            new_msr &= ~MSR_HVB;
+        } else {
+            cpu_abort(cs, "Trying to deliver HV exception (MSR) %d with "
+                      "no HV support\n", excp);
+        }
+    }
+
     /* If any alternate SRR register are defined, duplicate saved values */
     if (asrr0 != -1) {
         env->spr[asrr0] = env->spr[srr0];
-- 
2.9.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]