Re: [PATCH v4 01/17] iommu: Add hwpt_type with user_data for domain_alloc_user op

From: Yi Liu
Date: Tue Oct 17 2023 - 04:49:57 EST


On 2023/10/17 02:17, Nicolin Chen wrote:
On Mon, Oct 16, 2023 at 08:54:07AM -0300, Jason Gunthorpe wrote:
On Mon, Oct 16, 2023 at 11:28:15AM +0800, Yi Liu wrote:
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.

I'm not keen on that, what if a driver needs another type in the
future? You'd want to make the invalidation data format part of the
domain allocation?

The invalidation data has hwpt_id so it's tied to a hwpt and its
hwpt->domain. Would it be reasonable to have a different type of
invalidation data for the same type of hwpt?

this seems like what Jason asks. A type of hwpt can have two kinds
of invalidation data types. Is it really possible?

With this being asked, I added it for our next version. At this
moment, it only does a sanity job:

// API
__iommu_copy_struct_from_user(void *dst_data,
const struct iommu_user_data *src_data,
unsigned int data_type, size_t data_len,
size_t min_len)
{
if (src_data->type != data_type)
return -EINVAL;

// Caller
rc = iommu_copy_struct_from_user(&user_cfg, user_data,
IOMMU_HWPT_DATA_SELFTEST, iotlb);
if (rc)
return ERR_PTR(rc);

Thanks
Nic

--
Regards,
Yi Liu