qemu-ppc
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v18 5/7] ppc: spapr: Handle "ibm,nmi-register" and "ibm,nmi-i

From: Ganesh
Subject: Re: [PATCH v18 5/7] ppc: spapr: Handle "ibm,nmi-register" and "ibm,nmi-interlock" RTAS calls
Date: Thu, 9 Jan 2020 00:19:12 +0530
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 


On 1/8/20 6:34 AM, David Gibson wrote:

On Tue, Jan 07, 2020 at 11:57:08AM +0530, Ganesh wrote:

On 1/3/20 7:49 AM, David Gibson wrote:

On Thu, Jan 02, 2020 at 01:21:09PM +0530, Ganesh Goudar wrote:

From: Aravinda Prasad <address@hidden>

This patch adds support in QEMU to handle "ibm,nmi-register"
and "ibm,nmi-interlock" RTAS calls.

The machine check notification address is saved when the
OS issues "ibm,nmi-register" RTAS call.

This patch also handles the case when multiple processors
experience machine check at or about the same time by
handling "ibm,nmi-interlock" call. In such cases, as per
PAPR, subsequent processors serialize waiting for the first
processor to issue the "ibm,nmi-interlock" call. The second
processor that also received a machine check error waits
till the first processor is done reading the error log.
The first processor issues "ibm,nmi-interlock" call
when the error log is consumed.

[Move fwnmi registration to .apply hook]
Signed-off-by: Ganesh Goudar <address@hidden>
Signed-off-by: Aravinda Prasad <address@hidden>
---
   hw/ppc/spapr_caps.c    |  6 +++++
   hw/ppc/spapr_rtas.c    | 58 ++++++++++++++++++++++++++++++++++++++++++
   include/hw/ppc/spapr.h |  7 ++++-
   3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 3001098601..e922419cfb 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -502,6 +502,12 @@ static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, 
uint8_t val,
       if (!val) {
           return; /* Disabled by default */
       }
+
+    if (!spapr->fwnmi_calls_registered && !kvmppc_set_fwnmi()) {

So, we definitely need the kvmppc_set_fwnmi() call here.  But in the
case where we *do* have KVM, but the call fails, we should fail the
.apply hook, rather than ignoring it silently.

As we've discussed although TCG behaviour with fwnmi isn't 100%
correct, it's close enough to pass for most purposes - so it's
reasonable to continue if the cap is selected.  But if the cap is
selected and we're running with KVM we *must* enable the capability in
KVM or we're not providing the environment the user requested.


+        /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
+        spapr_fwnmi_register();

We discussed registering the hypercalls here, but I thought after that
I suggested just always registering them, and having them bail out
when called if the cap is not set.  I see that you've implemented that
bailout for register, though not for interlock.  I think that's
simpler than registering them here.

something like this?, with bailout in interlock as well.

{
     if (!val) {
         return; /* Disabled by default */
     }

     if (kvm_enabled()) {
         if (kvmppc_set_fwnmi() < 0) {
             error_report("Could not enable fwnmi capability");
             exit(1)
     }

     if (!spapr->fwnmi_calls_registered) {
         /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
         spapr_fwnmi_register();
         spapr->fwnmi_calls_registered = true;
     }
}

Uh.. no.. not really.  I was suggesting registering the calls
unconditionally, but in each of the call implementations you have

Yes I got your point, here I am just trying to avoid registering the
calls more than once, otherwise we may hit an assert.



     if(!fwnmi_enabed)
          return H_FUNCTION;

or maybe H_NOT_AVAILABLE.

Sure, ill add this check in interlock call also.



+        spapr->fwnmi_calls_registered = true;
+    }
   }
   SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 2c066a372d..54b142f35b 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -400,6 +400,56 @@ static void rtas_get_power_level(PowerPCCPU *cpu, 
SpaprMachineState *spapr,
       rtas_st(rets, 1, 100);
   }
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+                                  SpaprMachineState *spapr,
+                                  uint32_t token, uint32_t nargs,
+                                  target_ulong args,
+                                  uint32_t nret, target_ulong rets)
+{
+    hwaddr rtas_addr;
+
+    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
+    rtas_addr = spapr_get_rtas_addr();
+    if (!rtas_addr) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
+    spapr->guest_machine_check_addr = rtas_ld(args, 1);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
+                                   SpaprMachineState *spapr,
+                                   uint32_t token, uint32_t nargs,
+                                   target_ulong args,
+                                   uint32_t nret, target_ulong rets)
+{
+    if (spapr->guest_machine_check_addr == -1) {
+        /* NMI register not called */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    if (spapr->mc_status != cpu->vcpu_id) {
+        /* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    /*
+     * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+     * hence unset mc_status.
+     */
+    spapr->mc_status = -1;
+    qemu_cond_signal(&spapr->mc_delivery_cond);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
   static struct rtas_call {
       const char *name;
       spapr_rtas_fn fn;
@@ -503,6 +553,14 @@ hwaddr spapr_get_rtas_addr(void)
       return (hwaddr)fdt32_to_cpu(*rtas_data);
   }
+void spapr_fwnmi_register(void)
+{
+    spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
+                        rtas_ibm_nmi_register);
+    spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
+                        rtas_ibm_nmi_interlock);
+}
+
   static void core_rtas_register_types(void)
   {
       spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 652a5514e8..a90e677cc3 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -218,6 +218,8 @@ struct SpaprMachineState {
       unsigned gpu_numa_id;
       SpaprTpmProxy *tpm_proxy;
+
+    bool fwnmi_calls_registered;
   };
   #define H_SUCCESS         0
@@ -656,8 +658,10 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong 
opcode,
   #define RTAS_IBM_REMOVE_PE_DMA_WINDOW           (RTAS_TOKEN_BASE + 0x28)
   #define RTAS_IBM_RESET_PE_DMA_WINDOW            (RTAS_TOKEN_BASE + 0x29)
   #define RTAS_IBM_SUSPEND_ME                     (RTAS_TOKEN_BASE + 0x2A)
+#define RTAS_IBM_NMI_REGISTER                   (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_IBM_NMI_INTERLOCK                  (RTAS_TOKEN_BASE + 0x2C)
-#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x2D)
   /* RTAS ibm,get-system-parameter token values */
   #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS      20
@@ -908,4 +912,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr 
pagesize,
   void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
   hwaddr spapr_get_rtas_addr(void);
+void spapr_fwnmi_register(void);
   #endif /* HW_SPAPR_H */
reply via email to
[Prev in Thread] Current Thread [Next in Thread]