[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sergey Temerkhanov]

This commit modifies ARM64-specific ACPI PCI support functions to
properly set the numa_node property on host bridges and thus on PCI
devices. Such support requires _PXM objects to be set in DSDT/SSDT
tables describing PCI root bridges

Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
---
 arch/arm64/kernel/pci.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4f0e3ebfea4b..c45c7a26f984 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
 
 int pcibus_to_node(struct pci_bus *bus)
 {
-	return dev_to_node(&bus->dev);
+	struct pci_config_window *cfg = bus->sysdata;
+	struct acpi_device *adev = NULL;
+	struct device *dev;
+
+	if (!acpi_disabled)
+		adev = to_acpi_device(cfg->parent);
+
+	dev = adev ? &adev->dev : &bus->dev;
+
+	return dev_to_node(dev);
 }
 EXPORT_SYMBOL(pcibus_to_node);
 
@@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct pci_bus *bus, *child;
 	struct acpi_pci_root_ops *root_ops;
 
+	set_dev_node(&root->device->dev, node);
+
 	ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
 	if (!ri)
 		return NULL;
-- 
2.12.1

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sergey Temerkhanov]

This commit modifies ARM64-specific ACPI PCI support functions to
properly set the numa_node property on host bridges and thus on PCI
devices. Such support requires _PXM objects to be set in DSDT/SSDT
tables describing PCI root bridges

Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
---
 arch/arm64/kernel/pci.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4f0e3ebfea4b..c45c7a26f984 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
 
 int pcibus_to_node(struct pci_bus *bus)
 {
-	return dev_to_node(&bus->dev);
+	struct pci_config_window *cfg = bus->sysdata;
+	struct acpi_device *adev = NULL;
+	struct device *dev;
+
+	if (!acpi_disabled)
+		adev = to_acpi_device(cfg->parent);
+
+	dev = adev ? &adev->dev : &bus->dev;
+
+	return dev_to_node(dev);
 }
 EXPORT_SYMBOL(pcibus_to_node);
 
@@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct pci_bus *bus, *child;
 	struct acpi_pci_root_ops *root_ops;
 
+	set_dev_node(&root->device->dev, node);
+
 	ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
 	if (!ri)
 		return NULL;
-- 
2.12.1

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

Hi Sergey,

