qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 09/16] hw/vfio/platform: add vfio-platform su


From: Eric Auger
Subject: Re: [Qemu-devel] [PATCH v7 09/16] hw/vfio/platform: add vfio-platform support
Date: Thu, 27 Nov 2014 15:05:18 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

On 11/26/2014 03:46 PM, Eric Auger wrote:
> 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 <address@hidden>
>>>>>>> Signed-off-by: Eric Auger <address@hidden>
>>>>
>>>> [...]
>>>>
>>>>>>> +/*
>>>>>>> + * 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)

Hi Alex,

I believe I understand your proposal but the issue is to pass the
platform bus first_irq parameter which is needed to compute the absolute
IRQ number (=irqfd GSI). VFIO device does not have this info. Platform
bus doesn't have it either. Only machine file has the info.

The "irq now populated" notifier function callback would be called in
platform bus platform_bus_init_notify or link_sysbus_device I guess,
already executed in a machine-init-done notifier. The callback would
need to be called with sbdev and first_irq param to fulfill its task
(check of VFIO type, IRQFD setup). So I need to pass first_irq to
platform_bus. Do you agree? Can I add an API?

Besides there would be a single callback per platform bus. Wouldn't it
be worth to add an infrastructure to add/remove misc "binding_done"
notifiers and call all registered functions in link_sysbus_device? This
does not change the issue of passing the first_irq param ;-)

Eric

> 
> Thanks for your guidance
> 
> Eric
>>
>>
>> Alex
>>
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]