From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Un5Hz-00087d-3X for qemu-devel@nongnu.org; Thu, 13 Jun 2013 07:02:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Un5Ht-0002TO-7y for qemu-devel@nongnu.org; Thu, 13 Jun 2013 07:01:59 -0400 Message-ID: <51B9A69D.4070808@suse.de> Date: Thu, 13 Jun 2013 13:01:49 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1371069171-20377-1-git-send-email-scottwood@freescale.com> In-Reply-To: <1371069171-20377-1-git-send-email-scottwood@freescale.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] kvm/openpic: in-kernel mpic support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Wood , Alexander Graf Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org Am 12.06.2013 22:32, schrieb Scott Wood: > 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). >=20 > Note from Alex regarding kvm_irqchip_create(): >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Scott Wood > --- > v2: fix "llx" -> PRI_x64, and remove some broken leftover code > involving reg_base. > --- > default-configs/ppc-softmmu.mak | 1 + > default-configs/ppc64-softmmu.mak | 1 + > hw/intc/Makefile.objs | 1 + > hw/intc/openpic_kvm.c | 250 +++++++++++++++++++++++++++++= ++++++++ > hw/ppc/e500.c | 79 +++++++++++- > include/hw/ppc/openpic.h | 2 +- > 6 files changed, 328 insertions(+), 6 deletions(-) > create mode 100644 hw/intc/openpic_kvm.c >=20 > diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-soft= mmu.mak > index cc3587f..63255dc 100644 > --- a/default-configs/ppc-softmmu.mak > +++ b/default-configs/ppc-softmmu.mak > @@ -43,5 +43,6 @@ CONFIG_XILINX=3Dy > CONFIG_XILINX_ETHLITE=3Dy > CONFIG_OPENPIC=3Dy > CONFIG_E500=3D$(CONFIG_FDT) > +CONFIG_OPENPIC_KVM=3D$(and $(CONFIG_E500),$(CONFIG_KVM)) > # For PReP > CONFIG_MC146818RTC=3Dy > 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=3Dy > CONFIG_OPENPIC=3Dy > CONFIG_PSERIES=3D$(CONFIG_FDT) > CONFIG_E500=3D$(CONFIG_FDT) > +CONFIG_OPENPIC_KVM=3D$(and $(CONFIG_E500),$(CONFIG_KVM)) > # For pSeries > CONFIG_PCI_HOTPLUG=3Dy > # 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) +=3D grlib_irqmp.o > obj-$(CONFIG_IOAPIC) +=3D ioapic.o > obj-$(CONFIG_OMAP) +=3D omap_intc.o > obj-$(CONFIG_OPENPIC) +=3D openpic.o > +obj-$(CONFIG_OPENPIC_KVM) +=3D openpic_kvm.o > obj-$(CONFIG_SH4) +=3D sh_intc.o > diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c > new file mode 100644 > index 0000000..809b34b > --- /dev/null > +++ b/hw/intc/openpic_kvm.c > @@ -0,0 +1,250 @@ > +/* > + * KVM in-kernel OpenPIC > + * > + * Copyright 2013 Freescale Semiconductor, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaini= ng a copy > + * of this software and associated documentation files (the "Software"= ), to deal > + * in the Software without restriction, including without limitation t= he rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/o= r 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 incl= uded in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXP= RESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABI= LITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES O= R OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARI= SING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALI= NGS IN > + * THE SOFTWARE. > + */ > + > +#include > +#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; SysBusDevice parent_obj; please! http://wiki.qemu.org/QOMConventions > + 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 =3D opaque; > + struct kvm_device_attr attr; > + uint32_t val32 =3D val; > + int ret; > + > + attr.group =3D KVM_DEV_MPIC_GRP_REGISTER; > + attr.attr =3D addr; > + attr.addr =3D (uint64_t)(unsigned long)&val32; > + > + ret =3D ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr); > + if (ret < 0) { > + qemu_log_mask(LOG_UNIMP, "%s: %s %" PRIx64 "\n", __func__, > + strerror(errno), attr.attr); > + } > +} > + > +static uint64_t kvm_openpic_read(void *opaque, hwaddr addr, unsigned s= ize) > +{ > + KVMOpenPICState *opp =3D opaque; > + struct kvm_device_attr attr; > + uint32_t val =3D 0xdeadbeef; > + int ret; > + > + attr.group =3D KVM_DEV_MPIC_GRP_REGISTER; > + attr.attr =3D addr; > + attr.addr =3D (uint64_t)(unsigned long)&val; > + > + ret =3D ioctl(opp->fd, KVM_GET_DEVICE_ATTR, &attr); > + if (ret < 0) { > + qemu_log_mask(LOG_UNIMP, "%s: %s %" PRIx64 "\n", __func__, > + strerror(errno), attr.attr); > + return 0; > + } > + > + return val; > +} > + > +static const MemoryRegionOps kvm_openpic_mem_ops =3D { > + .write =3D kvm_openpic_write, > + .read =3D kvm_openpic_read, > + .endianness =3D DEVICE_BIG_ENDIAN, > + .impl =3D { > + .min_access_size =3D 4, > + .max_access_size =3D 4, > + }, > +}; > + > +static void kvm_openpic_region_add(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + KVMOpenPICState *opp =3D container_of(listener, KVMOpenPICState, > + mem_listener); > + struct kvm_device_attr attr; > + uint64_t reg_base; > + int ret; > + > + if (section->address_space !=3D &address_space_memory) { > + abort(); > + } > + > + reg_base =3D section->offset_within_address_space; > + > + attr.group =3D KVM_DEV_MPIC_GRP_MISC; > + attr.attr =3D KVM_DEV_MPIC_BASE_ADDR; > + attr.addr =3D (uint64_t)(unsigned long)®_base; > + > + ret =3D ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr); > + if (ret < 0) { > + fprintf(stderr, "%s: %s %" PRIx64 "\n", __func__, > + strerror(errno), reg_base); > + } > +} > + > +static void kvm_openpic_region_del(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + KVMOpenPICState *opp =3D container_of(listener, KVMOpenPICState, > + mem_listener); > + struct kvm_device_attr attr; > + uint64_t reg_base =3D 0; > + int ret; > + > + attr.group =3D KVM_DEV_MPIC_GRP_MISC; > + attr.attr =3D KVM_DEV_MPIC_BASE_ADDR; > + attr.addr =3D (uint64_t)(unsigned long)®_base; > + > + ret =3D ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr); > + if (ret < 0) { > + fprintf(stderr, "%s: %s %" PRIx64 "\n", __func__, > + strerror(errno), reg_base); > + } > +} > + > +static int kvm_openpic_init(SysBusDevice *dev) Please make this instance_init + realize functions - "dev" should rather be reserved for DeviceState. > +{ > + KVMState *s =3D kvm_state; > + KVMOpenPICState *opp =3D FROM_SYSBUS(typeof(*opp), dev); NACK, please introduce your own KVM_OPENPIC(obj) cast macro instead for new devices - has been a topic for several weeks and months now. > + int kvm_openpic_model; > + struct kvm_create_device cd =3D {0}; > + int ret, i; > + > + if (!kvm_check_extension(s, KVM_CAP_DEVICE_CTRL)) { > + return -EINVAL; > + } > + > + switch (opp->model) { > + case OPENPIC_MODEL_FSL_MPIC_20: > + kvm_openpic_model =3D KVM_DEV_TYPE_FSL_MPIC_20; > + break; > + > + case OPENPIC_MODEL_FSL_MPIC_42: > + kvm_openpic_model =3D KVM_DEV_TYPE_FSL_MPIC_42; > + break; > + > + default: > + return -EINVAL; > + } If there's only two supported enum-style options, why not make it two devices with the value set as a class field? > + > + cd.type =3D kvm_openpic_model; > + ret =3D kvm_vm_ioctl(s, KVM_CREATE_DEVICE, &cd); > + if (ret < 0) { > + qemu_log_mask(LOG_UNIMP, "%s: can't create device %d: %s\n", > + __func__, cd.type, strerror(errno)); > + return -EINVAL; > + } > + opp->fd =3D cd.fd; > + > + memory_region_init_io(&opp->mem, &kvm_openpic_mem_ops, opp, > + "kvm-openpic", 0x40000); > + > + sysbus_init_mmio(dev, &opp->mem); > + qdev_init_gpio_in(&dev->qdev, kvm_openpic_set_irq, OPENPIC_MAX_IRQ= ); > + > + opp->mem_listener.region_add =3D kvm_openpic_region_add; > + opp->mem_listener.region_add =3D kvm_openpic_region_del; > + memory_listener_register(&opp->mem_listener, &address_space_memory= ); > + > + /* indicate pic capabilities */ > + msi_supported =3D true; > + kvm_kernel_irqchip =3D true; > + kvm_async_interrupts_allowed =3D true; > + > + /* set up irq routing */ > + kvm_init_irq_routing(kvm_state); > + for (i =3D 0; i < 256; ++i) { > + kvm_irqchip_add_irq_route(kvm_state, i, 0, i); > + } > + > + kvm_irqfds_allowed =3D true; > + kvm_msi_via_irqfd_allowed =3D true; > + kvm_gsi_routing_allowed =3D true; > + > + return 0; > +} > + > +int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs) > +{ > + KVMOpenPICState *opp =3D FROM_SYSBUS(typeof(*opp), SYS_BUS_DEVICE(= d)); KVM_OPENPIC(d) > + struct kvm_enable_cap encap =3D {}; > + > + encap.cap =3D KVM_CAP_IRQ_MPIC; > + encap.args[0] =3D opp->fd; > + encap.args[1] =3D cs->cpu_index; > + > + return kvm_vcpu_ioctl(cs, KVM_ENABLE_CAP, &encap); > +} > + > +static Property kvm_openpic_properties[] =3D { > + DEFINE_PROP_UINT32("model", KVMOpenPICState, model, > + OPENPIC_MODEL_FSL_MPIC_20), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void kvm_openpic_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + SysBusDeviceClass *k =3D SYS_BUS_DEVICE_CLASS(klass); > + > + k->init =3D kvm_openpic_init; > + dc->props =3D kvm_openpic_properties; > + dc->reset =3D kvm_openpic_reset; > +} > + > +static const TypeInfo kvm_openpic_info =3D { > + .name =3D "kvm-openpic", > + .parent =3D TYPE_SYS_BUS_DEVICE, > + .instance_size =3D sizeof(KVMOpenPICState), > + .class_init =3D kvm_openpic_class_init, > +}; > + > +static void kvm_openpic_register_types(void) > +{ > + type_register_static(&kvm_openpic_info); > +} > + > +type_init(kvm_openpic_register_types) > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index 14e0547..f790ed1 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -448,18 +448,17 @@ static void ppce500_cpu_reset(void *opaque) > mmubooke_create_initial_mapping(env); > } > =20 > -static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion= *ccsr, > - qemu_irq **irqs) > +static DeviceState *ppce500_init_mpic_qemu(PPCE500Params *params, > + qemu_irq **irqs) > { > - qemu_irq *mpic; > DeviceState *dev; > SysBusDevice *s; > int i, j, k; > =20 > - mpic =3D g_new(qemu_irq, 256); > dev =3D qdev_create(NULL, "openpic"); > - qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus); > qdev_prop_set_uint32(dev, "model", params->mpic_version); > + qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus); > + > qdev_init_nofail(dev); > s =3D SYS_BUS_DEVICE(dev); > =20 > @@ -470,10 +469,80 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params = *params, MemoryRegion *ccsr, > } > } > =20 > + return dev; > +} > + > +static DeviceState *ppce500_init_mpic_kvm(PPCE500Params *params, > + qemu_irq **irqs) > +{ > + DeviceState *dev; > + CPUPPCState *env; > + CPUState *cs; > + int r; > + > + dev =3D qdev_create(NULL, "kvm-openpic"); > + qdev_prop_set_uint32(dev, "model", params->mpic_version); > + > + r =3D qdev_init(dev); > + if (r) { > + return NULL; > + } > + > + for (env =3D first_cpu; env !=3D NULL; env =3D env->next_cpu) { > + cs =3D ENV_GET_CPU(env); > + > + if (kvm_openpic_connect_vcpu(dev, cs)) { > + fprintf(stderr, "%s: failed to connect vcpu to irqchip\n", > + __func__); > + abort(); > + } > + } Note: My series on the list converts first_cpu to CPUState, so this may conflict depending on pull timing. Resolution would be s/env/cs/g. Andreas > + > + return dev; > +} > + > +static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion= *ccsr, > + qemu_irq **irqs) > +{ > + QemuOptsList *list; > + qemu_irq *mpic; > + DeviceState *dev =3D NULL; > + SysBusDevice *s; > + int i; > + > + mpic =3D g_new(qemu_irq, 256); > + > + if (kvm_enabled()) { > + bool irqchip_allowed =3D true, irqchip_required =3D false; > + > + list =3D qemu_find_opts("machine"); > + if (!QTAILQ_EMPTY(&list->head)) { > + irqchip_allowed =3D qemu_opt_get_bool(QTAILQ_FIRST(&list->= head), > + "kernel_irqchip", true= ); > + irqchip_required =3D qemu_opt_get_bool(QTAILQ_FIRST(&list-= >head), > + "kernel_irqchip", fal= se); > + } > + > + if (irqchip_allowed) { > + dev =3D ppce500_init_mpic_kvm(params, irqs); > + } > + > + if (irqchip_required && !dev) { > + fprintf(stderr, "%s: irqchip requested but unavailable\n", > + __func__); > + abort(); > + } > + } > + > + if (!dev) { > + dev =3D ppce500_init_mpic_qemu(params, irqs); > + } > + > for (i =3D 0; i < 256; i++) { > mpic[i] =3D qdev_get_gpio_in(dev, i); > } > =20 > + s =3D SYS_BUS_DEVICE(dev); > memory_region_add_subregion(ccsr, MPC8544_MPIC_REGS_OFFSET, > s->mmio[0].memory); > =20 > diff --git a/include/hw/ppc/openpic.h b/include/hw/ppc/openpic.h > index d873bb6..1fe4865 100644 > --- a/include/hw/ppc/openpic.h > +++ b/include/hw/ppc/openpic.h > @@ -24,6 +24,6 @@ enum { > #define OPENPIC_MAX_IRQ (OPENPIC_MAX_SRC + OPENPIC_MAX_IPI + \ > OPENPIC_MAX_TMR) > =20 > -DeviceState *kvm_openpic_create(BusState *bus, int model); > +int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs); > =20 > #endif /* __OPENPIC_H__ */ >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg