Re: [PATCH] ACPI: utils: Ignore leading root scope prefix in string _UID match

[Mario Limonciello <mario.limonciello@amd.com>]

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.