From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 15112C369A2 for ; Mon, 14 Apr 2025 15:40:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=iGXrY6D7FU19wN4Bhg4nWtc5sUSS9dum2nNQRB7Xhzk=; b=0j5Umq1kU5JuBATCFpPYS41/Ub 7cjrpb9NG2xF+dX+acYr8XaDZA4sHiRhu5FXqK/2kgsgcc0woIe1swqPDOIJ6Fma5Row5ILcJwI+k 7Eo9uG09i2TeJFVcqBW4wt9imvub6aF5lxR53zUhKlszWjXXBD+awCfTeeBFwLCcj00zjNLUSKZLg 2ysvA+++aPkrOMarVG3OQGMno6nEVhvkKs56UUPO88WltLONKmWO2AOqpsWKkrbTgSOt/OeYrdWW/ UakVEJOhk8aqQyurFdS6fEKTHKLfsMxY33Bn7yZKBZLOmbW0D18+GkMyFLsNg+Obji7MoC11luRn9 rUC34aPQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1u4Lv5-00000002fY8-1QnT; Mon, 14 Apr 2025 15:40:23 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1u4Lt3-00000002f9O-3xCs for linux-arm-kernel@lists.infradead.org; Mon, 14 Apr 2025 15:38:20 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 137701007; Mon, 14 Apr 2025 08:38:13 -0700 (PDT) Received: from [192.168.1.102] (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EE7993F66E; Mon, 14 Apr 2025 08:38:10 -0700 (PDT) Message-ID: <50a06ba8-0a99-40d2-8601-778ebf451f6a@arm.com> Date: Mon, 14 Apr 2025 16:37:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 4/4] iommu: Get DT/ACPI parsing into the proper probe path To: Johan Hovold Cc: Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , "Rafael J. Wysocki" , Len Brown , Russell King , Greg Kroah-Hartman , Danilo Krummrich , Stuart Yoder , Laurentiu Tudor , Nipun Gupta , Nikhil Agarwal , Joerg Roedel , Will Deacon , Rob Herring , Saravana Kannan , Bjorn Helgaas , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, iommu@lists.linux.dev, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, Charan Teja Kalla References: From: Robin Murphy Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250414_083818_076176_E991862C X-CRM114-Status: GOOD ( 26.56 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2025-04-11 9:02 am, Johan Hovold wrote: > Hi Robin, > > On Fri, Feb 28, 2025 at 03:46:33PM +0000, Robin Murphy wrote: >> In hindsight, there were some crucial subtleties overlooked when moving >> {of,acpi}_dma_configure() to driver probe time to allow waiting for >> IOMMU drivers with -EPROBE_DEFER, and these have become an >> ever-increasing source of problems. The IOMMU API has some fundamental >> assumptions that iommu_probe_device() is called for every device added >> to the system, in the order in which they are added. Calling it in a >> random order or not at all dependent on driver binding leads to >> malformed groups, a potential lack of isolation for devices with no >> driver, and all manner of unexpected concurrency and race conditions. >> We've attempted to mitigate the latter with point-fix bodges like >> iommu_probe_device_lock, but it's a losing battle and the time has come >> to bite the bullet and address the true source of the problem instead. > >> @@ -426,6 +438,12 @@ static int iommu_init_device(struct device *dev) >> ret = -ENODEV; >> goto err_free; >> } >> + /* >> + * And if we do now see any replay calls, they would indicate someone >> + * misusing the dma_configure path outside bus code. >> + */ >> + if (dev->driver) >> + dev_WARN(dev, "late IOMMU probe at driver bind, something fishy here!\n"); >> >> if (!try_module_get(ops->owner)) { >> ret = -EINVAL; >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >> index e10a68b5ffde..6b989a62def2 100644 >> --- a/drivers/iommu/of_iommu.c >> +++ b/drivers/iommu/of_iommu.c >> @@ -155,7 +155,12 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np, >> dev_iommu_free(dev); >> mutex_unlock(&iommu_probe_device_lock); >> >> - if (!err && dev->bus) >> + /* >> + * If we're not on the iommu_probe_device() path (as indicated by the >> + * initial dev->iommu) then try to simulate it. This should no longer >> + * happen unless of_dma_configure() is being misused outside bus code. >> + */ > > This assumption does not hold as there is nothing preventing iommu > driver probe from racing with a client driver probe. Not sure I follow - *this* assumption is that if we arrived here with dev->iommu already allocated then __iommu_probe_device() is already in progress for this device, either in the current callchain or on another thread, and so we can (and should) skip calling into it again. There's no ambiguity about that. >> + if (!err && dev->bus && !dev_iommu_present) >> err = iommu_probe_device(dev); >> >> if (err && err != -EPROBE_DEFER) > > I hit the (now moved) dev_WARN() on the ThinkPad T14s where the GPU SMMU > is probed late due to a clock dependency and can end up probing in > parallel with the GPU driver. And what *should* happen is that the GPU driver probe waits for the IOMMU driver probe to finish. Do you have fw_devlink enabled? > [ 3.805282] arm-smmu 3da0000.iommu: probing hardware configuration... > [ 3.806007] arm-smmu 3da0000.iommu: SMMUv2 with: > [ 3.806843] arm-smmu 3da0000.iommu: stage 1 translation > [ 3.807562] arm-smmu 3da0000.iommu: coherent table walk > [ 3.808253] arm-smmu 3da0000.iommu: stream matching with 24 register groups > [ 3.808957] arm-smmu 3da0000.iommu: 22 context banks (0 stage-2 only) > [ 3.809651] arm-smmu 3da0000.iommu: Supported page sizes: 0x61311000 > [ 3.810339] arm-smmu 3da0000.iommu: Stage-1: 48-bit VA -> 40-bit IPA > [ 3.811130] arm-smmu 3da0000.iommu: preserved 0 boot mappings > > [ 3.829042] platform 3d6a000.gmu: Adding to iommu group 8 > > [ 3.992050] ------------[ cut here ]------------ > [ 3.993045] adreno 3d00000.gpu: late IOMMU probe at driver bind, something fishy here! > [ 3.994058] WARNING: CPU: 9 PID: 343 at drivers/iommu/iommu.c:579 __iommu_probe_device+0x2b0/0x4ac > > [ 4.003272] CPU: 9 UID: 0 PID: 343 Comm: kworker/u50:2 Not tainted 6.15.0-rc1 #109 PREEMPT > [ 4.003276] Hardware name: LENOVO 21N2ZC5PUS/21N2ZC5PUS, BIOS N42ET83W (2.13 ) 10/04/2024 > > [ 4.025943] Call trace: > [ 4.025945] __iommu_probe_device+0x2b0/0x4ac (P) > [ 4.030453] iommu_probe_device+0x38/0x7c > [ 4.030455] of_iommu_configure+0x188/0x26c > [ 4.030457] of_dma_configure_id+0xcc/0x300 > [ 4.030460] platform_dma_configure+0x74/0xac > [ 4.030462] really_probe+0x74/0x38c Indeed this is exactly what is *not* supposed to be happening - does this patch help at all? https://lore.kernel.org/linux-iommu/09d901ad11b3a410fbb6e27f7d04ad4609c3fe4a.1741706365.git.robin.murphy@arm.com/ If not then I guess I do need to do something to explicitly distinguish the "iommu_device_register() is still running" state after all... Thanks, Robin. > [ 4.030464] __driver_probe_device+0x7c/0x160 > [ 4.030465] driver_probe_device+0x40/0x110 > [ 4.030467] __device_attach_driver+0xbc/0x158 > [ 4.030468] bus_for_each_drv+0x84/0xe0 > [ 4.030470] __device_attach+0xa8/0x1d4 > [ 4.030472] device_initial_probe+0x14/0x20 > [ 4.030473] bus_probe_device+0xb0/0xb4 > [ 4.030476] deferred_probe_work_func+0xa0/0xf4 > > [ 4.030501] ---[ end trace 0000000000000000 ]--- > [ 4.031269] adreno 3d00000.gpu: Adding to iommu group 9 > > Johan