Re: [PATCH] acpi,srat: Fix incorrect device handle check for Generic Initiator

[Shuai Xue <xueshuai@linux.alibaba.com>]

在 2025/9/11 17:11, Jonathan Cameron 写道:
> On Thu, 11 Sep 2025 12:28:00 +0800
> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> 
>> 在 2025/9/10 17:57, Jonathan Cameron 写道:
>>> On Wed, 10 Sep 2025 17:39:49 +0800
>>> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>>>    
>>>> The Generic Initiator Affinity Structure in SRAT table uses device
>>>> handle type field to indicate the device type. According to ACPI
>>>> specification, the device handle type value of 1 represents PCI device,
>>>> not 0.
>>>>
>>>> Fix this by defining explicit macros for device handle types and using
>>>> the correct check for PCI devices:
>>>>
>>>> - ACPI_SRAT_ACPI_DEVICE_HANDLE (0): ACPI device handle
>>>> - ACPI_SRAT_PCI_DEVICE_HANDLE (1): PCI device handle
>>>>
>>>> Fixes: 894c26a1c274 ("ACPI: Support Generic Initiator only domains")
>>>> Reported-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
>>>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>>>
>>> The actbl3.h additions need to go through acpcia and then have a link
>>> tag in here to show that it was merged.  Perhaps just fix it with a number
>>> for now and follow up with the acpcia stuff in the longer run?
>>
>> Sure, I will drop changes in actbl3.h to make it easy to progress.
>>
>>>
>>> Also note clearly this only affects a debug print - no functional bug.
>>> That may change whether people choose to backport this or not.
>>
>> Yes.
>>
>>>
>>> I'm curious on whether you are thinking of wiring this up so that
>>> we can set the appropriate nodes on PCI Devices other than by doing it
>>> with _PXM().  For obscure reasons there can be references both ways
>>> (so DSDT Device entry -> SRAT via _PXM, and SRAT -> Device via this field
>>> of generic initiators).
>>>
>>> For now we only implement the first one so all we need is a node to be
>>> instantiated for the GI to sit in.
>>>
>>> Come to think of it the fix that made PCI device entries in DSDT with _PXM
>>> turn up in the right place was reverted (for a problem with broken firmware
>>> on AMD threadripper systems - IIRC that I think is long solved).
>>> Not sure if that path even works today and the one this code is about has
>>> never been hooked up.
>>
>> You mean add a new map like pxm_to_node_map[]?
>>
>   
> Two things involved here. If a _PXM entry exists for the PCI device, then unreverting:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0d76bcc960e6057750fcf556b65da13f8bbdfd2b
> or something to replace that is needed.  There may still be a problem somewhere
> as we never entirely figured out the cause last time.  It is also possible this got
> fixed via another method in the meantime so first thing to do is to put some devices
> in a GI only node and see if the numa node value in sysfs is correct.
> 
> For the entry you are touching here to be passed to the actual set_dev_node() we'd
> probably do it by stashing any GI entries found when parsing SRAT in an xarray or similar and
> adding a lookup to see if we have a match in that xarray but not a _PXM in pci_acpi_setup().
> 
> Not particularly hard to do, but have to be very careful wrt to broken bios descriptions
> of the topologies.
> 
> Jonathan
> 

Hi, Jonathan,

Thanks for the detailed guide, it sounds a bit complicated to me :)

I send a v2 to fix device_handle_type check.

Thanks.
Shuai