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

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

[Shuai Xue]

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>
---
 drivers/acpi/numa/srat.c | 2 +-
 include/acpi/actbl3.h    | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index 53816dfab645..de71b370a275 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -237,7 +237,7 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 		struct acpi_srat_generic_affinity *p =
 			(struct acpi_srat_generic_affinity *)header;
 
-		if (p->device_handle_type == 0) {
+		if (p->device_handle_type == ACPI_SRAT_PCI_DEVICE_HANDLE) {
 			/*
 			 * For pci devices this may be the only place they
 			 * are assigned a proximity domain
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 79d3aa5a4bad..c8488614429c 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -284,6 +284,9 @@ struct acpi_srat_gic_its_affinity {
  * 6: ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY
  */
 
+#define ACPI_SRAT_APCI_DEVICE_HANDLE	(0)
+#define ACPI_SRAT_PCI_DEVICE_HANDLE	(1)
+
 #define ACPI_SRAT_DEVICE_HANDLE_SIZE	16
 
 struct acpi_srat_generic_affinity {
-- 
2.39.3

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

[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?

Also note clearly this only affects a debug print - no functional bug.
That may change whether people choose to backport this or not.

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.

Thanks for the fix!

Jonathan


> ---
>  drivers/acpi/numa/srat.c | 2 +-
>  include/acpi/actbl3.h    | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index 53816dfab645..de71b370a275 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -237,7 +237,7 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
>  		struct acpi_srat_generic_affinity *p =
>  			(struct acpi_srat_generic_affinity *)header;
>  
> -		if (p->device_handle_type == 0) {
> +		if (p->device_handle_type == ACPI_SRAT_PCI_DEVICE_HANDLE) {
>  			/*
>  			 * For pci devices this may be the only place they
>  			 * are assigned a proximity domain
> diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> index 79d3aa5a4bad..c8488614429c 100644
> --- a/include/acpi/actbl3.h
> +++ b/include/acpi/actbl3.h
> @@ -284,6 +284,9 @@ struct acpi_srat_gic_its_affinity {
>   * 6: ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY
>   */
>  
> +#define ACPI_SRAT_APCI_DEVICE_HANDLE	(0)
> +#define ACPI_SRAT_PCI_DEVICE_HANDLE	(1)
> +
>  #define ACPI_SRAT_DEVICE_HANDLE_SIZE	16
>  
>  struct acpi_srat_generic_affinity {

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

[Shuai Xue]

在 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[]?

> 
> Thanks for the fix!
> 
> Jonathan
> 
> 

Thanks.

Best Regards,
Shuai

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

[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


> > 
> > Thanks for the fix!
> > 
> > Jonathan
> > 
> >   
> 
> Thanks.
> 
> Best Regards,
> Shuai

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

[Shuai Xue]

在 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