Hi Jason,
On Tue, 2 Mar 2021 08:56:28 -0400, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> On Wed, Mar 03, 2021 at 04:35:39AM +0800, Liu Yi L wrote:
> >
> > +static int vfio_dev_bind_gpasid_fn(struct device *dev, void *data)
> > +{
> > + struct domain_capsule *dc = (struct domain_capsule *)data;
> > + unsigned long arg = *(unsigned long *)dc->data;
> > +
> > + return iommu_uapi_sva_bind_gpasid(dc->domain, dev,
> > + (void __user *)arg);
>
> This arg buisness is really tortured. The type should be set at the
> ioctl, not constantly passed down as unsigned long or worse void *.
>
> And why is this passing a __user pointer deep into an iommu_* API??
>
The idea was that IOMMU UAPI (not API) is independent of VFIO or other user
driver frameworks. The design is documented here:
Documentation/userspace-api/iommu.rst
IOMMU UAPI handles the type and sanitation of user provided data.
Could you be more specific about your concerns?
> > +/**
> > + * VFIO_IOMMU_NESTING_OP - _IOW(VFIO_TYPE, VFIO_BASE + 18,
> > + * struct vfio_iommu_type1_nesting_op)
> > + *
> > + * This interface allows userspace to utilize the nesting IOMMU
> > + * capabilities as reported in VFIO_IOMMU_TYPE1_INFO_CAP_NESTING
> > + * cap through VFIO_IOMMU_GET_INFO. For platforms which require
> > + * system wide PASID, PASID will be allocated by VFIO_IOMMU_PASID
> > + * _REQUEST.
> > + *
> > + * @data[] types defined for each op:
> > + * +=================+===============================================+
> > + * | NESTING OP | @data[] |
> > + * +=================+===============================================+
> > + * | BIND_PGTBL | struct iommu_gpasid_bind_data |
> > + * +-----------------+-----------------------------------------------+
> > + * | UNBIND_PGTBL | struct iommu_gpasid_bind_data |
> > + *
> > +-----------------+-----------------------------------------------+
>
> If the type is known why does the struct have a flex array?
>
This will be extended to other types in the next patches.
> Jason
Thanks,
Jacob
