[v5,1/4] s390x: split flic into kvm and non-kvm parts

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 <jfrei@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 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

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 <jfrei@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>   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 <jfrei@linux.vnet.ibm.com>
> + *            Cornelia Huck <cornelia.huck@de.ibm.com>
>    *
>    * 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 <sys/ioctl.h>
>   #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

On Thu, 08 May 2014 15:43:05 +0200
Alexander Graf <agraf@suse.de> wrote:

> 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 <jfrei@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >   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/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

> > +void s390_flic_init(void)
> >   {
> > +    DeviceState *dev;
> >       int r;

> > +    dev = s390_flic_kvm_create();
> 
> Why have this helper? Can't that logic live in the board file?

It could; but I think it is cleaner to contain this in the flic than to
have kvm-switches there.

On 08/05/14 15:03, 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 <jfrei@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  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 ++++--

Shouldnt we move this to include/hw/intc/ ?

Otherwise:
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>


>  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 <jfrei@linux.vnet.ibm.com>
> + *            Cornelia Huck <cornelia.huck@de.ibm.com>
>   *
>   * 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 <sys/ioctl.h>
>  #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();
> +    if (!dev) {
> +        dev = qdev_create(NULL, TYPE_QEMU_S390_FLIC);
> +        object_property_add_child(qdev_get_machine(), TYPE_QEMU_S390_FLIC,
> +                                  OBJECT(dev), NULL);
>      }
> -
> -    count = __get_all_irqs(flic, &buf, len);
> -    if (count < 0) {
> -        error_report("flic: couldn't retrieve irqs from kernel, rc %d",
> -                     count);
> -        /* Storing FLIC_FAILED into the count field here will cause the
> -         * target system to fail when attempting to load irqs from the
> -         * migration state */
> -        qemu_put_be64(f, FLIC_FAILED);
> -    } else {
> -        qemu_put_be64(f, count);
> -        qemu_put_buffer(f, (uint8_t *) buf,
> -                        count * sizeof(struct kvm_s390_irq));
> +    r = qdev_init(dev);
> +    if (r) {
> +        error_report("flic: couldn't create qdev");
>      }
> -    g_free(buf);
>  }
> 
> -/**
> - * kvm_flic_load - Load pending floating interrupts
> - * @f: QEMUFile containing migration state
> - * @opaque: pointer to flic device state
> - * @version_id: version id for migration
> - *
> - * Returns: value of flic_enqueue_irqs, -EINVAL on error
> - * Note: Do nothing when no interrupts where stored
> - * in QEMUFile
> - */
> -static int kvm_flic_load(QEMUFile *f, void *opaque, int version_id)
> -{
> -    uint64_t len = 0;
> -    uint64_t count = 0;
> -    void *buf = NULL;
> -    int r = 0;
> -
> -    if (version_id != FLIC_SAVEVM_VERSION) {
> -        r = -EINVAL;
> -        goto out;
> -    }
> -
> -    flic_enable_pfault((struct KVMS390FLICState *) opaque);
> -
> -    count = qemu_get_be64(f);
> -    len = count * sizeof(struct kvm_s390_irq);
> -    if (count == FLIC_FAILED) {
> -        r = -EINVAL;
> -        goto out;
> -    }
> -    if (count == 0) {
> -        r = 0;
> -        goto out;
> -    }
> -    buf = g_try_malloc0(len);
> -    if (!buf) {
> -        r = -ENOMEM;
> -        goto out;
> -    }
> -
> -    if (qemu_get_buffer(f, (uint8_t *) buf, len) != len) {
> -        r = -EINVAL;
> -        goto out_free;
> -    }
> -    r = flic_enqueue_irqs(buf, len, (struct KVMS390FLICState *) opaque);
> -
> -out_free:
> -    g_free(buf);
> -out:
> -    return r;
> -}
> -
> -static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
> -{
> -    KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> -    struct kvm_create_device cd = {0};
> -    int ret;
> -
> -    flic_state->fd = -1;
> -    if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
> -        trace_flic_no_device_api(errno);
> -        return;
> -    }
> -
> -    cd.type = KVM_DEV_TYPE_FLIC;
> -    ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> -    if (ret < 0) {
> -        trace_flic_create_device(errno);
> -        return;
> -    }
> -    flic_state->fd = cd.fd;
> -
> -    /* Register savevm handler for floating interrupts */
> -    register_savevm(NULL, "s390-flic", 0, 1, kvm_flic_save,
> -                    kvm_flic_load, (void *) flic_state);
> -}
> -
> -static void kvm_s390_flic_unrealize(DeviceState *dev, Error **errp)
> -{
> -    KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> -
> -    unregister_savevm(DEVICE(flic_state), "s390-flic", flic_state);
> -}
> -
> -static void kvm_s390_flic_reset(DeviceState *dev)
> -{
> -    KVMS390FLICState *flic = KVM_S390_FLIC(dev);
> -    struct kvm_device_attr attr = {
> -        .group = KVM_DEV_FLIC_CLEAR_IRQS,
> -    };
> -    int rc = 0;
> -
> -    if (flic->fd == -1) {
> -        return;
> -    }
> -
> -    flic_disable_wait_pfault(flic);
> -
> -    rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr);
> -    if (rc) {
> -        trace_flic_reset_failed(errno);
> -    }
> -
> -    flic_enable_pfault(flic);
> -}
> -
> -static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(oc);
> -
> -    dc->realize = kvm_s390_flic_realize;
> -    dc->unrealize = kvm_s390_flic_unrealize;
> -    dc->reset = kvm_s390_flic_reset;
> -}
> +static const TypeInfo qemu_s390_flic_info = {
> +    .name          = TYPE_QEMU_S390_FLIC,
> +    .parent        = TYPE_S390_FLIC_COMMON,
> +    .instance_size = sizeof(QEMUS390FLICState),
> +};
> 
> -static const TypeInfo kvm_s390_flic_info = {
> -    .name          = TYPE_KVM_S390_FLIC,
> +static const TypeInfo s390_flic_common_info = {
> +    .name          = TYPE_S390_FLIC_COMMON,
>      .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(KVMS390FLICState),
> -    .class_init    = kvm_s390_flic_class_init,
> +    .instance_size = sizeof(S390FLICState),
> +    .class_size    = sizeof(S390FLICStateClass),
>  };
> 
> -static void kvm_s390_flic_register_types(void)
> +static void qemu_s390_flic_register_types(void)
>  {
> -    type_register_static(&kvm_s390_flic_info);
> +    type_register_static(&s390_flic_common_info);
> +    type_register_static(&qemu_s390_flic_info);
>  }
> 
> -type_init(kvm_s390_flic_register_types)
> +type_init(qemu_s390_flic_register_types)
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> new file mode 100644
> index 0000000..70c1710
> --- /dev/null
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -0,0 +1,325 @@
> +/*
> + * QEMU S390x KVM floating interrupt controller (flic)
> + *
> + * Copyright 2014 IBM Corp.
> + * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com>
> + *            Cornelia Huck <cornelia.huck@de.ibm.com>
> + *
> + * 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 <sys/ioctl.h>
> +#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
> +
> +typedef struct KVMS390FLICState {
> +    S390FLICState parent_obj;
> +
> +    uint32_t fd;
> +} KVMS390FLICState;
> +
> +DeviceState *s390_flic_kvm_create(void)
> +{
> +    DeviceState *dev = NULL;
> +
> +    if (kvm_enabled()) {
> +        dev = qdev_create(NULL, TYPE_KVM_S390_FLIC);
> +        object_property_add_child(qdev_get_machine(), TYPE_KVM_S390_FLIC,
> +                                  OBJECT(dev), NULL);
> +    }
> +    return dev;
> +}
> +
> +/**
> + * 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)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_DEV_FLIC_APF_DISABLE_WAIT,
> +    };
> +    int rc;
> +
> +    rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr);
> +
> +    if (rc) {
> +        fprintf(stderr, "flic: couldn't disable pfault\n");
> +    }
> +}
> +
> +/** 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)
> +{
> +    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;
> +    }
> +
> +    count = __get_all_irqs(flic, &buf, len);
> +    if (count < 0) {
> +        error_report("flic: couldn't retrieve irqs from kernel, rc %d",
> +                     count);
> +        /* Storing FLIC_FAILED into the count field here will cause the
> +         * target system to fail when attempting to load irqs from the
> +         * migration state */
> +        qemu_put_be64(f, FLIC_FAILED);
> +    } else {
> +        qemu_put_be64(f, count);
> +        qemu_put_buffer(f, (uint8_t *) buf,
> +                        count * sizeof(struct kvm_s390_irq));
> +    }
> +    g_free(buf);
> +}
> +
> +/**
> + * kvm_flic_load - Load pending floating interrupts
> + * @f: QEMUFile containing migration state
> + * @opaque: pointer to flic device state
> + * @version_id: version id for migration
> + *
> + * Returns: value of flic_enqueue_irqs, -EINVAL on error
> + * Note: Do nothing when no interrupts where stored
> + * in QEMUFile
> + */
> +static int kvm_flic_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    uint64_t len = 0;
> +    uint64_t count = 0;
> +    void *buf = NULL;
> +    int r = 0;
> +
> +    if (version_id != FLIC_SAVEVM_VERSION) {
> +        r = -EINVAL;
> +        goto out;
> +    }
> +
> +    flic_enable_pfault((struct KVMS390FLICState *) opaque);
> +
> +    count = qemu_get_be64(f);
> +    len = count * sizeof(struct kvm_s390_irq);
> +    if (count == FLIC_FAILED) {
> +        r = -EINVAL;
> +        goto out;
> +    }
> +    if (count == 0) {
> +        r = 0;
> +        goto out;
> +    }
> +    buf = g_try_malloc0(len);
> +    if (!buf) {
> +        r = -ENOMEM;
> +        goto out;
> +    }
> +
> +    if (qemu_get_buffer(f, (uint8_t *) buf, len) != len) {
> +        r = -EINVAL;
> +        goto out_free;
> +    }
> +    r = flic_enqueue_irqs(buf, len, (struct KVMS390FLICState *) opaque);
> +
> +out_free:
> +    g_free(buf);
> +out:
> +    return r;
> +}
> +
> +static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
> +{
> +    KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> +    struct kvm_create_device cd = {0};
> +    int ret;
> +
> +    flic_state->fd = -1;
> +    if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
> +        trace_flic_no_device_api(errno);
> +        return;
> +    }
> +
> +    cd.type = KVM_DEV_TYPE_FLIC;
> +    ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> +    if (ret < 0) {
> +        trace_flic_create_device(errno);
> +        return;
> +    }
> +    flic_state->fd = cd.fd;
> +
> +    /* Register savevm handler for floating interrupts */
> +    register_savevm(NULL, "s390-flic", 0, 1, kvm_flic_save,
> +                    kvm_flic_load, (void *) flic_state);
> +}
> +
> +static void kvm_s390_flic_unrealize(DeviceState *dev, Error **errp)
> +{
> +    KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> +
> +    unregister_savevm(DEVICE(flic_state), "s390-flic", flic_state);
> +}
> +
> +static void kvm_s390_flic_reset(DeviceState *dev)
> +{
> +    KVMS390FLICState *flic = KVM_S390_FLIC(dev);
> +    struct kvm_device_attr attr = {
> +        .group = KVM_DEV_FLIC_CLEAR_IRQS,
> +    };
> +    int rc = 0;
> +
> +    if (flic->fd == -1) {
> +        return;
> +    }
> +
> +    flic_disable_wait_pfault(flic);
> +
> +    rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr);
> +    if (rc) {
> +        trace_flic_reset_failed(errno);
> +    }
> +
> +    flic_enable_pfault(flic);
> +}
> +
> +static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = kvm_s390_flic_realize;
> +    dc->unrealize = kvm_s390_flic_unrealize;
> +    dc->reset = kvm_s390_flic_reset;
> +}
> +
> +static const TypeInfo kvm_s390_flic_info = {
> +    .name          = TYPE_KVM_S390_FLIC,
> +    .parent        = TYPE_S390_FLIC_COMMON,
> +    .instance_size = sizeof(KVMS390FLICState),
> +    .class_init    = kvm_s390_flic_class_init,
> +};
> +
> +static void kvm_s390_flic_register_types(void)
> +{
> +    type_register_static(&kvm_s390_flic_info);
> +}
> +
> +type_init(kvm_s390_flic_register_types)
> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
> index 497b219..3bc60fd 100644
> --- a/include/hw/s390x/s390_flic.h
> +++ b/include/hw/s390x/s390_flic.h
> @@ -1,33 +1,62 @@
>  /*
> - * QEMU S390x KVM floating interrupt controller (flic)
> + * QEMU S390x floating interrupt controller (flic)
>   *
>   * Copyright 2014 IBM Corp.
>   * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com>
> + *            Cornelia Huck <cornelia.huck@de.ibm.com>
>   *
>   * 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.
>   */
> 
> -#ifndef __KVM_S390_FLIC_H
> -#define __KVM_S390_FLIC_H
> +#ifndef __HW_S390_FLIC_H
> +#define __HW_S390_FLIC_H
> 
>  #include "hw/sysbus.h"
> 
> -#define TYPE_KVM_S390_FLIC "s390-flic"
> +#define TYPE_S390_FLIC_COMMON "s390-flic"
> +#define S390_FLIC_COMMON(obj) \
> +    OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON)
> +
> +typedef struct S390FLICState {
> +    SysBusDevice parent_obj;
> +
> +} S390FLICState;
> +
> +#define S390_FLIC_COMMON_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(S390FLICStateClass, (klass), TYPE_S390_FLIC_COMMON)
> +#define S390_FLIC_COMMON_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(S390FLICStateClass, (obj), TYPE_S390_FLIC_COMMON)
> +
> +typedef struct S390FLICStateClass {
> +    DeviceClass parent_class;
> +
> +} S390FLICStateClass;
> +
> +#define TYPE_KVM_S390_FLIC "s390-flic-kvm"
>  #define KVM_S390_FLIC(obj) \
>      OBJECT_CHECK(KVMS390FLICState, (obj), TYPE_KVM_S390_FLIC)
> 
> -typedef struct KVMS390FLICState {
> -    SysBusDevice parent_obj;
> +#define TYPE_QEMU_S390_FLIC "s390-flic-qemu"
> +#define QEMU_S390_FLIC(obj) \
> +    OBJECT_CHECK(QEMUS390FLICState, (obj), TYPE_QEMU_S390_FLIC)
> 
> -    uint32_t fd;
> -} KVMS390FLICState;
> +typedef struct QEMUS390FLICState {
> +    S390FLICState parent_obj;
> +} QEMUS390FLICState;
> 
> -#ifdef CONFIG_KVM
>  void s390_flic_init(void);
> +
> +S390FLICState *s390_get_flic(void);
> +
> +#ifdef CONFIG_KVM
> +DeviceState *s390_flic_kvm_create(void);
>  #else
> -static inline void s390_flic_init(void) { }
> +static inline DeviceState *s390_flic_kvm_create(void)
> +{
> +    return NULL;
> +}
>  #endif
> 
> -#endif /* __KVM_S390_FLIC_H */
> +#endif /* __HW_S390_FLIC_H */
>

