target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests

Receive updates from SLOF about the updated rtas-base. A separate patch for SLOF [1] adds functionality to invoke a private HCALL whenever OS issues instantiate-rtas with a new rtas-base.

This is required as QEMU needs to know the updated rtas-base as it allocates error reporting structure in RTAS space upon a machine check exception.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-August/120386.html

Signed-off-by Aravinda Prasad <[email protected]>

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 waits till the first processor, which also received a machine check error, is done reading the error log. The first processor issues "ibm,nmi-interlock" call when the error log is consumed. This patch implements the releasing part of the error-log while subsequent patch (which builds error log) handles the locking part.

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c    |   36 ++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |   10 +++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

hw/ppc/spapr_rtas.c +0 -0

index 9869bc9..17c4672 100644
@@ -597,6 +597,38 @@ out:
     rtas_st(rets, 0, rc);

hw/ppc/spapr_rtas.c +24 -0

+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) {
+        /* NMI register not called */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+    } else {
+        /*
+         * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+         * hence unset mc_in_progress.
+         */
+        spapr->mc_in_progress = false;
+        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;

@@ -747,6 +779,10 @@ static void core_rtas_register_types(void)
                         rtas_get_sensor_state);
     spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector",
                         rtas_ibm_configure_connector);
+    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);
 }
 
 type_init(core_rtas_register_types)

include/hw/ppc/spapr.h +0 -0

index b5cadd7..de84a4e 100644

include/hw/ppc/spapr.h +5 -1

+    QemuCond mc_delivery_cond;
+
     /*< public >*/
     char *kvm_type;
 };

@@ -458,8 +464,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_IBM_SET_SLOT_RESET                 (RTAS_TOKEN_BASE + 0x23)
 #define RTAS_IBM_CONFIGURE_PE                   (RTAS_TOKEN_BASE + 0x24)
 #define RTAS_IBM_SLOT_ERROR_DETAIL              (RTAS_TOKEN_BASE + 0x25)
+#define RTAS_IBM_NMI_REGISTER                   (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_IBM_NMI_INTERLOCK                  (RTAS_TOKEN_BASE + 0x27)
 
-#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x28)
 
 /* RTAS ibm,get-system-parameter token values */
 #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS      20

Quoted 32 lines · Aravinda Prasad

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 waits till the first processor, which also received a machine check error, is done reading the error log. The first processor issues "ibm,nmi-interlock" call when the error log is consumed. This patch implements the releasing part of the error-log while subsequent patch (which builds error log) handles the locking part.

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c    |   36 ++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |   10 +++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

hw/ppc/spapr_rtas.c +0 -0

index 9869bc9..17c4672 100644
@@ -597,6 +597,38 @@ out:
     rtas_st(rets, 0, rc);
 }
 

hw/ppc/spapr_rtas.c out: · line 607

Quoted 8 lines

hw/ppc/spapr_rtas.c +8 -0

+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)
+{
+    spapr->mc_in_progress = false;
+    qemu_cond_init(&spapr->mc_delivery_cond);

I think initializing mc_in_progress and the condition variable should go in the overall initialization (in fact during system reset) instead of here. Things using these shouldn't be invoked until after the nmi_register, but it's safer to have the qemu internal variables initialized beforehand.

hw/ppc/spapr_rtas.c out: · line 610

Quoted 4 lines

hw/ppc/spapr_rtas.c +3 -0

+    spapr->guest_machine_check_addr = rtas_ld(args, 1);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}

It also looks like you need to add code to clear
guest_machine_check_addr if the system is reset

Quoted 43 lines

hw/ppc/spapr_rtas.c +24 -0

+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) {
+        /* NMI register not called */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+    } else {
+        /*
+         * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+         * hence unset mc_in_progress.
+         */
+        spapr->mc_in_progress = false;
+        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;

@@ -747,6 +779,10 @@ static void core_rtas_register_types(void)
                         rtas_get_sensor_state);
     spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector",
                         rtas_ibm_configure_connector);
+    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);
 }
 
 type_init(core_rtas_register_types)

include/hw/ppc/spapr.h +0 -0

index b5cadd7..de84a4e 100644
@@ -74,6 +74,12 @@ struct sPAPRMachineState {
     /* RTAS state */
     QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
 

include/hw/ppc/spapr.h struct sPAPRMachineState { · line 80

Quoted 4 lines

include/hw/ppc/spapr.h +4 -0

+    /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
+    target_ulong guest_machine_check_addr;
+    bool mc_in_progress;
+    int mc_cpu;

mc_cpu doesn't appear to be used.

I think guest_machine_check_addr and mc_in_progress will also need to be added to migration state.

Quoted 19 lines

include/hw/ppc/spapr.h +5 -1

+    QemuCond mc_delivery_cond;
+
     /*< public >*/
     char *kvm_type;
 };

@@ -458,8 +464,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_IBM_SET_SLOT_RESET                 (RTAS_TOKEN_BASE + 0x23)
 #define RTAS_IBM_CONFIGURE_PE                   (RTAS_TOKEN_BASE + 0x24)
 #define RTAS_IBM_SLOT_ERROR_DETAIL              (RTAS_TOKEN_BASE + 0x25)
+#define RTAS_IBM_NMI_REGISTER                   (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_IBM_NMI_INTERLOCK                  (RTAS_TOKEN_BASE + 0x27)
 
-#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_TOKEN_MAX                          (RTAS_TOKEN_BASE + 0x28)
 
 /* RTAS ibm,get-system-parameter token values */
 #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS      20

Memory error such as bit flips that cannot be corrected by hardware are passed on to the kernel for handling. If the memory address in error belongs to guest then guest kernel is responsible for taking suitable action. Patch [1] enhances KVM to exit guest with exit reason set to KVM_EXIT_NMI in such cases.

This patch handles KVM_EXIT_NMI exit. If the guest OS has registered the machine check handling routine by calling "ibm,nmi-register", then the handler builds the error log and invokes the registered handler else invokes the handler at 0x200.

[1] http://marc.info/?l=kvm&m=145024538523497

Signed-off-by Aravinda Prasad <[email protected]>

---
 cpus.c                   |    5 +++
 hw/ppc/spapr.c           |    6 +++
 include/qemu/main-loop.h |    8 ++++
 target-ppc/kvm.c         |   86 ++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm_ppc.h     |   81 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 186 insertions(+)

cpus.c +0 -0

index dddd056..7b7dd0f 100644

cpus.c +0 -0

 static int all_vcpus_paused(void)
 {
     CPUState *cpu;

hw/ppc/spapr.c +6 -0

index 05926a3..501dd70 100644
@@ -1556,6 +1556,12 @@ static void ppc_spapr_init(MachineState *machine)
         exit(1);
     }
     spapr->rtas_size = get_image_size(filename);
+
+    assert(spapr->rtas_size < RTAS_ERRLOG_OFFSET);
+
+    /* Resize blob to accommodate error log. */
+    spapr->rtas_size = RTAS_ERRLOG_OFFSET + sizeof(struct RtasMCELog);
+
     spapr->rtas_blob = g_malloc(spapr->rtas_size);
     if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
         error_report("Could not load LPAR rtas '%s'", filename);

include/qemu/main-loop.h +8 -0

index 9976909..c4d4446 100644
@@ -263,6 +263,14 @@ void qemu_mutex_lock_iothread(void);
  */
 void qemu_mutex_unlock_iothread(void);
 
+/**
+ * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
+ *
+ * This function atomically releases the main loop mutex and causes
+ * the calling thread to block on the condition.
+ */
+void qemu_cond_wait_iothread(QemuCond *cond);
+
 /* internal interfaces */
 
 void qemu_fd_register(int fd);

target-ppc/kvm.c +5 -0

index 110436d..2bbb46d 100644
@@ -1665,6 +1665,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         ret = 0;
         break;
 
+    case KVM_EXIT_NMI:
+        DPRINTF("handle NMI exception\n");
+        ret = kvm_handle_nmi(cpu);
+        break;
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;

target-ppc/kvm.c +4 -0

+
+    /*
+     * Properly set bits in MSR before we invoke the handler.
+     * SRR0/1, DAR and DSISR are properly set by KVM

target-ppc/kvm.c +57 -0

+    env->msr = msr;
+
+    if (!spapr->guest_machine_check_addr) {
+        /*
+         * If OS has not registered with "ibm,nmi-register"
+         * jump to 0x200
+         */
+        env->nip = 0x200;
+        return 0;
+    }
+
+    while (spapr->mc_in_progress) {
+        /*
+         * Check whether the same CPU got machine check error
+         * while still handling the mc error (i.e., before
+         * that CPU called "ibm,nmi-interlock"
+         */
+        if (spapr->mc_cpu == cpu->cpu_dt_id) {
+            qemu_system_guest_panicked();
+        }
+        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
+    }
+    spapr->mc_in_progress = true;
+    spapr->mc_cpu = cpu->cpu_dt_id;
+
+    /* Set error log fields */
+    mc_log.r3 = env->gpr[3];
+    mc_log.err_log.byte0 = 0;
+    mc_log.err_log.byte1 =
+        (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT);
+    mc_log.err_log.byte1 |=
+        (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT);
+    mc_log.err_log.byte2 =
+        (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT);
+    mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY;
+
+    if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) {
+        mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR;
+    } else {
+        mc_log.err_log.byte3 = 0;
+    }
+
+    /* Handle all Host/Guest LE/BE combinations */
+    if (env->msr & (1ULL << MSR_LE)) {
+        mc_log.r3 = cpu_to_le64(mc_log.r3);
+    } else {
+        mc_log.r3 = cpu_to_be64(mc_log.r3);
+    }
+
+    cpu_physical_memory_write(spapr->rtas_addr + RTAS_ERRLOG_OFFSET,
+                              &mc_log, sizeof(mc_log));
+
+    env->nip = spapr->guest_machine_check_addr;
+    env->gpr[3] = spapr->rtas_addr + RTAS_ERRLOG_OFFSET;
+
+    return 0;
+}

