On Thu, 12 Jun 2014 03:03:01 +1000 Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> This implements an NMI interface for s390 machine. > > This removes #ifdef s390 branch in qmp_inject_nmi so new s390's > nmi_monitor_handler() callback is going to be used for NMI. > > Since nmi_monitor_handler()-calling code is platform independent, > CPUState::cpu_index is used instead of S390CPU::env.cpu_num. > There should not be any change in behaviour as both @cpu_index and > @cpu_num are global CPU numbers. > > Also, s390_cpu_restart() takes care of preforming operations in > the specific CPU thread so no extra measure is required here either. > > Since the only error s390_cpu_restart() can return is ENOSYS, convert > it to QERR_UNSUPPORTED. > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > --- > Changes: > v6: > * supported NMI interface > > v5: > * added ENOSYS -> QERR_UNSUPPORTED, qapi/qmp/qerror.h was added for this > > v4: > * s/\<nmi\>/nmi_monitor_handler/ > > v3: > * now contains both old code removal and new code insertion, easier to > track changes > > --- > Is there any good reason to have @cpu_num in addition to @cpu_index? > Just asking :) > --- > cpus.c | 14 -------------- > hw/s390x/s390-virtio.c | 31 +++++++++++++++++++++++++++++++ > target-s390x/cpu.c | 1 + > 3 files changed, 32 insertions(+), 14 deletions(-) > > diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c > index 93c7ace..9c5b7b2 100644 > --- a/hw/s390x/s390-virtio.c > +++ b/hw/s390x/s390-virtio.c > @@ -38,6 +38,7 @@ > #include "hw/s390x/sclp.h" > #include "hw/s390x/s390_flic.h" > #include "hw/s390x/s390-virtio.h" > +#include "hw/nmi.h" > > //#define DEBUG_S390 > > @@ -51,6 +52,7 @@ > > #define MAX_BLK_DEVS 10 > #define ZIPL_FILENAME "s390-zipl.rom" > +#define TYPE_NMI_S390 "s390_nmi" I'd prefer "s390-nmi" instead. > > static VirtIOS390Bus *s390_bus; > static S390CPU **ipi_states; > @@ -277,6 +279,9 @@ static void s390_init(MachineState *machine) > > /* Create VirtIO network adapters */ > s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390"); > + > + object_property_add_child(OBJECT(machine), "nmi", > + object_new(TYPE_NMI_S390), NULL); This only adds the nmi interface to the old s390-virtio machine; we want this for the s390-virtio-ccw machine as well. > } > > static QEMUMachine s390_machine = { > @@ -295,8 +300,34 @@ static QEMUMachine s390_machine = { > .is_default = 1, > }; > > +static void s390_nmi(NMI *n, int cpu_index, Error **errp) > +{ > + CPUState *cs = qemu_get_cpu(cpu_index); > + > + if (s390_cpu_restart(S390_CPU(cs))) { > + error_set(errp, QERR_UNSUPPORTED); > + } > +} > + > +static void s390_nmi_class_init(ObjectClass *oc, void *data) > +{ > + NMIClass *nc = NMI_CLASS(oc); > + nc->nmi_monitor_handler = s390_nmi; > +} > + > +static const TypeInfo s390_nmi_info = { > + .name = TYPE_NMI_S390, > + .parent = TYPE_OBJECT, > + .class_init = s390_nmi_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_NMI }, > + { } > + }, > +}; > + > static void s390_machine_init(void) > { > + type_register_static(&s390_nmi_info); s390-virtio-ccw needs this as well. > qemu_register_machine(&s390_machine); > } The best way would probably be to put all nmi-related things into a new file that registers the nmi type and provides an s390_register_nmi() helper. > > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > index c3082b7..15142ff 100644 > --- a/target-s390x/cpu.c > +++ b/target-s390x/cpu.c > @@ -27,6 +27,7 @@ > #include "qemu-common.h" > #include "qemu/timer.h" > #include "hw/hw.h" > +#include "qapi/qmp/qerror.h" Leftover? > #ifndef CONFIG_USER_ONLY > #include "sysemu/arch_init.h" > #endif