RE: [PATCH v5 08/12] iommufd: Enforce pasid compatible domain for PASID-capable device

Tian, Kevin
2024-12-13T07:52:43Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Friday, December 13, 2024 3:20 PM
>
> On 2024/12/13 10:43, Tian, Kevin wrote:
> >
> > Here is my full picture:
> >
> > At domain allocation the driver should decide whether the setting of
> > ALLOC_PASID is compatible to the given domain type.
> >
> > If paging and iommu supports pasid then ALLOC_PASID is allowed. This
> > applies to all drivers. AMD driver will further select V1 vs. V2 according
> > to the flag bit.
> >
> > If nesting, AMR/ARM drivers will reject the bit as a CD/PASID table
> > cannot be attached to a PASID. Intel driver allows it if pasid is supported
> > by iommu.
>
> Following your opinion, I think the enforcement is something like this,
> it only checks pasid_compat for the PASID path.
>
> + if (idev->dev->iommu->max_pasids && pasid != IOMMU_NO_PASID
> &&
> !hwpt->pasid_compat)
> + return -EINVAL;

shouldn't it be:

if (!idev->dev->iommu->max_pasids ||
pasid == IOMMU_NO_PASID ||
!hwpt->pasid_compat)
return -EINVAL;

?

>
> This means the RID path is not surely be attached to pasid-comapt domain
> or not. either iommufd or iommu driver should do across check between the
> RID and PASID path. It is failing attaching non-pasid compat domain to RID
> if PASID has been attached, and vice versa, attaching PASIDs should be
> failed if RID has been attached to non pasid comapt domain. I doubt if this
> can be done easily as there is no lock between RID and PASID paths.

I'm not sure where that requirement comes from. Does AMD require RID
and PASID to use the same format when nesting is disabled? If yes, that's
still a driver burden to handle, not iommufd's...