On Wed, Jan 03, 2024 at 01:52:02PM -0400, Jason Gunthorpe wrote:
On Wed, Jan 03, 2024 at 09:06:23AM -0800, Nicolin Chen wrote:
On Wed, Jan 03, 2024 at 12:58:48PM -0400, Jason Gunthorpe wrote:
On Wed, Jan 03, 2024 at 08:48:46AM -0800, Nicolin Chen wrote:
You can pass the ctx to the invalidate op, it is already implied because the passed iommu_domain is linked to a single iommufd ctx.
The device virtual id lookup API needs something similar, yet it likely needs a viommu pointer (or its id) instead? As the table is attached to a viommu while an ictx can have multiple viommus, right?
Yes, when we get to an API for that it will have to be some op 'invalidate_viommu(..)' and it can get the necessary pointers.
OK! I will try that first.
The viommu object will have to be some driver object like the iommu_domain.
I drafted something like this, linking it to struct iommu_device:
+struct iommufd_viommu {
struct iommufd_object obj;struct iommufd_ctx *ictx;struct iommu_device *iommu_dev;struct iommufd_hwpt_paging *hwpt;/* array of struct iommufd_device, indexed by device virtual id */struct xarray device_ids;+};
The driver would have to create it and there would be some driver specific enclosing struct to go with it
Perhaps device_ids goes in the driver specific struct, I don't know.
+struct iommufd_viommu { + struct iommufd_object obj; + struct iommufd_ctx *ictx; + struct iommu_device *iommu_dev; + struct iommufd_hwpt_paging *hwpt; /* maybe unneeded */ + + int vmid; + + union iommu_driver_user_data { + struct iommu_driver_user_vtd; + struct iommu_driver_user_arm_smmuv3; + struct iommu_driver_user_amd_viommu; + }; +};
Then iommu_ops would need something like: iommu_user_alloc/free(struct iommu_device *iommu_dev, union *iommu_driver_user_data, int *vmid); iommu_user_set/unset_dev_id(union iommu_driver_user_data, struct device* dev. u32/u64 id); iommu_user_invalidate(union iommu_driver_user_data *iommu, struct iommu_user_data_array *array);
Comments and ideas on better naming convention?
Not sure it should have hwpt at all, probably vmid should come from the driver specific struct in some driver specific way
The idea having a hwpt pointer is to share the paging hwpt among the viommu objects. I don't think it "shouldn't have", yet likely we can avoid it depending on whether it will have some use or not in the context.
Nicolin