On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
> This commit modifies ARM64-specific ACPI PCI support functions to
> properly set the numa_node property on host bridges and thus on PCI
> devices. Such support requires _PXM objects to be set in DSDT/SSDT
> tables describing PCI root bridges
> 
> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> ---
>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 4f0e3ebfea4b..c45c7a26f984 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
>  
>  int pcibus_to_node(struct pci_bus *bus)
>  {
> -	return dev_to_node(&bus->dev);
> +	struct pci_config_window *cfg = bus->sysdata;

Well, this implicit cast is a guess (ie it might be a DT sysdata
and there you can't have a clue on what it really is).

> +	struct acpi_device *adev = NULL;
> +	struct device *dev;
> +
> +	if (!acpi_disabled)
> +		adev = to_acpi_device(cfg->parent);
> +
> +	dev = adev ? &adev->dev : &bus->dev;
> +
> +	return dev_to_node(dev);
>  }
>  EXPORT_SYMBOL(pcibus_to_node);
>  
> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	struct pci_bus *bus, *child;
>  	struct acpi_pci_root_ops *root_ops;
>  
> +	set_dev_node(&root->device->dev, node);

Ok I suspect this is one of those catch-22 issues with host bridge
creation/scan code (like domain nr), let me have a look I will send
you a patch next week when I figure out the call ordering required
to make this work, I want to make sure we can make this work with
DT and ACPI alike.

Thanks !
Lorenzo

> +
>  	ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>  	if (!ri)
>  		return NULL;
> -- 
> 2.12.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

Hi Sergey,

On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
> This commit modifies ARM64-specific ACPI PCI support functions to
> properly set the numa_node property on host bridges and thus on PCI
> devices. Such support requires _PXM objects to be set in DSDT/SSDT
> tables describing PCI root bridges
> 
> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> ---
>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 4f0e3ebfea4b..c45c7a26f984 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
>  
>  int pcibus_to_node(struct pci_bus *bus)
>  {
> -	return dev_to_node(&bus->dev);
> +	struct pci_config_window *cfg = bus->sysdata;

Well, this implicit cast is a guess (ie it might be a DT sysdata
and there you can't have a clue on what it really is).

> +	struct acpi_device *adev = NULL;
> +	struct device *dev;
> +
> +	if (!acpi_disabled)
> +		adev = to_acpi_device(cfg->parent);
> +
> +	dev = adev ? &adev->dev : &bus->dev;
> +
> +	return dev_to_node(dev);
>  }
>  EXPORT_SYMBOL(pcibus_to_node);
>  
> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	struct pci_bus *bus, *child;
>  	struct acpi_pci_root_ops *root_ops;
>  
> +	set_dev_node(&root->device->dev, node);

Ok I suspect this is one of those catch-22 issues with host bridge
creation/scan code (like domain nr), let me have a look I will send
you a patch next week when I figure out the call ordering required
to make this work, I want to make sure we can make this work with
DT and ACPI alike.

Thanks !
Lorenzo

> +
>  	ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>  	if (!ri)
>  		return NULL;
> -- 
> 2.12.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sergei Temerkhanov]

On Fri, Apr 7, 2017 at 10:18 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Hi Sergey,
>
> On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
>> This commit modifies ARM64-specific ACPI PCI support functions to
>> properly set the numa_node property on host bridges and thus on PCI
>> devices. Such support requires _PXM objects to be set in DSDT/SSDT
>> tables describing PCI root bridges
>>
>> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
>> ---
>>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
>> index 4f0e3ebfea4b..c45c7a26f984 100644
>> --- a/arch/arm64/kernel/pci.c
>> +++ b/arch/arm64/kernel/pci.c
>> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
>>
>>  int pcibus_to_node(struct pci_bus *bus)
>>  {
>> -     return dev_to_node(&bus->dev);
>> +     struct pci_config_window *cfg = bus->sysdata;
>
> Well, this implicit cast is a guess (ie it might be a DT sysdata
> and there you can't have a clue on what it really is).

Yes, but it's only accessed when acpi_disabled != 0. I just tried to
save few lines and brackets :-)

Regards,
Sergey

>
>> +     struct acpi_device *adev = NULL;
>> +     struct device *dev;
>> +
>> +     if (!acpi_disabled)
>> +             adev = to_acpi_device(cfg->parent);
>> +
>> +     dev = adev ? &adev->dev : &bus->dev;
>> +
>> +     return dev_to_node(dev);
>>  }
>>  EXPORT_SYMBOL(pcibus_to_node);
>>
>> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>       struct pci_bus *bus, *child;
>>       struct acpi_pci_root_ops *root_ops;
>>
>> +     set_dev_node(&root->device->dev, node);
>
> Ok I suspect this is one of those catch-22 issues with host bridge
> creation/scan code (like domain nr), let me have a look I will send
> you a patch next week when I figure out the call ordering required
> to make this work, I want to make sure we can make this work with
> DT and ACPI alike.
>
> Thanks !
> Lorenzo
>
>> +
>>       ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>>       if (!ri)
>>               return NULL;
>> --
>> 2.12.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sergei Temerkhanov]

On Fri, Apr 7, 2017 at 10:18 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Hi Sergey,
>
> On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
>> This commit modifies ARM64-specific ACPI PCI support functions to
>> properly set the numa_node property on host bridges and thus on PCI
>> devices. Such support requires _PXM objects to be set in DSDT/SSDT
>> tables describing PCI root bridges
>>
>> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
>> ---
>>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
>> index 4f0e3ebfea4b..c45c7a26f984 100644
>> --- a/arch/arm64/kernel/pci.c
>> +++ b/arch/arm64/kernel/pci.c
>> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
>>
>>  int pcibus_to_node(struct pci_bus *bus)
>>  {
>> -     return dev_to_node(&bus->dev);
>> +     struct pci_config_window *cfg = bus->sysdata;
>
> Well, this implicit cast is a guess (ie it might be a DT sysdata
> and there you can't have a clue on what it really is).

Yes, but it's only accessed when acpi_disabled != 0. I just tried to
save few lines and brackets :-)

Regards,
Sergey

>
>> +     struct acpi_device *adev = NULL;
>> +     struct device *dev;
>> +
>> +     if (!acpi_disabled)
>> +             adev = to_acpi_device(cfg->parent);
>> +
>> +     dev = adev ? &adev->dev : &bus->dev;
>> +
>> +     return dev_to_node(dev);
>>  }
>>  EXPORT_SYMBOL(pcibus_to_node);
>>
>> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>       struct pci_bus *bus, *child;
>>       struct acpi_pci_root_ops *root_ops;
>>
>> +     set_dev_node(&root->device->dev, node);
>
> Ok I suspect this is one of those catch-22 issues with host bridge
> creation/scan code (like domain nr), let me have a look I will send
> you a patch next week when I figure out the call ordering required
> to make this work, I want to make sure we can make this work with
> DT and ACPI alike.
>
> Thanks !
> Lorenzo
>
>> +
>>       ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>>       if (!ri)
>>               return NULL;
>> --
>> 2.12.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

