[PATCH 0/3] ppc: Add QOM interface for machine check injection

[PATCH 0/3] ppc: Add QOM interface for machine check injection

[Cédric Le Goater]

Hello,

This is a rework of Nick's patchset [*] adding mce injection for spapr
and pnv.  

Thanks,

C.

[*] https://lore.kernel.org/qemu-devel/20200325144147.221875-1-npiggin@gmail.com/

Changes :
 - moved definition under "hw/ppc/mce.h"
 - renamed to PPCMceInjection
 - simplified injection call in hmp_mce
 - extended with QMP support 
 - moved code under spapr_cpu_core.c pnv_core.c
 - removed superfluous cpu_synchronize_state()
 - clear previous setting in SPR_SRR1
 

Nicholas Piggin (3):
  ppc: Add QOM interface for machine check injection
  ppc/spapr: Implement mce injection
  ppc/pnv: Implement mce injection

 qapi/misc-target.json           | 26 +++++++++++++++
 include/hw/ppc/mce.h            | 31 ++++++++++++++++++
 include/hw/ppc/pnv_core.h       |  4 +++
 include/hw/ppc/spapr_cpu_core.h |  2 ++
 target/ppc/cpu.h                |  1 +
 hw/ppc/pnv.c                    |  3 ++
 hw/ppc/pnv_core.c               | 27 ++++++++++++++++
 hw/ppc/spapr.c                  |  4 +++
 hw/ppc/spapr_cpu_core.c         | 27 ++++++++++++++++
 target/ppc/excp_helper.c        | 12 +++++++
 target/ppc/monitor.c            | 56 +++++++++++++++++++++++++++++++++
 hmp-commands.hx                 | 20 +++++++++++-
 12 files changed, 212 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/ppc/mce.h

-- 
2.31.1

[PATCH 1/3] ppc: Add QOM interface for machine check injection

[Cédric Le Goater]

From: Nicholas Piggin <npiggin@gmail.com>

This implements a machine check injection framework and defines a
'mce' monitor command for ppc.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[ clg: - moved definition under "hw/ppc/mce.h"
       - renamed to PPCMceInjection
       - simplified injection call in hmp_mce
       - QMP support ]
Message-Id: <20200325144147.221875-4-npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 qapi/misc-target.json | 26 ++++++++++++++++++++
 include/hw/ppc/mce.h  | 31 ++++++++++++++++++++++++
 target/ppc/monitor.c  | 56 +++++++++++++++++++++++++++++++++++++++++++
 hmp-commands.hx       | 20 +++++++++++++++-
 4 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/ppc/mce.h

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 594fbd1577fa..b1456901893c 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -394,3 +394,29 @@
 #
 ##
 { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
+
+##
+# @mce:
+#
+# This command injects a machine check exception
+#
+# @cpu-index: CPU number on which to inject the machine check exception
+#
+# @srr1-mask : possible reasons for the exception
+#
+# @dsisr : more reasons
+#
+# @dar : effective address of next instruction
+#
+# @recovered : recoverable exception. Set MSR[RI]
+#
+# Since: 6.2
+#
+##
+{ 'command': 'mce',
+  'data': { 'cpu-index': 'uint32',
+            'srr1-mask': 'uint64',
+            'dsisr': 'uint32',
+            'dar': 'uint64',
+            'recovered': 'bool' },
+  'if': 'TARGET_PPC' }
diff --git a/include/hw/ppc/mce.h b/include/hw/ppc/mce.h
new file mode 100644
index 000000000000..b2b7dfa3342c
--- /dev/null
+++ b/include/hw/ppc/mce.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2021, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_PPC_MCE_H
+#define HW_PPC_MCE_H
+
+typedef struct PPCMceInjectionParams {
+    uint64_t srr1_mask;
+    uint32_t dsisr;
+    uint64_t dar;
+    bool recovered;
+} PPCMceInjectionParams;
+
+typedef struct PPCMceInjection PPCMceInjection;
+
+#define TYPE_PPC_MCE_INJECTION "ppc-mce-injection"
+#define PPC_MCE_INJECTION(obj) \
+    INTERFACE_CHECK(PPCMceInjection, (obj), TYPE_PPC_MCE_INJECTION)
+typedef struct PPCMceInjectionClass PPCMceInjectionClass;
+DECLARE_CLASS_CHECKERS(PPCMceInjectionClass, PPC_MCE_INJECTION,
+                       TYPE_PPC_MCE_INJECTION)
+
+struct PPCMceInjectionClass {
+    InterfaceClass parent_class;
+    void (*inject_mce)(CPUState *cs, PPCMceInjectionParams *p);
+};
+
+#endif
diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index a475108b2dbc..ae1a047e86de 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -23,11 +23,15 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-target.h"
 #include "cpu.h"
 #include "monitor/monitor.h"
 #include "qemu/ctype.h"
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
+#include "qapi/qmp/qdict.h"
+#include "hw/ppc/mce.h"
 
 static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
                                    int val)
