Linux IOMMU Development
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Mario Limonciello <mario.limonciello@amd.com>,
	Vasant Hegde <vasant.hegde@amd.com>,
	Joerg Roedel <jroedel@suse.de>, Sasha Levin <sashal@kernel.org>,
	joro@8bytes.org, iommu@lists.linux.dev
Subject: [PATCH AUTOSEL 6.15 033/118] iommu/amd: Allow matching ACPI HID devices without matching UIDs
Date: Tue,  3 Jun 2025 20:49:24 -0400	[thread overview]
Message-ID: <20250604005049.4147522-33-sashal@kernel.org> (raw)
In-Reply-To: <20250604005049.4147522-1-sashal@kernel.org>

From: Mario Limonciello <mario.limonciello@amd.com>

[ Upstream commit 51c33f333bbf7bdb6aa2a327e3a3e4bbb2591511 ]

A BIOS upgrade has changed the IVRS DTE UID for a device that no
longer matches the UID in the SSDT. In this case there is only
one ACPI device on the system with that _HID but the _UID mismatch.

IVRS:
```
              Subtable Type : F0 [Device Entry: ACPI HID Named Device]
                  Device ID : 0060
Data Setting (decoded below) : 40
                 INITPass : 0
                 EIntPass : 0
                 NMIPass : 0
                 Reserved : 0
                 System MGMT : 0
                 LINT0 Pass : 1
                 LINT1 Pass : 0
                   ACPI HID : "MSFT0201"
                   ACPI CID : 0000000000000000
                 UID Format : 02
                 UID Length : 09
                        UID : "\_SB.MHSP"
```

SSDT:
```
Device (MHSP)
{
    Name (_ADR, Zero)  // _ADR: Address
    Name (_HID, "MSFT0201")  // _HID: Hardware ID
    Name (_UID, One)  // _UID: Unique ID
```

To handle this case; while enumerating ACPI devices in
get_acpihid_device_id() count the number of matching ACPI devices with
a matching _HID. If there is exactly one _HID match then accept it even
if the UID doesn't match. Other operating systems allow this, but the
current IVRS spec leaves some ambiguity whether to allow or disallow it.
This should be clarified in future revisions of the spec. Output
'Firmware Bug' for this case to encourage it to be solved in the BIOS.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Link: https://lore.kernel.org/r/20250512173129.1274275-1-superm1@kernel.org
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis of the commit, the code changes, and the context
from similar commits, here is my assessment: **YES** This commit should
be backported to stable kernel trees for the following reasons: ## Bug
Fix Analysis 1. **Real-world hardware compatibility issue**: The commit
addresses a concrete problem where BIOS upgrades change IVRS DTE UIDs
that no longer match the UIDs in SSDT, causing ACPI HID device matching
to fail. This is a genuine hardware compatibility issue affecting real
systems. 2. **Minimal, targeted change**: The modification is confined
to a single function (`get_acpihid_device_id()`) in
`drivers/iommu/amd/iommu.c`. The change adds fallback logic without
altering the primary matching path, making it low-risk. 3.
**Conservative approach**: The fix only allows UID mismatch when there's
exactly one HID match (`hid_count == 1`), preventing ambiguous matches.
It maintains strict validation by returning `-EINVAL` for multiple HID
matches. 4. **Follows established patterns**: Similar to the reference
commits (all marked "YES"), this addresses ACPI device matching issues
in the AMD IOMMU subsystem, a pattern we've seen consistently
backported. ## Code Change Analysis The modification transforms the
original simple loop: ```c list_for_each_entry(p, &acpihid_map, list) {
if (acpi_dev_hid_uid_match(adev, p->hid, p->uid[0] ? p->uid : NULL)) {
if (entry) *entry = p; return p->devid; } } return -EINVAL; ``` Into a
more robust matching algorithm that: - First attempts exact HID+UID
matching (preserving original behavior) - Falls back to HID-only
matching when exactly one device matches - Logs firmware bugs
appropriately with `FW_BUG` - Rejects ambiguous multi-device scenarios
## Risk Assessment - **Low regression risk**: The primary matching path
remains unchanged - **Backward compatibility**: Systems with correct
BIOS behavior continue working identically - **Forward compatibility**:
Handles broken BIOS scenarios gracefully - **Contained scope**: Changes
are isolated to AMD IOMMU ACPI device identification ## Comparison with
Reference Commits This commit follows the same pattern as the "YES"
reference commits: - **Similar scope**: ACPI HID device matching in AMD
IOMMU (like commits #1, #2, #3, #4) - **Bug fix nature**: Addresses real
hardware compatibility issues - **Minimal code changes**: Small,
contained modifications - **Critical subsystem**: IOMMU functionality is
essential for system operation The commit contrasts with reference
commit #5 (marked "NO") which introduced new kernel parameters - a
feature addition rather than a bug fix. This commit represents exactly
the type of important bug fix that stable kernels are designed to
include: it resolves real-world hardware compatibility issues with
minimal risk and follows established successful patterns in the same
subsystem.

 drivers/iommu/amd/iommu.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index f34209b08b4c5..a05e0eb1729bf 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -241,7 +241,9 @@ static inline int get_acpihid_device_id(struct device *dev,
 					struct acpihid_map_entry **entry)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
-	struct acpihid_map_entry *p;
+	struct acpihid_map_entry *p, *p1 = NULL;
+	int hid_count = 0;
+	bool fw_bug;
 
 	if (!adev)
 		return -ENODEV;
@@ -249,12 +251,33 @@ static inline int get_acpihid_device_id(struct device *dev,
 	list_for_each_entry(p, &acpihid_map, list) {
 		if (acpi_dev_hid_uid_match(adev, p->hid,
 					   p->uid[0] ? p->uid : NULL)) {
-			if (entry)
-				*entry = p;
-			return p->devid;
+			p1 = p;
+			fw_bug = false;
+			hid_count = 1;
+			break;
+		}
+
+		/*
+		 * Count HID matches w/o UID, raise FW_BUG but allow exactly one match
+		 */
+		if (acpi_dev_hid_match(adev, p->hid)) {
+			p1 = p;
+			hid_count++;
+			fw_bug = true;
 		}
 	}
-	return -EINVAL;
+
+	if (!p1)
+		return -EINVAL;
+	if (fw_bug)
+		dev_err_once(dev, FW_BUG "No ACPI device matched UID, but %d device%s matched HID.\n",
+			     hid_count, hid_count > 1 ? "s" : "");
+	if (hid_count > 1)
+		return -EINVAL;
+	if (entry)
+		*entry = p1;
+
+	return p1->devid;
 }
 
 static inline int get_device_sbdf_id(struct device *dev)
-- 
2.39.5


       reply	other threads:[~2025-06-04  0:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250604005049.4147522-1-sashal@kernel.org>
2025-06-04  0:49 ` Sasha Levin [this message]
2025-06-04  0:50 ` [PATCH AUTOSEL 6.15 075/118] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04  0:50 ` [PATCH AUTOSEL 6.15 092/118] iommu: Avoid introducing more races Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250604005049.4147522-33-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=jroedel@suse.de \
    --cc=mario.limonciello@amd.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=vasant.hegde@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox