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

[Alexander Graf <agraf@suse.de>]

On 27.11.14 18:34, Eric Auger wrote:
> On 11/27/2014 06:24 PM, Alexander Graf wrote:
>>
>>
>> On 27.11.14 18:13, Eric Auger wrote:
>>> On 11/27/2014 04:55 PM, Alexander Graf wrote:
>>>>
>>>>
>>>> On 27.11.14 16:28, Alexander Graf wrote:
>>>>>
>>>>>
>>>>> On 27.11.14 16:14, Eric Auger wrote:
>>>>>> On 11/27/2014 03:35 PM, Alexander Graf wrote:
>>>>>>>
>>>>>>>

[...]

>>>>>
>>>>> That should be easy - make it a link property. In fact, this would be
>>>>> one of those cases where not generalizing the code would've been a good
>>>>> idea.
>>> In that case the machine (init done) callback would be used to pass the
>>> vgic handle to each vfio device. Registered by the machine file, isn't
>>> it. Aren't we exactly at the same state you wanted to improve initially
>>> where the notifier is registered by the machine file, not belonging to
>>> the VFIO device, just replacing first_irq param by vgic_handle which
>>> eventually ends up as a link.
>>>
>>> This notifier still cannot be registered by the VFIO device finalize fn
>>> since the VFIO device has no handle to the interrupt controller. kind of
>>> chicken & egg problem.
>>>>>
>>>>> If device creation would live in the machine file, the machine could
>>>>> automatically set the link. Maybe you can still get there somehow? You
>>>>> could add a machine callback in the device allocation function.
>>>>
>>>> If this gets too messy, I think doing a machine attribute would work as
>>>> well here. Check out the way we pass the e500-ccsr object on e500:
>>>>
>>>>
>>>> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/pci-host/ppce500.c;h=1b4c0f00236e8005c261da527d416fe6a053b353;hb=HEAD#l337
>>>>
>>>>
>>>> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;h=2832fc0da444d89737768f7c4dcb0638e2625750;hb=HEAD#l873
>>>
>>> looks OK indeed
>>>>
>>>> I think doing an actual link would be cleaner, but at least the above
>>>> gets you to an acceptable state that can still be improved with links
>>>> later - the basic idea is the same :).
>>>
>>>
>>> and why not "simply" a qemu_register_reset passing the vgic handle as
>>> opaque.
>>
>> Who would register this reset callback? It'd have to be someone who
>> knows both the VFIO device as well as the vGIC device.
> the machine file would. reset callback implemented in vfio-platform.c,
> looping on all instances. ~ as today for the notifier but without the
> dangling pointer. not sure you will like it though ;-)

Ah, so you would do the actual VFIO call inside the machine file? Or
would you call a VFIO function when you see that a device is VFIO and
trigger the connection at that point? That would work too I suppose.

>>
>> The reset idea could work as replacement for the notifier though. So you
>> could have the VFIO device register a reset callback in which it asks
>> the vgic for the number and registers the IRQ with KVM.
> arghh, still the problem of passing the vgic handle. I used the reset cb
> registration by the machine file to do that. Of course if we use your
> machine property trick we can do the registration by the VFIO driver
> itself.

Yup, either way works IMHO :).


Alex