From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33752) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Usvn2-00064e-I8 for qemu-devel@nongnu.org; Sat, 29 Jun 2013 10:06:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Usvmz-0008NV-On for qemu-devel@nongnu.org; Sat, 29 Jun 2013 10:06:12 -0400 Message-ID: <51CEE9C8.4010002@suse.de> Date: Sat, 29 Jun 2013 16:06:00 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1372513521-10050-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1372513521-10050-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] RFCv3 kvm irqfd: support msimessage to irq translation in PHB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Anthony Liguori , "Michael S . Tsirkin" , Jan Kiszka , Alexander Graf , qemu-devel@nongnu.org, Alex Williamson , qemu-ppc@nongnu.org, Paolo Bonzini , Paul Mackerras , David Gibson Am 29.06.2013 15:45, schrieb Alexey Kardashevskiy: > On PPC64 systems MSI Messages are translated to system IRQ in a PCI > host bridge. This is already supported for emulated MSI/MSIX but > not for irqfd where the current QEMU allocates IRQ numbers from > irqchip and maps MSIMessages to those IRQ in the host kernel. >=20 > The patch extends irqfd support in order to avoid unnecessary > mapping and reuse the one which already exists in a PCI host bridge. >=20 > Specifically, a map_msi callback is added to PCIBus and pci_bus_map_msi= () > to PCI API. The latter returns -1 if a specific PHB does not provide > with any trsnslation so the existing code will work. >=20 > Signed-off-by: Alexey Kardashevskiy >=20 > --- >=20 > Looks like we agreed that in general PHB is the right place for this, > not KVM, so I am trying again. >=20 > Probably something should be done to kvm_irqchip_update_msi_route() > as well but I do not really understand what exactly. Any suggestions? >=20 >=20 > --- > hw/misc/vfio.c | 7 +++++-- > hw/pci/pci.c | 13 +++++++++++++ > hw/ppc/spapr_pci.c | 6 ++++++ > hw/virtio/virtio-pci.c | 2 +- > include/hw/pci/pci.h | 4 ++++ > include/hw/pci/pci_bus.h | 1 + > include/sysemu/kvm.h | 2 +- > kvm-all.c | 7 ++++++- > 8 files changed, 37 insertions(+), 5 deletions(-) >=20 > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index 52fb036..59911bb 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -624,7 +624,9 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev,= unsigned int nr, > * Attempt to enable route through KVM irqchip, > * default to userspace handling if unavailable. > */ > - vector->virq =3D msg ? kvm_irqchip_add_msi_route(kvm_state, *msg) = : -1; > + > + vector->virq =3D msg ? > + kvm_irqchip_add_msi_route(kvm_state, vdev->pdev.bus, *msg)= : -1; Please don't access device-internal parent fields like ->pdev. To obtain a PCIDevice you can use PCI_DEVICE(vdev). But in this case isn't that just a complicated way to access the function's argument pdev? > if (vector->virq < 0 || > kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt, > vector->virq) < 0) { > @@ -792,7 +794,8 @@ retry: > * Attempt to enable route through KVM irqchip, > * default to userspace handling if unavailable. > */ > - vector->virq =3D kvm_irqchip_add_msi_route(kvm_state, msg); > + vector->virq =3D kvm_irqchip_add_msi_route(kvm_state, vdev->pd= ev.bus, > + msg); Ditto. > if (vector->virq < 0 || > kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interru= pt, > vector->virq) < 0) { > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 61b681a..543f172 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1240,6 +1240,19 @@ void pci_device_set_intx_routing_notifier(PCIDev= ice *dev, > dev->intx_routing_notifier =3D notifier; > } > =20 > +void pci_bus_set_map_msi_fn(PCIBus *bus, pci_map_msi_fn map_msi_fn) > +{ > + bus->map_msi =3D map_msi_fn; > +} > + > +int pci_bus_map_msi(PCIBus *bus, MSIMessage msg) > +{ > + if (bus->map_msi) { > + return bus->map_msi(bus, msg); > + } > + return -1; > +} > + > /* > * PCI-to-PCI bridge specification > * 9.1: Interrupt routing. Table 9-1 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 23dbc0e..bae4faf 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -486,6 +486,11 @@ static void spapr_msi_write(void *opaque, hwaddr a= ddr, > qemu_irq_pulse(xics_get_qirq(spapr->icp, irq)); > } > =20 > +static int spapr_msi_get_irq(PCIBus *bus, MSIMessage msg) > +{ > + return msg.data; > +} > + > static const MemoryRegionOps spapr_msi_ops =3D { > /* There is no .read as the read result is undefined by PCI spec *= / > .read =3D NULL, > @@ -657,6 +662,7 @@ static int spapr_phb_init(SysBusDevice *s) > =20 > sphb->lsi_table[i].irq =3D irq; > } > + pci_bus_set_map_msi_fn(bus, spapr_msi_get_irq); > =20 > return 0; > } > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index b070b64..06a4e13 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -481,7 +481,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIPr= oxy *proxy, > int ret; > =20 > if (irqfd->users =3D=3D 0) { > - ret =3D kvm_irqchip_add_msi_route(kvm_state, msg); > + ret =3D kvm_irqchip_add_msi_route(kvm_state, proxy->pci_dev.bu= s, msg); PCIDevice *pci_dev =3D PCI_DEVICE(proxy); and pci_dev->bus please. Andreas > if (ret < 0) { > return ret; > } > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 6ef1f97..8c1edd6 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -332,6 +332,7 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev); > typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level); > typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num); > typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); > +typedef int (*pci_map_msi_fn)(PCIBus *bus, MSIMessage msg); > =20 > typedef enum { > PCI_HOTPLUG_DISABLED, > @@ -375,6 +376,9 @@ bool pci_intx_route_changed(PCIINTxRoute *old, PCII= NTxRoute *new); > void pci_bus_fire_intx_routing_notifier(PCIBus *bus); > void pci_device_set_intx_routing_notifier(PCIDevice *dev, > PCIINTxRoutingNotifier notif= ier); > +void pci_bus_set_map_msi_fn(PCIBus *bus, pci_map_msi_fn map_msi_fn); > +int pci_bus_map_msi(PCIBus *bus, MSIMessage msg); > + > void pci_device_reset(PCIDevice *dev); > void pci_bus_reset(PCIBus *bus); > =20 > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h > index 66762f6..81efd2b 100644 > --- a/include/hw/pci/pci_bus.h > +++ b/include/hw/pci/pci_bus.h > @@ -16,6 +16,7 @@ struct PCIBus { > pci_set_irq_fn set_irq; > pci_map_irq_fn map_irq; > pci_route_irq_fn route_intx_to_irq; > + pci_map_msi_fn map_msi; > pci_hotplug_fn hotplug; > DeviceState *hotplug_qdev; > void *irq_opaque; > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index f404d16..1bf2abe 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -305,8 +305,8 @@ static inline void cpu_synchronize_post_init(CPUSta= te *cpu) > } > } > =20 > -int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg); > int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg= ); > +int kvm_irqchip_add_msi_route(KVMState *s, PCIBus *pbus, MSIMessage ms= g); > void kvm_irqchip_release_virq(KVMState *s, int virq); > =20 > int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n, int = virq); > diff --git a/kvm-all.c b/kvm-all.c > index 1f81cca..3b7710d 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1180,11 +1180,16 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessag= e msg) > return kvm_set_irq(s, route->kroute.gsi, 1); > } > =20 > -int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg) > +int kvm_irqchip_add_msi_route(KVMState *s, PCIBus *pbus, MSIMessage ms= g) > { > struct kvm_irq_routing_entry kroute; > int virq; > =20 > + virq =3D pci_bus_map_msi(pbus, msg); > + if (virq >=3D 0) { > + return virq; > + } > + > if (!kvm_gsi_routing_enabled()) { > return -ENOSYS; > } >=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