On 11/26/2014 12:20 PM, Alexander Graf wrote:
>
>
> 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.phillips@xxxxxxxxxx>
>>>>>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
>>>
>>> [...]
>>>
>>>>>> +/*
>>>>>> + * 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.
Hi Alex,
ok I work on 2)
Thanks for your guidance
Eric
>
>
> Alex
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
