RE: [PATCH v2 04/12] iommu/vt-d: Move scalable mode ATS enablement to probe path

[Tian, Kevin]

> From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Monday, February 24, 2025 1:16 PM
> 
> Device ATS is currently enabled when a domain is attached to the device
> and disabled when the domain is detached. This creates a limitation:
> when the IOMMU is operating in scalable mode and IOPF is enabled, the
> device's domain cannot be changed.

I got what this patch does, but not this description. Can you elaborate
about the limitation?

> @@ -1556,12 +1528,22 @@ domain_context_mapping(struct dmar_domain
> *domain, struct device *dev)
>  	struct device_domain_info *info = dev_iommu_priv_get(dev);
>  	struct intel_iommu *iommu = info->iommu;
>  	u8 bus = info->bus, devfn = info->devfn;
> +	struct pci_dev *pdev;
> +	int ret;
> 
>  	if (!dev_is_pci(dev))
>  		return domain_context_mapping_one(domain, iommu, bus,
> devfn);
> 
> -	return pci_for_each_dma_alias(to_pci_dev(dev),
> -				      domain_context_mapping_cb, domain);
> +	pdev = to_pci_dev(dev);
> +	ret = pci_for_each_dma_alias(pdev, domain_context_mapping_cb,
> domain);
> +	if (ret)
> +		return ret;
> +
> +	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
> +	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> +		info->ats_enabled = 1;
> +
> +	return 0;
>  }

It'd be good to add a note here for why legacy mode still requires
dynamic toggle at attach/detach time. It's not obvious w/o knowing
the story about legacy + identity.

btw the same enabling logic occurs in multiple places. Probably
you can still make a helper for that.

Otherwise,

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>