diff mbox series

[3/5] nmi: add MCE class for implementing machine check injection commands

Message ID 20200325144147.221875-4-npiggin@gmail.com
State New
Headers show
Series ppc: sreset and machine check injection | expand

Commit Message

Nicholas Piggin March 25, 2020, 2:41 p.m. UTC
Like commit 9cb805fd26 ("cpus: Define callback for QEMU "nmi" command")
this implements a machine check injection command framework and defines
a monitor command for ppc.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hmp-commands.hx              | 20 +++++++++++-
 hw/core/nmi.c                | 61 ++++++++++++++++++++++++++++++++++++
 include/hw/nmi.h             | 20 ++++++++++++
 include/monitor/hmp-target.h |  1 -
 include/monitor/hmp.h        |  1 +
 monitor/hmp-cmds.c           |  1 +
 target/ppc/monitor.c         | 11 +++++++
 7 files changed, 113 insertions(+), 2 deletions(-)

Comments

Cédric Le Goater March 25, 2020, 4:46 p.m. UTC | #1
On 3/25/20 3:41 PM, Nicholas Piggin wrote:
> Like commit 9cb805fd26 ("cpus: Define callback for QEMU "nmi" command")
> this implements a machine check injection command framework and defines
> a monitor command for ppc.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Looks good to me,

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C. 

