Re: [PATCH v2 01/11] iommu: Add new iommu op to create domains owned by userspace

[Nicolin Chen]

On Wed, May 24, 2023 at 05:02:19AM +0000, Tian, Kevin wrote:
 
> > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Sent: Saturday, May 20, 2023 2:45 AM
> >
> > On Fri, May 19, 2023 at 08:47:45AM +0000, Tian, Kevin wrote:
> > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > > Sent: Thursday, May 11, 2023 10:39 PM
> > > > @@ -229,6 +238,15 @@ struct iommu_iotlb_gather {
> > > >   *           after use. Return the data buffer if success, or ERR_PTR on
> > > >   *           failure.
> > > >   * @domain_alloc: allocate iommu domain
> > > > + * @domain_alloc_user: allocate user iommu domain
> > > > + * @domain_alloc_user_data_len: return the required length of the user
> > > > data
> > > > + *                              to allocate a specific type user iommu domain.
> > > > + *                              @hwpt_type is defined as enum iommu_hwpt_type
> > > > + *                              in include/uapi/linux/iommufd.h. The returned
> > > > + *                              length is the corresponding sizeof driver data
> > > > + *                              structures in include/uapi/linux/iommufd.h.
> > > > + *                              -EOPNOTSUPP would be returned if the input
> > > > + *                              @hwpt_type is not supported by the driver.
> > >
> > > Can this be merged with earlier @hw_info callback? That will already
> > > report a list of supported hwpt types. is there a problem to further
> > > describe the data length for each type in that interface?
> >
> > Yi and I had a last minute talk before he sent this version
> > actually... This version of hw_info no longer reports a list
> > of supported hwpt types. We previously did that in a bitmap,
> > but we found that a bitmap will not be sufficient eventually
> > if there are more than 64 hwpt_types.
> >
> > And this domain_alloc_user_data_len might not be necessary,
> > because in this version the IOMMUFD core doesn't really care
> > about the actual data_len since it copies the data into the
> > ucmd_buffer, i.e. we would probably only need a bool op like
> > "hwpt_type_is_supported".
> >
> 
> Or just pass to the @domain_alloc_user ops which should fail
> if the type is not supported?

The domain_alloc_user returns NULL, which then would be turned
into an ENOMEM error code. It might be confusing from the user
space perspective. Having an op at least allows the user space
to realize that something is wrong with the input structure?

Thanks
Nic