target-ppc/kvm_ppc.h +81 -0

index 5c1d334..ea3345b 100644
@@ -53,6 +53,87 @@ void kvmppc_hash64_free_pteg(uint64_t token);
 void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
                              target_ulong pte0, target_ulong pte1);
 bool kvmppc_has_cap_fixup_hcalls(void);
+int kvm_handle_nmi(PowerPCCPU *cpu);
+
+/* Offset from rtas-base where error log is placed */
+#define RTAS_ERRLOG_OFFSET       0x200
+
+#define RTAS_ELOG_SEVERITY_SHIFT         0x5
+#define RTAS_ELOG_DISPOSITION_SHIFT      0x3
+#define RTAS_ELOG_INITIATOR_SHIFT        0x4
+
+/*
+ * Only required RTAS event severity, disposition, initiator
+ * target and type are copied from arch/powerpc/include/asm/rtas.h
+ */
+
+/* RTAS event severity */
+#define RTAS_SEVERITY_ERROR_SYNC    0x3
+
+/* RTAS event disposition */
+#define RTAS_DISP_NOT_RECOVERED     0x2
+
+/* RTAS event initiator */
+#define RTAS_INITIATOR_MEMORY       0x4
+
+/* RTAS event target */
+#define RTAS_TARGET_MEMORY          0x4
+
+/* RTAS event type */
+#define RTAS_TYPE_ECC_UNCORR        0x09
+
+/*
+ * Currently KVM only passes on the uncorrected machine
+ * check memory error to guest. Other machine check errors
+ * such as SLB multi-hit and TLB multi-hit are recovered
+ * in KVM and are not passed on to guest.
+ *
+ * DSISR Bit for uncorrected machine check error. Based
+ * on arch/powerpc/include/asm/mce.h
+ */
+#define PPC_BIT(bit)                (0x8000000000000000ULL >> bit)
+#define P7_DSISR_MC_UE              (PPC_BIT(48))  /* P8 too */
+
+/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */
+struct rtas_error_log {
+    /* Byte 0 */
+    uint8_t     byte0;          /* Architectural version */
+
+    /* Byte 1 */
+    uint8_t     byte1;
+    /* XXXXXXXX
+     * XXX      3: Severity level of error
+     *    XX    2: Degree of recovery
+     *      X   1: Extended log present?
+     *       XX 2: Reserved
+     */
+
+    /* Byte 2 */
+    uint8_t     byte2;
+    /* XXXXXXXX
+     * XXXX     4: Initiator of event
+     *     XXXX 4: Target of failed operation
+     */
+    uint8_t     byte3;          /* General event or error*/
+    __be32      extended_log_length;    /* length in bytes */
+    unsigned char   buffer[1];      /* Start of extended log */
+                                /* Variable length.      */
+};
+
+/*
+ * Data format in RTAS-Blob
+ *
+ * This structure contains error information related to Machine
+ * Check exception. This is filled up and copied to rtas-blob
+ * upon machine check exception. The address of rtas-blob is
+ * passed on to OS registered machine check notification
+ * routines upon machine check exception
+ */
+struct RtasMCELog {
+    target_ulong r3;
+    struct rtas_error_log err_log;
+};
+
 
 #else
 

