Re: [Qemu-devel] [PATCH 8/9] kvm/openpic: in-kernel mpic support

[Alexander Graf]

On 01.05.2013, at 03:48, Scott Wood wrote:

> Enables support for the in-kernel MPIC that thas been merged into the
> KVM next branch.  This includes irqfd/KVM_IRQ_LINE support from Alex
> Graf (along with some other improvements).
> 
> Note from Alex regarding kvm_irqchip_create():
> 
>  On x86, one would call kvm_irqchip_create() to initialize an
>  in-kernel interrupt controller.  That function then goes ahead and
>  initializes global capability variables as well as the default irq
>  routing table.
> 
>  On ppc, we can't call kvm_irqchip_create() because we can have
>  different types of interrupt controllers.  So we want to do all the
>  things that function would do for us in the in-kernel device init
>  handler.
> 
> Signed-off-by: Scott Wood <address@hidden>
> ---
> default-configs/ppc-softmmu.mak   |    1 +
> default-configs/ppc64-softmmu.mak |    1 +
> hw/intc/Makefile.objs             |    1 +
> hw/intc/openpic_kvm.c             |  256 +++++++++++++++++++++++++++++++++++++
> hw/ppc/e500.c                     |   79 +++++++++++-
> include/hw/ppc/openpic.h          |    2 +-
> 6 files changed, 334 insertions(+), 6 deletions(-)
> create mode 100644 hw/intc/openpic_kvm.c
> 
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index cc3587f..63255dc 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -43,5 +43,6 @@ CONFIG_XILINX=y
> CONFIG_XILINX_ETHLITE=y
> CONFIG_OPENPIC=y
> CONFIG_E500=$(CONFIG_FDT)
> +CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
> # For PReP
> CONFIG_MC146818RTC=y
> diff --git a/default-configs/ppc64-softmmu.mak 
> b/default-configs/ppc64-softmmu.mak
> index 884ea8a..e3c0c68 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -44,6 +44,7 @@ CONFIG_XILINX_ETHLITE=y
> CONFIG_OPENPIC=y
> CONFIG_PSERIES=$(CONFIG_FDT)
> CONFIG_E500=$(CONFIG_FDT)
> +CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
> # For pSeries
> CONFIG_PCI_HOTPLUG=y
> # For PReP
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 718d97a..837ef19 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -20,4 +20,5 @@ obj-$(CONFIG_GRLIB) += grlib_irqmp.o
> obj-$(CONFIG_IOAPIC) += ioapic.o
> obj-$(CONFIG_OMAP) += omap_intc.o
> obj-$(CONFIG_OPENPIC) += openpic.o
> +obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
> obj-$(CONFIG_SH4) += sh_intc.o
> diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
> new file mode 100644
> index 0000000..e57ae2f
> --- /dev/null
> +++ b/hw/intc/openpic_kvm.c
> @@ -0,0 +1,256 @@
> +/*
> + * KVM in-kernel OpenPIC
> + *
> + * Copyright 2013 Freescale Semiconductor, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include <sys/ioctl.h>
> +#include "exec/address-spaces.h"
> +#include "hw/hw.h"
> +#include "hw/ppc/openpic.h"
> +#include "hw/pci/msi.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/kvm.h"
> +#include "qemu/log.h"
> +
> +typedef struct KVMOpenPICState {
> +    SysBusDevice busdev;
> +    MemoryRegion mem;
> +    MemoryListener mem_listener;
> +    uint32_t fd;
> +    uint32_t model;
> +} KVMOpenPICState;
> +
> +static void kvm_openpic_set_irq(void *opaque, int n_IRQ, int level)
> +{
> +    kvm_set_irq(kvm_state, n_IRQ, level);
> +}
> +
> +static void kvm_openpic_reset(DeviceState *d)
> +{
> +    qemu_log_mask(LOG_UNIMP, "%s: unimplemented\n", __func__);
> +}
> +
> +static void kvm_openpic_write(void *opaque, hwaddr addr, uint64_t val,
> +                              unsigned size)
> +{
> +    KVMOpenPICState *opp = opaque;
> +    struct kvm_device_attr attr;
> +    uint32_t val32 = val;
> +    int ret;
> +
> +    attr.group = KVM_DEV_MPIC_GRP_REGISTER;
> +    attr.attr = addr;
> +    attr.addr = (uint64_t)(unsigned long)&val32;
> +
> +    ret = ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr);
> +    if (ret < 0) {
> +        qemu_log_mask(LOG_UNIMP, "%s: %s %llx\n", __func__,
> +                      strerror(errno), attr.attr);
> +    }
> +}
> +
> +static uint64_t kvm_openpic_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    KVMOpenPICState *opp = opaque;
> +    struct kvm_device_attr attr;
> +    uint32_t val = 0xdeadbeef;
> +    int ret;
> +
> +    attr.group = KVM_DEV_MPIC_GRP_REGISTER;
> +    attr.attr = addr;
> +    attr.addr = (uint64_t)(unsigned long)&val;
> +
> +    ret = ioctl(opp->fd, KVM_GET_DEVICE_ATTR, &attr);
> +    if (ret < 0) {
> +        qemu_log_mask(LOG_UNIMP, "%s: %s %llx\n", __func__,
> +                      strerror(errno), attr.attr);

%llx on __u64. Please use PRIx64 instead. Same thing a few times below.

> +        return 0;
> +    }
> +
> +    return val;
> +}
> +
> +static const MemoryRegionOps kvm_openpic_mem_ops = {
> +    .write = kvm_openpic_write,
> +    .read  = kvm_openpic_read,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static void kvm_openpic_region_add(MemoryListener *listener,
> +                                   MemoryRegionSection *section)
> +{
> +    KVMOpenPICState *opp = container_of(listener, KVMOpenPICState,
> +                                        mem_listener);
> +    struct kvm_device_attr attr;
> +    uint64_t reg_base;
> +    int ret;
> +
> +    if (section->address_space != &address_space_memory) {
> +        abort();
> +    }
> +
> +    if (reg_base) {

Check on uninitialized variable.

> +        fprintf(stderr, "%s: reg_base already %llx\n", __func__, reg_base);
> +    }
> +
> +    reg_base = section->offset_within_address_space;
> +
> +    attr.group = KVM_DEV_MPIC_GRP_MISC;
> +    attr.attr = KVM_DEV_MPIC_BASE_ADDR;
> +    attr.addr = (uint64_t)(unsigned long)&reg_base;
> +
> +    ret = ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr);
> +    if (ret < 0) {
> +        fprintf(stderr, "%s: %s %llx\n", __func__, strerror(errno), 
> reg_base);
> +    }
> +}
> +
> +static void kvm_openpic_region_del(MemoryListener *listener,
> +                                   MemoryRegionSection *section)
> +{
> +    KVMOpenPICState *opp = container_of(listener, KVMOpenPICState,
> +                                        mem_listener);
> +    struct kvm_device_attr attr;
> +    uint64_t reg_base = 0;
> +    int ret;
> +
> +    if (!reg_base) {
> +        return;

This branch is always hit.


Alex