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 <eric.au...@linaro.org>
>>
>> ---
>> 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*/
>
>
>
