lkml.org 
[lkml]   [2023]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 2/3] vfio: add bus master feature to device feature ioctl
From


On 8/4/2023 3:30 AM, Alex Williamson wrote:
> On Thu, 3 Aug 2023 20:02:52 +0530
> Nipun Gupta <nipun.gupta@amd.com> wrote:
>
>> add bus mastering control to VFIO_DEVICE_FEATURE IOCTL. The VFIO user
>> can use this feature to enable or disable the Bus Mastering of a
>> device bound to VFIO.
>>
>> Co-developed-by: Shubham Rohila <shubham.rohila@amd.com>
>> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
>> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
>>
>> Changes in v5:
>> - This patch is newly added which proposes a new flag
>> VFIO_DEVICE_FEATURE_BUS_MASTER in VFIO_DEVICE_FEATURE IOCTL.
>>
>> ---
>> include/uapi/linux/vfio.h | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index 20c804bdc09c..05350a2f1eab 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -1287,6 +1287,22 @@ struct vfio_device_feature_mig_data_size {
>>
>> #define VFIO_DEVICE_FEATURE_MIG_DATA_SIZE 9
>>
>> +/**
>> + * Upon VFIO_DEVICE_FEATURE_SET, allow the BUS mastering for the device to be
>> + * set or clear based on the operation specified in op flag.
>> + *
>> + * If the BUS MASTER of the device is configured to CLEAR,
>> + * all the incoming DMA from the device will be blocked.
>> + * If the BUS MASTER of the device is configured to SET (enable),
>> + * device would be able to do DMA to host memory.
>
> CDX is clearly not the only bus that has the concept of controlling a
> device's ability to perform DMA, so I'm concerned about this
> description. For example someone with no prior vfio-pci experience
> might be confused reading the uAPI and then not having support for this
> feature in vfio-pci.
>
> One option would be to make this CDX specific through the name, ie.
> VFIO_DEVICE_FEATURE_CDX_BUS_MASTER, but maybe it's sufficient to leave
> it general but explain here that this is only implemented for devices
> which require bus master control and have no means in the in-band
> device interface to provide that control. Going on to state that PCI
> bus master is controlled in-band through config space and that this is
> initially only provided for CDX devices would be useful. Of course the
> PROBE interface can be used to determine if this is available for a
> given device.

Agreed. Will update the comment to reflect the same.

>
>> + */
>> +struct vfio_device_feature_bus_master {
>> + __u32 op;
>> +#define VFIO_DEVICE_FEATURE_SET_MASTER 0 /* Set Bus Master */
>> +#define VFIO_DEVICE_FEATURE_CLEAR_MASTER 1 /* Clear Bus Master */
>
> Ultimately it doesn't matter, but this is a strange choice, set = 0,
> clear = 1. I think the reverse would be better for raw debugging.

Yes, makes sense to use 1 for set and 0 for clear. Will fix in next respin.

Thanks,
Nipun

> Thanks,
>
> Alex
>
>> +};
>> +#define VFIO_DEVICE_FEATURE_BUS_MASTER 9
>> +
>> /* -------- API for Type1 VFIO IOMMU -------- */
>>
>> /**
>

\
 
 \ /
  Last update: 2023-08-08 18:29    [W:0.055 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site