Hi Sergei,

On Fri, Apr 07, 2017 at 11:31:12AM -0700, Sergei Temerkhanov wrote:
> On Fri, Apr 7, 2017 at 10:18 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > Hi Sergey,
> >
> > On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
> >> This commit modifies ARM64-specific ACPI PCI support functions to
> >> properly set the numa_node property on host bridges and thus on PCI
> >> devices. Such support requires _PXM objects to be set in DSDT/SSDT
> >> tables describing PCI root bridges
> >>
> >> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> >> ---
> >>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> >> index 4f0e3ebfea4b..c45c7a26f984 100644
> >> --- a/arch/arm64/kernel/pci.c
> >> +++ b/arch/arm64/kernel/pci.c
> >> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
> >>
> >>  int pcibus_to_node(struct pci_bus *bus)
> >>  {
> >> -     return dev_to_node(&bus->dev);
> >> +     struct pci_config_window *cfg = bus->sysdata;
> >
> > Well, this implicit cast is a guess (ie it might be a DT sysdata
> > and there you can't have a clue on what it really is).
> 
> Yes, but it's only accessed when acpi_disabled != 0. I just tried to
> save few lines and brackets :-)

I have put together a patch, I will drop it on linux-pci next week
so that we can all have a look and test it.

Thanks !
Lorenzo

> Regards,
> Sergey
> 
> >
> >> +     struct acpi_device *adev = NULL;
> >> +     struct device *dev;
> >> +
> >> +     if (!acpi_disabled)
> >> +             adev = to_acpi_device(cfg->parent);
> >> +
> >> +     dev = adev ? &adev->dev : &bus->dev;
> >> +
> >> +     return dev_to_node(dev);
> >>  }
> >>  EXPORT_SYMBOL(pcibus_to_node);
> >>
> >> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >>       struct pci_bus *bus, *child;
> >>       struct acpi_pci_root_ops *root_ops;
> >>
> >> +     set_dev_node(&root->device->dev, node);
> >
> > Ok I suspect this is one of those catch-22 issues with host bridge
> > creation/scan code (like domain nr), let me have a look I will send
> > you a patch next week when I figure out the call ordering required
> > to make this work, I want to make sure we can make this work with
> > DT and ACPI alike.
> >
> > Thanks !
> > Lorenzo
> >
> >> +
> >>       ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> >>       if (!ri)
> >>               return NULL;
> >> --
> >> 2.12.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

Hi Sergei,

