Re: [Qemu-devel] [PATCH v2] hw/vfio/platform: Add Qualcomm Technologies, Inc HIDMA device support

[Alexander Graf]

> On 18 Aug 2016, at 05:37, Auger Eric <address@hidden> wrote:
> 
> Hi Shanker,
> 
> Adding Alex in CC for (*)
> 
> On 14/08/2016 17:42, Shanker Donthineni wrote:
>> This patch introduces the Qualcomm Technologies, Inc HIDMA device and
>> allows passthrough the host HIDMA device to a guest machine using the
>> vfio-platform framework.
>> 
>> A platform device tree node is created for the guest machine which
>> contains the compat string 'qcom,hidma-1.0', mmio regions, active high
>> SPIs, and an optional property dma-coherent.
>> 
>> Signed-off-by: Vikram Sethi <address@hidden>
>> Signed-off-by: Shanker Donthineni <address@hidden>
>> ---
>> Changes sicnce v1:
>>  combined patches [v1 1/2] and [v1 2/2].
> Some general comments:
> - I preferred the previous series organization where we had the creation
> of the VFIO device first and its sysbus-fdt dynamic instantiation in a
> separate patch.
> 
> Peter requested sysbus-fdt stops growing and advised to split the fine
> into generic helpers and specific dt creation functions in separate
> files. This sounds the right moment to do it with looming new VFIO devices.
> 
> (*) Also I am now reconsidering the relevance of creating specific VFIO
> devices per compat string. At the begining of VFIO QEMU integration
> history we made that choice, advised by Alex (Graf), considering the
> QEMU VFIO device could not be generic. With a little more experience now
> we could see the specialization is currently done in the dt creation
> function (sysbus-fdt) and in the kernel reset module. So I would now
> advocate using a non abstract base VFIO device that could be
> instantiated passing its compat string as property. Creating
> hw/vfio/qcom-hidma.c and include/hw/vfio/vfio-qcom-hidma.h then would
> become useless. Alex, what is your feeling now?

Back when we set up the rule we were concerned with a few things that a generic 
sysbus devices can’t implement properly:

  * generic reset
  * power control
  * clock control

I guess the first two are mostly a matter of the in-kernel driver, not the user 
space one. Clock control hopefully isn’t much of a thing with real hardware ;).

So yes, if you can make a generic driver work, I’m not opposed.


Alex