On Mon, 12 May 2014 10:01:31 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 08/05/14 15:03, 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 <jfrei@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  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 ++++--
> 
> Shouldnt we move this to include/hw/intc/ ?

Current code is a bit undecided: Some interrupt controllers (like arm
gic) have their headers in include/hw/intc/, some (like openpic and
xics) in the architecture specific directories. Should we decide to
collect all of those headers in include/hw/intc/, I vote for doing that
in a general sweep.

> 
> Otherwise:
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> 
> >  5 files changed, 399 insertions(+), 299 deletions(-)
> >  create mode 100644 hw/intc/s390_flic_kvm.c

On 12/05/14 10:09, Cornelia Huck wrote:
> On Mon, 12 May 2014 10:01:31 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 08/05/14 15:03, 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 <jfrei@linux.vnet.ibm.com>
>>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>> ---
>>>  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 ++++--
>>
>> Shouldnt we move this to include/hw/intc/ ?
> 
> Current code is a bit undecided: Some interrupt controllers (like arm
> gic) have their headers in include/hw/intc/, some (like openpic and
> xics) in the architecture specific directories. Should we decide to
> collect all of those headers in include/hw/intc/, I vote for doing that
> in a general sweep.

Makes a lot of sense.

> 
>>
>> Otherwise:
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>
>>
>>>  5 files changed, 399 insertions(+), 299 deletions(-)
>>>  create mode 100644 hw/intc/s390_flic_kvm.c