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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276066807236213BCFDD2538CC32@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Tue, 25 Feb 2025 07:28:52 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>, Joerg Roedel <joro@...tes.org>, "Will
 Deacon" <will@...nel.org>, Robin Murphy <robin.murphy@....com>, "Jason
 Gunthorpe" <jgg@...pe.ca>
CC: "Jiang, Dave" <dave.jiang@...el.com>, Vinod Koul <vkoul@...nel.org>,
	Fenghua Yu <fenghuay@...dia.com>, Zhangfei Gao <zhangfei.gao@...aro.org>,
	Zhou Wang <wangzhou1@...ilicon.com>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 04/12] iommu/vt-d: Move scalable mode ATS enablement to
 probe path

> From: Lu Baolu <baolu.lu@...ux.intel.com>
> 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@...el.com>