Received-SPF: pass (google.com: domain of linux-kernel+bounces-178754-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <243d441d-dda8-442a-a495-83bf9725a14c@nvidia.com>
Date: Tue, 14 May 2024 14:27:38 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 5/7] iommu/dma: Make limit checks self-contained
To: Robin Murphy <robin.murphy@arm.com>, Joerg Roedel <joro@8bytes.org>,
 Christoph Hellwig <hch@lst.de>
Cc: Vineet Gupta <vgupta@kernel.org>, Russell King <linux@armlinux.org.uk>,
 Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>,
 Huacai Chen <chenhuacai@kernel.org>, WANG Xuerui <kernel@xen0n.name>,
 Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
 Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt
 <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>,
 Lorenzo Pieralisi <lpieralisi@kernel.org>, Hanjun Guo
 <guohanjun@huawei.com>, Sudeep Holla <sudeep.holla@arm.com>,
 "K. Y. Srinivasan" <kys@microsoft.com>,
 Haiyang Zhang <haiyangz@microsoft.com>, Wei Liu <wei.liu@kernel.org>,
 Dexuan Cui <decui@microsoft.com>,
 Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
 David Woodhouse <dwmw2@infradead.org>, Lu Baolu <baolu.lu@linux.intel.com>,
 Niklas Schnelle <schnelle@linux.ibm.com>,
 Matthew Rosato <mjrosato@linux.ibm.com>,
 Gerald Schaefer <gerald.schaefer@linux.ibm.com>,
 Jean-Philippe Brucker <jean-philippe@linaro.org>,
 Rob Herring <robh+dt@kernel.org>, Frank Rowand <frowand.list@gmail.com>,
 Marek Szyprowski <m.szyprowski@samsung.com>, Jason Gunthorpe <jgg@ziepe.ca>,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-acpi@vger.kernel.org, iommu@lists.linux.dev,
 devicetree@vger.kernel.org, Jason Gunthorpe <jgg@nvidia.com>,
 "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
References: <cover.1713523152.git.robin.murphy@arm.com>
 <e28a114243d1e79eb3609aded034f8529521333f.1713523152.git.robin.murphy@arm.com>
