[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api s
From: |
Kirti Wankhede |
Subject: |
Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags |
Date: |
Fri, 21 Oct 2016 08:30:53 +0530 |
On 10/21/2016 2:52 AM, Alex Williamson wrote:
> On Fri, 21 Oct 2016 02:44:37 +0530
> Kirti Wankhede <address@hidden> wrote:
>
...
>>>>>>
>>>>>> +extern const char *vfio_device_api_string(u32 flags);
>>>>>> +
>>>>>> struct pci_dev;
>>>>>> #ifdef CONFIG_EEH
>>>>>> extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
>>>>>
>>>>> Couldn't this simply be a #define in the uapi header?
>>>>>
>>>>> #define VFIO_DEVICE_PCI_API_STRING "vfio-pci"
>>>>>
>>>>> I don't really see why we need a lookup function.
>>>>>
>>>>
>>>> String is tightly coupled with the FLAG, right?
>>>> Instead user need to take care of making sure to return proper string,
>>>> and don't mis-match the string, I think having function is easier.
>>>
>>> That's exactly why I proposed putting the #define string in the uapi,
>>> by that I mean the vfio uapi header. That keeps the tight coupling to
>>> the flag, they're both defined in the same place, plus it gives
>>> userspace a reference so they're not just inventing a string to compare
>>> against. IOW, the vendor driver simply does an sprintf of
>>> VFIO_DEVICE_PCI_API_STRING and userspace (ie. libvirt) can do a strcmp
>>> with VFIO_DEVICE_PCI_API_STRING from the same header and everybody
>>> arrives at the same result.
>>>
>>>> Vendor driver should decide the type of device they want to expose and
>>>> set the flag, using this function vendor driver would return string
>>>> which is based on flag they set.
>>>
>>> Being a function adds no intrinsic value and being in a uapi header does
>>> add value to userspace. Thanks,
>>>
>>
>> Ok. The strings should be in uapi, but having function (like below) to
>> return proper string based on flag would be good to have for vendor driver.
>>
>> +const char *vfio_device_api_string(u32 flags)
>> +{
>> + if (flags & VFIO_DEVICE_FLAGS_PCI)
>> + return VFIO_DEVICE_API_PCI_STRING;
>> +
>> + if (flags & VFIO_DEVICE_FLAGS_PLATFORM)
>> + return VFIO_DEVICE_API_PLATFORM_STRING;
>> +
>> + if (flags & VFIO_DEVICE_FLAGS_AMBA)
>> + return VFIO_DEVICE_API_AMBA_STRING;
>> +
>> + return "";
>> +}
>> +EXPORT_SYMBOL(vfio_device_api_string);
>
> I disagree, it's pointless maintenance overhead. It's yet another
> function that we need to care about for kABI and it offers almost no
> value. Thanks,
>
If any vendor driver sets VFIO_DEVICE_FLAGS_PLATFORM flag but sets
VFIO_DEVICE_API_PCI_STRING, we don't have a way to verify this in kernel
driver. Is that acceptable?
Kirti
- Re: [Qemu-devel] [PATCH v9 05/12] vfio: Introduce common function to add capabilities, (continued)
- [Qemu-devel] [PATCH v9 07/12] vfio: Introduce vfio_set_irqs_validate_and_prepare(), Kirti Wankhede, 2016/10/17
- [Qemu-devel] [PATCH v9 09/12] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare(), Kirti Wankhede, 2016/10/17
- [Qemu-devel] [PATCH v9 08/12] vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare(), Kirti Wankhede, 2016/10/17
- [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Kirti Wankhede, 2016/10/17
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Alex Williamson, 2016/10/20
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Kirti Wankhede, 2016/10/20
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Alex Williamson, 2016/10/20
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Kirti Wankhede, 2016/10/20
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Alex Williamson, 2016/10/20
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags,
Kirti Wankhede <=
- Re: [Qemu-devel] [PATCH v9 10/12] vfio: Add function to get device_api string from vfio_device_info.flags, Alex Williamson, 2016/10/20
[Qemu-devel] [PATCH v9 11/12] docs: Add Documentation for Mediated devices, Kirti Wankhede, 2016/10/17
[Qemu-devel] [PATCH v9 12/12] docs: Sample driver to demonstrate how to use Mediated device framework., Kirti Wankhede, 2016/10/17
Re: [Qemu-devel] [PATCH v9 00/12] Add Mediated device support, Alex Williamson, 2016/10/17
Re: [Qemu-devel] [PATCH v9 00/12] Add Mediated device support, Jike Song, 2016/10/24