Quoted 29 lines · Aravinda Prasad

Memory error such as bit flips that cannot be corrected by hardware are passed on to the kernel for handling. If the memory address in error belongs to guest then guest kernel is responsible for taking suitable action. Patch [1] enhances KVM to exit guest with exit reason set to KVM_EXIT_NMI in such cases.

This patch handles KVM_EXIT_NMI exit. If the guest OS has registered the machine check handling routine by calling "ibm,nmi-register", then the handler builds the error log and invokes the registered handler else invokes the handler at 0x200.

[1] http://marc.info/?l=kvm&m=145024538523497

Signed-off-by Aravinda Prasad <[email protected]>

---
 cpus.c                   |    5 +++
 hw/ppc/spapr.c           |    6 +++
 include/qemu/main-loop.h |    8 ++++
 target-ppc/kvm.c         |   86 ++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm_ppc.h     |   81 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 186 insertions(+)

cpus.c +0 -0

index dddd056..7b7dd0f 100644

cpus.c void qemu_mutex_unlock_iothread(void) · line 1161

Quoted 8 lines

cpus.c +5 -0

@@ -1154,6 +1154,11 @@ void qemu_mutex_unlock_iothread(void)
     qemu_mutex_unlock(&qemu_global_mutex);
 }
 
+void qemu_cond_wait_iothread(QemuCond *cond)
+{
+    qemu_cond_wait(cond, &qemu_global_mutex);
+}
+

Even though it's trivial, this generic change should probably go in a separate patch from the papr specific pieces.

Speaking of which, I think it's sufficiently trivial you could just inline it in the header.

Quoted 57 lines

cpus.c +0 -0

 static int all_vcpus_paused(void)
 {
     CPUState *cpu;

hw/ppc/spapr.c +6 -0

index 05926a3..501dd70 100644
@@ -1556,6 +1556,12 @@ static void ppc_spapr_init(MachineState *machine)
         exit(1);
     }
     spapr->rtas_size = get_image_size(filename);
+
+    assert(spapr->rtas_size < RTAS_ERRLOG_OFFSET);
+
+    /* Resize blob to accommodate error log. */
+    spapr->rtas_size = RTAS_ERRLOG_OFFSET + sizeof(struct RtasMCELog);
+
     spapr->rtas_blob = g_malloc(spapr->rtas_size);
     if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
         error_report("Could not load LPAR rtas '%s'", filename);

include/qemu/main-loop.h +8 -0

index 9976909..c4d4446 100644
@@ -263,6 +263,14 @@ void qemu_mutex_lock_iothread(void);
  */
 void qemu_mutex_unlock_iothread(void);
 
