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

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

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.

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 66b0e1d5a98c..d33df02f0f2d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4123,7 +4123,7 @@ static void intel_iommu_domain_free(struct iommu_domain
*domain)
 }

 static int prepare_domain_attach_device(struct iommu_domain *domain,
-                                       struct device *dev)
+                                       struct device *dev, ioasid_t pasid)
 {
        struct dmar_domain *dmar_domain = to_dmar_domain(domain);
        struct intel_iommu *iommu;
@@ -4136,7 +4136,8 @@ static int prepare_domain_attach_device(struct
iommu_domain *domain,
        if (dmar_domain->force_snooping && !ecap_sc_support(iommu->ecap))
                return -EINVAL;

-       if (domain->dirty_ops && !slads_supported(iommu))
+       if (domain->dirty_ops &&
+           (!slads_supported(iommu) || pasid != IOMMU_NO_PASID))
                return -EINVAL;

        /* check if this iommu agaw is sufficient for max mapped address */
@@ -4174,7 +4175,7 @@ static int intel_iommu_attach_device(struct iommu_domain
*domain,
        if (info->domain)
                device_block_translation(dev);

-       ret = prepare_domain_attach_device(domain, dev);
+       ret = prepare_domain_attach_device(domain, dev, IOMMU_NO_PASID);
        if (ret)
                return ret;

@@ -4795,7 +4796,7 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain
*domain,
        if (context_copied(iommu, info->bus, info->devfn))
                return -EBUSY;

-       ret = prepare_domain_attach_device(domain, dev);
+       ret = prepare_domain_attach_device(domain, dev, pasid);
        if (ret)
                return ret;