From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55629) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WiObW-0003aS-7w for qemu-devel@nongnu.org; Thu, 08 May 2014 09:43:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WiObL-0000kt-GV for qemu-devel@nongnu.org; Thu, 08 May 2014 09:43:18 -0400 Received: from cantor2.suse.de ([195.135.220.15]:51926 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WiObL-0000kW-7Y for qemu-devel@nongnu.org; Thu, 08 May 2014 09:43:07 -0400 Message-ID: <536B89E9.7050800@suse.de> Date: Thu, 08 May 2014 15:43:05 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1399554218-8262-1-git-send-email-cornelia.huck@de.ibm.com> <1399554218-8262-2-git-send-email-cornelia.huck@de.ibm.com> In-Reply-To: <1399554218-8262-2-git-send-email-cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 1/4] s390x: split flic into kvm and non-kvm parts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: borntraeger@de.ibm.com, qemu-devel@nongnu.org On 05/08/2014 03:03 PM, Cornelia Huck wrote: > Introduce a common parent class for both cases, where kvm and non-kvm > can hook up callbacks. This will be used by follow-on patches for > adapter registration and mapping. > > We now always have a flic, regardless of whether we use kvm; the > non-kvm implementation just doesn't do anything. > > Reviewed-by: Jens Freimann > Signed-off-by: Cornelia Huck > --- > default-configs/s390x-softmmu.mak | 3 +- > hw/intc/Makefile.objs | 1 + > hw/intc/s390_flic.c | 318 ++++-------------------------------- > hw/intc/s390_flic_kvm.c | 325 +++++++++++++++++++++++++++++++++++++ > include/hw/s390x/s390_flic.h | 51 ++++-- > 5 files changed, 399 insertions(+), 299 deletions(-) > create mode 100644 hw/intc/s390_flic_kvm.c > > diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak > index d843dc0..126d88d 100644 > --- a/default-configs/s390x-softmmu.mak > +++ b/default-configs/s390x-softmmu.mak > @@ -1,3 +1,4 @@ > CONFIG_VIRTIO=y > CONFIG_SCLPCONSOLE=y > -CONFIG_S390_FLIC=$(CONFIG_KVM) > +CONFIG_S390_FLIC=y > +CONFIG_S390_FLIC_KVM=$(CONFIG_KVM) > diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > index c8a2318..843864a 100644 > --- a/hw/intc/Makefile.objs > +++ b/hw/intc/Makefile.objs > @@ -26,3 +26,4 @@ obj-$(CONFIG_XICS) += xics.o > obj-$(CONFIG_XICS_KVM) += xics_kvm.o > obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o > obj-$(CONFIG_S390_FLIC) += s390_flic.o > +obj-$(CONFIG_S390_FLIC_KVM) += s390_flic_kvm.o > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index b2ef3e3..7dc8c7d 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -1,322 +1,66 @@ > /* > - * QEMU S390x KVM floating interrupt controller (flic) > + * QEMU S390x floating interrupt controller (flic) > * > * Copyright 2014 IBM Corp. > * Author(s): Jens Freimann > + * Cornelia Huck > * > * This work is licensed under the terms of the GNU GPL, version 2 or (at > * your option) any later version. See the COPYING file in the top-level > * directory. > */ > > -#include > #include "qemu/error-report.h" > #include "hw/sysbus.h" > -#include "sysemu/kvm.h" > #include "migration/qemu-file.h" > #include "hw/s390x/s390_flic.h" > #include "trace.h" > > -#define FLIC_SAVE_INITIAL_SIZE getpagesize() > -#define FLIC_FAILED (-1UL) > -#define FLIC_SAVEVM_VERSION 1 > - > -void s390_flic_init(void) > -{ > - DeviceState *dev; > - int r; > - > - if (kvm_enabled()) { > - dev = qdev_create(NULL, "s390-flic"); > - object_property_add_child(qdev_get_machine(), "s390-flic", > - OBJECT(dev), NULL); > - r = qdev_init(dev); > - if (r) { > - error_report("flic: couldn't create qdev"); > - } > - } > -} > - > -/** > - * flic_get_all_irqs - store all pending irqs in buffer > - * @buf: pointer to buffer which is passed to kernel > - * @len: length of buffer > - * @flic: pointer to flic device state > - * > - * Returns: -ENOMEM if buffer is too small, > - * -EINVAL if attr.group is invalid, > - * -EFAULT if copying to userspace failed, > - * on success return number of stored interrupts > - */ > -static int flic_get_all_irqs(KVMS390FLICState *flic, > - void *buf, int len) > -{ > - struct kvm_device_attr attr = { > - .group = KVM_DEV_FLIC_GET_ALL_IRQS, > - .addr = (uint64_t) buf, > - .attr = len, > - }; > - int rc; > - > - rc = ioctl(flic->fd, KVM_GET_DEVICE_ATTR, &attr); > - > - return rc == -1 ? -errno : rc; > -} > - > -static void flic_enable_pfault(KVMS390FLICState *flic) > -{ > - struct kvm_device_attr attr = { > - .group = KVM_DEV_FLIC_APF_ENABLE, > - }; > - int rc; > - > - rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr); > - > - if (rc) { > - fprintf(stderr, "flic: couldn't enable pfault\n"); > - } > -} > - > -static void flic_disable_wait_pfault(KVMS390FLICState *flic) > +S390FLICState *s390_get_flic(void) > { > - struct kvm_device_attr attr = { > - .group = KVM_DEV_FLIC_APF_DISABLE_WAIT, > - }; > - int rc; > - > - rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr); > + S390FLICState *fs; > > - if (rc) { > - fprintf(stderr, "flic: couldn't disable pfault\n"); > + fs = S390_FLIC_COMMON(object_resolve_path(TYPE_KVM_S390_FLIC, NULL)); > + if (!fs) { > + fs = S390_FLIC_COMMON(object_resolve_path(TYPE_QEMU_S390_FLIC, NULL)); > } > + return fs; > } > > -/** flic_enqueue_irqs - returns 0 on success > - * @buf: pointer to buffer which is passed to kernel > - * @len: length of buffer > - * @flic: pointer to flic device state > - * > - * Returns: -EINVAL if attr.group is unknown > - */ > -static int flic_enqueue_irqs(void *buf, uint64_t len, > - KVMS390FLICState *flic) > -{ > - int rc; > - struct kvm_device_attr attr = { > - .group = KVM_DEV_FLIC_ENQUEUE, > - .addr = (uint64_t) buf, > - .attr = len, > - }; > - > - rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr); > - > - return rc ? -errno : 0; > -} > - > -/** > - * __get_all_irqs - store all pending irqs in buffer > - * @flic: pointer to flic device state > - * @buf: pointer to pointer to a buffer > - * @len: length of buffer > - * > - * Returns: return value of flic_get_all_irqs > - * Note: Retry and increase buffer size until flic_get_all_irqs > - * either returns a value >= 0 or a negative error code. > - * -ENOMEM is an exception, which means the buffer is too small > - * and we should try again. Other negative error codes can be > - * -EFAULT and -EINVAL which we ignore at this point > - */ > -static int __get_all_irqs(KVMS390FLICState *flic, > - void **buf, int len) > +void s390_flic_init(void) > { > + DeviceState *dev; > int r; > > - do { > - /* returns -ENOMEM if buffer is too small and number > - * of queued interrupts on success */ > - r = flic_get_all_irqs(flic, *buf, len); > - if (r >= 0) { > - break; > - } > - len *= 2; > - *buf = g_try_realloc(*buf, len); > - if (!buf) { > - return -ENOMEM; > - } > - } while (r == -ENOMEM && len <= KVM_S390_FLIC_MAX_BUFFER); > - > - return r; > -} > - > -/** > - * kvm_flic_save - Save pending floating interrupts > - * @f: QEMUFile containing migration state > - * @opaque: pointer to flic device state > - * > - * Note: Pass buf and len to kernel. Start with one page and > - * increase until buffer is sufficient or maxium size is > - * reached > - */ > -static void kvm_flic_save(QEMUFile *f, void *opaque) > -{ > - KVMS390FLICState *flic = opaque; > - int len = FLIC_SAVE_INITIAL_SIZE; > - void *buf; > - int count; > - > - flic_disable_wait_pfault((struct KVMS390FLICState *) opaque); > - > - buf = g_try_malloc0(len); > - if (!buf) { > - /* Storing FLIC_FAILED into the count field here will cause the > - * target system to fail when attempting to load irqs from the > - * migration state */ > - error_report("flic: couldn't allocate memory"); > - qemu_put_be64(f, FLIC_FAILED); > - return; > + dev = s390_flic_kvm_create(); Why have this helper? Can't that logic live in the board file? Alex