From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Auger Subject: Re: [PATCH v12 4/9] hw/vfio/platform: add capability to start IRQ propagation Date: Fri, 17 Apr 2015 17:31:29 +0200 Message-ID: <55312751.1000506@linaro.org> References: <1426785402-2091-1-git-send-email-eric.auger@linaro.org> <1426785402-2091-5-git-send-email-eric.auger@linaro.org> <1429221884.10086.38.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 62B2F4DF0C for ; Fri, 17 Apr 2015 11:26:09 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hky54DIMMvLt for ; Fri, 17 Apr 2015 11:26:08 -0400 (EDT) Received: from mail-wg0-f54.google.com (mail-wg0-f54.google.com [74.125.82.54]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 108B34DF0B for ; Fri, 17 Apr 2015 11:26:07 -0400 (EDT) Received: by wgyo15 with SMTP id o15so117229851wgy.2 for ; Fri, 17 Apr 2015 08:34:04 -0700 (PDT) In-Reply-To: <1429221884.10086.38.camel@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Alex Williamson Cc: eric.auger@st.com, patches@linaro.org, qemu-devel@nongnu.org, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu List-Id: kvmarm@lists.cs.columbia.edu Hi Alex, On 04/17/2015 12:04 AM, Alex Williamson wrote: > On Thu, 2015-03-19 at 17:16 +0000, Eric Auger wrote: >> Add a reset notify function that enables to start the propagation of >> interrupts to the guest. >> >> Signed-off-by: Eric Auger >> >> --- >> v10 -> v11: >> - comment reword >> >> v8 -> v9: >> - handle failure in vfio_irq_starter >> --- >> hw/vfio/platform.c | 64 +++++++++++++++++++++++++++++++++++++++++ >> include/hw/vfio/vfio-platform.h | 8 ++++++ >> 2 files changed, 72 insertions(+) >> >> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c >> index 31c2701..361e01b 100644 >> --- a/hw/vfio/platform.c >> +++ b/hw/vfio/platform.c >> @@ -572,6 +572,70 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp) >> } >> } >> >> +/** >> + * vfio_irq_starter: Start IRQ forwarding for a specific VFIO device >> + * @sbdev: Sysbus device handle >> + * @opaque: DeviceState handle of the interrupt controller the device >> + * is attached to >> + * >> + * The function retrieves the absolute GSI number each VFIO IRQ >> + * corresponds to and start forwarding. >> + */ > > Using "forwarding" here is really making me look for your platform's EOI > trick (IRQ Forwarding), which isn't here. sure > >> +static int vfio_irq_starter(SysBusDevice *sbdev, void *opaque) >> +{ >> + DeviceState *intc = (DeviceState *)opaque; >> + VFIOPlatformDevice *vdev; >> + VFIOINTp *intp; >> + qemu_irq irq; >> + int gsi, ret; >> + >> + if (object_dynamic_cast(OBJECT(sbdev), TYPE_VFIO_PLATFORM)) { >> + vdev = VFIO_PLATFORM_DEVICE(sbdev); >> + >> + QLIST_FOREACH(intp, &vdev->intp_list, next) { >> + gsi = 0; >> + while (1) { >> + irq = qdev_get_gpio_in(intc, gsi); >> + if (irq == intp->qemuirq) { >> + break; >> + } >> + gsi++; >> + } > > A for() loop would be more compact here, but is there some other exit > condition we can add? gsi < INT_MAX? Currently I do not see any way to retrieve the number of input GPIOs. This is static in core/qdev.c. either I leave as is or introduce getters. > >> + intp->virtualID = gsi; >> + ret = vdev->start_irq_fn(intp); >> + if (ret) { >> + error_report("%s unable to start propagation of IRQ index %d", >> + vdev->vbasedev.name, intp->pin); >> + exit(1); >> + } >> + } >> + } >> + return 0; >> +} >> + >> +/** >> + * vfio_kick_irqs: start IRQ forwarding for all the VFIO devices >> + * attached to the platform bus >> + * @data: Device state handle of the interrupt controller the VFIO IRQs >> + * must be bound to >> + * >> + * Binding to the platform bus IRQs happens on a machine init done >> + * notifier registered by the platform bus. Only at that time the >> + * absolute virtual IRQ = GSI is known and allows to setup IRQFD. >> + * This function typically can be called in a reset notifier registered >> + * by the machine file. >> + */ > > So there's not actually an irqfd setup done here yet and what is > currently done by the above starter function doesn't depend at all on > the GSI. Do you perhaps want to setup the vfio->eventfd signaling > during your initfn and then only connect the eventfd->irqfd to KVM in > the reset function? Sort of how vfio-pci does the routing update. Not sure I get what you mean here. In above starter I call start_irq_fn. This latter starts the injection depending on the chosen technique, 1) user side handling, 2) standalone irqfd, 2) irqfd + ARM forwarding. For starting 2) and 3) I actually use the GSI set above by intp->virtualID = gsi; See vfio_start_irqfd_injection. Indeed I noticed in VFIO-PCI you first started eventfd and then irqfd but I thought I did not need to do that. What is the problem starting VFIO signaling on reset (notifier) only? Besides, moving back to 2-step settup would not fix my issue of being able to know the gsi to attach to my irqfd. I need to further investigate this PCI INTx routing notifier... Best Regards Eric > >> +void vfio_kick_irqs(void *data) >> +{ >> + DeviceState *intc = (DeviceState *)data; >> + DeviceState *dev = >> + qdev_find_recursive(sysbus_get_default(), TYPE_PLATFORM_BUS_DEVICE); >> + PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(dev); >> + >> + assert(pbus->done_gathering); >> + foreach_dynamic_sysbus_device(vfio_irq_starter, intc); >> +} >> + >> static const VMStateDescription vfio_platform_vmstate = { >> .name = TYPE_VFIO_PLATFORM, >> .unmigratable = 1, >> diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h >> index 31893a3..c9ee898 100644 >> --- a/include/hw/vfio/vfio-platform.h >> +++ b/include/hw/vfio/vfio-platform.h >> @@ -77,4 +77,12 @@ typedef struct VFIOPlatformDeviceClass { >> #define VFIO_PLATFORM_DEVICE_GET_CLASS(obj) \ >> OBJECT_GET_CLASS(VFIOPlatformDeviceClass, (obj), TYPE_VFIO_PLATFORM) >> >> +/** >> + * vfio_kick_irqs - reset notifier that starts IRQ injection >> + * for all VFIO dynamic sysbus devices attached to the platform bus. >> + * >> + * @opaque: handle to the interrupt controller DeviceState >> + */ >> +void vfio_kick_irqs(void *opaque); >> + >> #endif /*HW_VFIO_VFIO_PLATFORM_H*/ > > >