Linux ACPI
 help / color / mirror / Atom feed
* [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