@@ -76,6 +80,48 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
     dump_mmu(env1);
 }
 
+void qmp_mce(uint32_t cpu_index, uint64_t srr1_mask, uint32_t dsisr,
+             uint64_t dar, bool recovered, Error **errp)
+{
+    PPCMceInjection *mce = (PPCMceInjection *)
+        object_dynamic_cast(qdev_get_machine(), TYPE_PPC_MCE_INJECTION);
+    CPUState *cs;
+
+    if (!mce) {
+        error_setg(errp, "MCE injection not supported on this machine");
+        return;
+    }
+
+    cs = qemu_get_cpu(cpu_index);
+
+    if (cs != NULL) {
+        PPCMceInjectionClass *mcec = PPC_MCE_INJECTION_GET_CLASS(mce);
+        PPCMceInjectionParams p = {
+            .srr1_mask = srr1_mask,
+            .dsisr = dsisr,
+            .dar = dar,
+            .recovered = recovered,
+        };
+        mcec->inject_mce(cs, &p);
+    }
+}
+
+void hmp_mce(Monitor *mon, const QDict *qdict)
+{
+    uint32_t cpu_index = qdict_get_int(qdict, "cpu_index");
+    uint64_t srr1_mask = qdict_get_int(qdict, "srr1_mask");
+    uint32_t dsisr = qdict_get_int(qdict, "dsisr");
+    uint64_t dar = qdict_get_int(qdict, "dar");
+    bool recovered = qdict_get_int(qdict, "recovered");
+    Error *err = NULL;
+
+    qmp_mce(cpu_index, srr1_mask, dsisr, dar, recovered, &err);
+    if (err) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+}
+
 const MonitorDef monitor_defs[] = {
     { "fpscr", offsetof(CPUPPCState, fpscr) },
     /* Next instruction pointer */
@@ -156,3 +202,13 @@ int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
 
     return -EINVAL;
 }
+
+static const TypeInfo type_infos[] = {
+    {
+        .name = TYPE_PPC_MCE_INJECTION,
+        .parent = TYPE_INTERFACE,
+        .class_size = sizeof(PPCMceInjectionClass),
+    },
+};
+
+DEFINE_TYPES(type_infos);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index cf723c69acb7..15d939ae096e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1461,12 +1461,30 @@ ERST
         .cmd        = hmp_mce,
     },
 
-#endif
 SRST
 ``mce`` *cpu* *bank* *status* *mcgstatus* *addr* *misc*
   Inject an MCE on the given CPU (x86 only).
 ERST
 
