Re: [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Mar 13, 2025 at 05:35:26AM -0700, Yi Liu wrote:
> Per the definition of IOMMU_HWPT_ALLOC_PASID, iommufd needs to enforce
> the RID to use PASID-compatible domain if PASID has been attached, and
> vice versa. The PASID path has already enforced it. This adds the
> enforcement in the RID path.
> 
> This enforcement requires a lock across the RID and PASID attach path,
> the idev->igroup->lock is used as both the RID and the PASID path holds
> it.
> 
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> v8 -> v9: iommufd_hwpt_pasid_compat() is moved to prior patch
> 	  I didn't include r-b from Kevin and Jason as this patch has
> 	  quite a few change compared with v8. It's mainly due to the
> 	  introduction of pasid_attach for both RID and PASID path.
> ---
>  drivers/iommu/iommufd/device.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index bceaee7376c2..5db6dc705d44 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -503,8 +503,29 @@ static int iommufd_hwpt_pasid_compat(struct iommufd_hw_pagetable *hwpt,
>  				     struct iommufd_device *idev,
>  				     ioasid_t pasid)
>  {
> -	if (pasid != IOMMU_NO_PASID && !hwpt->pasid_compat)
> -		return -EINVAL;
> +	struct iommufd_group *igroup = idev->igroup;
> +
> +	lockdep_assert_held(&igroup->lock);
> +
> +	if (pasid == IOMMU_NO_PASID) {
> +		unsigned long start = IOMMU_NO_PASID;
> +
> +		if (!hwpt->pasid_compat &&
> +		    xa_find_after(&igroup->pasid_attach,
> +				  &start, UINT_MAX, XA_PRESENT))
> +			return -EINVAL;
> +	}
> +
> +	if (pasid != IOMMU_NO_PASID) {

just use else

> +		struct iommufd_attach_handle *handle;
> +
> +		handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
> +
> +		if (!hwpt->pasid_compat ||

I probably wouldn't put the hwpt test inside this if statement. The
hwpt test is always needed, this if statement is about checking if the
RID is compatible.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason