> From: Alex Williamson
> Sent: Saturday, April 4, 2020 1:50 AM
[...]
> > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > > > index 9e843a1..298ac80 100644
> > > > --- a/include/uapi/linux/vfio.h
> > > > +++ b/include/uapi/linux/vfio.h
> > > > @@ -794,6 +794,47 @@ struct vfio_iommu_type1_dma_unmap {
> > > > #define VFIO_IOMMU_ENABLE _IO(VFIO_TYPE, VFIO_BASE + 15)
> > > > #define VFIO_IOMMU_DISABLE _IO(VFIO_TYPE, VFIO_BASE + 16)
> > > >
> > > > +/*
> > > > + * PASID (Process Address Space ID) is a PCIe concept which
> > > > + * has been extended to support DMA isolation in fine-grain.
> > > > + * With device assigned to user space (e.g. VMs), PASID alloc
> > > > + * and free need to be system wide. This structure defines
> > > > + * the info for pasid alloc/free between user space and kernel
> > > > + * space.
> > > > + *
> > > > + * @flag=VFIO_IOMMU_PASID_ALLOC, refer to the @alloc_pasid
> > > > + * @flag=VFIO_IOMMU_PASID_FREE, refer to @free_pasid
> > > > + */
> > > > +struct vfio_iommu_type1_pasid_request {
> > > > + __u32 argsz;
> > > > +#define VFIO_IOMMU_PASID_ALLOC (1 << 0)
> > > > +#define VFIO_IOMMU_PASID_FREE (1 << 1)
> > > > + __u32 flags;
> > > > + union {
> > > > + struct {
> > > > + __u32 min;
> > > > + __u32 max;
> > > > + __u32 result;
> > > > + } alloc_pasid;
> > > > + __u32 free_pasid;
> > > > + };
> > >
> > > We seem to be using __u8 data[] lately where the struct at data is
> > > defined by the flags. should we do that here?
> >
> > yeah, I can do that. BTW. Do you want to let the structure in the
> > lately patch share the same structure with this one? As I can foresee,
> > the two structures would look like similar as both of them include
> > argsz, flags and data[] fields. The difference is the definition of
> > flags. what about your opinion?
> >
> > struct vfio_iommu_type1_pasid_request {
> > __u32 argsz;
> > #define VFIO_IOMMU_PASID_ALLOC (1 << 0)
> > #define VFIO_IOMMU_PASID_FREE (1 << 1)
> > __u32 flags;
> > __u8 data[];
> > };
> >
> > struct vfio_iommu_type1_bind {
> > __u32 argsz;
> > __u32 flags;
> > #define VFIO_IOMMU_BIND_GUEST_PGTBL (1 << 0)
> > #define VFIO_IOMMU_UNBIND_GUEST_PGTBL (1 << 1)
> > __u8 data[];
> > };
>
>
> Yes, I was even wondering the same for the cache invalidate ioctl, or
> whether this is going too far for a general purpose "everything related
> to PASIDs" ioctl. We need to factor usability into the equation too.
> I'd be interested in opinions from others here too. Clearly I don't
> like single use, throw-away ioctls, but I can find myself on either
> side of the argument that allocation, binding, and invalidating are all
> within the domain of PASIDs and could fall within a single ioctl or
> they each represent different facets of managing PASIDs and should have
> separate ioctls. Thanks,
>
Looking at uapi/linux/iommu.h:
* Invalidations by %IOMMU_INV_GRANU_DOMAIN don't take any argument other than
* @version and @cache.
Although intel-iommu handles only PASID-related invalidation now, I
suppose other vendors (or future usages?) may allow non-pasid
based invalidation too based on above comment.
Thanks
Kevin
