On 2023/10/14 01:56, Nicolin Chen wrote:
On Fri, Oct 13, 2023 at 11:04:56AM -0300, Jason Gunthorpe wrote:
On Fri, Oct 13, 2023 at 12:33:13PM +0800, Yi Liu wrote:
not really. Below the users of the struct iommu_user_data in my current
iommufd_nesting branch. Only the domain_alloc_user op has type as there
can be multiple vendor specific alloc data types. Basically, I'm ok to
make the change you suggested, just not sure if it is good to add type
as it is only needed by one path.
I don't think we should ever have an opaque data blob without a type
tag..
I can add those "missing" data types, and then a driver will be
responsible for sanitizing the type along with the data_len.
I notice that the enum iommu_hwpt_data_type in the posted patch
is confined to the alloc_user uAPI. Perhaps we should share it
with invalidate too:
invalidation path does not need a type field today as the data
type is vendor specific, vendor driver should know the data type
when calls in.
/**
* enum iommu_hwpt_data_type - IOMMU HWPT Data Type
* @IOMMU_HWPT_DATA_NONE: no data
* @IOMMU_HWPT_DATA_VTD_S1: Intel VT-d stage-1 page table
* @IOMMU_HWPT_DATA_ARM_SMMUV3: ARM SMMUv3 Context Descriptor Table
*/
enum iommu_hwpt_data_type {
IOMMU_HWPT_DATA_NONE,
IOMMU_HWPT_DATA_VTD_S1,
IOMMU_HWPT_DATA_ARM_SMMUV3,
};
Though inevitably we'd have to define a separate data group for
things like set_dev_data that is related to idev v.s. hwpt:
yes, the type field is in separate data group per path.
// IOMMU_DEV_DATA_TYPE sounds like an IOMMU device, other than a
// passthrough device, so renaming to "_IDEV_" here. And perhaps
// "set_dev_data" could be "set_idev_data" too? Any better name?
/**
* enum iommu_idev_data_type - Data Type for a Device behind an IOMMU
* @IOMMU_IDEV_DATA_NONE: no data
* @IOMMU_IDEV_DATA_ARM_SMMUV3: ARM SMMUv3 specific device data
*/
enum iommu_idev_data_type {
IOMMU_IDEV_DATA_NONE,
IOMMU_IDEV_DATA_ARM_SMMUV3,
};
/**
* struct iommu_idev_data_arm_smmuv3 - ARM SMMUv3 specific device data
* @sid: The Stream ID that is assigned in the user space
*
* The SMMUv3 specific user space data for a device that is behind an SMMU HW.
* The guest-level user data should be linked to the host-level kernel data,
* which will be used by user space cache invalidation commands.
*/
struct iommu_idev_data_arm_smmuv3 {
__u32 sid;
};
/**
* struct iommu_set_idev_data - ioctl(IOMMU_SET_IDEV_DATA)
* @size: sizeof(struct iommu_set_idev_data)
* @dev_id: The device to set an iommu specific device data
* @data_uptr: User pointer of the device user data
* @data_len: Length of the device user data
*
* The device data must be unset using ioctl(IOMMU_UNSET_IDEV_DATA), before
* another ioctl(IOMMU_SET_IDEV_DATA) call or before the device itself gets
* unbind'd from the iommufd context.
*/
struct iommu_set_idev_data {
__u32 size;
__u32 dev_id;
__aligned_u64 data_uptr;
__u32 data_len;
};
Thanks
Nic
--
Regards,
Yi Liu