On Fri, Apr 07, 2017 at 11:31:12AM -0700, Sergei Temerkhanov wrote:
> On Fri, Apr 7, 2017 at 10:18 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > Hi Sergey,
> >
> > On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
> >> This commit modifies ARM64-specific ACPI PCI support functions to
> >> properly set the numa_node property on host bridges and thus on PCI
> >> devices. Such support requires _PXM objects to be set in DSDT/SSDT
> >> tables describing PCI root bridges
> >>
> >> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> >> ---
> >>  arch/arm64/kernel/pci.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> >> index 4f0e3ebfea4b..c45c7a26f984 100644
> >> --- a/arch/arm64/kernel/pci.c
> >> +++ b/arch/arm64/kernel/pci.c
> >> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
> >>
> >>  int pcibus_to_node(struct pci_bus *bus)
> >>  {
> >> -     return dev_to_node(&bus->dev);
> >> +     struct pci_config_window *cfg = bus->sysdata;
> >
> > Well, this implicit cast is a guess (ie it might be a DT sysdata
> > and there you can't have a clue on what it really is).
> 
> Yes, but it's only accessed when acpi_disabled != 0. I just tried to
> save few lines and brackets :-)

I have put together a patch, I will drop it on linux-pci next week
so that we can all have a look and test it.

Thanks !
Lorenzo

> Regards,
> Sergey
> 
> >
> >> +     struct acpi_device *adev = NULL;
> >> +     struct device *dev;
> >> +
> >> +     if (!acpi_disabled)
> >> +             adev = to_acpi_device(cfg->parent);
> >> +
> >> +     dev = adev ? &adev->dev : &bus->dev;
> >> +
> >> +     return dev_to_node(dev);
> >>  }
> >>  EXPORT_SYMBOL(pcibus_to_node);
> >>
> >> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >>       struct pci_bus *bus, *child;
> >>       struct acpi_pci_root_ops *root_ops;
> >>
> >> +     set_dev_node(&root->device->dev, node);
> >
> > Ok I suspect this is one of those catch-22 issues with host bridge
> > creation/scan code (like domain nr), let me have a look I will send
> > you a patch next week when I figure out the call ordering required
> > to make this work, I want to make sure we can make this work with
> > DT and ACPI alike.
> >
> > Thanks !
> > Lorenzo
> >
> >> +
> >>       ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> >>       if (!ri)
> >>               return NULL;
> >> --
> >> 2.12.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> >> the body of a message to majordomo at vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sinan Kaya]

On 4/12/2017 1:27 PM, Sinan Kaya wrote:
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

I didn't realize that we were pushing sysdata to the child buses. No
issues then. 

I was under the impression that sysdata only existed for the host bridge
itself and was not shared with the rest of the bus.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sinan Kaya]

On 4/12/2017 1:27 PM, Sinan Kaya wrote:
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

I didn't realize that we were pushing sysdata to the child buses. No
issues then. 

I was under the impression that sysdata only existed for the host bridge
itself and was not shared with the rest of the bus.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sinan Kaya]

Hi Lorenzo,

On 4/12/2017 1:04 PM, Lorenzo Pieralisi wrote:
> On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
>> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
>>>  int pcibus_to_node(struct pci_bus *bus)  {
>>> -	return dev_to_node(&bus->dev);
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	struct acpi_device *adev = NULL;
>>> +	struct device *dev;
>>> +
>>> +	if (!acpi_disabled)
>>> +		adev = to_acpi_device(cfg->parent);
>>> +
>>
>> I see a problem here that NUMA node information is read from the
>> parent device.  PCI bus can have multiple levels of switches and
>> bridges. The NUMA information is only present on the host bridge.
>>
>> This code only works if the endpoint is directly connected to the root
>> port. 
> 
> That's not what this code does. This code retrieves the struct device
> backing the ACPI device representing the PNP0A08 host bridge and its
> dev->numa_node value (that was set in pci_acpi_scan_root()).

pcibus_to_node() gets called from two places. The first place is 
pci_register_host_bridge() like you mentioned. 

The second place is pci_device_add(). pci_device_add() gets called for
every single device that is getting added to the system. 

> 
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

Yeah, that's what I was going to ask next about how other arches solve
this. Common solution to DT/ACPI is preferable as the issue exists in
both places unless DT has some other mechanism hidden behind the scenes.

> 
> This way pcibus_to_node() would become straightforward (ie as it
> is now - provided the above is doable):
> 
> dev_to_node(&bus->dev);
> 
> This is suspiciously similar to the domain number song and dance
> except that the NUMA node now is in the struct pci_bus->dev->numa_node
> instead of struct pci_bus->domain_nr.

I think this looks like the _CCA dance we are doing.

> 
> Lorenzo
> 

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sinan Kaya]

Hi Lorenzo,

