* [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match
@ 2026-08-31 18:01 Mario Limonciello
2026-09-04 13:26 ` Rafael J. Wysocki (Intel)
0 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2026-08-31 18:01 UTC (permalink / raw)
To: rafael, maciej.wieczor-retman, pawel.chmielewski, lenb
Cc: Mario Limonciello, linux-acpi, acpica-devel
Firmware may express an ACPI namespace path used as a _UID with or
without the leading root scope character ('\'). For example, an AMD
IVRS IVHD ACPI HID device entry may carry a character UID of
"\_SB.MHSP" while the corresponding device's _UID evaluates to
"_SB.MHSP" (or vice versa). This semantic difference is due to how
Windows PnP enumerates and uses devices.
acpi_str_uid_match() compared the two strings verbatim, so such
entries failed to match on the UID even though they refer to the same
object. In the AMD IOMMU case (get_acpihid_device_id()) this caused
the exact HID+UID match to be missed and the code to fall through to
the HID-only path, spuriously raising a FW_BUG.
Skip a single leading '\' on either string before comparing so that
paths that differ only by the root scope prefix are treated as a
match. The integer _UID path is unaffected.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
include/acpi/acpi_bus.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 1a45e0d521d8e..e0bec35953ef1 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -835,7 +835,15 @@ static inline bool acpi_str_uid_match(struct acpi_device *adev, const char *uid2
{
const char *uid1 = acpi_device_uid(adev);
- return uid1 && uid2 && !strcmp(uid1, uid2);
+ if (!uid1 || !uid2)
+ return false;
+
+ if (*uid1 == '\\')
+ uid1++;
+ if (*uid2 == '\\')
+ uid2++;
+
+ return !strcmp(uid1, uid2);
}
static inline bool acpi_int_uid_match(struct acpi_device *adev, u64 uid2)
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-08-31 18:01 [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match Mario Limonciello @ 2026-09-04 13:26 ` Rafael J. Wysocki (Intel) 2026-09-04 17:32 ` Mario Limonciello 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-09-04 13:26 UTC (permalink / raw) To: Mario Limonciello Cc: rafael, maciej.wieczor-retman, pawel.chmielewski, lenb, linux-acpi, acpica-devel On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > Firmware may express an ACPI namespace path used as a _UID with or > without the leading root scope character ('\'). For example, an AMD > IVRS IVHD ACPI HID device entry may carry a character UID of > "\_SB.MHSP" while the corresponding device's _UID evaluates to > "_SB.MHSP" (or vice versa). This semantic difference is due to how > Windows PnP enumerates and uses devices. Can you please elaborate a bit more? I would expect _UID to return the string without the leading backslash, so where does the other one come from, exactly? > acpi_str_uid_match() compared the two strings verbatim, so such > entries failed to match on the UID even though they refer to the same > object. In the AMD IOMMU case (get_acpihid_device_id()) this caused > the exact HID+UID match to be missed and the code to fall through to > the HID-only path, spuriously raising a FW_BUG. > > Skip a single leading '\' on either string before comparing so that > paths that differ only by the root scope prefix are treated as a > match. The integer _UID path is unaffected. But this sort of assumes that the string returned by _UID will always be a namespace path, but is that the case really? > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > include/acpi/acpi_bus.h | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 1a45e0d521d8e..e0bec35953ef1 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -835,7 +835,15 @@ static inline bool acpi_str_uid_match(struct acpi_device *adev, const char *uid2 > { > const char *uid1 = acpi_device_uid(adev); > > - return uid1 && uid2 && !strcmp(uid1, uid2); > + if (!uid1 || !uid2) > + return false; > + > + if (*uid1 == '\\') > + uid1++; > + if (*uid2 == '\\') > + uid2++; > + > + return !strcmp(uid1, uid2); > } > > static inline bool acpi_int_uid_match(struct acpi_device *adev, u64 uid2) > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-09-04 13:26 ` Rafael J. Wysocki (Intel) @ 2026-09-04 17:32 ` Mario Limonciello 2026-09-04 17:58 ` Rafael J. Wysocki (Intel) 0 siblings, 1 reply; 7+ messages in thread From: Mario Limonciello @ 2026-09-04 17:32 UTC (permalink / raw) To: Rafael J. Wysocki (Intel) Cc: maciej.wieczor-retman, pawel.chmielewski, lenb, linux-acpi, acpica-devel On 9/4/26 08:26, Rafael J. Wysocki (Intel) wrote: > On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> Firmware may express an ACPI namespace path used as a _UID with or >> without the leading root scope character ('\'). For example, an AMD >> IVRS IVHD ACPI HID device entry may carry a character UID of >> "\_SB.MHSP" while the corresponding device's _UID evaluates to >> "_SB.MHSP" (or vice versa). This semantic difference is due to how >> Windows PnP enumerates and uses devices. > > Can you please elaborate a bit more? > > I would expect _UID to return the string without the leading > backslash, so where does the other one come from, exactly? It comes from the ACPI IVRS table. https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB p306-307 talk about this field. Here is a sample entry decoded with iasl -d (from BIOS on an affected system) [223h 0547 001h] Subtable Type : F0 [Device Entry: ACPI HID Named Device] [224h 0548 002h] Device ID : 0068 [226h 0550 001h] Data Setting (decoded below) : 40 INITPass : 0 EIntPass : 0 NMIPass : 0 Reserved : 0 System MGMT : 0 LINT0 Pass : 1 LINT1 Pass : 0 [227h 0551 008h] ACPI HID : "MSFT0201" [22Fh 0559 008h] ACPI CID : 0000000000000000 [237h 0567 001h] UID Format : 02 [238h 0568 001h] UID Length : 09 [239h 0569 009h] UID : "\_SB.XHSP" Setting this field to _SB.XHSP does fix the issue for Linux, but this has problems on Windows. So my hope was to let \_SB.XHSP work for Linux too. > >> acpi_str_uid_match() compared the two strings verbatim, so such >> entries failed to match on the UID even though they refer to the same >> object. In the AMD IOMMU case (get_acpihid_device_id()) this caused >> the exact HID+UID match to be missed and the code to fall through to >> the HID-only path, spuriously raising a FW_BUG. >> >> Skip a single leading '\' on either string before comparing so that >> paths that differ only by the root scope prefix are treated as a >> match. The integer _UID path is unaffected. > > But this sort of assumes that the string returned by _UID will always > be a namespace path, but is that the case really? For IVRS entries this would be true since this is what is in the spec: > If defined as a character string, the ACPI UID marks the instances of > DMA-capable devices with the defined DeviceID (e.g. IOMMU visible > Routing ID). It should match the ACPI device name\x02space strings with > unit number, but without a trailing \0 character (as the UID length > specifies the size of the string already). But I don't know universally it would be true. I suppose one possible modification could be to look for the length of the string being at least 2 on the string before incrementing the pointer. > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> include/acpi/acpi_bus.h | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h >> index 1a45e0d521d8e..e0bec35953ef1 100644 >> --- a/include/acpi/acpi_bus.h >> +++ b/include/acpi/acpi_bus.h >> @@ -835,7 +835,15 @@ static inline bool acpi_str_uid_match(struct acpi_device *adev, const char *uid2 >> { >> const char *uid1 = acpi_device_uid(adev); >> >> - return uid1 && uid2 && !strcmp(uid1, uid2); >> + if (!uid1 || !uid2) >> + return false; >> + >> + if (*uid1 == '\\') >> + uid1++; >> + if (*uid2 == '\\') >> + uid2++; >> + >> + return !strcmp(uid1, uid2); >> } >> >> static inline bool acpi_int_uid_match(struct acpi_device *adev, u64 uid2) >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-09-04 17:32 ` Mario Limonciello @ 2026-09-04 17:58 ` Rafael J. Wysocki (Intel) 2026-09-10 9:22 ` Andy Shevchenko 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-09-04 17:58 UTC (permalink / raw) To: Mario Limonciello Cc: Rafael J. Wysocki (Intel), maciej.wieczor-retman, pawel.chmielewski, lenb, linux-acpi, acpica-devel, Andy Shevchenko +Andy On Fri, Sep 4, 2026 at 7:32 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > > > On 9/4/26 08:26, Rafael J. Wysocki (Intel) wrote: > > On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> Firmware may express an ACPI namespace path used as a _UID with or > >> without the leading root scope character ('\'). For example, an AMD > >> IVRS IVHD ACPI HID device entry may carry a character UID of > >> "\_SB.MHSP" while the corresponding device's _UID evaluates to > >> "_SB.MHSP" (or vice versa). This semantic difference is due to how > >> Windows PnP enumerates and uses devices. > > > > Can you please elaborate a bit more? > > > > I would expect _UID to return the string without the leading > > backslash, so where does the other one come from, exactly? > > It comes from the ACPI IVRS table. > > https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB > > p306-307 talk about this field. > > Here is a sample entry decoded with iasl -d > (from BIOS on an affected system) > > [223h 0547 001h] Subtable Type : F0 [Device Entry: ACPI > HID Named Device] > [224h 0548 002h] Device ID : 0068 > [226h 0550 001h] Data Setting (decoded below) : 40 > INITPass : 0 > EIntPass : 0 > NMIPass : 0 > Reserved : 0 > System MGMT : 0 > LINT0 Pass : 1 > LINT1 Pass : 0 > [227h 0551 008h] ACPI HID : "MSFT0201" > [22Fh 0559 008h] ACPI CID : 0000000000000000 > [237h 0567 001h] UID Format : 02 > [238h 0568 001h] UID Length : 09 > [239h 0569 009h] UID : "\_SB.XHSP" > > Setting this field to _SB.XHSP does fix the issue for Linux, but this > has problems on Windows. So my hope was to let \_SB.XHSP work for Linux > too. So how does Linux process this table? Maybe the leading backslash can be removed in that path? But I guess it may not help because _UID may contain a string with a leading backslash. > > > >> acpi_str_uid_match() compared the two strings verbatim, so such > >> entries failed to match on the UID even though they refer to the same > >> object. In the AMD IOMMU case (get_acpihid_device_id()) this caused > >> the exact HID+UID match to be missed and the code to fall through to > >> the HID-only path, spuriously raising a FW_BUG. > >> > >> Skip a single leading '\' on either string before comparing so that > >> paths that differ only by the root scope prefix are treated as a > >> match. The integer _UID path is unaffected. > > > > But this sort of assumes that the string returned by _UID will always > > be a namespace path, but is that the case really? > > For IVRS entries this would be true since this is what is in the spec: > > > If defined as a character string, the ACPI UID marks the instances of > > DMA-capable devices with the defined DeviceID (e.g. IOMMU visible > > Routing ID). It should match the ACPI device name space strings with > > unit number, but without a trailing \0 character (as the UID length > > specifies the size of the string already). > > But I don't know universally it would be true. In general, they can be free-form strings AFAICS. > I suppose one possible modification could be to look for the length of > the string being at least 2 on the string before incrementing the pointer. I guess this change can be made because it will only possibly cause problems when the only difference between the supplied UID and the string returned by _UID is the leading backslash and they are not supposed to match, which is unlikely to happen. However, the kerneldoc comment of acpi_str_uid_match() needs to be updated to mention this. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-09-04 17:58 ` Rafael J. Wysocki (Intel) @ 2026-09-10 9:22 ` Andy Shevchenko 2026-09-10 10:56 ` Rafael J. Wysocki (Intel) 0 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2026-09-10 9:22 UTC (permalink / raw) To: Rafael J. Wysocki (Intel) Cc: Mario Limonciello, maciej.wieczor-retman, pawel.chmielewski, lenb, linux-acpi, acpica-devel On Fri, Sep 04, 2026 at 07:58:56PM +0200, Rafael J. Wysocki (Intel) wrote: > +Andy Thank you for Cc:ing me. And sorry for the late reply, too many emails in the box... > On Fri, Sep 4, 2026 at 7:32 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: > > On 9/4/26 08:26, Rafael J. Wysocki (Intel) wrote: > > > On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello > > > <mario.limonciello@amd.com> wrote: > > >> > > >> Firmware may express an ACPI namespace path used as a _UID with or > > >> without the leading root scope character ('\'). For example, an AMD > > >> IVRS IVHD ACPI HID device entry may carry a character UID of > > >> "\_SB.MHSP" while the corresponding device's _UID evaluates to > > >> "_SB.MHSP" (or vice versa). This semantic difference is due to how > > >> Windows PnP enumerates and uses devices. > > > > > > Can you please elaborate a bit more? > > > > > > I would expect _UID to return the string without the leading > > > backslash, so where does the other one come from, exactly? > > > > It comes from the ACPI IVRS table. > > > > https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB Link is behind SSO, which Intel employee most likely can't get :-) > > p306-307 talk about this field. > > > > Here is a sample entry decoded with iasl -d > > (from BIOS on an affected system) > > > > [223h 0547 001h] Subtable Type : F0 [Device Entry: ACPI > > HID Named Device] > > [224h 0548 002h] Device ID : 0068 > > [226h 0550 001h] Data Setting (decoded below) : 40 > > INITPass : 0 > > EIntPass : 0 > > NMIPass : 0 > > Reserved : 0 > > System MGMT : 0 > > LINT0 Pass : 1 > > LINT1 Pass : 0 > > [227h 0551 008h] ACPI HID : "MSFT0201" > > [22Fh 0559 008h] ACPI CID : 0000000000000000 > > [237h 0567 001h] UID Format : 02 > > [238h 0568 001h] UID Length : 09 > > [239h 0569 009h] UID : "\_SB.XHSP" As far as I can tell this is weird (very unusual) interpretation of UID field. Whoever created a specification should be informed about this. > > Setting this field to _SB.XHSP does fix the issue for Linux, but this > > has problems on Windows. So my hope was to let \_SB.XHSP work for Linux > > too. > So how does Linux process this table? Maybe the leading backslash can > be removed in that path? > > But I guess it may not help because _UID may contain a string with a > leading backslash. > > > >> acpi_str_uid_match() compared the two strings verbatim, so such > > >> entries failed to match on the UID even though they refer to the same > > >> object. In the AMD IOMMU case (get_acpihid_device_id()) this caused > > >> the exact HID+UID match to be missed and the code to fall through to > > >> the HID-only path, spuriously raising a FW_BUG. > > >> > > >> Skip a single leading '\' on either string before comparing so that > > >> paths that differ only by the root scope prefix are treated as a > > >> match. The integer _UID path is unaffected. > > > > > > But this sort of assumes that the string returned by _UID will always > > > be a namespace path, but is that the case really? > > > > For IVRS entries this would be true since this is what is in the spec: > > > > > If defined as a character string, the ACPI UID marks the instances of > > > DMA-capable devices with the defined DeviceID (e.g. IOMMU visible > > > Routing ID). It should match the ACPI device name space strings with > > > unit number, but without a trailing \0 character (as the UID length > > > specifies the size of the string already). > > > > But I don't know universally it would be true. > > In general, they can be free-form strings AFAICS. Exactly, the semantic of the content is just arbitrary set of characters that is agreed to be unique (enough). > > I suppose one possible modification could be to look for the length of > > the string being at least 2 on the string before incrementing the pointer. > > I guess this change can be made because it will only possibly cause > problems when the only difference between the supplied UID and the > string returned by _UID is the leading backslash and they are not > supposed to match, which is unlikely to happen. > > However, the kerneldoc comment of acpi_str_uid_match() needs to be > updated to mention this. I don't see the direct question to me, I assume you want my opinion about the implementation. Ideally the specification has to be fixed, so we do not interpret UID as some "special" string. In a new version of the table specifications it can be amended with a new field, for example. If we need to solve the old behaviour, the skipping leading \ (and backslash only) might be good enough quirk to make sure that the rest of the callers (and possible future interpretations) won't be affected. TL;DR: Make quirk as narrow as possible to avoid false positive triggering. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-09-10 9:22 ` Andy Shevchenko @ 2026-09-10 10:56 ` Rafael J. Wysocki (Intel) 2026-09-10 17:13 ` Mario Limonciello 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-09-10 10:56 UTC (permalink / raw) To: Andy Shevchenko, Mario Limonciello Cc: maciej.wieczor-retman, pawel.chmielewski, linux-acpi, acpica-devel On Thu, Sep 10, 2026 at 11:22 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Sep 04, 2026 at 07:58:56PM +0200, Rafael J. Wysocki (Intel) wrote: > > +Andy > > Thank you for Cc:ing me. And sorry for the late reply, too many emails > in the box... > > > On Fri, Sep 4, 2026 at 7:32 PM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > > > On 9/4/26 08:26, Rafael J. Wysocki (Intel) wrote: > > > > On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello > > > > <mario.limonciello@amd.com> wrote: > > > >> > > > >> Firmware may express an ACPI namespace path used as a _UID with or > > > >> without the leading root scope character ('\'). For example, an AMD > > > >> IVRS IVHD ACPI HID device entry may carry a character UID of > > > >> "\_SB.MHSP" while the corresponding device's _UID evaluates to > > > >> "_SB.MHSP" (or vice versa). This semantic difference is due to how > > > >> Windows PnP enumerates and uses devices. > > > > > > > > Can you please elaborate a bit more? > > > > > > > > I would expect _UID to return the string without the leading > > > > backslash, so where does the other one come from, exactly? > > > > > > It comes from the ACPI IVRS table. > > > > > > https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB > > Link is behind SSO, which Intel employee most likely can't get :-) Yeah, good point. Link: tags should point to stuff that is available to everyone potentially interested. > > > p306-307 talk about this field. > > > > > > Here is a sample entry decoded with iasl -d > > > (from BIOS on an affected system) > > > > > > [223h 0547 001h] Subtable Type : F0 [Device Entry: ACPI > > > HID Named Device] > > > [224h 0548 002h] Device ID : 0068 > > > [226h 0550 001h] Data Setting (decoded below) : 40 > > > INITPass : 0 > > > EIntPass : 0 > > > NMIPass : 0 > > > Reserved : 0 > > > System MGMT : 0 > > > LINT0 Pass : 1 > > > LINT1 Pass : 0 > > > [227h 0551 008h] ACPI HID : "MSFT0201" > > > [22Fh 0559 008h] ACPI CID : 0000000000000000 > > > [237h 0567 001h] UID Format : 02 > > > [238h 0568 001h] UID Length : 09 > > > [239h 0569 009h] UID : "\_SB.XHSP" > > As far as I can tell this is weird (very unusual) interpretation of UID field. > Whoever created a specification should be informed about this. > > > > Setting this field to _SB.XHSP does fix the issue for Linux, but this > > > has problems on Windows. So my hope was to let \_SB.XHSP work for Linux > > > too. > > > So how does Linux process this table? Maybe the leading backslash can > > be removed in that path? > > > > But I guess it may not help because _UID may contain a string with a > > leading backslash. > > > > > >> acpi_str_uid_match() compared the two strings verbatim, so such > > > >> entries failed to match on the UID even though they refer to the same > > > >> object. In the AMD IOMMU case (get_acpihid_device_id()) this caused > > > >> the exact HID+UID match to be missed and the code to fall through to > > > >> the HID-only path, spuriously raising a FW_BUG. > > > >> > > > >> Skip a single leading '\' on either string before comparing so that > > > >> paths that differ only by the root scope prefix are treated as a > > > >> match. The integer _UID path is unaffected. > > > > > > > > But this sort of assumes that the string returned by _UID will always > > > > be a namespace path, but is that the case really? > > > > > > For IVRS entries this would be true since this is what is in the spec: > > > > > > > If defined as a character string, the ACPI UID marks the instances of > > > > DMA-capable devices with the defined DeviceID (e.g. IOMMU visible > > > > Routing ID). It should match the ACPI device name space strings with > > > > unit number, but without a trailing \0 character (as the UID length > > > > specifies the size of the string already). > > > > > > But I don't know universally it would be true. > > > > In general, they can be free-form strings AFAICS. > > Exactly, the semantic of the content is just arbitrary set of characters > that is agreed to be unique (enough). > > > > I suppose one possible modification could be to look for the length of > > > the string being at least 2 on the string before incrementing the pointer. > > > > I guess this change can be made because it will only possibly cause > > problems when the only difference between the supplied UID and the > > string returned by _UID is the leading backslash and they are not > > supposed to match, which is unlikely to happen. > > > > However, the kerneldoc comment of acpi_str_uid_match() needs to be > > updated to mention this. > > I don't see the direct question to me, I assume you want my opinion about > the implementation. Ideally the specification has to be fixed, so we do not > interpret UID as some "special" string. In a new version of the table > specifications it can be amended with a new field, for example. > > If we need to solve the old behaviour, the skipping leading \ (and backslash > only) might be good enough quirk to make sure that the rest of the callers > (and possible future interpretations) won't be affected. TL;DR: Make quirk > as narrow as possible to avoid false positive triggering. That is a good point either, but I think that if two UID strings differ only by a leading backslash, it is kind of reasonable to assume that they were intended to not differ (or conversely, it is hard to believe that someone would intentionally add a leading backslash to distinguish one UID string from another). So I'm going to apply this, but I will remove the Link: tag mentioned above. Thanks for the feedback! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match 2026-09-10 10:56 ` Rafael J. Wysocki (Intel) @ 2026-09-10 17:13 ` Mario Limonciello 0 siblings, 0 replies; 7+ messages in thread From: Mario Limonciello @ 2026-09-10 17:13 UTC (permalink / raw) To: Rafael J. Wysocki (Intel), Andy Shevchenko Cc: maciej.wieczor-retman, pawel.chmielewski, linux-acpi, acpica-devel On 9/10/26 05:56, Rafael J. Wysocki (Intel) wrote: > On Thu, Sep 10, 2026 at 11:22 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: >> >> On Fri, Sep 04, 2026 at 07:58:56PM +0200, Rafael J. Wysocki (Intel) wrote: >>> +Andy >> >> Thank you for Cc:ing me. And sorry for the late reply, too many emails >> in the box... >> >>> On Fri, Sep 4, 2026 at 7:32 PM Mario Limonciello >>> <mario.limonciello@amd.com> wrote: >>>> On 9/4/26 08:26, Rafael J. Wysocki (Intel) wrote: >>>>> On Mon, Aug 31, 2026 at 8:02 PM Mario Limonciello >>>>> <mario.limonciello@amd.com> wrote: >>>>>> >>>>>> Firmware may express an ACPI namespace path used as a _UID with or >>>>>> without the leading root scope character ('\'). For example, an AMD >>>>>> IVRS IVHD ACPI HID device entry may carry a character UID of >>>>>> "\_SB.MHSP" while the corresponding device's _UID evaluates to >>>>>> "_SB.MHSP" (or vice versa). This semantic difference is due to how >>>>>> Windows PnP enumerates and uses devices. >>>>> >>>>> Can you please elaborate a bit more? >>>>> >>>>> I would expect _UID to return the string without the leading >>>>> backslash, so where does the other one come from, exactly? >>>> >>>> It comes from the ACPI IVRS table. >>>> >>>> https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB >> >> Link is behind SSO, which Intel employee most likely can't get :-) > > Yeah, good point. > > Link: tags should point to stuff that is available to everyone > potentially interested. > >>>> p306-307 talk about this field. >>>> >>>> Here is a sample entry decoded with iasl -d >>>> (from BIOS on an affected system) >>>> >>>> [223h 0547 001h] Subtable Type : F0 [Device Entry: ACPI >>>> HID Named Device] >>>> [224h 0548 002h] Device ID : 0068 >>>> [226h 0550 001h] Data Setting (decoded below) : 40 >>>> INITPass : 0 >>>> EIntPass : 0 >>>> NMIPass : 0 >>>> Reserved : 0 >>>> System MGMT : 0 >>>> LINT0 Pass : 1 >>>> LINT1 Pass : 0 >>>> [227h 0551 008h] ACPI HID : "MSFT0201" >>>> [22Fh 0559 008h] ACPI CID : 0000000000000000 >>>> [237h 0567 001h] UID Format : 02 >>>> [238h 0568 001h] UID Length : 09 >>>> [239h 0569 009h] UID : "\_SB.XHSP" >> >> As far as I can tell this is weird (very unusual) interpretation of UID field. >> Whoever created a specification should be informed about this. >> >>>> Setting this field to _SB.XHSP does fix the issue for Linux, but this >>>> has problems on Windows. So my hope was to let \_SB.XHSP work for Linux >>>> too. >> >>> So how does Linux process this table? Maybe the leading backslash can >>> be removed in that path? >>> >>> But I guess it may not help because _UID may contain a string with a >>> leading backslash. >>> >>>>>> acpi_str_uid_match() compared the two strings verbatim, so such >>>>>> entries failed to match on the UID even though they refer to the same >>>>>> object. In the AMD IOMMU case (get_acpihid_device_id()) this caused >>>>>> the exact HID+UID match to be missed and the code to fall through to >>>>>> the HID-only path, spuriously raising a FW_BUG. >>>>>> >>>>>> Skip a single leading '\' on either string before comparing so that >>>>>> paths that differ only by the root scope prefix are treated as a >>>>>> match. The integer _UID path is unaffected. >>>>> >>>>> But this sort of assumes that the string returned by _UID will always >>>>> be a namespace path, but is that the case really? >>>> >>>> For IVRS entries this would be true since this is what is in the spec: >>>> >>>> > If defined as a character string, the ACPI UID marks the instances of >>>> > DMA-capable devices with the defined DeviceID (e.g. IOMMU visible >>>> > Routing ID). It should match the ACPI device name space strings with >>>> > unit number, but without a trailing \0 character (as the UID length >>>> > specifies the size of the string already). >>>> >>>> But I don't know universally it would be true. >>> >>> In general, they can be free-form strings AFAICS. >> >> Exactly, the semantic of the content is just arbitrary set of characters >> that is agreed to be unique (enough). >> >>>> I suppose one possible modification could be to look for the length of >>>> the string being at least 2 on the string before incrementing the pointer. >>> >>> I guess this change can be made because it will only possibly cause >>> problems when the only difference between the supplied UID and the >>> string returned by _UID is the leading backslash and they are not >>> supposed to match, which is unlikely to happen. >>> >>> However, the kerneldoc comment of acpi_str_uid_match() needs to be >>> updated to mention this. >> >> I don't see the direct question to me, I assume you want my opinion about >> the implementation. Ideally the specification has to be fixed, so we do not >> interpret UID as some "special" string. In a new version of the table >> specifications it can be amended with a new field, for example. >> >> If we need to solve the old behaviour, the skipping leading \ (and backslash >> only) might be good enough quirk to make sure that the rest of the callers >> (and possible future interpretations) won't be affected. TL;DR: Make quirk >> as narrow as possible to avoid false positive triggering. > > That is a good point either, but I think that if two UID strings > differ only by a leading backslash, it is kind of reasonable to assume > that they were intended to not differ (or conversely, it is hard to > believe that someone would intentionally add a leading backslash to > distinguish one UID string from another). > > So I'm going to apply this, but I will remove the Link: tag mentioned above. > > Thanks for the feedback! Thanks guys! And sorry for sharing the wrong public URL, I didn't realize I shared an SSO one. The important bit was copied into the thread anyway. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-10 17:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-31 18:01 [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match Mario Limonciello 2026-09-04 13:26 ` Rafael J. Wysocki (Intel) 2026-09-04 17:32 ` Mario Limonciello 2026-09-04 17:58 ` Rafael J. Wysocki (Intel) 2026-09-10 9:22 ` Andy Shevchenko 2026-09-10 10:56 ` Rafael J. Wysocki (Intel) 2026-09-10 17:13 ` Mario Limonciello
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox