Re: [PATCH v4 07/12] iommufd: Add data structure for Intel VT-d stage-1 cache invalidation

[Jason Gunthorpe]

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?

It means we have to allocate memory on this path, that doesn't seem
like the right direction for a performance improvement..

Having the driver copy in a loop might be better

Jason