> ---
>  hmp-commands.hx              | 20 +++++++++++-
>  hw/core/nmi.c                | 61 ++++++++++++++++++++++++++++++++++++
>  include/hw/nmi.h             | 20 ++++++++++++
>  include/monitor/hmp-target.h |  1 -
>  include/monitor/hmp.h        |  1 +
>  monitor/hmp-cmds.c           |  1 +
>  target/ppc/monitor.c         | 11 +++++++
>  7 files changed, 113 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 7f0f3974ad..4a9089b431 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1581,12 +1581,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",
> diff --git a/hw/core/nmi.c b/hw/core/nmi.c
> index 481c4b3c7e..2a79500967 100644
> --- a/hw/core/nmi.c
> +++ b/hw/core/nmi.c
> @@ -86,3 +86,64 @@ static void nmi_register_types(void)
>  }
> 
>  type_init(nmi_register_types)
> +
> +struct do_mce_s {
> +    const QDict *qdict;
> +    Error *err;
> +    bool handled;
> +};
> +
> +static void mce_children(Object *o, struct do_mce_s *ns);
> +
> +static int do_mce(Object *o, void *opaque)
> +{
> +    struct do_mce_s *ms = opaque;
> +    MCEState *m = (MCEState *) object_dynamic_cast(o, TYPE_MCE);
> +
> +    if (m) {
> +        MCEClass *mc = MCE_GET_CLASS(m);
> +
> +        ms->handled = true;
> +        mc->mce_monitor_handler(m, ms->qdict, &ms->err);
> +        if (ms->err) {
> +            return -1;
> +        }
> +    }
> +    mce_children(o, ms);
> +
> +    return 0;
> +}
> +
> +static void mce_children(Object *o, struct do_mce_s *ms)
> +{
> +    object_child_foreach(o, do_mce, ms);
> +}
> +
> +void mce_monitor_handle(const QDict *qdict, Error **errp)
> +{
> +    struct do_mce_s ms = {
> +        .qdict = qdict,
> +        .err = NULL,
> +        .handled = false
> +    };
> +
> +    mce_children(object_get_root(), &ms);
> +    if (ms.handled) {
> +        error_propagate(errp, ms.err);
> +    } else {
> +        error_setg(errp, QERR_UNSUPPORTED);
> +    }
> +}
> +
> +static const TypeInfo mce_info = {
> +    .name          = TYPE_MCE,
> +    .parent        = TYPE_INTERFACE,
> +    .class_size    = sizeof(MCEClass),
> +};
> +
> +static void mce_register_types(void)
> +{
> +    type_register_static(&mce_info);
> +}
> +
> +type_init(mce_register_types)
> diff --git a/include/hw/nmi.h b/include/hw/nmi.h
> index fe37ce3ad8..de39d95c9a 100644
> --- a/include/hw/nmi.h
> +++ b/include/hw/nmi.h
> @@ -43,4 +43,24 @@ typedef struct NMIClass {
> 
>  void nmi_monitor_handle(int cpu_index, Error **errp);
> 
> +
> +#define TYPE_MCE "mce"
> +
> +#define MCE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(MCEClass, (klass), TYPE_MCE)
> +#define MCE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(MCEClass, (obj), TYPE_MCE)
> +#define MCE(obj) \
> +     INTERFACE_CHECK(MCEState, (obj), TYPE_MCE)
> +
> +typedef struct MCEState MCEState;
> +
> +typedef struct MCEClass {
> +    InterfaceClass parent_class;
> +
> +    void (*mce_monitor_handler)(MCEState *n, const QDict *qdict, Error **errp);
> +} MCEClass;
> +
> +void mce_monitor_handle(const QDict *qdict, Error **errp);
> +
>  #endif /* NMI_H */
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 8b7820a3ad..afb8f5bca2 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -45,7 +45,6 @@ CPUState *mon_get_cpu(void);
> 
>  void hmp_info_mem(Monitor *mon, const QDict *qdict);
>  void hmp_info_tlb(Monitor *mon, const QDict *qdict);
> -void hmp_mce(Monitor *mon, const QDict *qdict);
>  void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
>  void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
> 
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index e33ca5a911..f747a5e214 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -54,6 +54,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
>  void hmp_cont(Monitor *mon, const QDict *qdict);
>  void hmp_system_wakeup(Monitor *mon, const QDict *qdict);
>  void hmp_nmi(Monitor *mon, const QDict *qdict);
> +void hmp_mce(Monitor *mon, const QDict *qdict);
>  void hmp_set_link(Monitor *mon, const QDict *qdict);
>  void hmp_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_loadvm(Monitor *mon, const QDict *qdict);
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 58724031ea..3664ef2a4f 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -52,6 +52,7 @@
>  #include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
>  #include "hw/rdma/rdma.h"
> +#include "hw/nmi.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
> 
> diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
> index a5a177d717..6daf543efc 100644
> --- a/target/ppc/monitor.c
> +++ b/target/ppc/monitor.c
> @@ -28,6 +28,8 @@
>  #include "qemu/ctype.h"
>  #include "monitor/hmp-target.h"
>  #include "monitor/hmp.h"
> +#include "qapi/qmp/qdict.h"
> +#include "hw/nmi.h"
> 
>  static target_long monitor_get_ccr(const struct MonitorDef *md, int val)
>  {
> @@ -72,6 +74,15 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>      dump_mmu(env1);
>  }
> 
> +void hmp_mce(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +
> +    mce_monitor_handle(qdict, &err);
> +
> +    hmp_handle_error(mon, err);
> +}
> +
>  const MonitorDef monitor_defs[] = {
>      { "fpscr", offsetof(CPUPPCState, fpscr) },
>      /* Next instruction pointer */
>
David Gibson March 31, 2020, 12:22 a.m. UTC | #2
On Thu, Mar 26, 2020 at 12:41:45AM +1000, Nicholas Piggin wrote:
> Like commit 9cb805fd26 ("cpus: Define callback for QEMU "nmi" command")
> this implements a machine check injection command framework and defines
> a monitor command for ppc.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

So, AFAICT, both x86 and ppc have something called an MCE, and while
I'm guessing they're somewhat related, they don't work quite the same
way - different information provided to the handler and so forth.

I think it's reasonable to overload the "mce" HMP command based on
target for the different types.  However, I think calling the
interface classes which are specific to the ppc type just "mce" could
be pretty confusing.

In addition, I think this is adding an HMP command to inject the event
without any corresponding way of injecting via QMP.  I believe that's
frowned upon.

> ---
>  hmp-commands.hx              | 20 +++++++++++-
>  hw/core/nmi.c                | 61 ++++++++++++++++++++++++++++++++++++
>  include/hw/nmi.h             | 20 ++++++++++++
>  include/monitor/hmp-target.h |  1 -
>  include/monitor/hmp.h        |  1 +
>  monitor/hmp-cmds.c           |  1 +
>  target/ppc/monitor.c         | 11 +++++++
>  7 files changed, 113 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 7f0f3974ad..4a9089b431 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1581,12 +1581,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",
> diff --git a/hw/core/nmi.c b/hw/core/nmi.c
> index 481c4b3c7e..2a79500967 100644
> --- a/hw/core/nmi.c
> +++ b/hw/core/nmi.c
> @@ -86,3 +86,64 @@ static void nmi_register_types(void)
>  }
>  
>  type_init(nmi_register_types)
> +
> +struct do_mce_s {
> +    const QDict *qdict;
> +    Error *err;
> +    bool handled;
> +};
> +
> +static void mce_children(Object *o, struct do_mce_s *ns);
> +
> +static int do_mce(Object *o, void *opaque)
> +{
> +    struct do_mce_s *ms = opaque;
> +    MCEState *m = (MCEState *) object_dynamic_cast(o, TYPE_MCE);
> +
> +    if (m) {
> +        MCEClass *mc = MCE_GET_CLASS(m);
> +
> +        ms->handled = true;
> +        mc->mce_monitor_handler(m, ms->qdict, &ms->err);
> +        if (ms->err) {
> +            return -1;
> +        }
> +    }
> +    mce_children(o, ms);
> +
> +    return 0;
> +}
> +
> +static void mce_children(Object *o, struct do_mce_s *ms)
> +{
> +    object_child_foreach(o, do_mce, ms);
> +}
> +
> +void mce_monitor_handle(const QDict *qdict, Error **errp)
> +{
> +    struct do_mce_s ms = {
> +        .qdict = qdict,
> +        .err = NULL,
> +        .handled = false
> +    };
> +
> +    mce_children(object_get_root(), &ms);
> +    if (ms.handled) {
> +        error_propagate(errp, ms.err);
> +    } else {
> +        error_setg(errp, QERR_UNSUPPORTED);
> +    }
> +}
> +
> +static const TypeInfo mce_info = {
> +    .name          = TYPE_MCE,
> +    .parent        = TYPE_INTERFACE,
> +    .class_size    = sizeof(MCEClass),
> +};
> +
> +static void mce_register_types(void)
> +{
> +    type_register_static(&mce_info);
> +}
> +
> +type_init(mce_register_types)
> diff --git a/include/hw/nmi.h b/include/hw/nmi.h
> index fe37ce3ad8..de39d95c9a 100644
> --- a/include/hw/nmi.h
> +++ b/include/hw/nmi.h
> @@ -43,4 +43,24 @@ typedef struct NMIClass {
>  
>  void nmi_monitor_handle(int cpu_index, Error **errp);
>  
> +
> +#define TYPE_MCE "mce"
> +
> +#define MCE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(MCEClass, (klass), TYPE_MCE)
> +#define MCE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(MCEClass, (obj), TYPE_MCE)
> +#define MCE(obj) \
> +     INTERFACE_CHECK(MCEState, (obj), TYPE_MCE)
> +
> +typedef struct MCEState MCEState;
> +
> +typedef struct MCEClass {
> +    InterfaceClass parent_class;
> +
> +    void (*mce_monitor_handler)(MCEState *n, const QDict *qdict, Error **errp);
> +} MCEClass;
> +
> +void mce_monitor_handle(const QDict *qdict, Error **errp);
> +
>  #endif /* NMI_H */
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 8b7820a3ad..afb8f5bca2 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -45,7 +45,6 @@ CPUState *mon_get_cpu(void);
>  
>  void hmp_info_mem(Monitor *mon, const QDict *qdict);
>  void hmp_info_tlb(Monitor *mon, const QDict *qdict);
> -void hmp_mce(Monitor *mon, const QDict *qdict);
>  void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
>  void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
>  
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index e33ca5a911..f747a5e214 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -54,6 +54,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
>  void hmp_cont(Monitor *mon, const QDict *qdict);
>  void hmp_system_wakeup(Monitor *mon, const QDict *qdict);
>  void hmp_nmi(Monitor *mon, const QDict *qdict);
> +void hmp_mce(Monitor *mon, const QDict *qdict);
>  void hmp_set_link(Monitor *mon, const QDict *qdict);
>  void hmp_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_loadvm(Monitor *mon, const QDict *qdict);
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 58724031ea..3664ef2a4f 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -52,6 +52,7 @@
>  #include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
>  #include "hw/rdma/rdma.h"
> +#include "hw/nmi.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
>  
> diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
> index a5a177d717..6daf543efc 100644
> --- a/target/ppc/monitor.c
> +++ b/target/ppc/monitor.c
> @@ -28,6 +28,8 @@
>  #include "qemu/ctype.h"
>  #include "monitor/hmp-target.h"
>  #include "monitor/hmp.h"
> +#include "qapi/qmp/qdict.h"
> +#include "hw/nmi.h"
>  
>  static target_long monitor_get_ccr(const struct MonitorDef *md, int val)
>  {
> @@ -72,6 +74,15 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>      dump_mmu(env1);
>  }
>  
> +void hmp_mce(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +
> +    mce_monitor_handle(qdict, &err);
> +
> +    hmp_handle_error(mon, err);
> +}
> +
>  const MonitorDef monitor_defs[] = {
>      { "fpscr", offsetof(CPUPPCState, fpscr) },
>      /* Next instruction pointer */
Nicholas Piggin April 3, 2020, 8:04 a.m. UTC | #3
David Gibson's on March 31, 2020 10:22 am:
> On Thu, Mar 26, 2020 at 12:41:45AM +1000, Nicholas Piggin wrote:
>> Like commit 9cb805fd26 ("cpus: Define callback for QEMU "nmi" command")
>> this implements a machine check injection command framework and defines
>> a monitor command for ppc.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> So, AFAICT, both x86 and ppc have something called an MCE, and while
> I'm guessing they're somewhat related, they don't work quite the same
> way - different information provided to the handler and so forth.
> 
> I think it's reasonable to overload the "mce" HMP command based on
> target for the different types.  However, I think calling the
> interface classes which are specific to the ppc type just "mce" could
> be pretty confusing.

Okay. So, convert i386 first?

> In addition, I think this is adding an HMP command to inject the event
> without any corresponding way of injecting via QMP.  I believe that's
> frowned upon.

I attempted that but didn't get too far. I guess it's more of a
special test than a management function (nmi has valid uses in 
administering a machine), so maybe we can get an exemption. One issue
is different QMP command for powerpc vs x86.

I think error injection as a general concept might be valid there, but
the better interface for that level would be higher up, e.g, not
specifying register settings but rather "simulate uncorrected memory
error on this byte".

Do you think that is reasonable reason to avoid adding QMP for this
nasty low level thing?

Thanks,
Nick
David Gibson April 6, 2020, 6:45 a.m. UTC | #4
On Fri, Apr 03, 2020 at 06:04:47PM +1000, Nicholas Piggin wrote:
> David Gibson's on March 31, 2020 10:22 am:
> > On Thu, Mar 26, 2020 at 12:41:45AM +1000, Nicholas Piggin wrote:
> >> Like commit 9cb805fd26 ("cpus: Define callback for QEMU "nmi" command")
> >> this implements a machine check injection command framework and defines
> >> a monitor command for ppc.
> >> 
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > 
> > So, AFAICT, both x86 and ppc have something called an MCE, and while
> > I'm guessing they're somewhat related, they don't work quite the same
> > way - different information provided to the handler and so forth.
> > 
> > I think it's reasonable to overload the "mce" HMP command based on
> > target for the different types.  However, I think calling the
> > interface classes which are specific to the ppc type just "mce" could
> > be pretty confusing.
> 
> Okay. So, convert i386 first?

Uh.. not necessarily.  But call your version PpcMce or something
instead.

> > In addition, I think this is adding an HMP command to inject the event
> > without any corresponding way of injecting via QMP.  I believe that's
> > frowned upon.
> 
> I attempted that but didn't get too far. I guess it's more of a
> special test than a management function (nmi has valid uses in 
> administering a machine), so maybe we can get an exemption. One issue
> is different QMP command for powerpc vs x86.
> 
> I think error injection as a general concept might be valid there, but
> the better interface for that level would be higher up, e.g, not
> specifying register settings but rather "simulate uncorrected memory
> error on this byte".
> 
> Do you think that is reasonable reason to avoid adding QMP for this
> nasty low level thing?

Hrm, I doubt it will convince the qemu community at large.  I think
the view is that QMP is the "real" management interface for qemu - HMP
is somewhere in between a shim for convenience and something needed
for backwards compat.
diff mbox series

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7f0f3974ad..4a9089b431 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1581,12 +1581,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",
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 481c4b3c7e..2a79500967 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -86,3 +86,64 @@  static void nmi_register_types(void)
 }
 
 type_init(nmi_register_types)
+
+struct do_mce_s {
+    const QDict *qdict;
+    Error *err;
+    bool handled;
+};
+
+static void mce_children(Object *o, struct do_mce_s *ns);
+
+static int do_mce(Object *o, void *opaque)
+{
+    struct do_mce_s *ms = opaque;
+    MCEState *m = (MCEState *) object_dynamic_cast(o, TYPE_MCE);
+
+    if (m) {
+        MCEClass *mc = MCE_GET_CLASS(m);
+
+        ms->handled = true;
+        mc->mce_monitor_handler(m, ms->qdict, &ms->err);
+        if (ms->err) {
+            return -1;
+        }
+    }
+    mce_children(o, ms);
+
+    return 0;
+}
+
+static void mce_children(Object *o, struct do_mce_s *ms)
+{
+    object_child_foreach(o, do_mce, ms);
+}
+
+void mce_monitor_handle(const QDict *qdict, Error **errp)
+{
+    struct do_mce_s ms = {
+        .qdict = qdict,
+        .err = NULL,
+        .handled = false
+    };
+
+    mce_children(object_get_root(), &ms);
+    if (ms.handled) {
+        error_propagate(errp, ms.err);
+    } else {
+        error_setg(errp, QERR_UNSUPPORTED);
+    }
+}
+
+static const TypeInfo mce_info = {
+    .name          = TYPE_MCE,
+    .parent        = TYPE_INTERFACE,
+    .class_size    = sizeof(MCEClass),
+};
+
+static void mce_register_types(void)
+{
+    type_register_static(&mce_info);
+}
+
+type_init(mce_register_types)
diff --git a/include/hw/nmi.h b/include/hw/nmi.h
index fe37ce3ad8..de39d95c9a 100644
--- a/include/hw/nmi.h
+++ b/include/hw/nmi.h
@@ -43,4 +43,24 @@  typedef struct NMIClass {
 
 void nmi_monitor_handle(int cpu_index, Error **errp);
 
+
+#define TYPE_MCE "mce"
+
+#define MCE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(MCEClass, (klass), TYPE_MCE)
+#define MCE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(MCEClass, (obj), TYPE_MCE)
+#define MCE(obj) \
+     INTERFACE_CHECK(MCEState, (obj), TYPE_MCE)
+
+typedef struct MCEState MCEState;
+
+typedef struct MCEClass {
+    InterfaceClass parent_class;
+
+    void (*mce_monitor_handler)(MCEState *n, const QDict *qdict, Error **errp);
+} MCEClass;
+
+void mce_monitor_handle(const QDict *qdict, Error **errp);
+
 #endif /* NMI_H */
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index 8b7820a3ad..afb8f5bca2 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -45,7 +45,6 @@  CPUState *mon_get_cpu(void);
 
 void hmp_info_mem(Monitor *mon, const QDict *qdict);
 void hmp_info_tlb(Monitor *mon, const QDict *qdict);
-void hmp_mce(Monitor *mon, const QDict *qdict);
 void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
 void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
 
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index e33ca5a911..f747a5e214 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -54,6 +54,7 @@  void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
 void hmp_cont(Monitor *mon, const QDict *qdict);
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict);
 void hmp_nmi(Monitor *mon, const QDict *qdict);
+void hmp_mce(Monitor *mon, const QDict *qdict);
 void hmp_set_link(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_loadvm(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 58724031ea..3664ef2a4f 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -52,6 +52,7 @@ 
 #include "exec/ramlist.h"
 #include "hw/intc/intc.h"
 #include "hw/rdma/rdma.h"
+#include "hw/nmi.h"
 #include "migration/snapshot.h"
 #include "migration/misc.h"
 
diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index a5a177d717..6daf543efc 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -28,6 +28,8 @@ 
 #include "qemu/ctype.h"
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
+#include "qapi/qmp/qdict.h"
+#include "hw/nmi.h"
 
 static target_long monitor_get_ccr(const struct MonitorDef *md, int val)
 {
@@ -72,6 +74,15 @@  void hmp_info_tlb(Monitor *mon, const QDict *qdict)
     dump_mmu(env1);
 }
 
+void hmp_mce(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    mce_monitor_handle(qdict, &err);
+
+    hmp_handle_error(mon, err);
+}
+
 const MonitorDef monitor_defs[] = {
     { "fpscr", offsetof(CPUPPCState, fpscr) },
     /* Next instruction pointer */