From: Jon Hunter <jonathanh@nvidia.com>
Content-Language: en-US
In-Reply-To: <e28a114243d1e79eb3609aded034f8529521333f.1713523152.git.robin.murphy@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?M3FjeUhXVWxLaFUxT25sRWx6UlpmUHFkdjByVnY1SGovTFNsU3RrSlVhTys5?=
 =?utf-8?B?cEpWMkhZbzNBRzlEc3JnTFAxcFRNWHEwdGJVUjdWVkRWbTlkRm9Mb05FSzJT?=
 =?utf-8?B?K29Pek9tYXF2aDhyMS84cUlxZThsd2xzcy8yc29QbnR6NXBVL3RRVFpSNmNT?=
 =?utf-8?B?bTJHVXVqV1RtWXMxSEovcndocElIbVJWd0wrbHZoREFvZklnYjJuMDJXaGty?=
 =?utf-8?B?dW5rUmVPRzZleVFPbUppK3luRVZnUk4wOTlhaUtJaUdIb21EdzQ2YjBsYTU3?=
 =?utf-8?B?QjB3ekkwS2Z2SXZteHBManBpVi9zcERiUTA4MWtSeTVQcEtwakt4cmpCWTUx?=
 =?utf-8?B?ak5JVTNsTnY2N2t1bFFwSXdUMDAxeE1GUWdSSmVTcWQ1N2ViSHZVNTF2V05V?=
 =?utf-8?B?OEt5V2dFc1MzUkZuUUxSYkFlZkxYajZyQ2pxY0lLL3NJMDRYTms2b2JGQnBj?=
 =?utf-8?B?clBtdE5wR2NvckZaUDY2Y1lBVG45WGYyd3hLN0JTQnlZdklBZDFKbnZzQ3JI?=
 =?utf-8?B?QUREeFhhTVBGeEg1elRqTlBXUkR5SWpjSll5MnY3RW5NME1Uc1RYN1JQTVFP?=
 =?utf-8?B?Sy9lRVVRRWsveGtDb0dvSS9Eak5zOVMwb0wrTEwrSGwzK0hTa0k0dDZYRGEx?=
 =?utf-8?B?WWdod0tYdGg1RzhmaFVKZDF2NVpMalBZU0VLN2hyejIrMTZ5RndqdXE5dHJD?=
 =?utf-8?B?eXkzaEEranYzbVE1aGFWZktQbnc1Q0cxWXNTdTMwOGo5UkZLSDAxTklPaHRL?=
 =?utf-8?B?QjdUY0JBc3hZRndkZDJKK1h2WFFmMThtcDVuL2poNlVFMHYzVVk5aWdwN3Z2?=
 =?utf-8?B?T2MyS09Zd1BXVlY1UWFaRzdUVTZ3UXNqbTluV0FzYmVHWmgyeXVRUEVxNFdF?=
 =?utf-8?B?U2pBQ2tUQXhxL1lxMlBIdkg3dnA1U1VQbEJYNTA4OTV4YTNIaW1oZ2NTSjNT?=
 =?utf-8?B?cG56U3RXQnk5R2ttQ0VKQXBuekdja04yM0YxSkRpaFZCMkdMZGR0b2paY2RQ?=
 =?utf-8?B?alM1T3RIdVk2VWV3RTZzYTlyWE5tQXUzOEVCSFY0Wm5jWWdBcUZSRVF2QWRo?=
 =?utf-8?B?eFUyaWlsMTlMQ2FEb2dzYXZUOHpqa01aTmo5eklPdks2WUFKSkk0SUpiSUFv?=
 =?utf-8?B?eXMyWTJHZ2c5cVBBR0xsMDIzejltdTVMYWloQXVRYlNsWDhwNGUzeVpVby9M?=
 =?utf-8?B?T1Y1MkJFYnhqbGhRUndqR2dDZHFTRksxMnVreHN1ZWhVMWlNVnhOaFRrWU5O?=
 =?utf-8?B?eGE4UzFxZ08rMExDOEIwWDhxek9ackxTUEhZWVl5VGU2Ukl4eXNScWkyMkpv?=
 =?utf-8?B?UUZhTTFxc2hBaDJnRDhLREVkMExEanJVWDBFM3lBelhwSGxEMGVQd1ZMVEF4?=
 =?utf-8?B?TlF5ekIvRDRpQWd2SlJMclBPQlgrQlcrNHJoTm9qaUQzZ2ordFhwSzBqT1Bi?=
 =?utf-8?B?Zmk0b0RPQjU1RzNFVVl6d1ljNFc4QU5yaXhSQnYwL05lZTRwNzZWUElrTmwv?=
 =?utf-8?B?R1VTbDlaZE5HZ1c1MFZqSFFLbDhsSUJmb2t4Zy9CRFBTMUtvTC9ib3F0SmpT?=
 =?utf-8?B?RklXRkVkQ0ZiL3JWdjl4NkNERzJCQ3FSZTRUQ2daVmFyOXFqU3Y4SFpmYUpK?=
 =?utf-8?B?cHdFSnk3YnhlWGYyTnJTa3J2S2diRjg1RmRQV1YxZk5jVTZsT3AyZDZvR3ls?=
 =?utf-8?B?Q1hldHI3L0M1WUkzU3dvYU9PeXB4d24vbHVCdVRkb21iNlU1Zm1oSEJGU3JD?=
 =?utf-8?B?RElkUXpZTFM1V0Rzb0FEUUsvQjk4U2FXd0J3OXM3R3FRTW1XNHVHcTIvQ1E5?=
 =?utf-8?B?Q2FPRlZhSVRiaDhlMUhvZjBGdlpmMXZsN2R3VEJiZkw2Nnd0V3U5Qi9Ud1Fo?=
 =?utf-8?B?Vzg2cnBZYUtOdTExUjE1eVhBalhUZktYZzJrdkVKUzhSbzlWNmNEVjdSQ3l2?=
 =?utf-8?B?U2JsaGJFRGRBOWtSaEhWTjVRTkZVMWFhdi9Uandnc3NHN21QWCtZb0gramMy?=
 =?utf-8?B?SDVyamErZHVRMHNSMEpWOHZmNnp2amZxTUp2YVljNTdxSUlpSjhaMUF1YVY0?=
 =?utf-8?B?MTQrWmwyNmpTeW50WGdlWUp6b2lzMkcvMUp3QnpoajR2TC9ZTEJIbDZFTmxF?=
 =?utf-8?B?dmoxeVNuOStFKzJORDNnL1FFeTE1WVRTa0NqRTArZE5jU1RPNFBIY01vUzBn?=
 =?utf-8?Q?fNWL8UmtF1qRKCWRm7pc0CCVv/uNiqt1HfWb8O1KDrG3?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f75e6c8-828a-4123-e235-08dc7419a6df
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5444.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 May 2024 13:27:50.4553
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: l0e3O+hvCPaXWn7VYGd/QFPOCPxXqsCPEtxj0fEsswS1JcmA2gg/Ca5MLwprsVxJ9hyzrVP/TImGhX5DCPcc6w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB7595