+/**
+ * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
+ *
+ * This function atomically releases the main loop mutex and causes
+ * the calling thread to block on the condition.
+ */
+void qemu_cond_wait_iothread(QemuCond *cond);
+
 /* internal interfaces */
 
 void qemu_fd_register(int fd);

target-ppc/kvm.c +5 -0

index 110436d..2bbb46d 100644
@@ -1665,6 +1665,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         ret = 0;
         break;
 
+    case KVM_EXIT_NMI:
+        DPRINTF("handle NMI exception\n");
+        ret = kvm_handle_nmi(cpu);
+        break;
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;

target-ppc/kvm.c int kvm_arch_msi_data_to_gsi(uint32_t data) · line 2496

Quoted 8 lines

target-ppc/kvm.c +5 -0

@@ -2484,3 +2489,84 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
 {
     return data & 0xffff;
 }
+
+int kvm_handle_nmi(PowerPCCPU *cpu)
+{
+    struct RtasMCELog mc_log;
+    CPUPPCState *env = &cpu->env;

You go from cpu to env here..

target-ppc/kvm.c int kvm_arch_msi_data_to_gsi(uint32_t data) · line 2501

Quoted 5 lines

target-ppc/kvm.c +5 -0

+    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    target_ulong msr = 0;
+
+    cpu_synchronize_state(CPU(ppc_env_get_cpu(env)));

Then back again awkwardly, although you still have the cpu variable.

Quoted 10 lines

target-ppc/kvm.c +9 -0

+
+    /*
+     * Properly set bits in MSR before we invoke the handler.
+     * SRR0/1, DAR and DSISR are properly set by KVM
+     */
+    if (!(*pcc->interrupts_big_endian)(cpu)) {
+        msr |= (1ULL << MSR_LE);
+    }
+

target-ppc/kvm.c int kvm_arch_msi_data_to_gsi(uint32_t data) · line 2515

Quoted 5 lines

target-ppc/kvm.c +5 -0

+    if (env->msr && (1ULL << MSR_SF)) {
+        msr |= (1ULL << MSR_SF);
+    }
+
+    msr |= (1ULL << MSR_ME);

Based on earlier discussions, sounds like assert(msr & (1ULL << MSR_ME)) would actually be correct here.

Quoted 151 lines

target-ppc/kvm.c +57 -0

+    env->msr = msr;
+
+    if (!spapr->guest_machine_check_addr) {
+        /*
+         * If OS has not registered with "ibm,nmi-register"
+         * jump to 0x200
+         */
+        env->nip = 0x200;
+        return 0;
+    }
+
+    while (spapr->mc_in_progress) {
+        /*
+         * Check whether the same CPU got machine check error
+         * while still handling the mc error (i.e., before
+         * that CPU called "ibm,nmi-interlock"
+         */
+        if (spapr->mc_cpu == cpu->cpu_dt_id) {
+            qemu_system_guest_panicked();
+        }
+        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
+    }
+    spapr->mc_in_progress = true;
+    spapr->mc_cpu = cpu->cpu_dt_id;
+
+    /* Set error log fields */
+    mc_log.r3 = env->gpr[3];
+    mc_log.err_log.byte0 = 0;
+    mc_log.err_log.byte1 =
+        (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT);
+    mc_log.err_log.byte1 |=
+        (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT);
+    mc_log.err_log.byte2 =
+        (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT);
+    mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY;
+
+    if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) {
+        mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR;
+    } else {
+        mc_log.err_log.byte3 = 0;
+    }
+
+    /* Handle all Host/Guest LE/BE combinations */
+    if (env->msr & (1ULL << MSR_LE)) {
+        mc_log.r3 = cpu_to_le64(mc_log.r3);
+    } else {
+        mc_log.r3 = cpu_to_be64(mc_log.r3);
+    }
+
+    cpu_physical_memory_write(spapr->rtas_addr + RTAS_ERRLOG_OFFSET,
+                              &mc_log, sizeof(mc_log));
+
+    env->nip = spapr->guest_machine_check_addr;
+    env->gpr[3] = spapr->rtas_addr + RTAS_ERRLOG_OFFSET;
+
+    return 0;
+}

