Re: [Xen-devel] [RFC PATCH v1 15/21] ARM: NUMA: Extract MPIDR from MADT table

2017-03-02 Thread Julien Grall



On 02/03/17 16:41, Vijay Kilari wrote:

On Thu, Mar 2, 2017 at 9:58 PM, Julien Grall  wrote:

Hello Vijay,

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


From: Vijaya Kumar K 

Parse MADT table and extract MPIDR for all
CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries
and store in cpu_uid_to_hwid[].

This mapping is used by SRAT table parsing to
extract MPIDR of the CPU ID.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/Makefile  |   1 +
 xen/arch/arm/acpi_numa.c   | 122
+
 xen/arch/arm/numa.c|   1 +



This new file should go in xen/arch/arm/acpi/


shouldn't be in xen/arch/arm/numa/?


If you introduce a numa directory then move in it. Otherwise acpi/.



+static int __init acpi_parse_madt_handler(struct acpi_subtable_header
*header,
+  const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *p =
+   container_of(header, struct acpi_madt_generic_interrupt,
header);
+
+if ( BAD_MADT_ENTRY(p, end) )
+{
+/* Though MADT is invalid, we disable NUMA by calling bad_srat()
*/
+bad_srat();
+return -EINVAL;
+}
+
+acpi_table_print_madt_entry(header);
+acpi_map_cpu_to_mpidr(p);
+
+return 0;
+}



Why do you need to parse the MADT again? Can't this be done in the parsing
made in acpi/boot.c?

I will check. But I see that this is done quite late in smp_init().


Then rework the code.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [RFC PATCH v1 15/21] ARM: NUMA: Extract MPIDR from MADT table

2017-03-02 Thread Vijay Kilari
On Thu, Mar 2, 2017 at 9:58 PM, Julien Grall  wrote:
> Hello Vijay,
>
> On 09/02/17 15:57, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Parse MADT table and extract MPIDR for all
>> CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries
>> and store in cpu_uid_to_hwid[].
>>
>> This mapping is used by SRAT table parsing to
>> extract MPIDR of the CPU ID.
>>
>> Signed-off-by: Vijaya Kumar 
>> ---
>>  xen/arch/arm/Makefile  |   1 +
>>  xen/arch/arm/acpi_numa.c   | 122
>> +
>>  xen/arch/arm/numa.c|   1 +
>
>
> This new file should go in xen/arch/arm/acpi/

shouldn't be in xen/arch/arm/numa/?
>
>
>>  xen/include/asm-arm/acpi.h |   2 +
>>  4 files changed, 126 insertions(+)
>>
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 7694485..8c5e67b 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -51,6 +51,7 @@ obj-y += vpsci.o
>>  obj-y += vuart.o
>>  obj-$(CONFIG_NUMA) += numa.o
>>  obj-$(CONFIG_NUMA) += dt_numa.o
>> +obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o
>>
>>  #obj-bin-y += o
>>
>> diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
>> new file mode 100644
>> index 000..3ee87f2
>> --- /dev/null
>> +++ b/xen/arch/arm/acpi_numa.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * ACPI based NUMA setup
>> + *
>> + * Copyright (C) 2016 - Cavium Inc.
>> + * Vijaya Kumar K 
>> + *
>> + * Reads the ACPI MADT and SRAT table to setup NUMA information.
>> + *
>> + * Contains Excerpts from x86 implementation
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>
>
> Xen is GPLv2, please update the license accordingly.
>
>
>> + *
>> + * 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.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +extern nodemask_t numa_nodes_parsed;
>> +struct uid_to_mpidr {
>> +u32 uid;
>> +u64 mpidr;
>> +};
>> +
>> +/* Holds mapping of CPU id to MPIDR read from MADT */
>> +static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly;
>> +
>> +static __init void bad_srat(void)
>> +{
>> +int i;
>> +
>> +printk(KERN_ERR "SRAT: SRAT not used.\n");
>> +acpi_numa = -1;
>> +for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
>> +pxm2node[i].node = NUMA_NO_NODE;
>> +}
>> +
>> +static u64 acpi_get_cpu_mpidr(int uid)
>> +{
>> +int i;
>> +
>> +if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid ==
>> uid &&
>> + cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID )
>> +return cpu_uid_to_hwid[uid].mpidr;
>
>
> Please don't make a special case. This makes more complicate to read the
> code.
>
> We should just loop to find the entry matching the UID.
>
>> +
>> +for ( i = 0; i < NR_CPUS; i++ )
>
>
> You can limit the loop by keeping an the number of element in the array.
OK.
>
>
>> +{
>> +if ( cpu_uid_to_hwid[i].uid == uid )
>> +return cpu_uid_to_hwid[i].mpidr;
>> +}
>> +
>> +return MPIDR_INVALID;
>> +}
>> +
>> +static void __init
>> +acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor)
>> +{
>> +static int cpus = 0;
>> +
>> +u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
>> +
>> +if ( mpidr == MPIDR_INVALID )
>> +{
>> +printk("Skip MADT cpu entry with invalid MPIDR\n");
>> +bad_srat();
>> +return;
>> +}
>> +
>> +cpu_uid_to_hwid[cpus].mpidr = mpidr;
>> +cpu_uid_to_hwid[cpus].uid = processor->uid;
>> +
>> +cpus++;
>> +}
>> +
>> +static int __init acpi_parse_madt_handler(struct acpi_subtable_header
>> *header,
>> +  const unsigned long end)
>> +{
>> +struct acpi_madt_generic_interrupt *p =
>> +   container_of(header, struct acpi_madt_generic_interrupt,
>> header);
>> +
>> +if ( BAD_MADT_ENTRY(p, end) )
>> +{
>> +/* Though MADT is invalid, we disable NUMA by calling bad_srat()
>> */
>> +bad_srat();
>> +return -EINVAL;
>> +}
>> +
>> +acpi_table_print_madt_entry(header);
>> +acpi_map_cpu_to_mpidr(p);
>> +
>> +return 0;
>> +}
>
>
> Why do you need to parse the MADT again? Can't this be done in the parsing
> made in acpi/boot.c?
I will check. But I see that this is done quite late in smp_init().

___
Xen-devel mailing list
Xen-devel@lists.xen.org

Re: [Xen-devel] [RFC PATCH v1 15/21] ARM: NUMA: Extract MPIDR from MADT table

2017-03-02 Thread Julien Grall

Hello Vijay,

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

From: Vijaya Kumar K 

Parse MADT table and extract MPIDR for all
CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries
and store in cpu_uid_to_hwid[].

This mapping is used by SRAT table parsing to
extract MPIDR of the CPU ID.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/Makefile  |   1 +
 xen/arch/arm/acpi_numa.c   | 122 +
 xen/arch/arm/numa.c|   1 +


This new file should go in xen/arch/arm/acpi/


 xen/include/asm-arm/acpi.h |   2 +
 4 files changed, 126 insertions(+)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 7694485..8c5e67b 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -51,6 +51,7 @@ obj-y += vpsci.o
 obj-y += vuart.o
 obj-$(CONFIG_NUMA) += numa.o
 obj-$(CONFIG_NUMA) += dt_numa.o
+obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o

 #obj-bin-y += o

diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
new file mode 100644
index 000..3ee87f2
--- /dev/null
+++ b/xen/arch/arm/acpi_numa.c
@@ -0,0 +1,122 @@
+/*
+ * ACPI based NUMA setup
+ *
+ * Copyright (C) 2016 - Cavium Inc.
+ * Vijaya Kumar K 
+ *
+ * Reads the ACPI MADT and SRAT table to setup NUMA information.
+ *
+ * Contains Excerpts from x86 implementation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.


Xen is GPLv2, please update the license accordingly.


+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+extern nodemask_t numa_nodes_parsed;
+struct uid_to_mpidr {
+u32 uid;
+u64 mpidr;
+};
+
+/* Holds mapping of CPU id to MPIDR read from MADT */
+static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly;
+
+static __init void bad_srat(void)
+{
+int i;
+
+printk(KERN_ERR "SRAT: SRAT not used.\n");
+acpi_numa = -1;
+for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
+pxm2node[i].node = NUMA_NO_NODE;
+}
+
+static u64 acpi_get_cpu_mpidr(int uid)
+{
+int i;
+
+if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid == uid 
&&
+ cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID )
+return cpu_uid_to_hwid[uid].mpidr;


Please don't make a special case. This makes more complicate to read the 
code.


We should just loop to find the entry matching the UID.


+
+for ( i = 0; i < NR_CPUS; i++ )


You can limit the loop by keeping an the number of element in the array.


+{
+if ( cpu_uid_to_hwid[i].uid == uid )
+return cpu_uid_to_hwid[i].mpidr;
+}
+
+return MPIDR_INVALID;
+}
+
+static void __init
+acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor)
+{
+static int cpus = 0;
+
+u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
+
+if ( mpidr == MPIDR_INVALID )
+{
+printk("Skip MADT cpu entry with invalid MPIDR\n");
+bad_srat();
+return;
+}
+
+cpu_uid_to_hwid[cpus].mpidr = mpidr;
+cpu_uid_to_hwid[cpus].uid = processor->uid;
+
+cpus++;
+}
+
+static int __init acpi_parse_madt_handler(struct acpi_subtable_header *header,
+  const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *p =
+   container_of(header, struct acpi_madt_generic_interrupt, 
header);
+
+if ( BAD_MADT_ENTRY(p, end) )
+{
+/* Though MADT is invalid, we disable NUMA by calling bad_srat() */
+bad_srat();
+return -EINVAL;
+}
+
+acpi_table_print_madt_entry(header);
+acpi_map_cpu_to_mpidr(p);
+
+return 0;
+}


Why do you need to parse the MADT again? Can't this be done in the 
parsing made in acpi/boot.c?



+
+void __init acpi_map_uid_to_mpidr(void)
+{
+int i;


unsigned int.


+
+for ( i  = 0; i < NR_CPUS; i++ )
+{
+cpu_uid_to_hwid[i].mpidr = MPIDR_INVALID;
+cpu_uid_to_hwid[i].uid = -1;
+}
+
+acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+acpi_parse_madt_handler, 0);
+}
+
+void __init acpi_numa_arch_fixup(void) {}


Missing emacs magic.


diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index 31dc552..5c49347 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 


Why this include? This patch should compile without it.


 #include 
 #include 
 #include 
diff --git a/xen/include/asm-arm/acpi.h 

[Xen-devel] [RFC PATCH v1 15/21] ARM: NUMA: Extract MPIDR from MADT table

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

Parse MADT table and extract MPIDR for all
CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries
and store in cpu_uid_to_hwid[].

This mapping is used by SRAT table parsing to
extract MPIDR of the CPU ID.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/Makefile  |   1 +
 xen/arch/arm/acpi_numa.c   | 122 +
 xen/arch/arm/numa.c|   1 +
 xen/include/asm-arm/acpi.h |   2 +
 4 files changed, 126 insertions(+)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 7694485..8c5e67b 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -51,6 +51,7 @@ obj-y += vpsci.o
 obj-y += vuart.o
 obj-$(CONFIG_NUMA) += numa.o
 obj-$(CONFIG_NUMA) += dt_numa.o
+obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o
 
 #obj-bin-y += o
 
diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
new file mode 100644
index 000..3ee87f2
--- /dev/null
+++ b/xen/arch/arm/acpi_numa.c
@@ -0,0 +1,122 @@
+/*
+ * ACPI based NUMA setup
+ *
+ * Copyright (C) 2016 - Cavium Inc.
+ * Vijaya Kumar K 
+ *
+ * Reads the ACPI MADT and SRAT table to setup NUMA information.
+ *
+ * Contains Excerpts from x86 implementation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+extern nodemask_t numa_nodes_parsed;
+struct uid_to_mpidr {
+u32 uid;
+u64 mpidr;
+};
+
+/* Holds mapping of CPU id to MPIDR read from MADT */
+static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly;
+
+static __init void bad_srat(void)
+{
+int i;
+
+printk(KERN_ERR "SRAT: SRAT not used.\n");
+acpi_numa = -1;
+for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
+pxm2node[i].node = NUMA_NO_NODE;
+}
+
+static u64 acpi_get_cpu_mpidr(int uid)
+{
+int i;
+
+if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid == uid 
&&
+ cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID )
+return cpu_uid_to_hwid[uid].mpidr;
+
+for ( i = 0; i < NR_CPUS; i++ )
+{
+if ( cpu_uid_to_hwid[i].uid == uid )
+return cpu_uid_to_hwid[i].mpidr;
+}
+
+return MPIDR_INVALID;
+}
+
+static void __init
+acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor)
+{
+static int cpus = 0;
+
+u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
+
+if ( mpidr == MPIDR_INVALID )
+{
+printk("Skip MADT cpu entry with invalid MPIDR\n");
+bad_srat();
+return;
+}
+
+cpu_uid_to_hwid[cpus].mpidr = mpidr;
+cpu_uid_to_hwid[cpus].uid = processor->uid;
+
+cpus++;
+}
+
+static int __init acpi_parse_madt_handler(struct acpi_subtable_header *header,
+  const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *p =
+   container_of(header, struct acpi_madt_generic_interrupt, 
header);
+
+if ( BAD_MADT_ENTRY(p, end) )
+{
+/* Though MADT is invalid, we disable NUMA by calling bad_srat() */
+bad_srat();
+return -EINVAL;
+}
+
+acpi_table_print_madt_entry(header);
+acpi_map_cpu_to_mpidr(p);
+
+return 0;
+}
+
+void __init acpi_map_uid_to_mpidr(void)
+{
+int i;
+
+for ( i  = 0; i < NR_CPUS; i++ )
+{
+cpu_uid_to_hwid[i].mpidr = MPIDR_INVALID;
+cpu_uid_to_hwid[i].uid = -1;
+}
+
+acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+acpi_parse_madt_handler, 0);
+}
+
+void __init acpi_numa_arch_fixup(void) {}
diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index 31dc552..5c49347 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
index 9f954d3..b1f36f4 100644
--- a/xen/include/asm-arm/acpi.h
+++ b/xen/include/asm-arm/acpi.h
@@ -68,6 +68,8 @@ static inline void enable_acpi(void)
 {
 acpi_disabled = 0;
 }
+
+void acpi_map_uid_to_mpidr(void);
 #else
 #define acpi_disabled (1)
 #define disable_acpi()
-- 
2.7.4


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