Hi Robin,

On 19/04/2024 17:54, Robin Murphy wrote:
> It's now easy to retrieve the device's DMA limits if we want to check
> them against the domain aperture, so do that ourselves instead of
> relying on them being passed through the callchain.
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Tested-by: Hanjun Guo <guohanjun@huawei.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/dma-iommu.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index a3039005b696..f542eabaefa4 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -660,19 +660,16 @@ static void iommu_dma_init_options(struct iommu_dma_options *options,
>   /**
>    * iommu_dma_init_domain - Initialise a DMA mapping domain
>    * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
> - * @base: IOVA at which the mappable address space starts
> - * @limit: Last address of the IOVA space
>    * @dev: Device the domain is being initialised for
>    *
> - * @base and @limit + 1 should be exact multiples of IOMMU page granularity to
> - * avoid rounding surprises. If necessary, we reserve the page at address 0
> + * If the geometry and dma_range_map include address 0, we reserve that page
>    * to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but
>    * any change which could make prior IOVAs invalid will fail.
>    */
> -static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
> -				 dma_addr_t limit, struct device *dev)
> +static int iommu_dma_init_domain(struct iommu_domain *domain, struct device *dev)
>   {
>   	struct iommu_dma_cookie *cookie = domain->iova_cookie;
> +	const struct bus_dma_region *map = dev->dma_range_map;
>   	unsigned long order, base_pfn;
>   	struct iova_domain *iovad;
>   	int ret;
> @@ -684,18 +681,18 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   
>   	/* Use the smallest supported page size for IOVA granularity */
>   	order = __ffs(domain->pgsize_bitmap);
> -	base_pfn = max_t(unsigned long, 1, base >> order);
> +	base_pfn = 1;
>   
>   	/* Check the domain allows at least some access to the device... */
> -	if (domain->geometry.force_aperture) {
> +	if (map) {
> +		dma_addr_t base = dma_range_map_min(map);
>   		if (base > domain->geometry.aperture_end ||
> -		    limit < domain->geometry.aperture_start) {
> +		    dma_range_map_max(map) < domain->geometry.aperture_start) {
>   			pr_warn("specified DMA range outside IOMMU capability\n");
>   			return -EFAULT;
>   		}
>   		/* ...then finally give it a kicking to make sure it fits */
> -		base_pfn = max_t(unsigned long, base_pfn,
> -				domain->geometry.aperture_start >> order);
> +		base_pfn = max(base, domain->geometry.aperture_start) >> order;
>   	}
>   
>   	/* start_pfn is always nonzero for an already-initialised domain */
> @@ -1760,7 +1757,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
>   	 * underlying IOMMU driver needs to support via the dma-iommu layer.
>   	 */
>   	if (iommu_is_dma_domain(domain)) {
> -		if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
> +		if (iommu_dma_init_domain(domain, dev))
>   			goto out_err;
>   		dev->dma_ops = &iommu_dma_ops;
>   	}


I have noticed some random test failures on Tegra186 and Tegra194 and 
bisect is pointing to this commit. Reverting this along with the various 
dependencies does fix the problem. On Tegra186 CPU hotplug is failing 
and on Tegra194 suspend is failing. Unfortunately, on neither platform 
do I see any particular crash but the boards hang somewhere.

If you have any ideas on things we can try let me know.

Cheers
Jon

-- 
nvpublic