target-ppc/kvm_ppc.h +81 -0

index 5c1d334..ea3345b 100644
@@ -53,6 +53,87 @@ void kvmppc_hash64_free_pteg(uint64_t token);
 void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
                              target_ulong pte0, target_ulong pte1);
 bool kvmppc_has_cap_fixup_hcalls(void);
+int kvm_handle_nmi(PowerPCCPU *cpu);
+
+/* Offset from rtas-base where error log is placed */
+#define RTAS_ERRLOG_OFFSET       0x200
+
+#define RTAS_ELOG_SEVERITY_SHIFT         0x5
+#define RTAS_ELOG_DISPOSITION_SHIFT      0x3
+#define RTAS_ELOG_INITIATOR_SHIFT        0x4
+
+/*
+ * Only required RTAS event severity, disposition, initiator
+ * target and type are copied from arch/powerpc/include/asm/rtas.h
+ */
+
+/* RTAS event severity */
+#define RTAS_SEVERITY_ERROR_SYNC    0x3
+
+/* RTAS event disposition */
+#define RTAS_DISP_NOT_RECOVERED     0x2
+
+/* RTAS event initiator */
+#define RTAS_INITIATOR_MEMORY       0x4
+
+/* RTAS event target */
+#define RTAS_TARGET_MEMORY          0x4
+
+/* RTAS event type */
+#define RTAS_TYPE_ECC_UNCORR        0x09
+
+/*
+ * Currently KVM only passes on the uncorrected machine
+ * check memory error to guest. Other machine check errors
+ * such as SLB multi-hit and TLB multi-hit are recovered
+ * in KVM and are not passed on to guest.
+ *
+ * DSISR Bit for uncorrected machine check error. Based
+ * on arch/powerpc/include/asm/mce.h
+ */
+#define PPC_BIT(bit)                (0x8000000000000000ULL >> bit)
+#define P7_DSISR_MC_UE              (PPC_BIT(48))  /* P8 too */
+
+/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */
+struct rtas_error_log {
+    /* Byte 0 */
+    uint8_t     byte0;          /* Architectural version */
+
+    /* Byte 1 */
+    uint8_t     byte1;
+    /* XXXXXXXX
+     * XXX      3: Severity level of error
+     *    XX    2: Degree of recovery
+     *      X   1: Extended log present?
+     *       XX 2: Reserved
+     */
+
+    /* Byte 2 */
+    uint8_t     byte2;
+    /* XXXXXXXX
+     * XXXX     4: Initiator of event
+     *     XXXX 4: Target of failed operation
+     */
+    uint8_t     byte3;          /* General event or error*/
+    __be32      extended_log_length;    /* length in bytes */
+    unsigned char   buffer[1];      /* Start of extended log */
+                                /* Variable length.      */
+};
+
+/*
+ * Data format in RTAS-Blob
+ *
+ * This structure contains error information related to Machine
+ * Check exception. This is filled up and copied to rtas-blob
+ * upon machine check exception. The address of rtas-blob is
+ * passed on to OS registered machine check notification
+ * routines upon machine check exception
+ */
+struct RtasMCELog {
+    target_ulong r3;
+    struct rtas_error_log err_log;
+};
+
 
 #else
 

Introduce a new KVM capability to control how KVM behaves on machine check exception.

Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.

This is required to avoid problem if a new kernel/KVM is used with an old QEMU. As old QEMU might not understand the new NMI exit type and treat it as a fatal error, even though the guest could have actually handled the error if the exception was delivered to guest's 0x200 interrupt vector.

PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS in KVM code, but still not reflected in QEMU code.

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c       |   17 +++++++++++++++++
 linux-headers/linux/kvm.h |    1 +
 target-ppc/kvm.c          |    2 ++
 3 files changed, 20 insertions(+)

hw/ppc/spapr_rtas.c +17 -0

