diff mbox series

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

Message ID 20211013214042.618918-2-clg@kaod.org
State New
Headers show
Series ppc: Add QOM interface for machine check injection | expand

Commit Message

Cédric Le Goater Oct. 13, 2021, 9:40 p.m. UTC 
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

Comments

Nicholas Piggin Oct. 15, 2021, 2:05 a.m. UTC | #1 
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
Cédric Le Goater Dec. 16, 2021, 5:30 p.m. UTC | #2 
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",
>
Cédric Le Goater Feb. 8, 2022, 3:55 p.m. UTC | #3 
[ 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.
diff mbox series

Patch

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",