On 26.11.14 11:48, Eric Auger wrote:
> On 11/26/2014 11:24 AM, Alexander Graf wrote:
>>
>>
>> On 26.11.14 10:45, Eric Auger wrote:
>>> On 11/05/2014 11:29 AM, Alexander Graf wrote:
>>>>
>>>>
>>>> On 31.10.14 15:05, Eric Auger wrote:
>>>>> Minimal VFIO platform implementation supporting
>>>>> - register space user mapping,
>>>>> - IRQ assignment based on eventfds handled on qemu side.
>>>>>
>>>>> irqfd kernel acceleration comes in a subsequent patch.
>>>>>
>>>>> Signed-off-by: Kim Phillips <kim.phill...@linaro.org>
>>>>> Signed-off-by: Eric Auger <eric.au...@linaro.org>
>>
>> [...]
>>
>>>>> +/*
>>>>> + * Mechanics to program/start irq injection on machine init done
>>>>> notifier:
>>>>> + * this is needed since at finalize time, the device IRQ are not yet
>>>>> + * bound to the platform bus IRQ. It is assumed here dynamic
>>>>> instantiation
>>>>> + * always is used. Binding to the platform bus IRQ happens on a machine
>>>>> + * init done notifier registered by the machine file. After its execution
>>>>> + * we execute a new notifier that actually starts the injection. When
>>>>> using
>>>>> + * irqfd, programming the injection consists in associating eventfds to
>>>>> + * GSI number,ie. virtual IRQ number
>>>>> + */
>>>>> +
>>>>> +typedef struct VfioIrqStarterNotifierParams {
>>>>> + unsigned int platform_bus_first_irq;
>>>>> + Notifier notifier;
>>>>> +} VfioIrqStarterNotifierParams;
>>>>> +
>>>>> +typedef struct VfioIrqStartParams {
>>>>> + PlatformBusDevice *pbus;
>>>>> + int platform_bus_first_irq;
>>>>> +} VfioIrqStartParams;
>>>>> +
>>>>> +/* Start injection of IRQ for a specific VFIO device */
>>>>> +static int vfio_irq_starter(SysBusDevice *sbdev, void *opaque)
>>>>> +{
>>>>> + int i;
>>>>> + VfioIrqStartParams *p = opaque;
>>>>> + VFIOPlatformDevice *vdev;
>>>>> + VFIODevice *vbasedev;
>>>>> + uint64_t irq_number;
>>>>> + PlatformBusDevice *pbus = p->pbus;
>>>>> + int platform_bus_first_irq = p->platform_bus_first_irq;
>>>>> +
>>>>> + if (object_dynamic_cast(OBJECT(sbdev), TYPE_VFIO_PLATFORM)) {
>>>>> + vdev = VFIO_PLATFORM_DEVICE(sbdev);
>>>>> + vbasedev = &vdev->vbasedev;
>>>>> + for (i = 0; i < vbasedev->num_irqs; i++) {
>>>>> + irq_number = platform_bus_get_irqn(pbus, sbdev, i)
>>>>> + + platform_bus_first_irq;
>>>>> + vfio_start_irq_injection(sbdev, i, irq_number);
>>>>> + }
>>>>> + }
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/* loop on all VFIO platform devices and start their IRQ injection */
>>>>> +static void vfio_irq_starter_notify(Notifier *notifier, void *data)
>>>>> +{
>>>>> + VfioIrqStarterNotifierParams *p =
>>>>> + container_of(notifier, VfioIrqStarterNotifierParams, notifier);
>>>>> + DeviceState *dev =
>>>>> + qdev_find_recursive(sysbus_get_default(),
>>>>> TYPE_PLATFORM_BUS_DEVICE);
>>>>> + PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(dev);
>>>>> +
>>>>> + if (pbus->done_gathering) {
>>>>> + VfioIrqStartParams data = {
>>>>> + .pbus = pbus,
>>>>> + .platform_bus_first_irq = p->platform_bus_first_irq,
>>>>> + };
>>>>> +
>>>>> + foreach_dynamic_sysbus_device(vfio_irq_starter, &data);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/* registers the machine init done notifier that will start VFIO IRQ */
>>>>> +void vfio_register_irq_starter(int platform_bus_first_irq)
>>>>> +{
>>>>> + VfioIrqStarterNotifierParams *p =
>>>>> g_new(VfioIrqStarterNotifierParams, 1);
>>>>> +
>>>>> + p->platform_bus_first_irq = platform_bus_first_irq;
>>>>> + p->notifier.notify = vfio_irq_starter_notify;
>>>>> + qemu_add_machine_init_done_notifier(&p->notifier);
>>>>
>>>> Could you add a notifier for each device instead? Then the notifier
>>>> would be part of the vfio device struct and not some dangling random
>>>> pointer :).
>>>>
>>>> Of course instead of foreach_dynamic_sysbus_device() you would directly
>>>> know the device you're dealing with and only handle a single device per
>>>> notifier.
>>>
>>> Hi Alex,
>>>
>>> I don't see how to practically follow your request:
>>>
>>> - at machine init time, VFIO devices are not yet instantiated so I
>>> cannot call foreach_dynamic_sysbus_device() there - I was definitively
>>> wrong in my first reply :-().
>>>
>>> - I can't register a per VFIO device notifier in the VFIO device
>>> finalize function because this latter is called after the platform bus
>>> instantiation. So the IRQ binding notifier (registered in platform bus
>>> finalize fn) would be called after the IRQ starter notifier.
>>>
>>> - then to simplify things a bit I could use a qemu_register_reset in
>>> place of a machine init done notifier (would relax the call order
>>> constraint) but the problem consists in passing the platform bus first
>>> irq (all the more so you requested it became part of a const struct)
>>>
>>> Do I miss something?
>>
>> So the basic idea is that the device itself calls
>> qemu_add_machine_init_done_notifier() in its realize function. The
>> Notifier struct would be part of the device state which means you can
>> cast yourself into the VFIO device state.
>
> humm, the vfio device is instantiated in the cmd line so after the
> machine init. This means 1st the platform bus binding notifier is
> registered (in platform bus realize) and then VFIO irq starter notifiers
> are registered (in VFIO realize). Notifiers beeing executed in the
> reverse order of their registration, this would fail. Am I wrong?
Bleks. Ok, I see 2 ways out of this:
1) Create a TailNotifier and convert the machine_init_done notifiers
to this
2) Add an "irq now populated" notifier function callback in a new
PlatformBusDeviceClass struct that you use to describe the
PlatformBusDevice class. Call all children's notifiers from the
machine_init notifier in the platform bus.
The more I think about it, the more I prefer option 2 I think.
Alex