+#endif
+
+#if defined(TARGET_PPC)
+
+    {
+        .name       = "mce",
+        .args_type  = "cpu_index:i,srr1_mask:l,dsisr:i,dar:l,recovered:i",
+        .params     = "cpu srr1_mask dsisr dar recovered",
+        .help       = "inject a MCE on the given CPU",
+        .cmd        = hmp_mce,
+    },
+
+SRST
+``mce`` *cpu* *srr1_mask* *dsisr* *dar* *recovered*
+  Inject an MCE on the given CPU (PPC only).
+ERST
+
+#endif
+
     {
         .name       = "getfd",
         .args_type  = "fdname:s",
-- 
2.31.1

[PATCH 2/3] ppc/spapr: Implement mce injection

[Cédric Le Goater]

From: Nicholas Piggin <npiggin@gmail.com>

This implements mce injection for spapr.

  (qemu) mce 0 0x200000 0x80 0xdeadbeef 1

    Disabling lock debugging due to kernel taint
    MCE: CPU0: machine check (Severe) Host SLB Multihit DAR: 00000000deadbeef [Recovered]
    MCE: CPU0: machine check (Severe) Host SLB Multihit [Recovered]
    MCE: CPU0: PID: 495 Comm: a NIP: [0000000130ee07c8]
    MCE: CPU0: Initiator CPU
    MCE: CPU0: Unknown
[   71.567193] MCE: CPU0: NIP: [c0000000000d7f6c] plpar_hcall_norets+0x1c/0x28
[   71.567249] MCE: CPU0: Initiator CPU
[   71.567308] MCE: CPU0: Unknown

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[ clg: - simplified injection and moved code under spapr_cpu_core.c ]
Message-Id: <20200325144147.221875-5-npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/spapr_cpu_core.h |  2 ++
 hw/ppc/spapr.c                  |  4 ++++
 hw/ppc/spapr_cpu_core.c         | 27 +++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
index dab3dfc76c0a..6734c7a89640 100644
--- a/include/hw/ppc/spapr_cpu_core.h
+++ b/include/hw/ppc/spapr_cpu_core.h
@@ -9,6 +9,7 @@
 #ifndef HW_SPAPR_CPU_CORE_H
 #define HW_SPAPR_CPU_CORE_H
 
+#include "hw/ppc/mce.h"
 #include "hw/cpu/core.h"
 #include "hw/qdev-core.h"
 #include "target/ppc/cpu-qom.h"
@@ -40,6 +41,7 @@ const char *spapr_get_cpu_core_type(const char *cpu_type);
 void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip,
                                target_ulong r1, target_ulong r3,
                                target_ulong r4);
+void spapr_cpu_inject_mce(CPUState *cs, PPCMceInjectionParams *p);
 
 typedef struct SpaprCpuState {
     uint64_t vpa_addr;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 087449f93871..c4ff63a79313 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -56,6 +56,7 @@
 #include "hw/core/cpu.h"
 
 #include "hw/ppc/ppc.h"
+#include "hw/ppc/mce.h"
 #include "hw/loader.h"
 
 #include "hw/ppc/fdt.h"
@@ -4522,6 +4523,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
     XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
     VofMachineIfClass *vmc = VOF_MACHINE_CLASS(oc);
+    PPCMceInjectionClass *mcec = PPC_MCE_INJECTION_CLASS(oc);
 
     mc->desc = "pSeries Logical Partition (PAPR compliant)";
     mc->ignore_boot_device_suffixes = true;
@@ -4615,6 +4617,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     vmc->client_architecture_support = spapr_vof_client_architecture_support;
     vmc->quiesce = spapr_vof_quiesce;
     vmc->setprop = spapr_vof_setprop;
+    mcec->inject_mce = spapr_cpu_inject_mce;
 }
 
 static const TypeInfo spapr_machine_info = {
@@ -4635,6 +4638,7 @@ static const TypeInfo spapr_machine_info = {
         { TYPE_INTERRUPT_STATS_PROVIDER },
         { TYPE_XIVE_FABRIC },
         { TYPE_VOF_MACHINE_IF },
+        { TYPE_PPC_MCE_INJECTION },
         { }
     },
 };
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 58e7341cb784..360efc16b1d6 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -9,6 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/cpu/core.h"
+#include "hw/core/cpu.h"
 #include "hw/ppc/spapr_cpu_core.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
@@ -19,12 +20,38 @@
 #include "sysemu/kvm.h"
 #include "target/ppc/kvm_ppc.h"
 #include "hw/ppc/ppc.h"
+#include "hw/ppc/mce.h"
 #include "target/ppc/mmu-hash64.h"
 #include "sysemu/numa.h"
 #include "sysemu/reset.h"
 #include "sysemu/hw_accel.h"
 #include "qemu/error-report.h"
 
+static void spapr_cpu_inject_mce_on_cpu(CPUState *cs, run_on_cpu_data data)
+{
+    PPCMceInjectionParams *params = (PPCMceInjectionParams *) data.host_ptr;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    uint64_t srr1_mce_bits = PPC_BITMASK(42, 45) | PPC_BIT(36);
+
+    cpu_synchronize_state(cs);
+
+    env->spr[SPR_SRR0] = env->nip;
+    env->spr[SPR_SRR1] = (env->msr & ~srr1_mce_bits) |
+                         (params->srr1_mask & srr1_mce_bits);
+    if (params->dsisr) {
+        env->spr[SPR_DSISR] = params->dsisr;
+        env->spr[SPR_DAR] = params->dar;
+    }
+
+    spapr_mce_req_event(cpu, params->recovered);
+}
+
+void spapr_cpu_inject_mce(CPUState *cs, PPCMceInjectionParams *p)
+{
+    run_on_cpu(cs, spapr_cpu_inject_mce_on_cpu, RUN_ON_CPU_HOST_PTR(p));
+}
+
 static void spapr_reset_vcpu(PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
-- 
2.31.1

[PATCH 3/3] ppc/pnv: Implement mce injection

[Cédric Le Goater]

From: Nicholas Piggin <npiggin@gmail.com>

This implements mce injection for pnv.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[ clg: - simplified injection and moved code under pnv_core.c
       - removed superfluous cpu_synchronize_state()
       - clear previous setting in SPR_SRR1 ]
Message-Id: <20200325144147.221875-6-npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv_core.h |  4 ++++
 target/ppc/cpu.h          |  1 +
 hw/ppc/pnv.c              |  3 +++
 hw/ppc/pnv_core.c         | 27 +++++++++++++++++++++++++++
 target/ppc/excp_helper.c  | 12 ++++++++++++
 5 files changed, 47 insertions(+)

diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index c22eab2e1f69..7ed7a52077ea 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -23,6 +23,7 @@
 #include "hw/cpu/core.h"
 #include "target/ppc/cpu.h"
 #include "qom/object.h"
+#include "hw/ppc/mce.h"
 
 #define TYPE_PNV_CORE "powernv-cpu-core"
 OBJECT_DECLARE_TYPE(PnvCore, PnvCoreClass,
@@ -70,4 +71,7 @@ struct PnvQuad {
     uint32_t quad_id;
     MemoryRegion xscom_regs;
 };
+
+void pnv_cpu_inject_mce(CPUState *cs, PPCMceInjectionParams *p);
+
 #endif /* PPC_PNV_CORE_H */
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index baa4e7c34d30..e0757e287718 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1273,6 +1273,7 @@ int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
 void ppc_cpu_do_interrupt(CPUState *cpu);
 bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req);
 void ppc_cpu_do_system_reset(CPUState *cs);
+void ppc_cpu_do_machine_check(CPUState *cs);
 void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector);
 extern const VMStateDescription vmstate_ppc_cpu;
 #endif
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 71e45515f136..374f48ea7f1b 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2001,6 +2001,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
     InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
+    PPCMceInjectionClass *mcec = PPC_MCE_INJECTION_CLASS(oc);
 
     mc->desc = "IBM PowerNV (Non-Virtualized)";
     mc->init = pnv_init;
