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

[Joao Martins <joao.m.martins@oracle.com>]

On 17/10/2023 15:16, Joao Martins wrote:
> 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.
> 
Minor change, I'm changing error code to -EINVAL to align with non-PASID case.