Re: [Qemu-arm] [PATCH 1/2] vfio/platform: make the vfio-platform device non abstract

[Sinan Kaya]

On 2/21/2017 9:59 AM, Auger Eric wrote:
> Hi Sinan,
> On 21/02/2017 15:45, Sinan Kaya wrote:
>> Hi Eric,
>> Thanks for the long anticipated patch.
>>
>> On 2/21/2017 8:18 AM, Eric Auger wrote:
>>> +    DEFINE_PROP_STRING("manufacturer", VFIOPlatformDevice, manufacturer),
>>> +    DEFINE_PROP_STRING("model", VFIOPlatformDevice, model),
>>
>> The OF string is not always of the format "manufacturer, model".
>>
>> HIDMA for instance uses a syntax of "qcom, hidma-1.0" and "qcom, hidma-2.0".
>> This syntax is used for cases where an IP gets used in multiple SOCs. 
>>
>> Maybe, we should call it just compatible-string and try not to make meaning
>> out of it.
>>
>> Sinan
>>
> 
> in  http://www.devicetree.org/specifications-pdf I found the description
> at the bottom.
> 
> I your case what would prevent from creating 2 entries in the table and
> then use either in qemu cmd line?
> 

I see your point. You are saying that hidma-1.0 would be the model and qcom
would be the manufacturer. 

This should work.

However, I have more concerns below.

> VFIO_PLATFORM_BINDING("qcom", "hidma-1.0", add_amd_hidma_fdt_node),
> VFIO_PLATFORM_BINDING("qcom", "hidma-2.0", add_amd_hidma_fdt_node),
> 
> The issue is the comma in the property is interpreted as a separator for
> options, hence that choice of having 2 separate options.

Note that the goal is to be able to instantiate any platform device by just
passing a compatible string and the object from the host. We don't want
to make changes to QEMU every time somebody comes up with a new platform device.
This step is unnecessary. 

A general purpose solution should be able to passthrough any platform device.

I have AHCI compatible SATA drive and HIDMA use cases for this purposes. The
code should be able to probe the given device with the compatible string without
requiring any changes to QEMU.

I didn't see this in the code. Maybe, it is just because I'm QEMU code
illiterate.

I shouldn't need to submit a patch to QEMU to add these two lines.

VFIO_PLATFORM_BINDING("qcom", "hidma-1.0", add_qcom_hidma_fdt_node),
VFIO_PLATFORM_BINDING("qcom", "hidma-2.0", add_qcom_hidma_fdt_node),


> 
> Besides I will fix the CONFIG_LINUX compilation issue asap.
> 
> Thanks
> 
> Eric
> 
> "
> The recommended format is "manufacturer,model", where manufacturer is a
> string describing the name of the manufacturer (such as a stock ticker
> symbol), and model specifies the model number.
> 
> Example:
> compatible = "fsl,mpc8641-uart", "ns16550";
> In this example, an operating system would first try to locate a device
> driver that supported
> fsl,mpc8641-uart. If a driver was not found, it would then try to locate
> a driver that supported the more general ns16550 device type.
> "
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.