On 4/12/2017 1:04 PM, Lorenzo Pieralisi wrote:
> On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
>> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
>>>  int pcibus_to_node(struct pci_bus *bus)  {
>>> -	return dev_to_node(&bus->dev);
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	struct acpi_device *adev = NULL;
>>> +	struct device *dev;
>>> +
>>> +	if (!acpi_disabled)
>>> +		adev = to_acpi_device(cfg->parent);
>>> +
>>
>> I see a problem here that NUMA node information is read from the
>> parent device.  PCI bus can have multiple levels of switches and
>> bridges. The NUMA information is only present on the host bridge.
>>
>> This code only works if the endpoint is directly connected to the root
>> port. 
> 
> That's not what this code does. This code retrieves the struct device
> backing the ACPI device representing the PNP0A08 host bridge and its
> dev->numa_node value (that was set in pci_acpi_scan_root()).

pcibus_to_node() gets called from two places. The first place is 
pci_register_host_bridge() like you mentioned. 

The second place is pci_device_add(). pci_device_add() gets called for
every single device that is getting added to the system. 

> 
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

Yeah, that's what I was going to ask next about how other arches solve
this. Common solution to DT/ACPI is preferable as the issue exists in
both places unless DT has some other mechanism hidden behind the scenes.

> 
> This way pcibus_to_node() would become straightforward (ie as it
> is now - provided the above is doable):
> 
> dev_to_node(&bus->dev);
> 
> This is suspiciously similar to the domain number song and dance
> except that the NUMA node now is in the struct pci_bus->dev->numa_node
> instead of struct pci_bus->domain_nr.

I think this looks like the _CCA dance we are doing.

> 
> Lorenzo
> 

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
> >  int pcibus_to_node(struct pci_bus *bus)  {
> > -	return dev_to_node(&bus->dev);
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	struct acpi_device *adev = NULL;
> > +	struct device *dev;
> > +
> > +	if (!acpi_disabled)
> > +		adev = to_acpi_device(cfg->parent);
> > +
> 
> I see a problem here that NUMA node information is read from the
> parent device.  PCI bus can have multiple levels of switches and
> bridges. The NUMA information is only present on the host bridge.
> 
> This code only works if the endpoint is directly connected to the root
> port. 

That's not what this code does. This code retrieves the struct device
backing the ACPI device representing the PNP0A08 host bridge and its
dev->numa_node value (that was set in pci_acpi_scan_root()).

I am not a big fan of this. I wonder if we could not make it DT/ACPI
agnostic by simply setting the numa_node in the pci_bus->dev field,
and propagate it downstream a PCI hierarcy (as we do with sysdata)
in pci_alloc_child_bus().

This way pcibus_to_node() would become straightforward (ie as it
is now - provided the above is doable):

dev_to_node(&bus->dev);

This is suspiciously similar to the domain number song and dance
except that the NUMA node now is in the struct pci_bus->dev->numa_node
instead of struct pci_bus->domain_nr.

Lorenzo

[PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Lorenzo Pieralisi]

On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
> >  int pcibus_to_node(struct pci_bus *bus)  {
> > -	return dev_to_node(&bus->dev);
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	struct acpi_device *adev = NULL;
> > +	struct device *dev;
> > +
> > +	if (!acpi_disabled)
> > +		adev = to_acpi_device(cfg->parent);
> > +
> 
> I see a problem here that NUMA node information is read from the
> parent device.  PCI bus can have multiple levels of switches and
> bridges. The NUMA information is only present on the host bridge.
> 
> This code only works if the endpoint is directly connected to the root
> port. 

That's not what this code does. This code retrieves the struct device
backing the ACPI device representing the PNP0A08 host bridge and its
dev->numa_node value (that was set in pci_acpi_scan_root()).

I am not a big fan of this. I wonder if we could not make it DT/ACPI
agnostic by simply setting the numa_node in the pci_bus->dev field,
and propagate it downstream a PCI hierarcy (as we do with sysdata)
in pci_alloc_child_bus().

This way pcibus_to_node() would become straightforward (ie as it
is now - provided the above is doable):

dev_to_node(&bus->dev);

This is suspiciously similar to the domain number song and dance
except that the NUMA node now is in the struct pci_bus->dev->numa_node
instead of struct pci_bus->domain_nr.

Lorenzo

Re: [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode

[Sinan Kaya]

On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
>  int pcibus_to_node(struct pci_bus *bus)  {
> -	return dev_to_node(&bus->dev);
> +	struct pci_config_window *cfg = bus->sysdata;
> +	struct acpi_device *adev = NULL;
> +	struct device *dev;
> +
> +	if (!acpi_disabled)
> +		adev = to_acpi_device(cfg->parent);
> +

I see a problem here that NUMA node information is read from the parent device.
PCI bus can have multiple levels of switches and bridges. The NUMA information
is only present on the host bridge.

This code only works if the endpoint is directly connected to the root port. 

You should follow a similar design pattern like pci_dma_configure() where
you go to the host bridge device from any pci device and read the _PXM from
the host bridge.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.