From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f173.google.com (mail-qk1-f173.google.com [209.85.222.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB863407568 for ; Wed, 1 Apr 2026 11:50:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775044206; cv=none; b=mFqc8ca+M1VZ6SM1l9aA3k86sUichAHTDfJcD1h92yMtQLJ1qSS36WhywJAJu9bZbEBSzTp1qLRQ0FmFD5zs+RXnPX+1+IKFpKUO7ldbjf2bcihcYIKX5C9Kec5S1g3muKdfdCp87u2ERJf6e5q9Toc0+iDdcmQnZK9MTKZYeig= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775044206; c=relaxed/simple; bh=N1+Ri8tyl8Kwm5BhyBgbC2qh62bga6Mp83akJRvVsJA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=A4GHvH1TnB5dY2CxYZ8EcOGYD7eJXKyLI2M8/QUwlDenYxRXRzoCCKSghaA8HlQDR0Ge+RtIaQxok13vBWGwwvYDr/pv9eWF6/f7HbD+vI6/+pC6QNURUVeEMkNFLX11MpPdc3LbJ2aM01gBvOpraoKD+yAAnYq0U39wHlVsAYA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=b9D3a059; arc=none smtp.client-ip=209.85.222.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="b9D3a059" Received: by mail-qk1-f173.google.com with SMTP id af79cd13be357-8cd8a189f44so591397585a.0 for ; Wed, 01 Apr 2026 04:50:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1775044204; x=1775649004; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yCRyd9qleOZMVIjgK7RhQpHFpKKn7hN1W8j3Towy8nQ=; b=b9D3a05953Hj09I908zLhRHOdPzMVkUexZAEK+UpRXiVHQx9wtx8w699TThP5LSEsL 9K2maOGtF+CyYwU4W6Fu2+oIfxYs+cicJzIfut3brCPGc8PlJlTfWt31L2ysAQHLbYpz iv9AIepXTKwg1oHBtlGkgB8uJ7KXJyARkWufy5fzKSxCBOHldCKj+2RLXv9BClRhO6cV cy5A96hvsb7UREo3OiQgMdD7I1zLtS6VeTBRVhI4rYXZ38t2QQSqzRv/3g7aCLEpA7c8 zeX27wWCjNkIsbFG7zr6q6vUekmfZXAiipwmbR6g9U5NoygBttAdSPpmhvVvg3zLeGCD VKKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775044204; x=1775649004; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yCRyd9qleOZMVIjgK7RhQpHFpKKn7hN1W8j3Towy8nQ=; b=K/bgBu9Eh0n4wDZU1nNAWFWxLu31m+26FxV85Zurnd/7IXOCOE/sNCuRhRUu6iGOuG ZUm4NvRvNAcnAZ09sRQgN/iKkoeJvaxKVY8ulFvwHQ0YfEPk1fRfAImO8nKRwdMcLAKJ 2iblVoE5VzzKxcLn8UmcPA8nuP4IqdprYUrZDkbtd5fcWMHE+cUjGWR/h5oNYbd8AIin DQK/u9jXBFsioratzjEP7aICNuEKDkUBuidO8xV/r/ncrL6TKmsDF0ft4mhOLis1TQql a6INUKb73N6rhZUeWsVbUte0koFYUCxyvKkEuJaqSEfv9mCMuir19+zh7mSH9IEnVUUC pDDQ== X-Forwarded-Encrypted: i=1; AJvYcCV6WU+VPCEJnQwhsOHkeI3F9h99sR4SkJ7pc/QnQqIp64KUKLBK6WiW8dEdl01dSEIEfFkg2e0BkM+2HGg=@vger.kernel.org X-Gm-Message-State: AOJu0Yym/BYGeMTteClOKEzgRQ1uWRIMbOqxLu1S5tY+HG3W/QMFhdQp tPbj1yZ8ErbJYKw2RyZq0cvT1KJt7bs798DGy5ASqbJA9u/N2GuAQyl4R2NK57zT/tQ= X-Gm-Gg: ATEYQzzzfIccuGQ5flZZjxhYTPk2iwSEwQ9BXDvLzvYi5CnElVdggy5F2lMbCvCPlFv zkHtY91R7EXyWgrN6Lxjq2iXLFyyaTF1ISbeUKUPCqehRhsdpmlb7H6moeUskVEyZgm8ZLmq405 kWvSrOjHpXFuEmAKrQJOyu4+Z8I8D1hj+zWLmHTCLNj/Jz8stszpnohzMBk2twhXIV6dAr10esM KYZOEE2Nb6JlSILXi7DUr0DwnqBjJWuJmgmkRxUrsLKufDx2LSbylso+0IqC44Ro0CIkVjHyOWp qp3B3FJcUYawoGv8RJOWDeqtmJVS7DF28Q/dzPy2bKNDsyX6ZS1md0ewZBUmhfVO0jeHvYhET4q oqLBx37YRkcLanbTrxR4Mwe+MtBq/hRIrVZBPGaDAtcgZ8bzZlgKnwv+WY7zCbcSzT9oi5GGRm9 lSuLp6Fec94FmFokSwcUbdrdbF8tZ5P9c= X-Received: by 2002:a05:620a:6910:b0:8cd:b90f:fc16 with SMTP id af79cd13be357-8d1b5c6bf23mr447046685a.68.1775044203818; Wed, 01 Apr 2026 04:50:03 -0700 (PDT) Received: from [10.11.12.108] ([79.115.63.48]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8d027db4770sm1309221285a.0.2026.04.01.04.49.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Apr 2026 04:50:03 -0700 (PDT) Message-ID: Date: Wed, 1 Apr 2026 14:49:54 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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: Robin Murphy , 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 , Bjorn Helgaas Cc: 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 , Peter Griffin , =?UTF-8?Q?Andr=C3=A9_Draszik?= , Juan Yescas , kernel-team@android.com References: <67b32e90-1f60-4bf5-b534-b4a901d5a796@linaro.org> <6c3b506e-8cc4-45ad-a801-326886c694c4@arm.com> Content-Language: en-US From: Tudor Ambarus In-Reply-To: <6c3b506e-8cc4-45ad-a801-326886c694c4@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi, Robin, Thanks a lot for the educative answers! And sorry for the late reply, I got sidetracked. On 3/23/26 10:49 PM, Robin Murphy wrote: > On 23/03/2026 5:18 pm, Tudor Ambarus wrote: >> Hi, Robin, >> >> On 2/28/25 5:46 PM, Robin Murphy wrote: >>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>> index a3b45b84f42b..1cec7074367a 100644 >>> --- a/drivers/iommu/iommu.c >>> +++ b/drivers/iommu/iommu.c >>> @@ -414,9 +414,21 @@ static int iommu_init_device(struct device *dev) >>>       if (!dev_iommu_get(dev)) >>>           return -ENOMEM; >>>       /* >>> -     * For FDT-based systems and ACPI IORT/VIOT, drivers register IOMMU >>> -     * instances with non-NULL fwnodes, and client devices should have been >>> -     * identified with a fwspec by this point. Otherwise, we can currently >>> +     * For FDT-based systems and ACPI IORT/VIOT, the common firmware parsing >>> +     * is buried in the bus dma_configure path. Properly unpicking that is >>> +     * still a big job, so for now just invoke the whole thing. The device >>> +     * already having a driver bound means dma_configure has already run and >>> +     * either found no IOMMU to wait for, or we're in its replay call right >>> +     * now, so either way there's no point calling it again. >>> +     */ >>> +    if (!dev->driver && dev->bus->dma_configure) { >>> +        mutex_unlock(&iommu_probe_device_lock); >>> +        dev->bus->dma_configure(dev); >>> +        mutex_lock(&iommu_probe_device_lock); >>> +    } >> >> I was chasing the "something fishy" dev_WARN on a 6.19+ downstream >> android kernel and while looking at the IOMMU code I couldn't help >> myself and ask whether we shall prevent concurrent execution of >> dma_configure(). >> >> It seems to me that while the IOMMU subsystem is executing >> dma_configure(), the deferred probe workqueue can concurrently pick up >> the same device, enter really_probe(), set dev->driver, and execute >> dma_configure(). Is it worth protecting against this? > > Yes, it's certainly still possible to hit a false-positive if thread A in iommu_device_register()->bus_iommu_probe() races against thread B attempting to bind, simply because thread B can set dev->driver long before it gets to any point where ends up serialising on iommu_probe_device_lock again, so thread A can observe that even while it is doing the IOMMU probe in the "correct" context. Other than the warning though, it's still functionally OK even if the "wrong" thread does end up finishing the probe, at least after 0c8e9c148e29 ("iommu: Avoid introducing more races"). I confirm I have 0c8e9c148e29 ("iommu: Avoid introducing more races") in my tree. If the concurrent execution is functionally safe and the dev->driver check is a known source of false positives during async probing, do you think it would worth switching the dev_WARN to dev_info? I'm thinking dev_WARN is a bit harsh, as it can disrupt CI pipelines that halt on warnings. >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index e61927b4d41f..5f0c1a8064b5 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -461,9 +461,19 @@ static int iommu_init_device(struct device *dev) >>           * already having a driver bound means dma_configure has already run and >>           * found no IOMMU to wait for, so there's no point calling it again. >>           */ >> -       if (!dev->iommu->fwspec && !dev->driver && dev->bus->dma_configure) { >> +       if (!dev->iommu->fwspec && !READ_ONCE(dev->driver) && >> +           dev->bus->dma_configure) { >>                  mutex_unlock(&iommu_probe_device_lock); >> -               dev->bus->dma_configure(dev); >> + >> +               /* >> +                * Serialize with really_probe(). Recheck dev->driver in case a >> +                * driver bound while we were waiting for the lock. >> +                */ >> +               device_lock(dev); >> +               if (!dev->driver) >> +                       dev->bus->dma_configure(dev); >> +               device_unlock(dev); > > Much as I can't wait to get rid of iommu_probe_device_lock, the main reason we still can't rely on device_lock() at the moment is not actually the remaining sketchy replay-dependers per the comment in __iommu_probe_device(), but more fundamentally that for most IOMMU drivers this will deadlock in that same iommu_device_register()->bus_iommu_probe() path, when the bus walk happens to stumble across the IOMMU device itself, which of course is already locked as it's still in the middle of its own driver bind. I couldn't see an easy, clean and reliable way to get around that, so that can got kicked down the road in order to get the "call of_xlate in the right order and make iommu_device_register() actually work" basics landed (and start shaking out all these other problems...) > Thank you for the detailed explanation. Between the self-deadlock on the IOMMU device itself, and the fact that a non-blocking device_trylock() would be unsafe (as it could fail due to a simple sysfs read, permanently orphaning the device from the bus walk), it's clear there is no clean locking fix for this TOCTOU window right now. Thanks, ta