Messages in this thread | | | Date | Wed, 9 Aug 2023 12:12:25 -0700 | From | Nicolin Chen <> | Subject | Re: [PATCH v4 09/12] iommu/vt-d: Add iotlb flush for nested domain |
| |
On Wed, Aug 09, 2023 at 01:24:56PM -0300, Jason Gunthorpe wrote: > On Wed, Aug 09, 2023 at 09:30:12AM +0000, Liu, Yi L wrote: > > > Yeah, adding new structures to ucmd_buffer may increase the size as > > well if the new one is larger. While for an array, if there is new entry, > > it is for sure to increase the size. I remember there is one tricky thing > > when handling the selftest type. E.g. it is defined as 0xbadbeef, if using > > it to index array, it would expire. So we have some special handling on > > it. If defining the things in iommu_ops, it is simpler. Selftest may be > > not so critical to determining the direction though. > > Maybe we are trying too hard to make it "easy" on the driver. > > Can't we just have the driver invoke some: > > driver_iommufd_invalidate_op(??? *opaque) > { > struct driver_base_struct args; > > rc = iommufd_get_args(opaque, &args, sizeof(args), > offsetof(args, last));
OK. So, IIUIC, the opaque should be:
struct iommu_user_data { void __user *data_uptr; size_t data_len; }user_data;
And core does basic sanity of data_uptr != NULL and data_len !=0 before passing this to driver, and then do a full sanity during the iommufd_get_args (or iommufd_get_user_data?) call.
> The whole point of this excercise was to avoid the mistake where > drivers code the uapi checks incorrectly. We can achieve the same > outcome by providing a mandatory helper.
OK. I will rework this part today.
> Similarly for managing the array of invalidation commands.
You mean an embedded uptr inside a driver user data struct right? Sure, that should go through the new helper too.
Thanks Nicolin
| |