@@ -2018,6 +2019,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
     mc->default_ram_id = "pnv.ram";
     ispc->print_info = pnv_pic_print_info;
     nc->nmi_monitor_handler = pnv_nmi;
+    mcec->inject_mce = pnv_cpu_inject_mce;
 
     object_class_property_add_bool(oc, "hb-mode",
                                    pnv_machine_get_hb, pnv_machine_set_hb);
@@ -2080,6 +2082,7 @@ static const TypeInfo types[] = {
         .interfaces = (InterfaceInfo[]) {
             { TYPE_INTERRUPT_STATS_PROVIDER },
             { TYPE_NMI },
+            { TYPE_PPC_MCE_INJECTION },
             { },
         },
     },
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 19e8eb885f71..868b361f99e5 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -25,12 +25,39 @@
 #include "target/ppc/cpu.h"
 #include "hw/ppc/ppc.h"
 #include "hw/ppc/pnv.h"
+#include "hw/ppc/mce.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_xscom.h"
 #include "hw/ppc/xics.h"
 #include "hw/qdev-properties.h"
 #include "helper_regs.h"
 
+static void pnv_cpu_inject_mce_on_cpu(CPUState *cs, run_on_cpu_data data)
+{
+    PPCMceInjectionParams *params = (PPCMceInjectionParams *) data.host_ptr;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    uint64_t srr1_mce_bits = PPC_BITMASK(42, 45) | PPC_BIT(36);
+
+    ppc_cpu_do_machine_check(cs);
+
+    env->spr[SPR_SRR1] = (env->msr & ~srr1_mce_bits) |
+                         (params->srr1_mask & srr1_mce_bits);
+    if (params->dsisr) {
+        env->spr[SPR_DSISR] = params->dsisr;
+        env->spr[SPR_DAR] = params->dar;
+    }
+
+    if (!params->recovered) {
+        env->msr &= ~MSR_RI;
+    }
+}
+
+void pnv_cpu_inject_mce(CPUState *cs, PPCMceInjectionParams *p)
+{
+    run_on_cpu(cs, pnv_cpu_inject_mce_on_cpu, RUN_ON_CPU_HOST_PTR(p));
+}
+
 static const char *pnv_core_cpu_typename(PnvCore *pc)
 {
     const char *core_type = object_class_get_name(object_get_class(OBJECT(pc)));
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b7d176792098..f383f1646cc3 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -379,6 +379,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
             cs->halted = 1;
             cpu_interrupt_exittb(cs);
         }