index 17c4672..53319da 100644
@@ -38,6 +38,8 @@
 #include <libfdt.h>
 #include "hw/ppc/spapr_drc.h"
 
+extern int cap_fwnmi;
+
 /* #define DEBUG_SPAPR */
 
 #ifdef DEBUG_SPAPR

@@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
                                   target_ulong args,
                                   uint32_t nret, target_ulong rets)
 {
+    int ret;
+    CPUState *cs = CPU(cpu);
+
+    if (!cap_fwnmi) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
     spapr->mc_in_progress = false;
     qemu_cond_init(&spapr->mc_delivery_cond);
     spapr->guest_machine_check_addr = rtas_ld(args, 1);
+
+    ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+    if (ret < 0) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
+
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 

linux-headers/linux/kvm.h +1 -0

index 683f713..2db1fba 100644
@@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_DISABLE_QUIRKS 116
 #define KVM_CAP_X86_SMM 117
 #define KVM_CAP_MULTI_ADDRESS_SPACE 118
+#define KVM_CAP_PPC_FWNMI 121
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

target-ppc/kvm.c +2 -0

index 2bbb46d..5339c04 100644
@@ -74,6 +74,7 @@ static int cap_ppc_watchdog;
 static int cap_papr;
 static int cap_htab_fd;
 static int cap_fixup_hcalls;
+int cap_fwnmi;
 
 static uint32_t debug_inst_opcode;
 

@@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
      * only activated after this by kvmppc_set_papr() */
     cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);
     cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL);
+    cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
 
     if (!cap_interrupt_level) {
         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "

Quoted 15 lines · Aravinda Prasad

Introduce a new KVM capability to control how KVM behaves on machine check exception.

Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.

This is required to avoid problem if a new kernel/KVM is used with an old QEMU. As old QEMU might not understand the new NMI exit type and treat it as a fatal error, even though the guest could have actually handled the error if the exception was delivered to guest's 0x200 interrupt vector.

Quoted 3 lines

PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS in KVM code, but still not reflected in QEMU code.

The commit message seems to be written as if this were the kernel patch adding the capability there, rather than the qemu patch using it.

Quoted 78 lines

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c       |   17 +++++++++++++++++
 linux-headers/linux/kvm.h |    1 +
 target-ppc/kvm.c          |    2 ++
 3 files changed, 20 insertions(+)

hw/ppc/spapr_rtas.c +17 -0

index 17c4672..53319da 100644
@@ -38,6 +38,8 @@
 #include <libfdt.h>
 #include "hw/ppc/spapr_drc.h"
 
+extern int cap_fwnmi;
+
 /* #define DEBUG_SPAPR */
 
 #ifdef DEBUG_SPAPR

@@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
                                   target_ulong args,
                                   uint32_t nret, target_ulong rets)
 {
+    int ret;
+    CPUState *cs = CPU(cpu);
+
+    if (!cap_fwnmi) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
     spapr->mc_in_progress = false;
     qemu_cond_init(&spapr->mc_delivery_cond);
     spapr->guest_machine_check_addr = rtas_ld(args, 1);
+
+    ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+    if (ret < 0) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
+
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 

linux-headers/linux/kvm.h +1 -0

index 683f713..2db1fba 100644
@@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_DISABLE_QUIRKS 116
 #define KVM_CAP_X86_SMM 117
 #define KVM_CAP_MULTI_ADDRESS_SPACE 118
+#define KVM_CAP_PPC_FWNMI 121
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

target-ppc/kvm.c +2 -0

index 2bbb46d..5339c04 100644
@@ -74,6 +74,7 @@ static int cap_ppc_watchdog;
 static int cap_papr;
 static int cap_htab_fd;
 static int cap_fixup_hcalls;
+int cap_fwnmi;
 
 static uint32_t debug_inst_opcode;
 

@@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
      * only activated after this by kvmppc_set_papr() */
     cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);
     cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL);
