Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-10 Thread Vijay Kilari
On Fri, Mar 3, 2017 at 8:52 PM, Jan Beulich  wrote:
 On 03.03.17 at 16:16,  wrote:
>> On Fri, Mar 3, 2017 at 8:22 PM, Julien Grall  wrote:
>>> int __init acpi_numa_init(void)
>>> {
>>> if (!acpi_parse_table()) {
>>> acpi_table_parse_srat(TYPE_CPU_AFFINITY);
>>
>> This is not defined for ARM. We have to make this also arch specific.
>> So all arch specific code from xen/drivers/acpi/numa.c should be moved
>> to arch specific to xen/arch/x86/srat.c
>
> There surely is a way to specify processor affinity on ARM?

In ARM, we use ACPI_SRAT_TYPE_GICC_AFFINITY type entry in SRAT
to extract cpu to proximity mapping

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Vijay Kilari
On Fri, Mar 3, 2017 at 8:22 PM, Julien Grall  wrote:
>
>
> On 03/03/17 14:45, Vijay Kilari wrote:
>>
>> On Fri, Mar 3, 2017 at 7:22 PM, Julien Grall  wrote:
>>>
>>>
>>>
>>> On 03/03/17 13:50, Vijay Kilari wrote:


 On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall 
 wrote:
>>>
>>>
>>> This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM}
>>> in
>>> common header.
>>>
>>> Also, x2apic and gicc are respectively x86-specific and arm-specific.
>>> So
>>> I
>>> think we should move the parsing in a separate arch-depend function
>>> to
>>> avoid
>>> those ifdery.
>>>
>>> By that I mean having a function acpi_table_arch_parse_srat that will
>>> parse
>>> x2apci on x86 and gicc on ARM. Jan, what do you think?
>>
>>
>>
>>
>> Linux also follows similar approach
>>
>>
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
>
>
>
>
> So? What are you trying to prove?
>
> The linux version is much readable than yours. Anyway, we should limit
> CONFIG_{X86,ARM} ifdefery in common code.
>
> Currently, I see no point to have those ifdefery when it is possible to
> add
> an arch-depend function.



 This is because in xen we have another level of config CONFIG_ACPI_NUMA.
 I have plans to reuse cpu and memory part next revision.
>>>
>>>
>>>
>>> This does not explain why you want to do like Linux.
>>
>>
>> Basically want to reuse xen/drivers/acpi/numa.c which is common for
>> both x86 and ARM.
>> If not, then we have move some arch specific as you mentioned.
>
>
> I think you misunderstood what I suggested. I only asked to do something
> like:

Got it.
>
> int __init acpi_numa_init(void)
> {
> if (!acpi_parse_table()) {
> acpi_table_parse_srat(TYPE_CPU_AFFINITY);

This is not defined for ARM. We have to make this also arch specific.
So all arch specific code from xen/drivers/acpi/numa.c should be moved
to arch specific to xen/arch/x86/srat.c

> acpi_table_parse_srat(TYPE_MEMORY_AFFINITY);
> acpi_table_arch_parse_srat();
> }
> }
>
> And then for x86
>
> void acpi_table_arch_parse_start(void)
> {
> acpi_table_parse_srat(TYPE_X2APIC_CPU_AFFINITY);
> }
>
> And for ARM
>
> void acpi_table_arch_parse_start(void)
> {
> acpi_table_parse_srat(TYPE_GICC_AFFINITY);
> }
>
>
> The code is still as common but a function is called for arch specific
> setup. This does not require any ifdefery.
>
>>
>> I have another idea where in, if all the NUMA ACPI code is programmed
>> under
>> CONFIG_NUMA and only initialization is kept under CONFIG_ACPI_NUMA
>> similar to x86
>> then we don't need to pollute this header much and limit the changes.
>>
>> I will try to implement this and see how simple it can go and let you
>> know. OK?
>
>
> I don't want to see the common header polluted with #ifdef CONFIG_X86 and
> #ifdef CONFIG_ARM. This is just not right.
>
> Cheers,
>
> --
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Jan Beulich
>>> On 03.03.17 at 16:16,  wrote:
> On Fri, Mar 3, 2017 at 8:22 PM, Julien Grall  wrote:
>> int __init acpi_numa_init(void)
>> {
>> if (!acpi_parse_table()) {
>> acpi_table_parse_srat(TYPE_CPU_AFFINITY);
> 
> This is not defined for ARM. We have to make this also arch specific.
> So all arch specific code from xen/drivers/acpi/numa.c should be moved
> to arch specific to xen/arch/x86/srat.c

There surely is a way to specify processor affinity on ARM?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Julien Grall



On 03/03/17 14:45, Vijay Kilari wrote:

On Fri, Mar 3, 2017 at 7:22 PM, Julien Grall  wrote:



On 03/03/17 13:50, Vijay Kilari wrote:


On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:


This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM}
in
common header.

Also, x2apic and gicc are respectively x86-specific and arm-specific.
So
I
think we should move the parsing in a separate arch-depend function to
avoid
those ifdery.

By that I mean having a function acpi_table_arch_parse_srat that will
parse
x2apci on x86 and gicc on ARM. Jan, what do you think?




Linux also follows similar approach


https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265




So? What are you trying to prove?

The linux version is much readable than yours. Anyway, we should limit
CONFIG_{X86,ARM} ifdefery in common code.

Currently, I see no point to have those ifdefery when it is possible to
add
an arch-depend function.



This is because in xen we have another level of config CONFIG_ACPI_NUMA.
I have plans to reuse cpu and memory part next revision.



This does not explain why you want to do like Linux.


Basically want to reuse xen/drivers/acpi/numa.c which is common for
both x86 and ARM.
If not, then we have move some arch specific as you mentioned.


I think you misunderstood what I suggested. I only asked to do something 
like:


int __init acpi_numa_init(void)
{
if (!acpi_parse_table()) {
acpi_table_parse_srat(TYPE_CPU_AFFINITY);
acpi_table_parse_srat(TYPE_MEMORY_AFFINITY);
acpi_table_arch_parse_srat();
}
}

And then for x86

void acpi_table_arch_parse_start(void)
{
acpi_table_parse_srat(TYPE_X2APIC_CPU_AFFINITY);
}

And for ARM

void acpi_table_arch_parse_start(void)
{
acpi_table_parse_srat(TYPE_GICC_AFFINITY);
}


The code is still as common but a function is called for arch specific 
setup. This does not require any ifdefery.




I have another idea where in, if all the NUMA ACPI code is programmed under
CONFIG_NUMA and only initialization is kept under CONFIG_ACPI_NUMA
similar to x86
then we don't need to pollute this header much and limit the changes.

I will try to implement this and see how simple it can go and let you know. OK?


I don't want to see the common header polluted with #ifdef CONFIG_X86 
and #ifdef CONFIG_ARM. This is just not right.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Vijay Kilari
On Fri, Mar 3, 2017 at 7:22 PM, Julien Grall  wrote:
>
>
> On 03/03/17 13:50, Vijay Kilari wrote:
>>
>> On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:
>
> This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM}
> in
> common header.
>
> Also, x2apic and gicc are respectively x86-specific and arm-specific.
> So
> I
> think we should move the parsing in a separate arch-depend function to
> avoid
> those ifdery.
>
> By that I mean having a function acpi_table_arch_parse_srat that will
> parse
> x2apci on x86 and gicc on ARM. Jan, what do you think?



 Linux also follows similar approach


 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
>>>
>>>
>>>
>>> So? What are you trying to prove?
>>>
>>> The linux version is much readable than yours. Anyway, we should limit
>>> CONFIG_{X86,ARM} ifdefery in common code.
>>>
>>> Currently, I see no point to have those ifdefery when it is possible to
>>> add
>>> an arch-depend function.
>>
>>
>> This is because in xen we have another level of config CONFIG_ACPI_NUMA.
>> I have plans to reuse cpu and memory part next revision.
>
>
> This does not explain why you want to do like Linux.

Basically want to reuse xen/drivers/acpi/numa.c which is common for
both x86 and ARM.
If not, then we have move some arch specific as you mentioned.

I have another idea where in, if all the NUMA ACPI code is programmed under
CONFIG_NUMA and only initialization is kept under CONFIG_ACPI_NUMA
similar to x86
then we don't need to pollute this header much and limit the changes.

I will try to implement this and see how simple it can go and let you know. OK?

>
> --
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Julien Grall



On 03/03/17 13:50, Vijay Kilari wrote:

On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:

This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
common header.

Also, x2apic and gicc are respectively x86-specific and arm-specific. So
I
think we should move the parsing in a separate arch-depend function to
avoid
those ifdery.

By that I mean having a function acpi_table_arch_parse_srat that will
parse
x2apci on x86 and gicc on ARM. Jan, what do you think?



Linux also follows similar approach

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265



So? What are you trying to prove?

The linux version is much readable than yours. Anyway, we should limit
CONFIG_{X86,ARM} ifdefery in common code.

Currently, I see no point to have those ifdefery when it is possible to add
an arch-depend function.


This is because in xen we have another level of config CONFIG_ACPI_NUMA.
I have plans to reuse cpu and memory part next revision.


This does not explain why you want to do like Linux.

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Vijay Kilari
On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:
> Hello Vijay,
>
> On 03/03/17 12:39, Vijay Kilari wrote:
>>
>> On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall 
>> wrote:

 diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
 index 50bf9f8..ce22e88 100644
 --- a/xen/drivers/acpi/numa.c
 +++ b/xen/drivers/acpi/numa.c
 @@ -25,9 +25,11 @@
  #include 
  #include 
  #include 
 +#include 
>>>
>>>
>>>
>>> Why do you need to inlucde xen/mm.h and ...
>>>
  #include 
  #include 
  #include 
 +#include 
>>>
>>>
>>>
>>> asm/mm.h?
>>
>>
>> I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
>> there is compilation error.
>
>
> Regarding asm/mm.h, it is already included by xen/mm.h. So no point to add
> it.
>
> Now regarding xen/mm.h, just saying there is a compilation error is not
> helpful. Can you expand why it is needed?

I remember just adding xen/mm.h has not solved the problem. Anyway I
will check this
when I work for next revision.

>
> [...]
>
>>> This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
>>> common header.
>>>
>>> Also, x2apic and gicc are respectively x86-specific and arm-specific. So
>>> I
>>> think we should move the parsing in a separate arch-depend function to
>>> avoid
>>> those ifdery.
>>>
>>> By that I mean having a function acpi_table_arch_parse_srat that will
>>> parse
>>> x2apci on x86 and gicc on ARM. Jan, what do you think?
>>
>>
>> Linux also follows similar approach
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
>
>
> So? What are you trying to prove?
>
> The linux version is much readable than yours. Anyway, we should limit
> CONFIG_{X86,ARM} ifdefery in common code.
>
> Currently, I see no point to have those ifdefery when it is possible to add
> an arch-depend function.

This is because in xen we have another level of config CONFIG_ACPI_NUMA.
I have plans to reuse cpu and memory part next revision.

Regards
Vijay

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Julien Grall

Hello Vijay,

On 03/03/17 12:39, Vijay Kilari wrote:

On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall  wrote:

diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
index 50bf9f8..ce22e88 100644
--- a/xen/drivers/acpi/numa.c
+++ b/xen/drivers/acpi/numa.c
@@ -25,9 +25,11 @@
 #include 
 #include 
 #include 
+#include 



Why do you need to inlucde xen/mm.h and ...


 #include 
 #include 
 #include 
+#include 



asm/mm.h?


I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
there is compilation error.


Regarding asm/mm.h, it is already included by xen/mm.h. So no point to 
add it.


Now regarding xen/mm.h, just saying there is a compilation error is not 
helpful. Can you expand why it is needed?


[...]


This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
common header.

Also, x2apic and gicc are respectively x86-specific and arm-specific. So I
think we should move the parsing in a separate arch-depend function to avoid
those ifdery.

By that I mean having a function acpi_table_arch_parse_srat that will parse
x2apci on x86 and gicc on ARM. Jan, what do you think?


Linux also follows similar approach
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265


So? What are you trying to prove?

The linux version is much readable than yours. Anyway, we should limit 
CONFIG_{X86,ARM} ifdefery in common code.


Currently, I see no point to have those ifdefery when it is possible to 
add an arch-depend function.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-03 Thread Vijay Kilari
On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall  wrote:
> (+ Jan as ACPI maintainer)
>
> Hello Vijay,
>
> On 09/02/17 15:57, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Register SRAT entry handler for type
>> ACPI_SRAT_TYPE_GICC_AFFINITY to parse SRAT table
>> and extract proximity for all CPU IDs.
>>
>> Signed-off-by: Vijaya Kumar 
>> ---
>>  xen/arch/arm/acpi_numa.c  | 55
>> +++
>>  xen/drivers/acpi/numa.c   | 37 +++
>>  xen/drivers/acpi/osl.c|  2 ++
>>  xen/include/acpi/actbl1.h | 17 ++-
>>  xen/include/xen/acpi.h| 39 +
>>  5 files changed, 149 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
>> index 3ee87f2..f659275 100644
>> --- a/xen/arch/arm/acpi_numa.c
>> +++ b/xen/arch/arm/acpi_numa.c
>> @@ -105,6 +105,61 @@ static int __init acpi_parse_madt_handler(struct
>> acpi_subtable_header *header,
>>  return 0;
>>  }
>>
>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>> +void __init acpi_numa_gicc_affinity_init(const struct
>> acpi_srat_gicc_affinity *pa)
>> +{
>> +int pxm, node;
>> +u64 mpidr = 0;
>
>
> mpidr does not need to be set to 0.
>
>> +static u32 cpus_in_srat;
>> +
>> +if ( srat_disabled() )
>> +return;
>> +
>> +if ( pa->header.length < sizeof(struct acpi_srat_gicc_affinity) )
>> +{
>> +printk(XENLOG_WARNING "SRAT: Invalid SRAT header length: %d\n",
>> +   pa->header.length);
>> +bad_srat();
>> +return;
>> +}
>> +
>> +if ( !(pa->flags & ACPI_SRAT_GICC_ENABLED) )
>> +return;
>> +
>> +if ( cpus_in_srat >= NR_CPUS )
>> +{
>> +printk(XENLOG_WARNING
>
>
> This should be XENLOG_ERROR.
OK
>
>> +   "SRAT: cpu_to_node_map[%d] is too small to fit all
>> cpus\n",
>> +   NR_CPUS);
>> +return;
>> +}
>> +
>> +pxm = pa->proximity_domain;
>> +node = setup_node(pxm);
>> +if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES )
>
>
> Looking at the implementation of setup_node, node will either be equal to
> NUMA_NO_NODE or valid. It is not possible to have node >= MAX_NUMNODES.
ok
>
>> +{
>> +printk(XENLOG_WARNING "SRAT: Too many proximity domains %d\n",
>> pxm);
>
>
> setup_node is already printing an error if we have too many proximity
> domains. So no need to duplicate twice.
ok
>
>> +bad_srat();
>> +return;
>> +}
>> +
>> +mpidr = acpi_get_cpu_mpidr(pa->acpi_processor_uid);
>> +if ( mpidr == MPIDR_INVALID )
>> +{
>> +printk(XENLOG_WARNING
>
>
> s/XENLOG_WARNING/XENLOG_ERROR/
>
>> +   "SRAT: PXM %d with ACPI ID %d has no valid MPIDR in
>> MADT\n",
>> +   pxm, pa->acpi_processor_uid);
>> +bad_srat();
>> +return;
>> +}
>> +
>> +node_set(node, numa_nodes_parsed);
>> +cpus_in_srat++;
>> +acpi_numa = 1;
>> +printk(XENLOG_INFO "SRAT: PXM %d -> MPIDR 0x%lx -> Node %d\n",
>> +   pxm, mpidr, node);
>> +}
>> +
>>  void __init acpi_map_uid_to_mpidr(void)
>>  {
>>  int i;
>> diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
>> index 50bf9f8..ce22e88 100644
>> --- a/xen/drivers/acpi/numa.c
>> +++ b/xen/drivers/acpi/numa.c
>> @@ -25,9 +25,11 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
>
> Why do you need to inlucde xen/mm.h and ...
>
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
>
> asm/mm.h?

I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
there is compilation error.

>
>
>>
>>  #define ACPI_NUMA  0x8000
>>  #define _COMPONENT ACPI_NUMA
>> @@ -105,6 +107,21 @@ void __init acpi_table_print_srat_entry(struct
>> acpi_subtable_header * header)
>> }
>>  #endif /* ACPI_DEBUG_OUTPUT */
>> break;
>> +   case ACPI_SRAT_TYPE_GICC_AFFINITY:
>> +#ifdef ACPI_DEBUG_OUTPUT
>> +   {
>> +   struct acpi_srat_gicc_affinity *p =
>> +   (struct acpi_srat_gicc_affinity *)header;
>> +   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>> + "SRAT Processor (acpi
>> id[0x%04x]) in"
>> + " proximity domain %d %s\n",
>> + p->acpi_processor_uid,
>> + p->proximity_domain,
>> + (p->flags &
>> ACPI_SRAT_GICC_ENABLED) ?
>> + "enabled" : "disabled");
>> +   }
>> +#endif /* ACPI_DEBUG_OUTPUT */
>> +   break;
>> default:
>> printk(KERN_WARNING PREFIX
>>"Found unsupported SRAT entry (type = %#x)\n",
>> @@ 

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-03-02 Thread Julien Grall

(+ Jan as ACPI maintainer)

Hello Vijay,

On 09/02/17 15:57, vijay.kil...@gmail.com wrote:

From: Vijaya Kumar K 

Register SRAT entry handler for type
ACPI_SRAT_TYPE_GICC_AFFINITY to parse SRAT table
and extract proximity for all CPU IDs.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/acpi_numa.c  | 55 +++
 xen/drivers/acpi/numa.c   | 37 +++
 xen/drivers/acpi/osl.c|  2 ++
 xen/include/acpi/actbl1.h | 17 ++-
 xen/include/xen/acpi.h| 39 +
 5 files changed, 149 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
index 3ee87f2..f659275 100644
--- a/xen/arch/arm/acpi_numa.c
+++ b/xen/arch/arm/acpi_numa.c
@@ -105,6 +105,61 @@ static int __init acpi_parse_madt_handler(struct 
acpi_subtable_header *header,
 return 0;
 }

+/* Callback for Proximity Domain -> ACPI processor UID mapping */
+void __init acpi_numa_gicc_affinity_init(const struct acpi_srat_gicc_affinity 
*pa)
+{
+int pxm, node;
+u64 mpidr = 0;


mpidr does not need to be set to 0.


+static u32 cpus_in_srat;
+
+if ( srat_disabled() )
+return;
+
+if ( pa->header.length < sizeof(struct acpi_srat_gicc_affinity) )
+{
+printk(XENLOG_WARNING "SRAT: Invalid SRAT header length: %d\n",
+   pa->header.length);
+bad_srat();
+return;
+}
+
+if ( !(pa->flags & ACPI_SRAT_GICC_ENABLED) )
+return;
+
+if ( cpus_in_srat >= NR_CPUS )
+{
+printk(XENLOG_WARNING


This should be XENLOG_ERROR.


+   "SRAT: cpu_to_node_map[%d] is too small to fit all cpus\n",
+   NR_CPUS);
+return;
+}
+
+pxm = pa->proximity_domain;
+node = setup_node(pxm);
+if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES )


Looking at the implementation of setup_node, node will either be equal 
to NUMA_NO_NODE or valid. It is not possible to have node >= MAX_NUMNODES.



+{
+printk(XENLOG_WARNING "SRAT: Too many proximity domains %d\n", pxm);


setup_node is already printing an error if we have too many proximity 
domains. So no need to duplicate twice.



+bad_srat();
+return;
+}
+
+mpidr = acpi_get_cpu_mpidr(pa->acpi_processor_uid);
+if ( mpidr == MPIDR_INVALID )
+{
+printk(XENLOG_WARNING


s/XENLOG_WARNING/XENLOG_ERROR/


+   "SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
+   pxm, pa->acpi_processor_uid);
+bad_srat();
+return;
+}
+
+node_set(node, numa_nodes_parsed);
+cpus_in_srat++;
+acpi_numa = 1;
+printk(XENLOG_INFO "SRAT: PXM %d -> MPIDR 0x%lx -> Node %d\n",
+   pxm, mpidr, node);
+}
+
 void __init acpi_map_uid_to_mpidr(void)
 {
 int i;
diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
index 50bf9f8..ce22e88 100644
--- a/xen/drivers/acpi/numa.c
+++ b/xen/drivers/acpi/numa.c
@@ -25,9 +25,11 @@
 #include 
 #include 
 #include 
+#include 


Why do you need to inlucde xen/mm.h and ...


 #include 
 #include 
 #include 
+#include 


asm/mm.h?



 #define ACPI_NUMA  0x8000
 #define _COMPONENT ACPI_NUMA
@@ -105,6 +107,21 @@ void __init acpi_table_print_srat_entry(struct 
acpi_subtable_header * header)
}
 #endif /* ACPI_DEBUG_OUTPUT */
