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

[Auger Eric]

Hi Sinan,

On 21/02/2017 16:10, Sinan Kaya wrote:
> 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.

Well we need to generate a device tree node for your device. We have not
proven yet the function that creates this latter can be generic. There
are some helpers that can read the host dt node, copy some properties
from the host to the guest. But still at the moment we can't make things
as generic as PCIe and this is why the platform passthrough still is
controversial. So this patch aims at lowering the integration efforts
but minimal specialization needs to be done in sysbus-fdt. Same on
kernel side with the reset module.

Thanks

Eric


> 
> 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.
>> "
>>
> 
>