Re: [Xen-devel] [RFC PATCH v1 06/21] ARM: NUMA: Parse CPU NUMA information

2017-02-22 Thread Julien Grall 

Hello Vijay,

On 22/02/17 10:46, Vijay Kilari wrote:

On Mon, Feb 20, 2017 at 11:02 PM, Julien Grall  wrote:

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


From: Vijaya Kumar K 

Parse CPU node and fetch numa-node-id information.
For each node-id found, update nodemask_t mask.



A link to the bindings would have been useful...


Call numa_init() from setup_mm() with start and end
pfn of the complete ram..



s/.././



Signed-off-by: Vijaya Kumar K 
---
 xen/arch/arm/Makefile |  1 +
 xen/arch/arm/bootfdt.c|  8 ++---
 xen/arch/arm/dt_numa.c| 72
+++
 xen/arch/arm/numa.c   | 14 +
 xen/arch/arm/setup.c  |  3 ++
 xen/include/asm-arm/numa.h| 14 +
 xen/include/xen/device_tree.h |  4 +++
 7 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b5d7a19..7694485 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -50,6 +50,7 @@ obj-y += vtimer.o
 obj-y += vpsci.o
 obj-y += vuart.o
 obj-$(CONFIG_NUMA) += numa.o
+obj-$(CONFIG_NUMA) += dt_numa.o



I would prefer if we introduce a directly numa within arm. This would make
the root cleaner.


As it is very specific to DT, I have introduced in this file. ACPI is
implemented
in separate file. Common arm specific numa code is in numa.c file.


Sorry I meant separate directory not not "directly".

Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH v1 06/21] ARM: NUMA: Parse CPU NUMA information

2017-02-22 Thread Vijay Kilari 
On Mon, Feb 20, 2017 at 11:02 PM, Julien Grall  wrote:
> Hello Vijay,
>
> On 09/02/17 15:56, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Parse CPU node and fetch numa-node-id information.
>> For each node-id found, update nodemask_t mask.
>
>
> A link to the bindings would have been useful...
>
>> Call numa_init() from setup_mm() with start and end
>> pfn of the complete ram..
>
>
> s/.././
>
>
>> Signed-off-by: Vijaya Kumar K 
>> ---
>>  xen/arch/arm/Makefile |  1 +
>>  xen/arch/arm/bootfdt.c|  8 ++---
>>  xen/arch/arm/dt_numa.c| 72
>> +++
>>  xen/arch/arm/numa.c   | 14 +
>>  xen/arch/arm/setup.c  |  3 ++
>>  xen/include/asm-arm/numa.h| 14 +
>>  xen/include/xen/device_tree.h |  4 +++
>>  7 files changed, 112 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index b5d7a19..7694485 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -50,6 +50,7 @@ obj-y += vtimer.o
>>  obj-y += vpsci.o
>>  obj-y += vuart.o
>>  obj-$(CONFIG_NUMA) += numa.o
>> +obj-$(CONFIG_NUMA) += dt_numa.o
>
>
> I would prefer if we introduce a directly numa within arm. This would make
> the root cleaner.

As it is very specific to DT, I have introduced in this file. ACPI is
implemented
in separate file. Common arm specific numa code is in numa.c file.

