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

[Eric Auger]

On 11/27/2014 06:51 PM, Alexander Graf wrote:
> 
> 
> 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?
yes in 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 :).
OK I suggest I do my next patch as is and you will tell me... it will be
easy to revert to machine prop anyway.

Thanks for your time!

Eric
> 
> 
> Alex
>