+        if (msr_pow) {
+            /* indicate that we resumed from power save mode */
+            msr |= 0x10000;
+        }
         if (env->msr_mask & MSR_HVB) {
             /*
              * ISA specifies HV, but can be delivered to guest with HV
@@ -1071,6 +1075,14 @@ void ppc_cpu_do_system_reset(CPUState *cs)
     powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
 }
 
+void ppc_cpu_do_machine_check(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_MCHECK);
+}
+
 void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
-- 
2.31.1

Re: [PATCH 1/3] ppc: Add QOM interface for machine check injection

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of October 14, 2021 7:40 am:
> From: Nicholas Piggin <npiggin@gmail.com>
> 
> This implements a machine check injection framework and defines a
> 'mce' monitor command for ppc.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> [ clg: - moved definition under "hw/ppc/mce.h"
>        - renamed to PPCMceInjection
>        - simplified injection call in hmp_mce
>        - QMP support ]

Nice, thanks for doing this.

> Message-Id: <20200325144147.221875-4-npiggin@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  qapi/misc-target.json | 26 ++++++++++++++++++++
>  include/hw/ppc/mce.h  | 31 ++++++++++++++++++++++++
>  target/ppc/monitor.c  | 56 +++++++++++++++++++++++++++++++++++++++++++
>  hmp-commands.hx       | 20 +++++++++++++++-
>  4 files changed, 132 insertions(+), 1 deletion(-)
>  create mode 100644 include/hw/ppc/mce.h
> 
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 594fbd1577fa..b1456901893c 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -394,3 +394,29 @@
>  #
>  ##
>  { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> +
> +##
> +# @mce:
> +#
> +# This command injects a machine check exception
> +#
> +# @cpu-index: CPU number on which to inject the machine check exception
> +#
> +# @srr1-mask : possible reasons for the exception

I would say this is implementation specific mask of bits that are 
inserted in the SRR1 register at interrupt-time (except RI - see 
@recovered) which indicate the cause of the exception.

These are not architected and may change from CPU to CPU. I.e., the
mask itself may change, not just the reasons.

> +#
> +# @dsisr : more reasons

This is value inserted into DSISR register, and is typically used
to indicate the cause of a "d-side" MCE. If this is 0 then both
DSISR and DAR registers are left unchanged.

> +#
> +# @dar : effective address of next instruction

This is the value inserted into the DAR register (if @dsisr was 
non-zero). It is implementation specific but is typically used
to indicate the effective address of the target address involved
in the mce for d-side exceptions.

I wonder if we should put an @asdr parameter there too -- I'm not
acutally sure if P10 implements that (getting clarification) but
the architecture at least allows it.

What's the go for updating this API? Can we just break it, or do
we need to version it or call a different name like mce2 etc if
we want to change it?

Thanks,
Nick

Re: [PATCH 1/3] ppc: Add QOM interface for machine check injection

[Cédric Le Goater]

On 10/13/21 23:40, Cédric Le Goater wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
> 
> This implements a machine check injection framework and defines a
> 'mce' monitor command for ppc.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> [ clg: - moved definition under "hw/ppc/mce.h"
>         - renamed to PPCMceInjection
>         - simplified injection call in hmp_mce
>         - QMP support ]
> Message-Id: <20200325144147.221875-4-npiggin@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>


I did not initially copy the QAPI and HMP maintainers :/

One important question is about the stability of the API. MCE is
implementation specific and may change with CPUs. How much of it
can we change once it is merged ? May be this is not the right
approach.


Thanks,

C.

> ---
>   qapi/misc-target.json | 26 ++++++++++++++++++++
>   include/hw/ppc/mce.h  | 31 ++++++++++++++++++++++++
>   target/ppc/monitor.c  | 56 +++++++++++++++++++++++++++++++++++++++++++
>   hmp-commands.hx       | 20 +++++++++++++++-
>   4 files changed, 132 insertions(+), 1 deletion(-)
>   create mode 100644 include/hw/ppc/mce.h
> 
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 594fbd1577fa..b1456901893c 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -394,3 +394,29 @@
>   #
>   ##
>   { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> +
> +##
> +# @mce:
> +#
> +# This command injects a machine check exception
> +#
> +# @cpu-index: CPU number on which to inject the machine check exception
> +#
> +# @srr1-mask : possible reasons for the exception
> +#
> +# @dsisr : more reasons
> +#
> +# @dar : effective address of next instruction
> +#
> +# @recovered : recoverable exception. Set MSR[RI]
> +#
> +# Since: 6.2
> +#
> +##
> +{ 'command': 'mce',
> +  'data': { 'cpu-index': 'uint32',
> +            'srr1-mask': 'uint64',
> +            'dsisr': 'uint32',
> +            'dar': 'uint64',
> +            'recovered': 'bool' },
> +  'if': 'TARGET_PPC' }
> diff --git a/include/hw/ppc/mce.h b/include/hw/ppc/mce.h
> new file mode 100644
> index 000000000000..b2b7dfa3342c
> --- /dev/null
> +++ b/include/hw/ppc/mce.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (c) 2021, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_PPC_MCE_H
> +#define HW_PPC_MCE_H
> +
> +typedef struct PPCMceInjectionParams {
> +    uint64_t srr1_mask;
> +    uint32_t dsisr;
> +    uint64_t dar;
> +    bool recovered;
> +} PPCMceInjectionParams;
> +
> +typedef struct PPCMceInjection PPCMceInjection;
> +
> +#define TYPE_PPC_MCE_INJECTION "ppc-mce-injection"
> +#define PPC_MCE_INJECTION(obj) \
> +    INTERFACE_CHECK(PPCMceInjection, (obj), TYPE_PPC_MCE_INJECTION)
> +typedef struct PPCMceInjectionClass PPCMceInjectionClass;
> +DECLARE_CLASS_CHECKERS(PPCMceInjectionClass, PPC_MCE_INJECTION,
> +                       TYPE_PPC_MCE_INJECTION)
> +
> +struct PPCMceInjectionClass {
> +    InterfaceClass parent_class;
> +    void (*inject_mce)(CPUState *cs, PPCMceInjectionParams *p);
> +};
> +
> +#endif
> diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
> index a475108b2dbc..ae1a047e86de 100644
> --- a/target/ppc/monitor.c
> +++ b/target/ppc/monitor.c
> @@ -23,11 +23,15 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-misc-target.h"
>   #include "cpu.h"
>   #include "monitor/monitor.h"
>   #include "qemu/ctype.h"
>   #include "monitor/hmp-target.h"
>   #include "monitor/hmp.h"
> +#include "qapi/qmp/qdict.h"
> +#include "hw/ppc/mce.h"
>   
>   static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
>                                      int val)
> @@ -76,6 +80,48 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>       dump_mmu(env1);
>   }
>   
> +void qmp_mce(uint32_t cpu_index, uint64_t srr1_mask, uint32_t dsisr,
> +             uint64_t dar, bool recovered, Error **errp)
> +{
> +    PPCMceInjection *mce = (PPCMceInjection *)
> +        object_dynamic_cast(qdev_get_machine(), TYPE_PPC_MCE_INJECTION);
> +    CPUState *cs;
> +
> +    if (!mce) {
> +        error_setg(errp, "MCE injection not supported on this machine");
> +        return;
> +    }
> +
> +    cs = qemu_get_cpu(cpu_index);
> +
> +    if (cs != NULL) {
> +        PPCMceInjectionClass *mcec = PPC_MCE_INJECTION_GET_CLASS(mce);
> +        PPCMceInjectionParams p = {
> +            .srr1_mask = srr1_mask,
> +            .dsisr = dsisr,
> +            .dar = dar,
> +            .recovered = recovered,
> +        };
> +        mcec->inject_mce(cs, &p);
> +    }
> +}
> +
> +void hmp_mce(Monitor *mon, const QDict *qdict)
> +{
> +    uint32_t cpu_index = qdict_get_int(qdict, "cpu_index");
> +    uint64_t srr1_mask = qdict_get_int(qdict, "srr1_mask");
> +    uint32_t dsisr = qdict_get_int(qdict, "dsisr");
> +    uint64_t dar = qdict_get_int(qdict, "dar");
> +    bool recovered = qdict_get_int(qdict, "recovered");
> +    Error *err = NULL;
> +
> +    qmp_mce(cpu_index, srr1_mask, dsisr, dar, recovered, &err);
> +    if (err) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +}
> +
>   const MonitorDef monitor_defs[] = {
>       { "fpscr", offsetof(CPUPPCState, fpscr) },
>       /* Next instruction pointer */
> @@ -156,3 +202,13 @@ int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
>   
>       return -EINVAL;
>   }
> +
> +static const TypeInfo type_infos[] = {
> +    {
> +        .name = TYPE_PPC_MCE_INJECTION,
> +        .parent = TYPE_INTERFACE,
> +        .class_size = sizeof(PPCMceInjectionClass),
> +    },
> +};
> +
> +DEFINE_TYPES(type_infos);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index cf723c69acb7..15d939ae096e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1461,12 +1461,30 @@ ERST
>           .cmd        = hmp_mce,
>       },
>   
> -#endif
>   SRST
>   ``mce`` *cpu* *bank* *status* *mcgstatus* *addr* *misc*
>     Inject an MCE on the given CPU (x86 only).
>   ERST
>   
> +#endif
> +
> +#if defined(TARGET_PPC)
> +
> +    {
> +        .name       = "mce",
> +        .args_type  = "cpu_index:i,srr1_mask:l,dsisr:i,dar:l,recovered:i",
> +        .params     = "cpu srr1_mask dsisr dar recovered",
> +        .help       = "inject a MCE on the given CPU",
> +        .cmd        = hmp_mce,
> +    },
> +
> +SRST
> +``mce`` *cpu* *srr1_mask* *dsisr* *dar* *recovered*
> +  Inject an MCE on the given CPU (PPC only).
> +ERST
> +
> +#endif
> +
>       {
>           .name       = "getfd",
>           .args_type  = "fdname:s",
> 

Re: [PATCH 1/3] ppc: Add QOM interface for machine check injection

[Cédric Le Goater]

[ Adding David who I think is the HMP maintainer and Fabiano who has
   been rewriting a lot of the PPC exception model ]

On 10/15/21 04:05, Nicholas Piggin wrote:
> Excerpts from Cédric Le Goater's message of October 14, 2021 7:40 am:
>> From: Nicholas Piggin <npiggin@gmail.com>
>>
>> This implements a machine check injection framework and defines a
>> 'mce' monitor command for ppc.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> [ clg: - moved definition under "hw/ppc/mce.h"
>>         - renamed to PPCMceInjection
>>         - simplified injection call in hmp_mce
>>         - QMP support ]
> 
> Nice, thanks for doing this.
>
>> Message-Id: <20200325144147.221875-4-npiggin@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   qapi/misc-target.json | 26 ++++++++++++++++++++
>>   include/hw/ppc/mce.h  | 31 ++++++++++++++++++++++++
>>   target/ppc/monitor.c  | 56 +++++++++++++++++++++++++++++++++++++++++++
>>   hmp-commands.hx       | 20 +++++++++++++++-
>>   4 files changed, 132 insertions(+), 1 deletion(-)
>>   create mode 100644 include/hw/ppc/mce.h
>>
>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> index 594fbd1577fa..b1456901893c 100644
>> --- a/qapi/misc-target.json
>> +++ b/qapi/misc-target.json
>> @@ -394,3 +394,29 @@
>>   #
>>   ##
>>   { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
>> +
>> +##
>> +# @mce:
>> +#
>> +# This command injects a machine check exception
>> +#
>> +# @cpu-index: CPU number on which to inject the machine check exception
>> +#
>> +# @srr1-mask : possible reasons for the exception
> 
> I would say this is implementation specific mask of bits that are
> inserted in the SRR1 register at interrupt-time (except RI - see
> @recovered) which indicate the cause of the exception.
> 
> These are not architected and may change from CPU to CPU. I.e., the
> mask itself may change, not just the reasons.
> 
>> +#
>> +# @dsisr : more reasons
> 
> This is value inserted into DSISR register, and is typically used
> to indicate the cause of a "d-side" MCE. If this is 0 then both
> DSISR and DAR registers are left unchanged.
> 
>> +#
>> +# @dar : effective address of next instruction
> 
> This is the value inserted into the DAR register (if @dsisr was
> non-zero). It is implementation specific but is typically used
> to indicate the effective address of the target address involved
> in the mce for d-side exceptions.
> 
> I wonder if we should put an @asdr parameter there too -- I'm not
> acutally sure if P10 implements that (getting clarification) but
> the architecture at least allows it.
> 
> What's the go for updating this API? Can we just break it, or do
> we need to version it or call a different name like mce2 etc if
> we want to change it?

I am not sure what the answer would be. May be David can tell ?

Thanks,

C.