From: Jason Gunthorpe jgg@nvidia.com Sent: Wednesday, August 2, 2023 9:48 PM
On Wed, Aug 02, 2023 at 07:41:05AM +0000, Tian, Kevin wrote:
+/**
- struct iommu_hwpt_vtd_s1_invalidate - Intel VT-d cache invalidation
(IOMMU_HWPT_TYPE_VTD_S1)
- @flags: Must be 0
- @entry_size: Size in bytes of each cache invalidation request
- @entry_nr_uptr: User pointer to the number of invalidation requests.
Kernel reads it to get the number of requests and
updates the buffer with the number of requests that
have been processed successfully. This pointer must
point to a __u32 type of memory location.
- @inv_data_uptr: Pointer to the cache invalidation requests
- The Intel VT-d specific invalidation data for a set of cache invalidation
- requests. Kernel loops the requests one-by-one and stops when
failure
- is encountered. The number of handled requests is reported to user
by
- writing the buffer pointed by @entry_nr_uptr.
- */
+struct iommu_hwpt_vtd_s1_invalidate {
- __u32 flags;
- __u32 entry_size;
- __aligned_u64 entry_nr_uptr;
- __aligned_u64 inv_data_uptr;
+};
I wonder whether this array can be defined directly in the common struct iommu_hwpt_invalidate so there is no need for underlying iommu driver to further deal with user buffers, including various minsz/backward compat. check.
You want to have an array and another chunk of data?
What is the array for? To do batching?
yes, it's for batching
It means we have to allocate memory on this path, that doesn't seem like the right direction for a performance improvement..
It reuses the ucmd_buffer to avoid memory allocation:
@@ -485,6 +485,12 @@ union ucmd_buffer { #ifdef CONFIG_IOMMUFD_TEST struct iommu_test_cmd test; #endif + /* + * hwpt_type specific structure used in the cache invalidation + * path. + */ + struct iommu_hwpt_vtd_s1_invalidate vtd; + struct iommu_hwpt_vtd_s1_invalidate_desc req_vtd; };
I don't quite like this way.
Having the driver copy in a loop might be better
Can you elaborate?