Re: [PATCH v3 19/19] iommu/intel: Access/Dirty bit support for SL domains

Joao Martins
2023-10-17T14:17:36Z

On 17/10/2023 13:41, Baolu Lu wrote:
> On 2023/10/17 18:51, Joao Martins wrote:
>> On 16/10/2023 14:01, Baolu Lu wrote:
>>> On 2023/10/16 20:59, Jason Gunthorpe wrote:
>>>> On Mon, Oct 16, 2023 at 08:58:42PM +0800, Baolu Lu wrote:
>>>>> On 2023/10/16 19:42, Jason Gunthorpe wrote:
>>>>>> On Mon, Oct 16, 2023 at 11:57:34AM +0100, Joao Martins wrote:
>>>>>>
>>>>>>> True. But to be honest, I thought we weren't quite there yet in PASID
>>>>>>> support
>>>>>>> from IOMMUFD perspective; hence why I didn't aim at it. Or do I have the
>>>>>>> wrong
>>>>>>> impression? From the code below, it clearly looks the driver does.
>>>>>> I think we should plan that this series will go before the PASID
>>>>>> series
>>>>> I know that PASID support in IOMMUFD is not yet available, but the VT-d
>>>>> driver already supports attaching a domain to a PASID, as required by
>>>>> the idxd driver for kernel DMA with PASID. Therefore, from the driver's
>>>>> perspective, dirty tracking should also be enabled for PASIDs.
>>>> As long as the driver refuses to attach a dirty track enabled domain
>>>> to PASID it would be fine for now.
>>> Yes. This works.
>> Baolu Lu, I am blocking PASID attachment this way; let me know if this matches
>> how would you have the driver refuse dirty tracking on PASID.
>
> Joao, how about blocking pasid attachment in intel_iommu_set_dev_pasid()
> directly?
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index c86ba5a3e75c..392b6ca9ce90 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4783,6 +4783,9 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain
> *domain,
>         if (context_copied(iommu, info->bus, info->devfn))
>                 return -EBUSY;
>
> +       if (domain->dirty_ops)
> +               return -EOPNOTSUPP;
> +
>         ret = prepare_domain_attach_device(domain, dev);
>         if (ret)
>                 return ret;

I was trying to centralize all the checks, but I can place it here if you prefer
this way.