>
>
>>
>>  #obj-bin-y += o
>>
>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
>> index 979f675..d1122d8 100644
>> --- a/xen/arch/arm/bootfdt.c
>> +++ b/xen/arch/arm/bootfdt.c
>> @@ -17,8 +17,8 @@
>>  #include 
>>  #include 
>>
>> -static bool_t __init device_tree_node_matches(const void *fdt, int node,
>> -  const char *match)
>> +bool_t __init device_tree_node_matches(const void *fdt, int node,
>> +   const char *match)
>>  {
>>  const char *name;
>>  size_t match_len;
>> @@ -63,8 +63,8 @@ static void __init device_tree_get_reg(const __be32
>> **cell, u32 address_cells,
>>  *size = dt_next_cell(size_cells, cell);
>>  }
>>
>> -static u32 __init device_tree_get_u32(const void *fdt, int node,
>> -  const char *prop_name, u32 dflt)
>> +u32 __init device_tree_get_u32(const void *fdt, int node,
>> +   const char *prop_name, u32 dflt)
>>  {
>>  const struct fdt_property *prop;
>>
>> diff --git a/xen/arch/arm/dt_numa.c b/xen/arch/arm/dt_numa.c
>> new file mode 100644
>> index 000..4b94c36
>> --- /dev/null
>> +++ b/xen/arch/arm/dt_numa.c
>> @@ -0,0 +1,72 @@
>> +/*
>> + * OF NUMA Parsing support.
>> + *
>> + * Copyright (C) 2015 - 2016 Cavium Inc.
>> + *
>> + * Some code extracts are taken from linux drivers/of/of_numa.c
>
>
> Please specify which code has been extract from Linux and the commit id.
>
> Looking at this patch only, none of this code is from Linux. Or it has been
> heavily modified.
It is heavily modified. I will drop this statement.

>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see .
>> + */
>> +
>> +#include 
>
>
> No need to include xen/config.h
>
>> +#include 
>
>
> This include should not be there as the device tree is not yet parsed.
>
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +nodemask_t numa_nodes_parsed;
>
>
> Why does this variable live in dt_numa.c when this is used by common and
> acpi code?
>
> Also, any variable/function exported should have there prototype in the
> associated header.

ok. will check.
>
>> +
>> +/*
>> + * Even though we connect cpus to numa domains later in SMP
>> + * init, we need to know the node ids now for all cpus.
>> +*/
>
>
> Coding style:
>
> /*
>  * ...
>
>  */
>
>> +static int __init dt_numa_process_cpu_node(const void *fdt, int node,
>> +   const char *name,
>> +   u32 address_cells, u32
>> size_cells)
>> +{
>> +u32 nid;
>> +
>> +nid = device_tree_get_u32(fdt, node, "numa-node-id", MAX_NUMNODES);
>> +
>> +if ( nid >= MAX_NUMNODES )
>> +printk(XENLOG_WARNING "NUMA: Node id %u exceeds maximum value\n",
>> nid);
>> +else
>> +node_set(nid, numa_nodes_parsed);
>> +
>> +return 0;
>> +}
>> +
>> +static int __init dt_numa_scan_cpu_node(const void *fdt, int node,
>

Re: [Xen-devel] [RFC PATCH v1 06/21] ARM: NUMA: Parse CPU NUMA information

2017-02-20 Thread Julien Grall 

Hello Vijay,

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

+static int __init dt_numa_scan_cpu_node(const void *fdt, int node,
+const char *name, int depth,
+u32 address_cells, u32 size_cells,
+void *data)
+


I forgot to mention this. Please drop this newline.



+{
+if ( device_tree_node_matches(fdt, node, "cpu") )
+return dt_numa_process_cpu_node(fdt, node, name, address_cells,
+size_cells);
+
+return 0;
+}


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH v1 06/21] ARM: NUMA: Parse CPU NUMA information

2017-02-20 Thread Julien Grall 

Hello Vijay,

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

From: Vijaya Kumar K 

Parse CPU node and fetch numa-node-id information.
For each node-id found, update nodemask_t mask.


A link to the bindings would have been useful...


Call numa_init() from setup_mm() with start and end
pfn of the complete ram..


s/.././



Signed-off-by: Vijaya Kumar K 
---
 xen/arch/arm/Makefile |  1 +
 xen/arch/arm/bootfdt.c|  8 ++---
 xen/arch/arm/dt_numa.c| 72 +++
 xen/arch/arm/numa.c   | 14 +
 xen/arch/arm/setup.c  |  3 ++
 xen/include/asm-arm/numa.h| 14 +
 xen/include/xen/device_tree.h |  4 +++
 7 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b5d7a19..7694485 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -50,6 +50,7 @@ obj-y += vtimer.o
 obj-y += vpsci.o
 obj-y += vuart.o
 obj-$(CONFIG_NUMA) += numa.o
+obj-$(CONFIG_NUMA) += dt_numa.o


I would prefer if we introduce a directly numa within arm. This would 
make the root cleaner.




 #obj-bin-y += o

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 979f675..d1122d8 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -17,8 +17,8 @@
 #include 
 #include 

-static bool_t __init device_tree_node_matches(const void *fdt, int node,
-  const char *match)
+bool_t __init device_tree_node_matches(const void *fdt, int node,
+   const char *match)
 {
 const char *name;
 size_t match_len;
@@ -63,8 +63,8 @@ static void __init device_tree_get_reg(const __be32 **cell, 
u32 address_cells,
 *size = dt_next_cell(size_cells, cell);
 }

-static u32 __init device_tree_get_u32(const void *fdt, int node,
-  const char *prop_name, u32 dflt)
+u32 __init device_tree_get_u32(const void *fdt, int node,
+   const char *prop_name, u32 dflt)
 {
 const struct fdt_property *prop;

diff --git a/xen/arch/arm/dt_numa.c b/xen/arch/arm/dt_numa.c
new file mode 100644
index 000..4b94c36
--- /dev/null
+++ b/xen/arch/arm/dt_numa.c
@@ -0,0 +1,72 @@
+/*
+ * OF NUMA Parsing support.
+ *
+ * Copyright (C) 2015 - 2016 Cavium Inc.
+ *
+ * Some code extracts are taken from linux drivers/of/of_numa.c


Please specify which code has been extract from Linux and the commit id.

Looking at this patch only, none of this code is from Linux. Or it has 
been heavily modified.



+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 


No need to include xen/config.h


+#include 


This include should not be there as the device tree is not yet parsed.


+#include 
+#include 
+#include 
+#include 
+#include 
+
+nodemask_t numa_nodes_parsed;


Why does this variable live in dt_numa.c when this is used by common and 
acpi code?


Also, any variable/function exported should have there prototype in the 
associated header.



+
+/*
+ * Even though we connect cpus to numa domains later in SMP
+ * init, we need to know the node ids now for all cpus.
+*/


Coding style:

/*
 * ...
 */


+static int __init dt_numa_process_cpu_node(const void *fdt, int node,
+   const char *name,
+   u32 address_cells, u32 size_cells)
+{
+u32 nid;
+
+nid = device_tree_get_u32(fdt, node, "numa-node-id", MAX_NUMNODES);
+
+if ( nid >= MAX_NUMNODES )
+printk(XENLOG_WARNING "NUMA: Node id %u exceeds maximum value\n", nid);
+else
+node_set(nid, numa_nodes_parsed);
+
+return 0;
+}
+
+static int __init dt_numa_scan_cpu_node(const void *fdt, int node,
+const char *name, int depth,
+u32 address_cells, u32 size_cells,
+void *data)
+
+{
+if ( device_tree_node_matches(fdt, node, "cpu") )
+return dt_numa_process_cpu_node(fdt, node, name, address_cells,
+size_cells);


This code is wrong. CPUs nodes can only be in /cpus and you cannot rely 
on the name to be "cpu" (see binding in 
Documentation/devicetree/bindings/arm/cpu.txt). The only way to check if 
it is a CPU is to look for the property device_type.



+
+return 0;
+}
+
+int __init dt_numa

[Xen-devel] [RFC PATCH v1 06/21] ARM: NUMA: Parse CPU NUMA information

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

Parse CPU node and fetch numa-node-id information.
For each node-id found, update nodemask_t mask.

Call numa_init() from setup_mm() with start and end
pfn of the complete ram..

Signed-off-by: Vijaya Kumar K 
---
 xen/arch/arm/Makefile |  1 +
 xen/arch/arm/bootfdt.c|  8 ++---
 xen/arch/arm/dt_numa.c| 72 +++
 xen/arch/arm/numa.c   | 14 +
 xen/arch/arm/setup.c  |  3 ++
 xen/include/asm-arm/numa.h| 14 +
 xen/include/xen/device_tree.h |  4 +++
 7 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b5d7a19..7694485 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -50,6 +50,7 @@ obj-y += vtimer.o
 obj-y += vpsci.o
 obj-y += vuart.o
 obj-$(CONFIG_NUMA) += numa.o
+obj-$(CONFIG_NUMA) += dt_numa.o
 
 #obj-bin-y += o
 
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 979f675..d1122d8 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -17,8 +17,8 @@
 #include 
 #include 
 
-static bool_t __init device_tree_node_matches(const void *fdt, int node,
-  const char *match)
+bool_t __init device_tree_node_matches(const void *fdt, int node,
+   const char *match)
 {
 const char *name;
 size_t match_len;
@@ -63,8 +63,8 @@ static void __init device_tree_get_reg(const __be32 **cell, 
u32 address_cells,
 *size = dt_next_cell(size_cells, cell);
 }
 
-static u32 __init device_tree_get_u32(const void *fdt, int node,
-  const char *prop_name, u32 dflt)
+u32 __init device_tree_get_u32(const void *fdt, int node,
+   const char *prop_name, u32 dflt)
 {
 const struct fdt_property *prop;
 
diff --git a/xen/arch/arm/dt_numa.c b/xen/arch/arm/dt_numa.c
new file mode 100644
index 000..4b94c36
--- /dev/null
+++ b/xen/arch/arm/dt_numa.c
@@ -0,0 +1,72 @@
+/*
+ * OF NUMA Parsing support.
+ *
+ * Copyright (C) 2015 - 2016 Cavium Inc.
+ *
+ * Some code extracts are taken from linux drivers/of/of_numa.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+nodemask_t numa_nodes_parsed;
+
+/*
+ * Even though we connect cpus to numa domains later in SMP
+ * init, we need to know the node ids now for all cpus.
+*/
+static int __init dt_numa_process_cpu_node(const void *fdt, int node,
+   const char *name,
+   u32 address_cells, u32 size_cells)
+{
+u32 nid;
+
+nid = device_tree_get_u32(fdt, node, "numa-node-id", MAX_NUMNODES);
+
+if ( nid >= MAX_NUMNODES )
+printk(XENLOG_WARNING "NUMA: Node id %u exceeds maximum value\n", nid);
+else
+node_set(nid, numa_nodes_parsed);
+
+return 0;
+}
+
+static int __init dt_numa_scan_cpu_node(const void *fdt, int node,
+const char *name, int depth,
+u32 address_cells, u32 size_cells,
+void *data)
+
+{
+if ( device_tree_node_matches(fdt, node, "cpu") )
+return dt_numa_process_cpu_node(fdt, node, name, address_cells,
+size_cells);
+
+return 0;
+}
+
+int __init dt_numa_init(void)
+{
+int ret;
+
+nodes_clear(numa_nodes_parsed);
+ret = device_tree_for_each_node((void *)device_tree_flattened,
+dt_numa_scan_cpu_node, NULL);
+return ret;
+}
diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index 59d09c7..9a7f0bb 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -21,6 +21,20 @@
 #include 
 #include 
 #include 
+#include 
+
+int __init numa_init(void)
+{
+int ret = 0;
+
+if ( numa_off )
+goto no_numa;
+
+ret = dt_numa_init();
+
+no_numa:
+return ret;
+}
 
 int __init arch_numa_setup(char *opt)
 {
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 049e449..b6618ca 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -753,6 +754,8 @@ void __init start_xen(unsigned long boot_phys_offset,
 /* Parse the ACPI tables for po