break;
+   case ACPI_SRAT_TYPE_GICC_AFFINITY:
+#ifdef ACPI_DEBUG_OUTPUT
+   {
+   struct acpi_srat_gicc_affinity *p =
+   (struct acpi_srat_gicc_affinity *)header;
+   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "SRAT Processor (acpi id[0x%04x]) in"
+ " proximity domain %d %s\n",
+ p->acpi_processor_uid,
+ p->proximity_domain,
+ (p->flags & ACPI_SRAT_GICC_ENABLED) ?
+ "enabled" : "disabled");
+   }
+#endif /* ACPI_DEBUG_OUTPUT */
+   break;
default:
printk(KERN_WARNING PREFIX
   "Found unsupported SRAT entry (type = %#x)\n",
@@ -185,6 +202,24 @@ int __init acpi_parse_srat(struct acpi_table_header *table)
return 0;
 }

+static int __init
+acpi_parse_gicc_affinity(struct acpi_subtable_header *header,
+const unsigned long end)
+{
+   const struct acpi_srat_gicc_affinity *processor_affinity
+   = (struct acpi_srat_gicc_affinity *)header;
+
+   if (!processor_affinity)
+   return -EINVAL;
+
+   acpi_table_print_srat_entry(header);
+
+   /* let architecture-dependent part to do it */
+   

[Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

2017-02-09 Thread vijay . kilari
From: Vijaya Kumar K 

Register SRAT entry handler for type
ACPI_SRAT_TYPE_GICC_AFFINITY to parse SRAT table
and extract proximity for all CPU IDs.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/acpi_numa.c  | 55 +++
 xen/drivers/acpi/numa.c   | 37 +++
 xen/drivers/acpi/osl.c|  2 ++
 xen/include/acpi/actbl1.h | 17 ++-
 xen/include/xen/acpi.h| 39 +
 5 files changed, 149 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
index 3ee87f2..f659275 100644
--- a/xen/arch/arm/acpi_numa.c
+++ b/xen/arch/arm/acpi_numa.c
@@ -105,6 +105,61 @@ static int __init acpi_parse_madt_handler(struct 
acpi_subtable_header *header,
 return 0;
 }
 
+/* Callback for Proximity Domain -> ACPI processor UID mapping */
+void __init acpi_numa_gicc_affinity_init(const struct acpi_srat_gicc_affinity 
*pa)
+{
+int pxm, node;
+u64 mpidr = 0;
+static u32 cpus_in_srat;
+
+if ( srat_disabled() )
+return;
+
+if ( pa->header.length < sizeof(struct acpi_srat_gicc_affinity) )
+{
+printk(XENLOG_WARNING "SRAT: Invalid SRAT header length: %d\n",
+   pa->header.length);
+bad_srat();
+return;
+}
+
+if ( !(pa->flags & ACPI_SRAT_GICC_ENABLED) )
+return;
+
+if ( cpus_in_srat >= NR_CPUS )
+{
+printk(XENLOG_WARNING
+   "SRAT: cpu_to_node_map[%d] is too small to fit all cpus\n",
+   NR_CPUS);
+return;
+}
+
+pxm = pa->proximity_domain;
+node = setup_node(pxm);
+if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES )
+{
+printk(XENLOG_WARNING "SRAT: Too many proximity domains %d\n", pxm);
+bad_srat();
+return;
+}
+
+mpidr = acpi_get_cpu_mpidr(pa->acpi_processor_uid);
+if ( mpidr == MPIDR_INVALID )
+{
+printk(XENLOG_WARNING
+   "SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
+   pxm, pa->acpi_processor_uid);
+bad_srat();
+return;
+}
+
+node_set(node, numa_nodes_parsed);
+cpus_in_srat++;
+acpi_numa = 1;
+printk(XENLOG_INFO "SRAT: PXM %d -> MPIDR 0x%lx -> Node %d\n",
+   pxm, mpidr, node);
+}
+
 void __init acpi_map_uid_to_mpidr(void)
 {
 int i;
diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
index 50bf9f8..ce22e88 100644
--- a/xen/drivers/acpi/numa.c
+++ b/xen/drivers/acpi/numa.c
@@ -25,9 +25,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 
 #define ACPI_NUMA  0x8000
 #define _COMPONENT ACPI_NUMA
@@ -105,6 +107,21 @@ void __init acpi_table_print_srat_entry(struct 
acpi_subtable_header * header)
}
 #endif /* ACPI_DEBUG_OUTPUT */
break;
+   case ACPI_SRAT_TYPE_GICC_AFFINITY:
+#ifdef ACPI_DEBUG_OUTPUT
+   {
+   struct acpi_srat_gicc_affinity *p =
+   (struct acpi_srat_gicc_affinity *)header;
+   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "SRAT Processor (acpi id[0x%04x]) in"
+ " proximity domain %d %s\n",
+ p->acpi_processor_uid,
+ p->proximity_domain,
+ (p->flags & ACPI_SRAT_GICC_ENABLED) ?
+ "enabled" : "disabled");
+   }
+#endif /* ACPI_DEBUG_OUTPUT */
+   break;
default:
printk(KERN_WARNING PREFIX
   "Found unsupported SRAT entry (type = %#x)\n",
@@ -185,6 +202,24 @@ int __init acpi_parse_srat(struct acpi_table_header *table)
return 0;
 }
 
+static int __init
+acpi_parse_gicc_affinity(struct acpi_subtable_header *header,
+const unsigned long end)
+{
+   const struct acpi_srat_gicc_affinity *processor_affinity
+   = (struct acpi_srat_gicc_affinity *)header;
+
+   if (!processor_affinity)
+   return -EINVAL;
+
+   acpi_table_print_srat_entry(header);
+
+   /* let architecture-dependent part to do it */
+   acpi_numa_gicc_affinity_init(processor_affinity);
+
+   return 0;
+}
+
 int __init
 acpi_table_parse_srat(int id, acpi_madt_entry_handler handler,
  unsigned int max_entries)
@@ -205,6 +240,8 @@ int __init acpi_numa_init(void)
acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
  acpi_parse_memory_affinity,
  NR_NODE_MEMBLKS);
+   acpi_table_parse_srat(ACPI_SRAT_TYPE_GICC_AFFINITY,
+