+    cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
 
     if (!cap_interrupt_level) {
         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "

Quoted 20 lines · David Gibson

Quoted 18 lines · Aravinda Prasad

Introduce a new KVM capability to control how KVM behaves on machine check exception.

Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.

This is required to avoid problem if a new kernel/KVM is used with an old QEMU. As old QEMU might not understand the new NMI exit type and treat it as a fatal error, even though the guest could have actually handled the error if the exception was delivered to guest's 0x200 interrupt vector.

PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS in KVM code, but still not reflected in QEMU code.

Quoted 4 lines

The commit message seems to be written as if this were the kernel patch adding the capability there, rather than the qemu patch using it.

I will reword it.

Regards,
Aravinda

Quoted 79 lines

Quoted 78 lines

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c       |   17 +++++++++++++++++
 linux-headers/linux/kvm.h |    1 +
 target-ppc/kvm.c          |    2 ++
 3 files changed, 20 insertions(+)

hw/ppc/spapr_rtas.c +17 -0

index 17c4672..53319da 100644
@@ -38,6 +38,8 @@
 #include <libfdt.h>
 #include "hw/ppc/spapr_drc.h"
 
+extern int cap_fwnmi;
+
 /* #define DEBUG_SPAPR */
 
 #ifdef DEBUG_SPAPR

@@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
                                   target_ulong args,
                                   uint32_t nret, target_ulong rets)
 {
+    int ret;
+    CPUState *cs = CPU(cpu);
+
+    if (!cap_fwnmi) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
     spapr->mc_in_progress = false;
     qemu_cond_init(&spapr->mc_delivery_cond);
     spapr->guest_machine_check_addr = rtas_ld(args, 1);
+
+    ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+    if (ret < 0) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
+
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 

linux-headers/linux/kvm.h +1 -0

index 683f713..2db1fba 100644
@@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_DISABLE_QUIRKS 116
 #define KVM_CAP_X86_SMM 117
 #define KVM_CAP_MULTI_ADDRESS_SPACE 118
+#define KVM_CAP_PPC_FWNMI 121
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

target-ppc/kvm.c +2 -0

index 2bbb46d..5339c04 100644
@@ -74,6 +74,7 @@ static int cap_ppc_watchdog;
 static int cap_papr;
 static int cap_htab_fd;
 static int cap_fixup_hcalls;
+int cap_fwnmi;
 
 static uint32_t debug_inst_opcode;
 

@@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
      * only activated after this by kvmppc_set_papr() */
     cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);
     cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL);
+    cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
 
     if (!cap_interrupt_level) {
         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "

Introduce a new KVM capability to control how KVM behaves on machine check exception.

Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.

This is required to avoid problem if a new kernel/KVM is used with an old QEMU. As old QEMU might not understand the new NMI exit type and treat it as a fatal error, even though the guest could have actually handled the error if the exception was delivered to guest's 0x200 interrupt vector.

PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS in KVM code, but still not reflected in QEMU code.

Signed-off-by Aravinda Prasad <[email protected]>

---
 hw/ppc/spapr_rtas.c       |   17 +++++++++++++++++
 linux-headers/linux/kvm.h |    1 +
 target-ppc/kvm.c          |    2 ++
 3 files changed, 20 insertions(+)

hw/ppc/spapr_rtas.c +17 -0

index 17c4672..53319da 100644
@@ -38,6 +38,8 @@
 #include <libfdt.h>
 #include "hw/ppc/spapr_drc.h"
 
+extern int cap_fwnmi;
+
 /* #define DEBUG_SPAPR */
 
 #ifdef DEBUG_SPAPR

@@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
                                   target_ulong args,
                                   uint32_t nret, target_ulong rets)
 {
+    int ret;
+    CPUState *cs = CPU(cpu);
+
+    if (!cap_fwnmi) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
     spapr->mc_in_progress = false;
     qemu_cond_init(&spapr->mc_delivery_cond);
     spapr->guest_machine_check_addr = rtas_ld(args, 1);
+
+    ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+    if (ret < 0) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
+
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 

linux-headers/linux/kvm.h +0 -0

index 683f713..2db1fba 100644