From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38693) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv1U9-0008NK-LS for qemu-devel@nongnu.org; Thu, 12 Jun 2014 05:39:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wv1U2-00024L-Cr for qemu-devel@nongnu.org; Thu, 12 Jun 2014 05:39:53 -0400 Message-ID: <5399755D.1080803@suse.de> Date: Thu, 12 Jun 2014 11:39:41 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402506183-29736-1-git-send-email-aik@ozlabs.ru> <1402506183-29736-3-git-send-email-aik@ozlabs.ru> <20140612083113.1e4cd667.cornelia.huck@de.ibm.com> <53997520.8020509@ozlabs.ru> In-Reply-To: <53997520.8020509@ozlabs.ru> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 2/4] target-s390x: Migrate to new NMI interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , Cornelia Huck Cc: Peter Maydell , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Alex Bligh , Paolo Bonzini , Luiz Capitulino , Richard Henderson On 12.06.14 11:38, Alexey Kardashevskiy wrote: > On 06/12/2014 04:31 PM, Cornelia Huck wrote: >> On Thu, 12 Jun 2014 03:03:01 +1000 >> Alexey Kardashevskiy 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 >>> --- >>> Changes: >>> v6: >>> * supported NMI interface >>> >>> v5: >>> * added ENOSYS -> QERR_UNSUPPORTED, qapi/qmp/qerror.h was added for this >>> >>> v4: >>> * s/\/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. > > I pushed some version to git@github.com:aik/qemu.git , branch nmi-v7 > Please have a look and give it a go - I do not have s390 kernel/images > handy. Thanks! > > It does not look like we really need a new file for NMI now. > Also, should I put it under #ifdef CONFIG_USER_ONLY on s390? The machine only ever gets compiled on for non-user, so why would you need any #ifdefs? Alex