[PATCH v2 0/4] ACPI based NUMA support for ARM64

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Ganapatrao Kulkarni]

 -v2 rebased on arm64-numa v7 patches.
 http://www.spinics.net/lists/arm-kernel/msg460813.html

 -v1 initial patches from Hanjun Guo

Hanjun Guo (4):
  acpi, numa: Use pr_fmt() instead of printk
  acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
  arm64, acpi, numa: NUMA support based on SRAT and SLIT
  acpi, numa: Enable ACPI based NUMA on ARM64

 arch/arm64/include/asm/acpi.h |   4 +
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/numa.c          |   7 +-
 drivers/acpi/Kconfig          |   4 +-
 drivers/acpi/numa.c           | 106 ++++++++++++---------
 include/linux/acpi.h          |  15 +++
 7 files changed, 306 insertions(+), 46 deletions(-)
 create mode 100644 arch/arm64/kernel/acpi_numa.c

-- 
1.8.1.4

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Ganapatrao Kulkarni]

 -v2 rebased on arm64-numa v7 patches.
 http://www.spinics.net/lists/arm-kernel/msg460813.html

 -v1 initial patches from Hanjun Guo

Hanjun Guo (4):
  acpi, numa: Use pr_fmt() instead of printk
  acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
  arm64, acpi, numa: NUMA support based on SRAT and SLIT
  acpi, numa: Enable ACPI based NUMA on ARM64

 arch/arm64/include/asm/acpi.h |   4 +
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/numa.c          |   7 +-
 drivers/acpi/Kconfig          |   4 +-
 drivers/acpi/numa.c           | 106 ++++++++++++---------
 include/linux/acpi.h          |  15 +++
 7 files changed, 306 insertions(+), 46 deletions(-)
 create mode 100644 arch/arm64/kernel/acpi_numa.c

-- 
1.8.1.4

[PATCH v2 1/4] acpi, numa: Use pr_fmt() instead of printk

[Ganapatrao Kulkarni]

From: Hanjun Guo <hanjun.guo@linaro.org>

Just do some cleanups to replace printk with pr_fmt().

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/numa.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 72b6e9e..4e427fc 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -18,6 +18,9 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -28,8 +31,6 @@
 #include <linux/nodemask.h>
 #include <linux/topology.h>
 
-#define PREFIX "ACPI: "
-
 #define ACPI_NUMA	0x80000000
 #define _COMPONENT	ACPI_NUMA
 ACPI_MODULE_NAME("numa");
@@ -187,9 +188,8 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 #endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 	default:
-		printk(KERN_WARNING PREFIX
-		       "Found unsupported SRAT entry (type = 0x%x)\n",
-		       header->type);
+		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
+			header->type);
 		break;
 	}
 }
@@ -222,7 +222,7 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
 	struct acpi_table_slit *slit = (struct acpi_table_slit *)table;
 
 	if (!slit_valid(slit)) {
-		printk(KERN_INFO "ACPI: SLIT table looks invalid. Not used.\n");
+		pr_info("SLIT table looks invalid. Not used.\n");
 		return -EINVAL;
 	}
 	acpi_numa_slit_init(slit);
@@ -233,12 +233,9 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
 void __init __weak
 acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 {
-	printk(KERN_WARNING PREFIX
-	       "Found unsupported x2apic [0x%08x] SRAT entry\n", pa->apic_id);
-	return;
+	pr_warn("Found unsupported x2apic [0x%08x] SRAT entry\n", pa->apic_id);
 }
 
-
 static int __init
 acpi_parse_x2apic_affinity(struct acpi_subtable_header *header,
 			   const unsigned long end)
-- 
1.8.1.4

[PATCH v2 1/4] acpi, numa: Use pr_fmt() instead of printk

[Ganapatrao Kulkarni]

From: Hanjun Guo <hanjun.guo@linaro.org>

Just do some cleanups to replace printk with pr_fmt().

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/numa.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 72b6e9e..4e427fc 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -18,6 +18,9 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -28,8 +31,6 @@
 #include <linux/nodemask.h>
 #include <linux/topology.h>
 
-#define PREFIX "ACPI: "
-
 #define ACPI_NUMA	0x80000000
 #define _COMPONENT	ACPI_NUMA
 ACPI_MODULE_NAME("numa");
@@ -187,9 +188,8 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 #endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 	default:
-		printk(KERN_WARNING PREFIX
-		       "Found unsupported SRAT entry (type = 0x%x)\n",
-		       header->type);
+		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
+			header->type);
 		break;
 	}
 }
@@ -222,7 +222,7 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
 	struct acpi_table_slit *slit = (struct acpi_table_slit *)table;
 
 	if (!slit_valid(slit)) {
-		printk(KERN_INFO "ACPI: SLIT table looks invalid. Not used.\n");
+		pr_info("SLIT table looks invalid. Not used.\n");
 		return -EINVAL;
 	}
 	acpi_numa_slit_init(slit);
@@ -233,12 +233,9 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
 void __init __weak
 acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 {
-	printk(KERN_WARNING PREFIX
-	       "Found unsupported x2apic [0x%08x] SRAT entry\n", pa->apic_id);
-	return;
+	pr_warn("Found unsupported x2apic [0x%08x] SRAT entry\n", pa->apic_id);
 }
 
-
 static int __init
 acpi_parse_x2apic_affinity(struct acpi_subtable_header *header,
 			   const unsigned long end)
-- 
1.8.1.4

[PATCH v2 2/4] acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()

[Ganapatrao Kulkarni]

Replace ACPI_DEBUG_PRINT() with pr_debug() in this patch as
ACPI_DEBUG_OUTPUT is controlled by ACPICA.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/numa.c | 54 +++++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 4e427fc..36683d0 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -129,64 +129,52 @@ EXPORT_SYMBOL(acpi_map_pxm_to_online_node);
 static void __init
 acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 {
-
-	ACPI_FUNCTION_NAME("acpi_table_print_srat_entry");
-
 	if (!header)
 		return;
 
 	switch (header->type) {
 
 	case ACPI_SRAT_TYPE_CPU_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_cpu_affinity *p =
 			    (struct acpi_srat_cpu_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Processor (id[0x%02x] eid[0x%02x]) in proximity domain %d %s\n",
-					  p->apic_id, p->local_sapic_eid,
-					  p->proximity_domain_lo,
-					  (p->flags & ACPI_SRAT_CPU_ENABLED)?
-					  "enabled" : "disabled"));
+			pr_debug("SRAT Processor (id[0x%02x] eid[0x%02x]) in proximity domain %d %s\n",
+				 p->apic_id, p->local_sapic_eid,
+				 p->proximity_domain_lo,
+				 (p->flags & ACPI_SRAT_CPU_ENABLED) ?
+				 "enabled" : "disabled");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 
 	case ACPI_SRAT_TYPE_MEMORY_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_mem_affinity *p =
 			    (struct acpi_srat_mem_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Memory (0x%lx length 0x%lx) in proximity domain %d %s%s%s\n",
-					  (unsigned long)p->base_address,
-					  (unsigned long)p->length,
-					  p->proximity_domain,
-					  (p->flags & ACPI_SRAT_MEM_ENABLED)?
-					  "enabled" : "disabled",
-					  (p->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)?
-					  " hot-pluggable" : "",
-					  (p->flags & ACPI_SRAT_MEM_NON_VOLATILE)?
-					  " non-volatile" : ""));
+			pr_debug("SRAT Memory (0x%lx length 0x%lx) in proximity domain %d %s%s%s\n",
+				 (unsigned long)p->base_address,
+				 (unsigned long)p->length,
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_MEM_ENABLED) ?
+				 "enabled" : "disabled",
+				 (p->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) ?
+				 " hot-pluggable" : "",
+				 (p->flags & ACPI_SRAT_MEM_NON_VOLATILE) ?
+				 " non-volatile" : "");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 
 	case ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_x2apic_cpu_affinity *p =
 			    (struct acpi_srat_x2apic_cpu_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Processor (x2apicid[0x%08x]) in"
-					  " proximity domain %d %s\n",
-					  p->apic_id,
-					  p->proximity_domain,
-					  (p->flags & ACPI_SRAT_CPU_ENABLED) ?
-					  "enabled" : "disabled"));
+			pr_debug("SRAT Processor (x2apicid[0x%08x]) in proximity domain %d %s\n",
+				 p->apic_id,
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_CPU_ENABLED) ?
+				 "enabled" : "disabled");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
+
 	default:
 		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
 			header->type);
-- 
1.8.1.4

[PATCH v2 2/4] acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()

[Ganapatrao Kulkarni]

Replace ACPI_DEBUG_PRINT() with pr_debug() in this patch as
ACPI_DEBUG_OUTPUT is controlled by ACPICA.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/numa.c | 54 +++++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 4e427fc..36683d0 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -129,64 +129,52 @@ EXPORT_SYMBOL(acpi_map_pxm_to_online_node);
 static void __init
 acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 {
-
-	ACPI_FUNCTION_NAME("acpi_table_print_srat_entry");
-
 	if (!header)
 		return;
 
 	switch (header->type) {
 
 	case ACPI_SRAT_TYPE_CPU_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_cpu_affinity *p =
 			    (struct acpi_srat_cpu_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Processor (id[0x%02x] eid[0x%02x]) in proximity domain %d %s\n",
-					  p->apic_id, p->local_sapic_eid,
-					  p->proximity_domain_lo,
-					  (p->flags & ACPI_SRAT_CPU_ENABLED)?
-					  "enabled" : "disabled"));
+			pr_debug("SRAT Processor (id[0x%02x] eid[0x%02x]) in proximity domain %d %s\n",
+				 p->apic_id, p->local_sapic_eid,
+				 p->proximity_domain_lo,
+				 (p->flags & ACPI_SRAT_CPU_ENABLED) ?
+				 "enabled" : "disabled");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 
 	case ACPI_SRAT_TYPE_MEMORY_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_mem_affinity *p =
 			    (struct acpi_srat_mem_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Memory (0x%lx length 0x%lx) in proximity domain %d %s%s%s\n",
-					  (unsigned long)p->base_address,
-					  (unsigned long)p->length,
-					  p->proximity_domain,
-					  (p->flags & ACPI_SRAT_MEM_ENABLED)?
-					  "enabled" : "disabled",
-					  (p->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)?
-					  " hot-pluggable" : "",
-					  (p->flags & ACPI_SRAT_MEM_NON_VOLATILE)?
-					  " non-volatile" : ""));
+			pr_debug("SRAT Memory (0x%lx length 0x%lx) in proximity domain %d %s%s%s\n",
+				 (unsigned long)p->base_address,
+				 (unsigned long)p->length,
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_MEM_ENABLED) ?
+				 "enabled" : "disabled",
+				 (p->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) ?
+				 " hot-pluggable" : "",
+				 (p->flags & ACPI_SRAT_MEM_NON_VOLATILE) ?
+				 " non-volatile" : "");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
 
 	case ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY:
-#ifdef ACPI_DEBUG_OUTPUT
 		{
 			struct acpi_srat_x2apic_cpu_affinity *p =
 			    (struct acpi_srat_x2apic_cpu_affinity *)header;
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "SRAT Processor (x2apicid[0x%08x]) in"
-					  " proximity domain %d %s\n",
-					  p->apic_id,
-					  p->proximity_domain,
-					  (p->flags & ACPI_SRAT_CPU_ENABLED) ?
-					  "enabled" : "disabled"));
+			pr_debug("SRAT Processor (x2apicid[0x%08x]) in proximity domain %d %s\n",
+				 p->apic_id,
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_CPU_ENABLED) ?
+				 "enabled" : "disabled");
 		}
-#endif				/* ACPI_DEBUG_OUTPUT */
 		break;
+
 	default:
 		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
 			header->type);
-- 
1.8.1.4

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Ganapatrao Kulkarni]

Introduce a new file to hold ACPI based NUMA information
parsing from SRAT and SLIT.

SRAT includes the CPU ACPI ID to Proximity Domain mappings
and memory ranges to Proximity Domain mapping.
SLIT has the information of inter node
distances(relative number for access latency).

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Ganapatrao Kulkarni <gkulkarni@caviumnetworks.com>
---
 arch/arm64/include/asm/acpi.h |   4 +
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/numa.c          |   7 +-
 4 files changed, 226 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/acpi_numa.c

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index caafd63..e171a18 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -83,6 +83,10 @@ static inline bool acpi_has_cpu_in_madt(void)
 static inline void arch_fix_phys_package_id(int num, u32 slot) { }
 void __init acpi_init_cpus(void);
 
+#ifdef CONFIG_ACPI_NUMA
+int arm64_acpi_numa_init(void);
+#endif /* CONFIG_ACPI_NUMA */
+
 #else
 static inline void acpi_init_cpus(void) { }
 #endif /* CONFIG_ACPI */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 7987763..555c4a5 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
 arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
+arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
new file mode 100644
index 0000000..8aee205
--- /dev/null
+++ b/arch/arm64/kernel/acpi_numa.c
@@ -0,0 +1,215 @@
+/*
+ * ACPI 5.1 based NUMA setup for ARM64
+ * Lots of code was borrowed from arch/x86/mm/srat.c
+ *
+ * Copyright 2004 Andi Kleen, SuSE Labs.
+ * Copyright (C) 2013-2014, Linaro Ltd.
+ *		Author: Hanjun Guo <hanjun.guo@linaro.org>
+ *
+ * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
+ *
+ * Called from acpi_numa_init while reading the SRAT and SLIT tables.
+ * Assumes all memory regions belonging to a single proximity domain
+ * are in one chunk. Holes between them will be included in the node.
+ */
+
+#define pr_fmt(fmt) "ACPI: NUMA: " fmt
+
+#include <linux/acpi.h>
+#include <linux/bitmap.h>
+#include <linux/bootmem.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/memblock.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+#include <linux/topology.h>
+
+#include <acpi/processor.h>
+#include <asm/numa.h>
+
+int acpi_numa __initdata;
+
+static __init int setup_node(int pxm)
+{
+	return acpi_map_pxm_to_node(pxm);
+}
+
+static __init void bad_srat(void)
+{
+	pr_err("SRAT not used.\n");
+	acpi_numa = -1;
+}
+
+static __init inline int srat_disabled(void)
+{
+	return acpi_numa < 0;
+}
+
+/*
+ * Callback for SLIT parsing.
+ * It will get the distance information presented by SLIT
+ * and init the distance matrix of numa nodes
+ */
+void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
+{
+	int i, j;
+
+	for (i = 0; i < slit->locality_count; i++) {
+		const int from_node = pxm_to_node(i);
+
+		if (from_node == NUMA_NO_NODE)
+			continue;
+
+		for (j = 0; j < slit->locality_count; j++) {
+			const int to_node = pxm_to_node(j);
+
+			if (to_node == NUMA_NO_NODE)
+				continue;
+
+			pr_info("SLIT: Distance[%d][%d] = %d\n",
+					from_node, to_node,
+					slit->entry[
+					slit->locality_count * i + j]);
+			numa_set_distance(from_node, to_node,
+				slit->entry[slit->locality_count * i + j]);
+		}
+	}
+}
+
+static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
+{
+	unsigned long madt_end, entry;
+	struct acpi_table_madt *madt;
+	acpi_size tbl_size;
+
+	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
+			(struct acpi_table_header **)&madt, &tbl_size)))
+		return -ENODEV;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		struct acpi_subtable_header *header =
+			(struct acpi_subtable_header *)entry;
+
+		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
+			struct acpi_madt_generic_interrupt *gicc =
+				container_of(header,
+				struct acpi_madt_generic_interrupt, header);
+
+			if ((gicc->flags & ACPI_MADT_ENABLED) &&
+			    (gicc->uid == acpi_id)) {
+				*mpidr = gicc->arm_mpidr;
+				early_acpi_os_unmap_memory(madt, tbl_size);
+				return 0;
+			}
+		}
+		entry += header->length;
+	}
+
+	early_acpi_os_unmap_memory(madt, tbl_size);
+	return -ENODEV;
+}
+
+/* Callback for Proximity Domain -> ACPI processor UID mapping */
+void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
+{
+	int pxm, node;
+	u64 mpidr;
+	static int cpus_in_srat;
+
+	if (srat_disabled())
+		return;
+
+	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
+		bad_srat();
+		return;
+	}
+
+	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
+		return;
+
+	if (cpus_in_srat >= NR_CPUS) {
+		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
+			     NR_CPUS);
+		return;
+	}
+
+	pxm = pa->proximity_domain;
+	node = setup_node(pxm);
+
+	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
+		pr_err("SRAT: Too many proximity domains %d\n", pxm);
+		bad_srat();
+		return;
+	}
+
+	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
+		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
+			pxm, pa->acpi_processor_uid);
+		bad_srat();
+		return;
+	}
+
+	cpu_to_node_map[cpus_in_srat] = node;
+	node_set(node, numa_nodes_parsed);
+	acpi_numa = 1;
+	cpus_in_srat++;
+	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
+			pxm, mpidr, node, cpus_in_srat);
+}
+
+/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
+int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
+{
+	u64 start, end;
+	int node, pxm;
+
+	if (srat_disabled())
+		return -EINVAL;
+
+	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
+		bad_srat();
+		return -EINVAL;
+	}
+
+	start = ma->base_address;
+	end = start + ma->length;
+	pxm = ma->proximity_domain;
+
+	node = setup_node(pxm);
+
+	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
+		pr_err("SRAT: Too many proximity domains.\n");
+		bad_srat();
+		return -EINVAL;
+	}
+
+	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
+		node, pxm,
+		(unsigned long long) start, (unsigned long long) end - 1);
+
+	if (numa_add_memblk(node, start, (end - start)) < 0) {
+		bad_srat();
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+void __init acpi_numa_arch_fixup(void) { }
+
+int __init arm64_acpi_numa_init(void)
+{
+	int ret;
+
+	ret = acpi_numa_init();
+	if (ret < 0)
+		return ret;
+
+	return srat_disabled() ? -EINVAL : 0;
+}
diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index 209c7a9..c2950fc 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -17,6 +17,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
@@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
 	int ret = -ENODEV;
 
 #ifdef CONFIG_OF_NUMA
-	if (!numa_off)
+	if (!numa_off && acpi_disabled)
 		ret = numa_init(arm64_of_numa_init);
 #endif
+#ifdef CONFIG_ACPI_NUMA
+	if (!numa_off && !acpi_disabled)
+		ret = numa_init(arm64_acpi_numa_init);
+#endif
 
 	if (ret)
 		numa_init(dummy_numa_init);
-- 
1.8.1.4

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Ganapatrao Kulkarni]

Introduce a new file to hold ACPI based NUMA information
parsing from SRAT and SLIT.

SRAT includes the CPU ACPI ID to Proximity Domain mappings
and memory ranges to Proximity Domain mapping.
SLIT has the information of inter node
distances(relative number for access latency).

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Ganapatrao Kulkarni <gkulkarni@caviumnetworks.com>
---
 arch/arm64/include/asm/acpi.h |   4 +
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/numa.c          |   7 +-
 4 files changed, 226 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/acpi_numa.c

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index caafd63..e171a18 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -83,6 +83,10 @@ static inline bool acpi_has_cpu_in_madt(void)
 static inline void arch_fix_phys_package_id(int num, u32 slot) { }
 void __init acpi_init_cpus(void);
 
+#ifdef CONFIG_ACPI_NUMA
+int arm64_acpi_numa_init(void);
+#endif /* CONFIG_ACPI_NUMA */
+
 #else
 static inline void acpi_init_cpus(void) { }
 #endif /* CONFIG_ACPI */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 7987763..555c4a5 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
 arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
+arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
new file mode 100644
index 0000000..8aee205
--- /dev/null
+++ b/arch/arm64/kernel/acpi_numa.c
@@ -0,0 +1,215 @@
+/*
+ * ACPI 5.1 based NUMA setup for ARM64
+ * Lots of code was borrowed from arch/x86/mm/srat.c
+ *
+ * Copyright 2004 Andi Kleen, SuSE Labs.
+ * Copyright (C) 2013-2014, Linaro Ltd.
+ *		Author: Hanjun Guo <hanjun.guo@linaro.org>
+ *
+ * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
+ *
+ * Called from acpi_numa_init while reading the SRAT and SLIT tables.
+ * Assumes all memory regions belonging to a single proximity domain
+ * are in one chunk. Holes between them will be included in the node.
+ */
+
+#define pr_fmt(fmt) "ACPI: NUMA: " fmt
+
+#include <linux/acpi.h>
+#include <linux/bitmap.h>
+#include <linux/bootmem.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/memblock.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+#include <linux/topology.h>
+
+#include <acpi/processor.h>
+#include <asm/numa.h>
+
+int acpi_numa __initdata;
+
+static __init int setup_node(int pxm)
+{
+	return acpi_map_pxm_to_node(pxm);
+}
+
+static __init void bad_srat(void)
+{
+	pr_err("SRAT not used.\n");
+	acpi_numa = -1;
+}
+
+static __init inline int srat_disabled(void)
+{
+	return acpi_numa < 0;
+}
+
+/*
+ * Callback for SLIT parsing.
+ * It will get the distance information presented by SLIT
+ * and init the distance matrix of numa nodes
+ */
+void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
+{
+	int i, j;
+
+	for (i = 0; i < slit->locality_count; i++) {
+		const int from_node = pxm_to_node(i);
+
+		if (from_node == NUMA_NO_NODE)
+			continue;
+
+		for (j = 0; j < slit->locality_count; j++) {
+			const int to_node = pxm_to_node(j);
+
+			if (to_node == NUMA_NO_NODE)
+				continue;
+
+			pr_info("SLIT: Distance[%d][%d] = %d\n",
+					from_node, to_node,
+					slit->entry[
+					slit->locality_count * i + j]);
+			numa_set_distance(from_node, to_node,
+				slit->entry[slit->locality_count * i + j]);
+		}
+	}
+}
+
+static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
+{
+	unsigned long madt_end, entry;
+	struct acpi_table_madt *madt;
+	acpi_size tbl_size;
+
+	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
+			(struct acpi_table_header **)&madt, &tbl_size)))
+		return -ENODEV;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		struct acpi_subtable_header *header =
+			(struct acpi_subtable_header *)entry;
+
+		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
+			struct acpi_madt_generic_interrupt *gicc =
+				container_of(header,
+				struct acpi_madt_generic_interrupt, header);
+
+			if ((gicc->flags & ACPI_MADT_ENABLED) &&
+			    (gicc->uid == acpi_id)) {
+				*mpidr = gicc->arm_mpidr;
+				early_acpi_os_unmap_memory(madt, tbl_size);
+				return 0;
+			}
+		}
+		entry += header->length;
+	}
+
+	early_acpi_os_unmap_memory(madt, tbl_size);
+	return -ENODEV;
+}
+
+/* Callback for Proximity Domain -> ACPI processor UID mapping */
+void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
+{
+	int pxm, node;
+	u64 mpidr;
+	static int cpus_in_srat;
+
+	if (srat_disabled())
+		return;
+
+	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
+		bad_srat();
+		return;
+	}
+
+	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
+		return;
+
+	if (cpus_in_srat >= NR_CPUS) {
+		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
+			     NR_CPUS);
+		return;
+	}
+
+	pxm = pa->proximity_domain;
+	node = setup_node(pxm);
+
+	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
+		pr_err("SRAT: Too many proximity domains %d\n", pxm);
+		bad_srat();
+		return;
+	}
+
+	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
+		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
+			pxm, pa->acpi_processor_uid);
+		bad_srat();
+		return;
+	}
+
+	cpu_to_node_map[cpus_in_srat] = node;
+	node_set(node, numa_nodes_parsed);
+	acpi_numa = 1;
+	cpus_in_srat++;
+	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
+			pxm, mpidr, node, cpus_in_srat);
+}
+
+/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
+int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
+{
+	u64 start, end;
+	int node, pxm;
+
+	if (srat_disabled())
+		return -EINVAL;
+
+	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
+		bad_srat();
+		return -EINVAL;
+	}
+
+	start = ma->base_address;
+	end = start + ma->length;
+	pxm = ma->proximity_domain;
+
+	node = setup_node(pxm);
+
+	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
+		pr_err("SRAT: Too many proximity domains.\n");
+		bad_srat();
+		return -EINVAL;
+	}
+
+	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
+		node, pxm,
+		(unsigned long long) start, (unsigned long long) end - 1);
+
+	if (numa_add_memblk(node, start, (end - start)) < 0) {
+		bad_srat();
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+void __init acpi_numa_arch_fixup(void) { }
+
+int __init arm64_acpi_numa_init(void)
+{
+	int ret;
+
+	ret = acpi_numa_init();
+	if (ret < 0)
+		return ret;
+
+	return srat_disabled() ? -EINVAL : 0;
+}
diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index 209c7a9..c2950fc 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -17,6 +17,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
@@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
 	int ret = -ENODEV;
 
 #ifdef CONFIG_OF_NUMA
-	if (!numa_off)
+	if (!numa_off && acpi_disabled)
 		ret = numa_init(arm64_of_numa_init);
 #endif
+#ifdef CONFIG_ACPI_NUMA
+	if (!numa_off && !acpi_disabled)
+		ret = numa_init(arm64_acpi_numa_init);
+#endif
 
 	if (ret)
 		numa_init(dummy_numa_init);
-- 
1.8.1.4

[PATCH v2 4/4] acpi, numa: Enable ACPI based NUMA on ARM64

[Ganapatrao Kulkarni]

Add function needed for cpu to node mapping, and enable
ACPI based NUMA for ARM64 in Kconfig

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/Kconfig |  4 ++--
 drivers/acpi/numa.c  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h | 15 +++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 25dbb76..06ea7dc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -280,8 +280,8 @@ config ACPI_THERMAL
 config ACPI_NUMA
 	bool "NUMA support"
 	depends on NUMA
-	depends on (X86 || IA64)
-	default y if IA64_GENERIC || IA64_SGI_SN2
+	depends on (X86 || IA64 || ARM64)
+	default y if IA64_GENERIC || IA64_SGI_SN2 || ARM64
 
 config ACPI_CUSTOM_DSDT_FILE
 	string "Custom DSDT Table file to include"
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 36683d0..807083a 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -175,6 +175,18 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 		}
 		break;
 
+	case ACPI_SRAT_TYPE_GICC_AFFINITY:
+		{
+			struct acpi_srat_gicc_affinity *p =
+			    (struct acpi_srat_gicc_affinity *)header;
+			pr_debug("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");
+		}
+		break;
+
 	default:
 		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
 			header->type);
@@ -260,6 +272,24 @@ acpi_parse_processor_affinity(struct acpi_subtable_header *header,
 	return 0;
 }
 
+static int __init
+acpi_parse_gicc_affinity(struct acpi_subtable_header *header,
+			      const unsigned long end)
+{
+	struct acpi_srat_gicc_affinity *processor_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;
+}
+
 static int __initdata parsed_numa_memblks;
 
 static int __init
@@ -304,6 +334,9 @@ int __init acpi_numa_init(void)
 {
 	int cnt = 0;
 
+	if (acpi_disabled)
+		return -EINVAL;
+
 	/*
 	 * Should not limit number with cpu num that is from NR_CPUS or nr_cpus=
 	 * SRAT cpu entries could have different order with that in MADT.
@@ -316,6 +349,8 @@ int __init acpi_numa_init(void)
 				     acpi_parse_x2apic_affinity, 0);
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_CPU_AFFINITY,
 				     acpi_parse_processor_affinity, 0);
+		acpi_table_parse_srat(ACPI_SRAT_TYPE_GICC_AFFINITY,
+				     acpi_parse_gicc_affinity, 0);
 		cnt = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 					    acpi_parse_memory_affinity,
 					    NR_NODE_MEMBLKS);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0548339..ff56702 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -170,8 +170,23 @@ void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
 
 /* the following four functions are architecture-dependent */
 void acpi_numa_slit_init (struct acpi_table_slit *slit);
+
+#if defined(CONFIG_X86) || defined(CONFIG_IA64)
 void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
+#else
+static inline void
+acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa) { }
+#endif
+
 void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
+
+#ifdef CONFIG_ARM64
+void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
+#else
+static inline void
+acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
+#endif
+
 int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
 
-- 
1.8.1.4

[PATCH v2 4/4] acpi, numa: Enable ACPI based NUMA on ARM64

[Ganapatrao Kulkarni]

Add function needed for cpu to node mapping, and enable
ACPI based NUMA for ARM64 in Kconfig

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/Kconfig |  4 ++--
 drivers/acpi/numa.c  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h | 15 +++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 25dbb76..06ea7dc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -280,8 +280,8 @@ config ACPI_THERMAL
 config ACPI_NUMA
 	bool "NUMA support"
 	depends on NUMA
-	depends on (X86 || IA64)
-	default y if IA64_GENERIC || IA64_SGI_SN2
+	depends on (X86 || IA64 || ARM64)
+	default y if IA64_GENERIC || IA64_SGI_SN2 || ARM64
 
 config ACPI_CUSTOM_DSDT_FILE
 	string "Custom DSDT Table file to include"
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 36683d0..807083a 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -175,6 +175,18 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 		}
 		break;
 
+	case ACPI_SRAT_TYPE_GICC_AFFINITY:
+		{
+			struct acpi_srat_gicc_affinity *p =
+			    (struct acpi_srat_gicc_affinity *)header;
+			pr_debug("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");
+		}
+		break;
+
 	default:
 		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
 			header->type);
@@ -260,6 +272,24 @@ acpi_parse_processor_affinity(struct acpi_subtable_header *header,
 	return 0;
 }
 
+static int __init
+acpi_parse_gicc_affinity(struct acpi_subtable_header *header,
+			      const unsigned long end)
+{
+	struct acpi_srat_gicc_affinity *processor_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;
+}
+
 static int __initdata parsed_numa_memblks;
 
 static int __init
@@ -304,6 +334,9 @@ int __init acpi_numa_init(void)
 {
 	int cnt = 0;
 
+	if (acpi_disabled)
+		return -EINVAL;
+
 	/*
 	 * Should not limit number with cpu num that is from NR_CPUS or nr_cpus=
 	 * SRAT cpu entries could have different order with that in MADT.
@@ -316,6 +349,8 @@ int __init acpi_numa_init(void)
 				     acpi_parse_x2apic_affinity, 0);
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_CPU_AFFINITY,
 				     acpi_parse_processor_affinity, 0);
+		acpi_table_parse_srat(ACPI_SRAT_TYPE_GICC_AFFINITY,
+				     acpi_parse_gicc_affinity, 0);
 		cnt = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 					    acpi_parse_memory_affinity,
 					    NR_NODE_MEMBLKS);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0548339..ff56702 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -170,8 +170,23 @@ void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
 
 /* the following four functions are architecture-dependent */
 void acpi_numa_slit_init (struct acpi_table_slit *slit);
+
+#if defined(CONFIG_X86) || defined(CONFIG_IA64)
 void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
+#else
+static inline void
+acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa) { }
+#endif
+
 void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
+
+#ifdef CONFIG_ARM64
+void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
+#else
+static inline void
+acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
+#endif
+
 int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
 
-- 
1.8.1.4

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Shannon Zhao]

Hi,

On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> +{
> +	int pxm, node;
> +	u64 mpidr;
> +	static int cpus_in_srat;
> +
> +	if (srat_disabled())
> +		return;
> +
> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> +		return;
> +
> +	if (cpus_in_srat >= NR_CPUS) {
> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> +			     NR_CPUS);
> +		return;
> +	}

arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
   pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
able to use all cpus\n",

-- 
Shannon

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Shannon Zhao]

Hi,

On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> +{
> +	int pxm, node;
> +	u64 mpidr;
> +	static int cpus_in_srat;
> +
> +	if (srat_disabled())
> +		return;
> +
> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> +		return;
> +
> +	if (cpus_in_srat >= NR_CPUS) {
> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> +			     NR_CPUS);
> +		return;
> +	}

arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
   pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
able to use all cpus\n",

-- 
Shannon

Re: [PATCH v2 0/4] ACPI based NUMA support for ARM64

[Shannon Zhao]

On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>  -v2 rebased on arm64-numa v7 patches.
>  http://www.spinics.net/lists/arm-kernel/msg460813.html
> 
>  -v1 initial patches from Hanjun Guo
> 
> Hanjun Guo (4):
>   acpi, numa: Use pr_fmt() instead of printk
>   acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
>   arm64, acpi, numa: NUMA support based on SRAT and SLIT
>   acpi, numa: Enable ACPI based NUMA on ARM64
> 
>  arch/arm64/include/asm/acpi.h |   4 +
>  arch/arm64/kernel/Makefile    |   1 +
>  arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/numa.c          |   7 +-
>  drivers/acpi/Kconfig          |   4 +-
>  drivers/acpi/numa.c           | 106 ++++++++++++---------
>  include/linux/acpi.h          |  15 +++
>  7 files changed, 306 insertions(+), 46 deletions(-)
>  create mode 100644 arch/arm64/kernel/acpi_numa.c
> 

I've tested this patch set on QEMU VM and guest prints below log:

ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x0 -> Node 0 cpu 1
ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x1 -> Node 0 cpu 2
ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x2 -> Node 1 cpu 3
ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x3 -> Node 1 cpu 4
ACPI: NUMA: SRAT: Node 0 PXM 0 [mem 0x40000000-0x5fffffff]
NUMA: Adding memblock [0x40000000 - 0x5fffffff] on node 0
ACPI: NUMA: SRAT: Node 1 PXM 1 [mem 0x60000000-0x7fffffff]
NUMA: Adding memblock [0x60000000 - 0x7fffffff] on node 1
Initmem setup node 0 [mem 0x40000000-0x5fffffff]
  NODE_DATA [mem 0x5fffe400-0x5fffffff]
Initmem setup node 1 [mem 0x60000000-0x7fffffff]
  NODE_DATA [mem 0x7effd400-0x7effefff]
Zone ranges:
  DMA      [mem 0x0000000040000000-0x000000007fffffff]
  Normal   empty
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000040000000-0x000000005fffffff]
  node   1: [mem 0x0000000060000000-0x000000007fffffff]
Initmem setup node 0 [mem 0x0000000040000000-0x000000005fffffff]
Initmem setup node 1 [mem 0x0000000060000000-0x000000007fffffff]

So, Tested-by: Shannon Zhao <shannon.zhao@linaro.org>

-- 
Shannon

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Shannon Zhao]

On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>  -v2 rebased on arm64-numa v7 patches.
>  http://www.spinics.net/lists/arm-kernel/msg460813.html
> 
>  -v1 initial patches from Hanjun Guo
> 
> Hanjun Guo (4):
>   acpi, numa: Use pr_fmt() instead of printk
>   acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
>   arm64, acpi, numa: NUMA support based on SRAT and SLIT
>   acpi, numa: Enable ACPI based NUMA on ARM64
> 
>  arch/arm64/include/asm/acpi.h |   4 +
>  arch/arm64/kernel/Makefile    |   1 +
>  arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/numa.c          |   7 +-
>  drivers/acpi/Kconfig          |   4 +-
>  drivers/acpi/numa.c           | 106 ++++++++++++---------
>  include/linux/acpi.h          |  15 +++
>  7 files changed, 306 insertions(+), 46 deletions(-)
>  create mode 100644 arch/arm64/kernel/acpi_numa.c
> 

I've tested this patch set on QEMU VM and guest prints below log:

ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x0 -> Node 0 cpu 1
ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x1 -> Node 0 cpu 2
ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x2 -> Node 1 cpu 3
ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x3 -> Node 1 cpu 4
ACPI: NUMA: SRAT: Node 0 PXM 0 [mem 0x40000000-0x5fffffff]
NUMA: Adding memblock [0x40000000 - 0x5fffffff] on node 0
ACPI: NUMA: SRAT: Node 1 PXM 1 [mem 0x60000000-0x7fffffff]
NUMA: Adding memblock [0x60000000 - 0x7fffffff] on node 1
Initmem setup node 0 [mem 0x40000000-0x5fffffff]
  NODE_DATA [mem 0x5fffe400-0x5fffffff]
Initmem setup node 1 [mem 0x60000000-0x7fffffff]
  NODE_DATA [mem 0x7effd400-0x7effefff]
Zone ranges:
  DMA      [mem 0x0000000040000000-0x000000007fffffff]
  Normal   empty
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000040000000-0x000000005fffffff]
  node   1: [mem 0x0000000060000000-0x000000007fffffff]
Initmem setup node 0 [mem 0x0000000040000000-0x000000005fffffff]
Initmem setup node 1 [mem 0x0000000060000000-0x000000007fffffff]

So, Tested-by: Shannon Zhao <shannon.zhao@linaro.org>

-- 
Shannon

Re: [PATCH v2 0/4] ACPI based NUMA support for ARM64

[Hanjun Guo]

On 11/27/2015 05:25 PM, Shannon Zhao wrote:
>
>
> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>>   -v2 rebased on arm64-numa v7 patches.
>>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>>
>>   -v1 initial patches from Hanjun Guo
>>
>> Hanjun Guo (4):
>>    acpi, numa: Use pr_fmt() instead of printk
>>    acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
>>    arm64, acpi, numa: NUMA support based on SRAT and SLIT
>>    acpi, numa: Enable ACPI based NUMA on ARM64
>>
>>   arch/arm64/include/asm/acpi.h |   4 +
>>   arch/arm64/kernel/Makefile    |   1 +
>>   arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
>>   arch/arm64/mm/numa.c          |   7 +-
>>   drivers/acpi/Kconfig          |   4 +-
>>   drivers/acpi/numa.c           | 106 ++++++++++++---------
>>   include/linux/acpi.h          |  15 +++
>>   7 files changed, 306 insertions(+), 46 deletions(-)
>>   create mode 100644 arch/arm64/kernel/acpi_numa.c
>>
>
> I've tested this patch set on QEMU VM and guest prints below log:
>
> ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x0 -> Node 0 cpu 1
> ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x1 -> Node 0 cpu 2
> ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x2 -> Node 1 cpu 3
> ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x3 -> Node 1 cpu 4
> ACPI: NUMA: SRAT: Node 0 PXM 0 [mem 0x40000000-0x5fffffff]
> NUMA: Adding memblock [0x40000000 - 0x5fffffff] on node 0
> ACPI: NUMA: SRAT: Node 1 PXM 1 [mem 0x60000000-0x7fffffff]
> NUMA: Adding memblock [0x60000000 - 0x7fffffff] on node 1
> Initmem setup node 0 [mem 0x40000000-0x5fffffff]
>    NODE_DATA [mem 0x5fffe400-0x5fffffff]
> Initmem setup node 1 [mem 0x60000000-0x7fffffff]
>    NODE_DATA [mem 0x7effd400-0x7effefff]
> Zone ranges:
>    DMA      [mem 0x0000000040000000-0x000000007fffffff]
>    Normal   empty
> Movable zone start for each node
> Early memory node ranges
>    node   0: [mem 0x0000000040000000-0x000000005fffffff]
>    node   1: [mem 0x0000000060000000-0x000000007fffffff]
> Initmem setup node 0 [mem 0x0000000040000000-0x000000005fffffff]
> Initmem setup node 1 [mem 0x0000000060000000-0x000000007fffffff]
>
> So, Tested-by: Shannon Zhao <shannon.zhao@linaro.org>

Hi Shannon, thanks a lot!

Thanks
Hanjun

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Hanjun Guo]

On 11/27/2015 05:25 PM, Shannon Zhao wrote:
>
>
> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>>   -v2 rebased on arm64-numa v7 patches.
>>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>>
>>   -v1 initial patches from Hanjun Guo
>>
>> Hanjun Guo (4):
>>    acpi, numa: Use pr_fmt() instead of printk
>>    acpi, numa: Replace ACPI_DEBUG_PRINT() with pr_debug()
>>    arm64, acpi, numa: NUMA support based on SRAT and SLIT
>>    acpi, numa: Enable ACPI based NUMA on ARM64
>>
>>   arch/arm64/include/asm/acpi.h |   4 +
>>   arch/arm64/kernel/Makefile    |   1 +
>>   arch/arm64/kernel/acpi_numa.c | 215 ++++++++++++++++++++++++++++++++++++++++++
>>   arch/arm64/mm/numa.c          |   7 +-
>>   drivers/acpi/Kconfig          |   4 +-
>>   drivers/acpi/numa.c           | 106 ++++++++++++---------
>>   include/linux/acpi.h          |  15 +++
>>   7 files changed, 306 insertions(+), 46 deletions(-)
>>   create mode 100644 arch/arm64/kernel/acpi_numa.c
>>
>
> I've tested this patch set on QEMU VM and guest prints below log:
>
> ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x0 -> Node 0 cpu 1
> ACPI: NUMA: SRAT: PXM 0 -> MPIDR 0x1 -> Node 0 cpu 2
> ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x2 -> Node 1 cpu 3
> ACPI: NUMA: SRAT: PXM 1 -> MPIDR 0x3 -> Node 1 cpu 4
> ACPI: NUMA: SRAT: Node 0 PXM 0 [mem 0x40000000-0x5fffffff]
> NUMA: Adding memblock [0x40000000 - 0x5fffffff] on node 0
> ACPI: NUMA: SRAT: Node 1 PXM 1 [mem 0x60000000-0x7fffffff]
> NUMA: Adding memblock [0x60000000 - 0x7fffffff] on node 1
> Initmem setup node 0 [mem 0x40000000-0x5fffffff]
>    NODE_DATA [mem 0x5fffe400-0x5fffffff]
> Initmem setup node 1 [mem 0x60000000-0x7fffffff]
>    NODE_DATA [mem 0x7effd400-0x7effefff]
> Zone ranges:
>    DMA      [mem 0x0000000040000000-0x000000007fffffff]
>    Normal   empty
> Movable zone start for each node
> Early memory node ranges
>    node   0: [mem 0x0000000040000000-0x000000005fffffff]
>    node   1: [mem 0x0000000060000000-0x000000007fffffff]
> Initmem setup node 0 [mem 0x0000000040000000-0x000000005fffffff]
> Initmem setup node 1 [mem 0x0000000060000000-0x000000007fffffff]
>
> So, Tested-by: Shannon Zhao <shannon.zhao@linaro.org>

Hi Shannon, thanks a lot!

Thanks
Hanjun

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Lorenzo Pieralisi]

On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:

[...]

> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 7987763..555c4a5 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
>  arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
> +arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o

Isn't it better to merge ACPI and DT support in one file (a bit like
what we did for smp.c) to remove some of this iffdeffery ?

>  
>  obj-y					+= $(arm64-obj-y) vdso/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
> new file mode 100644
> index 0000000..8aee205
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi_numa.c
> @@ -0,0 +1,215 @@
> +/*
> + * ACPI 5.1 based NUMA setup for ARM64
> + * Lots of code was borrowed from arch/x86/mm/srat.c
> + *
> + * Copyright 2004 Andi Kleen, SuSE Labs.
> + * Copyright (C) 2013-2014, Linaro Ltd.
> + *		Author: Hanjun Guo <hanjun.guo@linaro.org>
> + *
> + * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
> + *
> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
> + * Assumes all memory regions belonging to a single proximity domain
> + * are in one chunk. Holes between them will be included in the node.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/bitmap.h>
> +#include <linux/bootmem.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/memblock.h>
> +#include <linux/mmzone.h>
> +#include <linux/module.h>
> +#include <linux/topology.h>
> +
> +#include <acpi/processor.h>
> +#include <asm/numa.h>
> +
> +int acpi_numa __initdata;
> +
> +static __init int setup_node(int pxm)
> +{
> +	return acpi_map_pxm_to_node(pxm);
> +}

This function is not that useful given how it is used in the patch.

> +
> +static __init void bad_srat(void)
> +{
> +	pr_err("SRAT not used.\n");
> +	acpi_numa = -1;
> +}
> +
> +static __init inline int srat_disabled(void)
> +{
> +	return acpi_numa < 0;
> +}
> +
> +/*
> + * Callback for SLIT parsing.
> + * It will get the distance information presented by SLIT
> + * and init the distance matrix of numa nodes
> + */
> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < slit->locality_count; i++) {
> +		const int from_node = pxm_to_node(i);
> +
> +		if (from_node == NUMA_NO_NODE)
> +			continue;
> +
> +		for (j = 0; j < slit->locality_count; j++) {
> +			const int to_node = pxm_to_node(j);
> +
> +			if (to_node == NUMA_NO_NODE)
> +				continue;
> +
> +			pr_info("SLIT: Distance[%d][%d] = %d\n",
> +					from_node, to_node,
> +					slit->entry[
> +					slit->locality_count * i + j]);
> +			numa_set_distance(from_node, to_node,
> +				slit->entry[slit->locality_count * i + j]);
> +		}
> +	}
> +}

This function is an x86 copy'n'paste. ia64 just requires this callback
to save a slit table pointer (that can be moved to generic code and it is
initdata anyway), so my question is, do we really need to duplicate it ?

> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
> +{

Looks familiar. I guess you can't reuse the code in drivers/acpi
(acpi_map_cpuid()) only because that implies permanent table mappings to be
in place and you need to call this function before acpi_gbl_permanent_mmap
is set ?

> +	unsigned long madt_end, entry;
> +	struct acpi_table_madt *madt;
> +	acpi_size tbl_size;
> +
> +	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
> +			(struct acpi_table_header **)&madt, &tbl_size)))
> +		return -ENODEV;
> +
> +	entry = (unsigned long)madt;
> +	madt_end = entry + madt->header.length;
> +
> +	/* Parse all entries looking for a match. */
> +	entry += sizeof(struct acpi_table_madt);
> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +		struct acpi_subtable_header *header =
> +			(struct acpi_subtable_header *)entry;
> +
> +		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
> +			struct acpi_madt_generic_interrupt *gicc =
> +				container_of(header,
> +				struct acpi_madt_generic_interrupt, header);
> +
> +			if ((gicc->flags & ACPI_MADT_ENABLED) &&
> +			    (gicc->uid == acpi_id)) {
> +				*mpidr = gicc->arm_mpidr;
> +				early_acpi_os_unmap_memory(madt, tbl_size);
> +				return 0;
> +			}
> +		}
> +		entry += header->length;
> +	}
> +
> +	early_acpi_os_unmap_memory(madt, tbl_size);
> +	return -ENODEV;
> +}
> +
> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> +{
> +	int pxm, node;
> +	u64 mpidr;
> +	static int cpus_in_srat;
> +
> +	if (srat_disabled())
> +		return;
> +
> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> +		return;
> +
> +	if (cpus_in_srat >= NR_CPUS) {
> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> +			     NR_CPUS);
> +		return;
> +	}
> +
> +	pxm = pa->proximity_domain;
> +	node = setup_node(pxm);
> +
> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> +		pr_err("SRAT: Too many proximity domains %d\n", pxm);
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
> +		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
> +			pxm, pa->acpi_processor_uid);
> +		bad_srat();
> +		return;
> +	}
> +
> +	cpu_to_node_map[cpus_in_srat] = node;

This looks wrong. cpus_in_srat is a logical index, but I do not see why
it has to be sequential. You retrieve the mpidr for a given SRAT entry
and with that value you should retrieve the cpu_logical index that
corresponds to it (get_logical_index()), or maybe I am missing something
from the ACPI specs that enforce a SRAT entries ordering, on which we
should not rely upon anyway.

> +	node_set(node, numa_nodes_parsed);
> +	acpi_numa = 1;

What's the point if you are checking for < 0 in srat_disabled() ?

> +	cpus_in_srat++;
> +	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
> +			pxm, mpidr, node, cpus_in_srat);
> +}
> +
> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> +{
> +	u64 start, end;
> +	int node, pxm;
> +
> +	if (srat_disabled())
> +		return -EINVAL;
> +
> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	start = ma->base_address;
> +	end = start + ma->length;
> +	pxm = ma->proximity_domain;
> +
> +	node = setup_node(pxm);
> +
> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> +		pr_err("SRAT: Too many proximity domains.\n");
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
> +		node, pxm,
> +		(unsigned long long) start, (unsigned long long) end - 1);
> +
> +	if (numa_add_memblk(node, start, (end - start)) < 0) {
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

I do not see again any major changes compared to x86, numa_add_memblk()
has a different interface (size vs end) but apart from that it would
be nice to avoid rewriting the same code time and again.

> +
> +void __init acpi_numa_arch_fixup(void) { }

Sigh. How many of these useless callbacks are we forced to define ?

> +
> +int __init arm64_acpi_numa_init(void)
> +{
> +	int ret;
> +
> +	ret = acpi_numa_init();
> +	if (ret < 0)
> +		return ret;
> +
> +	return srat_disabled() ? -EINVAL : 0;
> +}
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index 209c7a9..c2950fc 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -17,6 +17,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/bootmem.h>
>  #include <linux/ctype.h>
>  #include <linux/init.h>
> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>  	int ret = -ENODEV;
>  
>  #ifdef CONFIG_OF_NUMA
> -	if (!numa_off)
> +	if (!numa_off && acpi_disabled)
>  		ret = numa_init(arm64_of_numa_init);
>  #endif
> +#ifdef CONFIG_ACPI_NUMA
> +	if (!numa_off && !acpi_disabled)
> +		ret = numa_init(arm64_acpi_numa_init);
> +#endif

See my comment above on DT/ACPI, this ifdeffery does not look great.

Thanks,
Lorenzo

>  
>  	if (ret)
>  		numa_init(dummy_numa_init);
> -- 
> 1.8.1.4
> 

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Lorenzo Pieralisi]

On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:

[...]

> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 7987763..555c4a5 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
>  arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
> +arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o

Isn't it better to merge ACPI and DT support in one file (a bit like
what we did for smp.c) to remove some of this iffdeffery ?

>  
>  obj-y					+= $(arm64-obj-y) vdso/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
> new file mode 100644
> index 0000000..8aee205
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi_numa.c
> @@ -0,0 +1,215 @@
> +/*
> + * ACPI 5.1 based NUMA setup for ARM64
> + * Lots of code was borrowed from arch/x86/mm/srat.c
> + *
> + * Copyright 2004 Andi Kleen, SuSE Labs.
> + * Copyright (C) 2013-2014, Linaro Ltd.
> + *		Author: Hanjun Guo <hanjun.guo@linaro.org>
> + *
> + * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
> + *
> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
> + * Assumes all memory regions belonging to a single proximity domain
> + * are in one chunk. Holes between them will be included in the node.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/bitmap.h>
> +#include <linux/bootmem.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/memblock.h>
> +#include <linux/mmzone.h>
> +#include <linux/module.h>
> +#include <linux/topology.h>
> +
> +#include <acpi/processor.h>
> +#include <asm/numa.h>
> +
> +int acpi_numa __initdata;
> +
> +static __init int setup_node(int pxm)
> +{
> +	return acpi_map_pxm_to_node(pxm);
> +}

This function is not that useful given how it is used in the patch.

> +
> +static __init void bad_srat(void)
> +{
> +	pr_err("SRAT not used.\n");
> +	acpi_numa = -1;
> +}
> +
> +static __init inline int srat_disabled(void)
> +{
> +	return acpi_numa < 0;
> +}
> +
> +/*
> + * Callback for SLIT parsing.
> + * It will get the distance information presented by SLIT
> + * and init the distance matrix of numa nodes
> + */
> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < slit->locality_count; i++) {
> +		const int from_node = pxm_to_node(i);
> +
> +		if (from_node == NUMA_NO_NODE)
> +			continue;
> +
> +		for (j = 0; j < slit->locality_count; j++) {
> +			const int to_node = pxm_to_node(j);
> +
> +			if (to_node == NUMA_NO_NODE)
> +				continue;
> +
> +			pr_info("SLIT: Distance[%d][%d] = %d\n",
> +					from_node, to_node,
> +					slit->entry[
> +					slit->locality_count * i + j]);
> +			numa_set_distance(from_node, to_node,
> +				slit->entry[slit->locality_count * i + j]);
> +		}
> +	}
> +}

This function is an x86 copy'n'paste. ia64 just requires this callback
to save a slit table pointer (that can be moved to generic code and it is
initdata anyway), so my question is, do we really need to duplicate it ?

> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
> +{

Looks familiar. I guess you can't reuse the code in drivers/acpi
(acpi_map_cpuid()) only because that implies permanent table mappings to be
in place and you need to call this function before acpi_gbl_permanent_mmap
is set ?

> +	unsigned long madt_end, entry;
> +	struct acpi_table_madt *madt;
> +	acpi_size tbl_size;
> +
> +	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
> +			(struct acpi_table_header **)&madt, &tbl_size)))
> +		return -ENODEV;
> +
> +	entry = (unsigned long)madt;
> +	madt_end = entry + madt->header.length;
> +
> +	/* Parse all entries looking for a match. */
> +	entry += sizeof(struct acpi_table_madt);
> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +		struct acpi_subtable_header *header =
> +			(struct acpi_subtable_header *)entry;
> +
> +		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
> +			struct acpi_madt_generic_interrupt *gicc =
> +				container_of(header,
> +				struct acpi_madt_generic_interrupt, header);
> +
> +			if ((gicc->flags & ACPI_MADT_ENABLED) &&
> +			    (gicc->uid == acpi_id)) {
> +				*mpidr = gicc->arm_mpidr;
> +				early_acpi_os_unmap_memory(madt, tbl_size);
> +				return 0;
> +			}
> +		}
> +		entry += header->length;
> +	}
> +
> +	early_acpi_os_unmap_memory(madt, tbl_size);
> +	return -ENODEV;
> +}
> +
> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> +{
> +	int pxm, node;
> +	u64 mpidr;
> +	static int cpus_in_srat;
> +
> +	if (srat_disabled())
> +		return;
> +
> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> +		return;
> +
> +	if (cpus_in_srat >= NR_CPUS) {
> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> +			     NR_CPUS);
> +		return;
> +	}
> +
> +	pxm = pa->proximity_domain;
> +	node = setup_node(pxm);
> +
> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> +		pr_err("SRAT: Too many proximity domains %d\n", pxm);
> +		bad_srat();
> +		return;
> +	}
> +
> +	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
> +		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
> +			pxm, pa->acpi_processor_uid);
> +		bad_srat();
> +		return;
> +	}
> +
> +	cpu_to_node_map[cpus_in_srat] = node;

This looks wrong. cpus_in_srat is a logical index, but I do not see why
it has to be sequential. You retrieve the mpidr for a given SRAT entry
and with that value you should retrieve the cpu_logical index that
corresponds to it (get_logical_index()), or maybe I am missing something
from the ACPI specs that enforce a SRAT entries ordering, on which we
should not rely upon anyway.

> +	node_set(node, numa_nodes_parsed);
> +	acpi_numa = 1;

What's the point if you are checking for < 0 in srat_disabled() ?

> +	cpus_in_srat++;
> +	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
> +			pxm, mpidr, node, cpus_in_srat);
> +}
> +
> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> +{
> +	u64 start, end;
> +	int node, pxm;
> +
> +	if (srat_disabled())
> +		return -EINVAL;
> +
> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	start = ma->base_address;
> +	end = start + ma->length;
> +	pxm = ma->proximity_domain;
> +
> +	node = setup_node(pxm);
> +
> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> +		pr_err("SRAT: Too many proximity domains.\n");
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
> +		node, pxm,
> +		(unsigned long long) start, (unsigned long long) end - 1);
> +
> +	if (numa_add_memblk(node, start, (end - start)) < 0) {
> +		bad_srat();
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

I do not see again any major changes compared to x86, numa_add_memblk()
has a different interface (size vs end) but apart from that it would
be nice to avoid rewriting the same code time and again.

> +
> +void __init acpi_numa_arch_fixup(void) { }

Sigh. How many of these useless callbacks are we forced to define ?

> +
> +int __init arm64_acpi_numa_init(void)
> +{
> +	int ret;
> +
> +	ret = acpi_numa_init();
> +	if (ret < 0)
> +		return ret;
> +
> +	return srat_disabled() ? -EINVAL : 0;
> +}
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index 209c7a9..c2950fc 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -17,6 +17,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/bootmem.h>
>  #include <linux/ctype.h>
>  #include <linux/init.h>
> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>  	int ret = -ENODEV;
>  
>  #ifdef CONFIG_OF_NUMA
> -	if (!numa_off)
> +	if (!numa_off && acpi_disabled)
>  		ret = numa_init(arm64_of_numa_init);
>  #endif
> +#ifdef CONFIG_ACPI_NUMA
> +	if (!numa_off && !acpi_disabled)
> +		ret = numa_init(arm64_acpi_numa_init);
> +#endif

See my comment above on DT/ACPI, this ifdeffery does not look great.

Thanks,
Lorenzo

>  
>  	if (ret)
>  		numa_init(dummy_numa_init);
> -- 
> 1.8.1.4
> 

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Hanjun Guo]

Hi Lorenzo,

Sorry for the late reply, just busy with other work, please
see my comments below.

On 2015/12/19 0:23, Lorenzo Pieralisi wrote:
> On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:
>
> [...]
>
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 7987763..555c4a5 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
>>   arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>>   arm64-obj-$(CONFIG_ACPI)		+= acpi.o
>>   arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
>> +arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o
>
> Isn't it better to merge ACPI and DT support in one file (a bit like
> what we did for smp.c) to remove some of this iffdeffery ?

It's definitely a good idea, but I'm not sure what's the blockers
ahead, I will try to do that in next version.

>
>>
>>   obj-y					+= $(arm64-obj-y) vdso/
>>   obj-m					+= $(arm64-obj-m)
>> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
>> new file mode 100644
>> index 0000000..8aee205
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi_numa.c
>> @@ -0,0 +1,215 @@
>> +/*
>> + * ACPI 5.1 based NUMA setup for ARM64
>> + * Lots of code was borrowed from arch/x86/mm/srat.c
>> + *
>> + * Copyright 2004 Andi Kleen, SuSE Labs.
>> + * Copyright (C) 2013-2014, Linaro Ltd.
>> + *		Author: Hanjun Guo <hanjun.guo@linaro.org>
>> + *
>> + * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
>> + *
>> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
>> + * Assumes all memory regions belonging to a single proximity domain
>> + * are in one chunk. Holes between them will be included in the node.
>> + */
>> +
>> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/bootmem.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mm.h>
>> +#include <linux/memblock.h>
>> +#include <linux/mmzone.h>
>> +#include <linux/module.h>
>> +#include <linux/topology.h>
>> +
>> +#include <acpi/processor.h>
>> +#include <asm/numa.h>
>> +
>> +int acpi_numa __initdata;
>> +
>> +static __init int setup_node(int pxm)
>> +{
>> +	return acpi_map_pxm_to_node(pxm);
>> +}
>
> This function is not that useful given how it is used in the patch.

Agree, just use acpi_map_pxm_to_node() will be fine.

>
>> +
>> +static __init void bad_srat(void)
>> +{
>> +	pr_err("SRAT not used.\n");
>> +	acpi_numa = -1;
>> +}
>> +
>> +static __init inline int srat_disabled(void)
>> +{
>> +	return acpi_numa < 0;
>> +}
>> +
>> +/*
>> + * Callback for SLIT parsing.
>> + * It will get the distance information presented by SLIT
>> + * and init the distance matrix of numa nodes
>> + */
>> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
>> +{
>> +	int i, j;
>> +
>> +	for (i = 0; i < slit->locality_count; i++) {
>> +		const int from_node = pxm_to_node(i);
>> +
>> +		if (from_node == NUMA_NO_NODE)
>> +			continue;
>> +
>> +		for (j = 0; j < slit->locality_count; j++) {
>> +			const int to_node = pxm_to_node(j);
>> +
>> +			if (to_node == NUMA_NO_NODE)
>> +				continue;
>> +
>> +			pr_info("SLIT: Distance[%d][%d] = %d\n",
>> +					from_node, to_node,
>> +					slit->entry[
>> +					slit->locality_count * i + j]);
>> +			numa_set_distance(from_node, to_node,
>> +				slit->entry[slit->locality_count * i + j]);
>> +		}
>> +	}
>> +}
>
> This function is an x86 copy'n'paste. ia64 just requires this callback
> to save a slit table pointer (that can be moved to generic code and it is
> initdata anyway), so my question is, do we really need to duplicate it ?

Sure we need it, no matter DT or ACPI, we need to figure out the
NUMA node distance and set it properly. For IA64, the slit table
pointer is saved but it also used to setup the NUMA node distance
as you can see the code in acpi_numa_arch_fixup() in arch/ia64/kernel
/acpi.c, it just init it in different time slot.

>
>> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
>> +{
>
> Looks familiar. I guess you can't reuse the code in drivers/acpi
> (acpi_map_cpuid()) only because that implies permanent table mappings to be
> in place and you need to call this function before acpi_gbl_permanent_mmap
> is set ?

Hey, you are reading my mind :). Yes, as you said, it also will
lead to early memory remap leak, any suggestion?

>
>> +	unsigned long madt_end, entry;
>> +	struct acpi_table_madt *madt;
>> +	acpi_size tbl_size;
>> +
>> +	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
>> +			(struct acpi_table_header **)&madt, &tbl_size)))
>> +		return -ENODEV;
>> +
>> +	entry = (unsigned long)madt;
>> +	madt_end = entry + madt->header.length;
>> +
>> +	/* Parse all entries looking for a match. */
>> +	entry += sizeof(struct acpi_table_madt);
>> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>> +		struct acpi_subtable_header *header =
>> +			(struct acpi_subtable_header *)entry;
>> +
>> +		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
>> +			struct acpi_madt_generic_interrupt *gicc =
>> +				container_of(header,
>> +				struct acpi_madt_generic_interrupt, header);
>> +
>> +			if ((gicc->flags & ACPI_MADT_ENABLED) &&
>> +			    (gicc->uid == acpi_id)) {
>> +				*mpidr = gicc->arm_mpidr;
>> +				early_acpi_os_unmap_memory(madt, tbl_size);
>> +				return 0;
>> +			}
>> +		}
>> +		entry += header->length;
>> +	}
>> +
>> +	early_acpi_os_unmap_memory(madt, tbl_size);
>> +	return -ENODEV;
>> +}
>> +
>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
>> +{
>> +	int pxm, node;
>> +	u64 mpidr;
>> +	static int cpus_in_srat;
>> +
>> +	if (srat_disabled())
>> +		return;
>> +
>> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>> +		return;
>> +
>> +	if (cpus_in_srat >= NR_CPUS) {
>> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
>> +			     NR_CPUS);
>> +		return;
>> +	}
>> +
>> +	pxm = pa->proximity_domain;
>> +	node = setup_node(pxm);
>> +
>> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>> +		pr_err("SRAT: Too many proximity domains %d\n", pxm);
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
>> +		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
>> +			pxm, pa->acpi_processor_uid);
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	cpu_to_node_map[cpus_in_srat] = node;
>
> This looks wrong. cpus_in_srat is a logical index, but I do not see why
> it has to be sequential. You retrieve the mpidr for a given SRAT entry
> and with that value you should retrieve the cpu_logical index that
> corresponds to it (get_logical_index()), or maybe I am missing something
> from the ACPI specs that enforce a SRAT entries ordering, on which we
> should not rely upon anyway.

No, you are right, there is no enforce of ordering of CPU in SRAT, I
will update it, good catch.

>
>> +	node_set(node, numa_nodes_parsed);
>> +	acpi_numa = 1;
>
> What's the point if you are checking for < 0 in srat_disabled() ?

acpi_numa will be set as -1 in bad_srat(), if that happens, we
will not going to parse SRAT anymore, did I understand your
question?

>
>> +	cpus_in_srat++;
>> +	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
>> +			pxm, mpidr, node, cpus_in_srat);
>> +}
>> +
>> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
>> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>> +{
>> +	u64 start, end;
>> +	int node, pxm;
>> +
>> +	if (srat_disabled())
>> +		return -EINVAL;
>> +
>> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	start = ma->base_address;
>> +	end = start + ma->length;
>> +	pxm = ma->proximity_domain;
>> +
>> +	node = setup_node(pxm);
>> +
>> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>> +		pr_err("SRAT: Too many proximity domains.\n");
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
>> +		node, pxm,
>> +		(unsigned long long) start, (unsigned long long) end - 1);
>> +
>> +	if (numa_add_memblk(node, start, (end - start)) < 0) {
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>
> I do not see again any major changes compared to x86, numa_add_memblk()
> has a different interface (size vs end) but apart from that it would
> be nice to avoid rewriting the same code time and again.

Let me see if I can rewrite it in next version.

>
>> +
>> +void __init acpi_numa_arch_fixup(void) { }
>
> Sigh. How many of these useless callbacks are we forced to define ?

Hmm, x86 also use a dummy function for it. I think we can fix it
in this way:

  - introduce a kconfig named ACPI_HAS_NUMA_ARCH_FIXUP

  - select it on IA64

  - introduce a stub function for x86 and ARM64 when
    !ACPI_HAS_NUMA_ARCH_FIXUP

  - remove the useless callbacks for x86 and ARM64

what do you think?

>
>> +
>> +int __init arm64_acpi_numa_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = acpi_numa_init();
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return srat_disabled() ? -EINVAL : 0;
>> +}
>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>> index 209c7a9..c2950fc 100644
>> --- a/arch/arm64/mm/numa.c
>> +++ b/arch/arm64/mm/numa.c
>> @@ -17,6 +17,7 @@
>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>    */
>>
>> +#include <linux/acpi.h>
>>   #include <linux/bootmem.h>
>>   #include <linux/ctype.h>
>>   #include <linux/init.h>
>> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>>   	int ret = -ENODEV;
>>
>>   #ifdef CONFIG_OF_NUMA
>> -	if (!numa_off)
>> +	if (!numa_off && acpi_disabled)
>>   		ret = numa_init(arm64_of_numa_init);
>>   #endif
>> +#ifdef CONFIG_ACPI_NUMA
>> +	if (!numa_off && !acpi_disabled)
>> +		ret = numa_init(arm64_acpi_numa_init);
>> +#endif
>
> See my comment above on DT/ACPI, this ifdeffery does not look great.

I will sync with Ganapatrao to see how to combine them together,
if any problem met, we will back to this discussion.

Thanks
Hanjun

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Hanjun Guo]

Hi Lorenzo,

Sorry for the late reply, just busy with other work, please
see my comments below.

On 2015/12/19 0:23, Lorenzo Pieralisi wrote:
> On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:
>
> [...]
>
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 7987763..555c4a5 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)			+= pci.o
>>   arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>>   arm64-obj-$(CONFIG_ACPI)		+= acpi.o
>>   arm64-obj-$(CONFIG_OF_NUMA)		+= of_numa.o
>> +arm64-obj-$(CONFIG_ACPI_NUMA)		+= acpi_numa.o
>
> Isn't it better to merge ACPI and DT support in one file (a bit like
> what we did for smp.c) to remove some of this iffdeffery ?

It's definitely a good idea, but I'm not sure what's the blockers
ahead, I will try to do that in next version.

>
>>
>>   obj-y					+= $(arm64-obj-y) vdso/
>>   obj-m					+= $(arm64-obj-m)
>> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
>> new file mode 100644
>> index 0000000..8aee205
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi_numa.c
>> @@ -0,0 +1,215 @@
>> +/*
>> + * ACPI 5.1 based NUMA setup for ARM64
>> + * Lots of code was borrowed from arch/x86/mm/srat.c
>> + *
>> + * Copyright 2004 Andi Kleen, SuSE Labs.
>> + * Copyright (C) 2013-2014, Linaro Ltd.
>> + *		Author: Hanjun Guo <hanjun.guo@linaro.org>
>> + *
>> + * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
>> + *
>> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
>> + * Assumes all memory regions belonging to a single proximity domain
>> + * are in one chunk. Holes between them will be included in the node.
>> + */
>> +
>> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/bootmem.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mm.h>
>> +#include <linux/memblock.h>
>> +#include <linux/mmzone.h>
>> +#include <linux/module.h>
>> +#include <linux/topology.h>
>> +
>> +#include <acpi/processor.h>
>> +#include <asm/numa.h>
>> +
>> +int acpi_numa __initdata;
>> +
>> +static __init int setup_node(int pxm)
>> +{
>> +	return acpi_map_pxm_to_node(pxm);
>> +}
>
> This function is not that useful given how it is used in the patch.

Agree, just use acpi_map_pxm_to_node() will be fine.

>
>> +
>> +static __init void bad_srat(void)
>> +{
>> +	pr_err("SRAT not used.\n");
>> +	acpi_numa = -1;
>> +}
>> +
>> +static __init inline int srat_disabled(void)
>> +{
>> +	return acpi_numa < 0;
>> +}
>> +
>> +/*
>> + * Callback for SLIT parsing.
>> + * It will get the distance information presented by SLIT
>> + * and init the distance matrix of numa nodes
>> + */
>> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
>> +{
>> +	int i, j;
>> +
>> +	for (i = 0; i < slit->locality_count; i++) {
>> +		const int from_node = pxm_to_node(i);
>> +
>> +		if (from_node == NUMA_NO_NODE)
>> +			continue;
>> +
>> +		for (j = 0; j < slit->locality_count; j++) {
>> +			const int to_node = pxm_to_node(j);
>> +
>> +			if (to_node == NUMA_NO_NODE)
>> +				continue;
>> +
>> +			pr_info("SLIT: Distance[%d][%d] = %d\n",
>> +					from_node, to_node,
>> +					slit->entry[
>> +					slit->locality_count * i + j]);
>> +			numa_set_distance(from_node, to_node,
>> +				slit->entry[slit->locality_count * i + j]);
>> +		}
>> +	}
>> +}
>
> This function is an x86 copy'n'paste. ia64 just requires this callback
> to save a slit table pointer (that can be moved to generic code and it is
> initdata anyway), so my question is, do we really need to duplicate it ?

Sure we need it, no matter DT or ACPI, we need to figure out the
NUMA node distance and set it properly. For IA64, the slit table
pointer is saved but it also used to setup the NUMA node distance
as you can see the code in acpi_numa_arch_fixup() in arch/ia64/kernel
/acpi.c, it just init it in different time slot.

>
>> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
>> +{
>
> Looks familiar. I guess you can't reuse the code in drivers/acpi
> (acpi_map_cpuid()) only because that implies permanent table mappings to be
> in place and you need to call this function before acpi_gbl_permanent_mmap
> is set ?

Hey, you are reading my mind :). Yes, as you said, it also will
lead to early memory remap leak, any suggestion?

>
>> +	unsigned long madt_end, entry;
>> +	struct acpi_table_madt *madt;
>> +	acpi_size tbl_size;
>> +
>> +	if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
>> +			(struct acpi_table_header **)&madt, &tbl_size)))
>> +		return -ENODEV;
>> +
>> +	entry = (unsigned long)madt;
>> +	madt_end = entry + madt->header.length;
>> +
>> +	/* Parse all entries looking for a match. */
>> +	entry += sizeof(struct acpi_table_madt);
>> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>> +		struct acpi_subtable_header *header =
>> +			(struct acpi_subtable_header *)entry;
>> +
>> +		if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
>> +			struct acpi_madt_generic_interrupt *gicc =
>> +				container_of(header,
>> +				struct acpi_madt_generic_interrupt, header);
>> +
>> +			if ((gicc->flags & ACPI_MADT_ENABLED) &&
>> +			    (gicc->uid == acpi_id)) {
>> +				*mpidr = gicc->arm_mpidr;
>> +				early_acpi_os_unmap_memory(madt, tbl_size);
>> +				return 0;
>> +			}
>> +		}
>> +		entry += header->length;
>> +	}
>> +
>> +	early_acpi_os_unmap_memory(madt, tbl_size);
>> +	return -ENODEV;
>> +}
>> +
>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
>> +{
>> +	int pxm, node;
>> +	u64 mpidr;
>> +	static int cpus_in_srat;
>> +
>> +	if (srat_disabled())
>> +		return;
>> +
>> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>> +		return;
>> +
>> +	if (cpus_in_srat >= NR_CPUS) {
>> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
>> +			     NR_CPUS);
>> +		return;
>> +	}
>> +
>> +	pxm = pa->proximity_domain;
>> +	node = setup_node(pxm);
>> +
>> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>> +		pr_err("SRAT: Too many proximity domains %d\n", pxm);
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
>> +		pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR in MADT\n",
>> +			pxm, pa->acpi_processor_uid);
>> +		bad_srat();
>> +		return;
>> +	}
>> +
>> +	cpu_to_node_map[cpus_in_srat] = node;
>
> This looks wrong. cpus_in_srat is a logical index, but I do not see why
> it has to be sequential. You retrieve the mpidr for a given SRAT entry
> and with that value you should retrieve the cpu_logical index that
> corresponds to it (get_logical_index()), or maybe I am missing something
> from the ACPI specs that enforce a SRAT entries ordering, on which we
> should not rely upon anyway.

No, you are right, there is no enforce of ordering of CPU in SRAT, I
will update it, good catch.

>
>> +	node_set(node, numa_nodes_parsed);
>> +	acpi_numa = 1;
>
> What's the point if you are checking for < 0 in srat_disabled() ?

acpi_numa will be set as -1 in bad_srat(), if that happens, we
will not going to parse SRAT anymore, did I understand your
question?

>
>> +	cpus_in_srat++;
>> +	pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
>> +			pxm, mpidr, node, cpus_in_srat);
>> +}
>> +
>> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
>> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>> +{
>> +	u64 start, end;
>> +	int node, pxm;
>> +
>> +	if (srat_disabled())
>> +		return -EINVAL;
>> +
>> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	start = ma->base_address;
>> +	end = start + ma->length;
>> +	pxm = ma->proximity_domain;
>> +
>> +	node = setup_node(pxm);
>> +
>> +	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>> +		pr_err("SRAT: Too many proximity domains.\n");
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
>> +		node, pxm,
>> +		(unsigned long long) start, (unsigned long long) end - 1);
>> +
>> +	if (numa_add_memblk(node, start, (end - start)) < 0) {
>> +		bad_srat();
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>
> I do not see again any major changes compared to x86, numa_add_memblk()
> has a different interface (size vs end) but apart from that it would
> be nice to avoid rewriting the same code time and again.

Let me see if I can rewrite it in next version.

>
>> +
>> +void __init acpi_numa_arch_fixup(void) { }
>
> Sigh. How many of these useless callbacks are we forced to define ?

Hmm, x86 also use a dummy function for it. I think we can fix it
in this way:

  - introduce a kconfig named ACPI_HAS_NUMA_ARCH_FIXUP

  - select it on IA64

  - introduce a stub function for x86 and ARM64 when
    !ACPI_HAS_NUMA_ARCH_FIXUP

  - remove the useless callbacks for x86 and ARM64

what do you think?

>
>> +
>> +int __init arm64_acpi_numa_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = acpi_numa_init();
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return srat_disabled() ? -EINVAL : 0;
>> +}
>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>> index 209c7a9..c2950fc 100644
>> --- a/arch/arm64/mm/numa.c
>> +++ b/arch/arm64/mm/numa.c
>> @@ -17,6 +17,7 @@
>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>    */
>>
>> +#include <linux/acpi.h>
>>   #include <linux/bootmem.h>
>>   #include <linux/ctype.h>
>>   #include <linux/init.h>
>> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>>   	int ret = -ENODEV;
>>
>>   #ifdef CONFIG_OF_NUMA
>> -	if (!numa_off)
>> +	if (!numa_off && acpi_disabled)
>>   		ret = numa_init(arm64_of_numa_init);
>>   #endif
>> +#ifdef CONFIG_ACPI_NUMA
>> +	if (!numa_off && !acpi_disabled)
>> +		ret = numa_init(arm64_acpi_numa_init);
>> +#endif
>
> See my comment above on DT/ACPI, this ifdeffery does not look great.

I will sync with Ganapatrao to see how to combine them together,
if any problem met, we will back to this discussion.

Thanks
Hanjun

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Ganapatrao Kulkarni]

Hi Lorenzo,


On Tue, Dec 22, 2015 at 6:04 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
> Hi Lorenzo,
>
> Sorry for the late reply, just busy with other work, please
> see my comments below.
>
> On 2015/12/19 0:23, Lorenzo Pieralisi wrote:
>>
>> On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:
>>
>> [...]
>>
>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>> index 7987763..555c4a5 100644
>>> --- a/arch/arm64/kernel/Makefile
>>> +++ b/arch/arm64/kernel/Makefile
>>> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)                       += pci.o
>>>   arm64-obj-$(CONFIG_ARMV8_DEPRECATED)  += armv8_deprecated.o
>>>   arm64-obj-$(CONFIG_ACPI)              += acpi.o
>>>   arm64-obj-$(CONFIG_OF_NUMA)           += of_numa.o
>>> +arm64-obj-$(CONFIG_ACPI_NUMA)          += acpi_numa.o
>>
>>
>> Isn't it better to merge ACPI and DT support in one file (a bit like
>> what we did for smp.c) to remove some of this iffdeffery ?
IMO, it is better to have it in separate files for the better readability.
however we can remove iffdeffery.

>
>
> It's definitely a good idea, but I'm not sure what's the blockers
> ahead, I will try to do that in next version.
>
>
>>
>>>
>>>   obj-y                                 += $(arm64-obj-y) vdso/
>>>   obj-m                                 += $(arm64-obj-m)
>>> diff --git a/arch/arm64/kernel/acpi_numa.c
>>> b/arch/arm64/kernel/acpi_numa.c
>>> new file mode 100644
>>> index 0000000..8aee205
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/acpi_numa.c
>>> @@ -0,0 +1,215 @@
>>> +/*
>>> + * ACPI 5.1 based NUMA setup for ARM64
>>> + * Lots of code was borrowed from arch/x86/mm/srat.c
>>> + *
>>> + * Copyright 2004 Andi Kleen, SuSE Labs.
>>> + * Copyright (C) 2013-2014, Linaro Ltd.
>>> + *             Author: Hanjun Guo <hanjun.guo@linaro.org>
>>> + *
>>> + * Reads the ACPI SRAT table to figure out what memory belongs to which
>>> CPUs.
>>> + *
>>> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
>>> + * Assumes all memory regions belonging to a single proximity domain
>>> + * are in one chunk. Holes between them will be included in the node.
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/bitmap.h>
>>> +#include <linux/bootmem.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/mm.h>
>>> +#include <linux/memblock.h>
>>> +#include <linux/mmzone.h>
>>> +#include <linux/module.h>
>>> +#include <linux/topology.h>
>>> +
>>> +#include <acpi/processor.h>
>>> +#include <asm/numa.h>
>>> +
>>> +int acpi_numa __initdata;
>>> +
>>> +static __init int setup_node(int pxm)
>>> +{
>>> +       return acpi_map_pxm_to_node(pxm);
>>> +}
>>
>>
>> This function is not that useful given how it is used in the patch.
>
>
> Agree, just use acpi_map_pxm_to_node() will be fine.
>
>
>>
>>> +
>>> +static __init void bad_srat(void)
>>> +{
>>> +       pr_err("SRAT not used.\n");
>>> +       acpi_numa = -1;
>>> +}
>>> +
>>> +static __init inline int srat_disabled(void)
>>> +{
>>> +       return acpi_numa < 0;
>>> +}
>>> +
>>> +/*
>>> + * Callback for SLIT parsing.
>>> + * It will get the distance information presented by SLIT
>>> + * and init the distance matrix of numa nodes
>>> + */
>>> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
>>> +{
>>> +       int i, j;
>>> +
>>> +       for (i = 0; i < slit->locality_count; i++) {
>>> +               const int from_node = pxm_to_node(i);
>>> +
>>> +               if (from_node == NUMA_NO_NODE)
>>> +                       continue;
>>> +
>>> +               for (j = 0; j < slit->locality_count; j++) {
>>> +                       const int to_node = pxm_to_node(j);
>>> +
>>> +                       if (to_node == NUMA_NO_NODE)
>>> +                               continue;
>>> +
>>> +                       pr_info("SLIT: Distance[%d][%d] = %d\n",
>>> +                                       from_node, to_node,
>>> +                                       slit->entry[
>>> +                                       slit->locality_count * i + j]);
>>> +                       numa_set_distance(from_node, to_node,
>>> +                               slit->entry[slit->locality_count * i +
>>> j]);
>>> +               }
>>> +       }
>>> +}
>>
>>
>> This function is an x86 copy'n'paste. ia64 just requires this callback
>> to save a slit table pointer (that can be moved to generic code and it is
>> initdata anyway), so my question is, do we really need to duplicate it ?
>
>
> Sure we need it, no matter DT or ACPI, we need to figure out the
> NUMA node distance and set it properly. For IA64, the slit table
> pointer is saved but it also used to setup the NUMA node distance
> as you can see the code in acpi_numa_arch_fixup() in arch/ia64/kernel
> /acpi.c, it just init it in different time slot.
>
>>
>>> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
>>> +{
>>
>>
>> Looks familiar. I guess you can't reuse the code in drivers/acpi
>> (acpi_map_cpuid()) only because that implies permanent table mappings to
>> be
>> in place and you need to call this function before acpi_gbl_permanent_mmap
>> is set ?
>
>
> Hey, you are reading my mind :). Yes, as you said, it also will
> lead to early memory remap leak, any suggestion?
>
>
>>
>>> +       unsigned long madt_end, entry;
>>> +       struct acpi_table_madt *madt;
>>> +       acpi_size tbl_size;
>>> +
>>> +       if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
>>> +                       (struct acpi_table_header **)&madt, &tbl_size)))
>>> +               return -ENODEV;
>>> +
>>> +       entry = (unsigned long)madt;
>>> +       madt_end = entry + madt->header.length;
>>> +
>>> +       /* Parse all entries looking for a match. */
>>> +       entry += sizeof(struct acpi_table_madt);
>>> +       while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>>> +               struct acpi_subtable_header *header =
>>> +                       (struct acpi_subtable_header *)entry;
>>> +
>>> +               if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
>>> +                       struct acpi_madt_generic_interrupt *gicc =
>>> +                               container_of(header,
>>> +                               struct acpi_madt_generic_interrupt,
>>> header);
>>> +
>>> +                       if ((gicc->flags & ACPI_MADT_ENABLED) &&
>>> +                           (gicc->uid == acpi_id)) {
>>> +                               *mpidr = gicc->arm_mpidr;
>>> +                               early_acpi_os_unmap_memory(madt,
>>> tbl_size);
>>> +                               return 0;
>>> +                       }
>>> +               }
>>> +               entry += header->length;
>>> +       }
>>> +
>>> +       early_acpi_os_unmap_memory(madt, tbl_size);
>>> +       return -ENODEV;
>>> +}
>>> +
>>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity
>>> *pa)
>>> +{
>>> +       int pxm, node;
>>> +       u64 mpidr;
>>> +       static int cpus_in_srat;
>>> +
>>> +       if (srat_disabled())
>>> +               return;
>>> +
>>> +       if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>>> +               return;
>>> +
>>> +       if (cpus_in_srat >= NR_CPUS) {
>>> +               pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small,
>>> may not be able to use all cpus\n",
>>> +                            NR_CPUS);
>>> +               return;
>>> +       }
>>> +
>>> +       pxm = pa->proximity_domain;
>>> +       node = setup_node(pxm);
>>> +
>>> +       if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>>> +               pr_err("SRAT: Too many proximity domains %d\n", pxm);
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
>>> +               pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR
>>> in MADT\n",
>>> +                       pxm, pa->acpi_processor_uid);
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       cpu_to_node_map[cpus_in_srat] = node;
>>
>>
>> This looks wrong. cpus_in_srat is a logical index, but I do not see why
>> it has to be sequential. You retrieve the mpidr for a given SRAT entry
>> and with that value you should retrieve the cpu_logical index that
>> corresponds to it (get_logical_index()), or maybe I am missing something
>> from the ACPI specs that enforce a SRAT entries ordering, on which we
>> should not rely upon anyway.
>
>
> No, you are right, there is no enforce of ordering of CPU in SRAT, I
> will update it, good catch.
>
>>
>>> +       node_set(node, numa_nodes_parsed);
>>> +       acpi_numa = 1;
>>
>>
>> What's the point if you are checking for < 0 in srat_disabled() ?
>
>
> acpi_numa will be set as -1 in bad_srat(), if that happens, we
> will not going to parse SRAT anymore, did I understand your
> question?
>
>
>>
>>> +       cpus_in_srat++;
>>> +       pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
>>> +                       pxm, mpidr, node, cpus_in_srat);
>>> +}
>>> +
>>> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings
>>> */
>>> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity
>>> *ma)
>>> +{
>>> +       u64 start, end;
>>> +       int node, pxm;
>>> +
>>> +       if (srat_disabled())
>>> +               return -EINVAL;
>>> +
>>> +       if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       start = ma->base_address;
>>> +       end = start + ma->length;
>>> +       pxm = ma->proximity_domain;
>>> +
>>> +       node = setup_node(pxm);
>>> +
>>> +       if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>>> +               pr_err("SRAT: Too many proximity domains.\n");
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
>>> +               node, pxm,
>>> +               (unsigned long long) start, (unsigned long long) end -
>>> 1);
>>> +
>>> +       if (numa_add_memblk(node, start, (end - start)) < 0) {
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>
>>
>> I do not see again any major changes compared to x86, numa_add_memblk()
>> has a different interface (size vs end) but apart from that it would
>> be nice to avoid rewriting the same code time and again.
numa_add_memblk() is not same.

>
>
> Let me see if I can rewrite it in next version.
>
>>
>>> +
>>> +void __init acpi_numa_arch_fixup(void) { }
>>
>>
>> Sigh. How many of these useless callbacks are we forced to define ?
>
>
> Hmm, x86 also use a dummy function for it. I think we can fix it
> in this way:
>
>  - introduce a kconfig named ACPI_HAS_NUMA_ARCH_FIXUP
>
>  - select it on IA64
>
>  - introduce a stub function for x86 and ARM64 when
>    !ACPI_HAS_NUMA_ARCH_FIXUP
>
>  - remove the useless callbacks for x86 and ARM64
>
> what do you think?
>
>
>>
>>> +
>>> +int __init arm64_acpi_numa_init(void)
>>> +{
>>> +       int ret;
>>> +
>>> +       ret = acpi_numa_init();
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       return srat_disabled() ? -EINVAL : 0;
>>> +}
>>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>>> index 209c7a9..c2950fc 100644
>>> --- a/arch/arm64/mm/numa.c
>>> +++ b/arch/arm64/mm/numa.c
>>> @@ -17,6 +17,7 @@
>>>    * along with this program.  If not, see
>>> <http://www.gnu.org/licenses/>.
>>>    */
>>>
>>> +#include <linux/acpi.h>
>>>   #include <linux/bootmem.h>
>>>   #include <linux/ctype.h>
>>>   #include <linux/init.h>
>>> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>>>         int ret = -ENODEV;
>>>
>>>   #ifdef CONFIG_OF_NUMA
>>> -       if (!numa_off)
>>> +       if (!numa_off && acpi_disabled)
>>>                 ret = numa_init(arm64_of_numa_init);
>>>   #endif
>>> +#ifdef CONFIG_ACPI_NUMA
>>> +       if (!numa_off && !acpi_disabled)
>>> +               ret = numa_init(arm64_acpi_numa_init);
>>> +#endif
>>
>>
>> See my comment above on DT/ACPI, this ifdeffery does not look great.
ok,  since both OF_NUMA and ACPI_NUMA are default y for ARM64, we can
remove the ifdef.
we can have like below.

void __init arm64_numa_init(void)
{
        int ret = -ENODEV;

        if (!numa_off && acpi_disabled)
                ret = numa_init(arm64_of_numa_init);
        else if (!numa_off && !acpi_disabled)
                ret = numa_init(arm64_acpi_numa_init);

        if (ret)
                numa_init(dummy_numa_init);
}

>
>
> I will sync with Ganapatrao to see how to combine them together,
> if any problem met, we will back to this discussion.
>
> Thanks
> Hanjun
thanks
Ganapat

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Ganapatrao Kulkarni]

Hi Lorenzo,


On Tue, Dec 22, 2015 at 6:04 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
> Hi Lorenzo,
>
> Sorry for the late reply, just busy with other work, please
> see my comments below.
>
> On 2015/12/19 0:23, Lorenzo Pieralisi wrote:
>>
>> On Tue, Nov 17, 2015 at 11:55:32PM +0530, Ganapatrao Kulkarni wrote:
>>
>> [...]
>>
>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>> index 7987763..555c4a5 100644
>>> --- a/arch/arm64/kernel/Makefile
>>> +++ b/arch/arm64/kernel/Makefile
>>> @@ -42,6 +42,7 @@ arm64-obj-$(CONFIG_PCI)                       += pci.o
>>>   arm64-obj-$(CONFIG_ARMV8_DEPRECATED)  += armv8_deprecated.o
>>>   arm64-obj-$(CONFIG_ACPI)              += acpi.o
>>>   arm64-obj-$(CONFIG_OF_NUMA)           += of_numa.o
>>> +arm64-obj-$(CONFIG_ACPI_NUMA)          += acpi_numa.o
>>
>>
>> Isn't it better to merge ACPI and DT support in one file (a bit like
>> what we did for smp.c) to remove some of this iffdeffery ?
IMO, it is better to have it in separate files for the better readability.
however we can remove iffdeffery.

>
>
> It's definitely a good idea, but I'm not sure what's the blockers
> ahead, I will try to do that in next version.
>
>
>>
>>>
>>>   obj-y                                 += $(arm64-obj-y) vdso/
>>>   obj-m                                 += $(arm64-obj-m)
>>> diff --git a/arch/arm64/kernel/acpi_numa.c
>>> b/arch/arm64/kernel/acpi_numa.c
>>> new file mode 100644
>>> index 0000000..8aee205
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/acpi_numa.c
>>> @@ -0,0 +1,215 @@
>>> +/*
>>> + * ACPI 5.1 based NUMA setup for ARM64
>>> + * Lots of code was borrowed from arch/x86/mm/srat.c
>>> + *
>>> + * Copyright 2004 Andi Kleen, SuSE Labs.
>>> + * Copyright (C) 2013-2014, Linaro Ltd.
>>> + *             Author: Hanjun Guo <hanjun.guo@linaro.org>
>>> + *
>>> + * Reads the ACPI SRAT table to figure out what memory belongs to which
>>> CPUs.
>>> + *
>>> + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
>>> + * Assumes all memory regions belonging to a single proximity domain
>>> + * are in one chunk. Holes between them will be included in the node.
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "ACPI: NUMA: " fmt
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/bitmap.h>
>>> +#include <linux/bootmem.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/mm.h>
>>> +#include <linux/memblock.h>
>>> +#include <linux/mmzone.h>
>>> +#include <linux/module.h>
>>> +#include <linux/topology.h>
>>> +
>>> +#include <acpi/processor.h>
>>> +#include <asm/numa.h>
>>> +
>>> +int acpi_numa __initdata;
>>> +
>>> +static __init int setup_node(int pxm)
>>> +{
>>> +       return acpi_map_pxm_to_node(pxm);
>>> +}
>>
>>
>> This function is not that useful given how it is used in the patch.
>
>
> Agree, just use acpi_map_pxm_to_node() will be fine.
>
>
>>
>>> +
>>> +static __init void bad_srat(void)
>>> +{
>>> +       pr_err("SRAT not used.\n");
>>> +       acpi_numa = -1;
>>> +}
>>> +
>>> +static __init inline int srat_disabled(void)
>>> +{
>>> +       return acpi_numa < 0;
>>> +}
>>> +
>>> +/*
>>> + * Callback for SLIT parsing.
>>> + * It will get the distance information presented by SLIT
>>> + * and init the distance matrix of numa nodes
>>> + */
>>> +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
>>> +{
>>> +       int i, j;
>>> +
>>> +       for (i = 0; i < slit->locality_count; i++) {
>>> +               const int from_node = pxm_to_node(i);
>>> +
>>> +               if (from_node == NUMA_NO_NODE)
>>> +                       continue;
>>> +
>>> +               for (j = 0; j < slit->locality_count; j++) {
>>> +                       const int to_node = pxm_to_node(j);
>>> +
>>> +                       if (to_node == NUMA_NO_NODE)
>>> +                               continue;
>>> +
>>> +                       pr_info("SLIT: Distance[%d][%d] = %d\n",
>>> +                                       from_node, to_node,
>>> +                                       slit->entry[
>>> +                                       slit->locality_count * i + j]);
>>> +                       numa_set_distance(from_node, to_node,
>>> +                               slit->entry[slit->locality_count * i +
>>> j]);
>>> +               }
>>> +       }
>>> +}
>>
>>
>> This function is an x86 copy'n'paste. ia64 just requires this callback
>> to save a slit table pointer (that can be moved to generic code and it is
>> initdata anyway), so my question is, do we really need to duplicate it ?
>
>
> Sure we need it, no matter DT or ACPI, we need to figure out the
> NUMA node distance and set it properly. For IA64, the slit table
> pointer is saved but it also used to setup the NUMA node distance
> as you can see the code in acpi_numa_arch_fixup() in arch/ia64/kernel
> /acpi.c, it just init it in different time slot.
>
>>
>>> +static int __init get_mpidr_in_madt(int acpi_id, u64 *mpidr)
>>> +{
>>
>>
>> Looks familiar. I guess you can't reuse the code in drivers/acpi
>> (acpi_map_cpuid()) only because that implies permanent table mappings to
>> be
>> in place and you need to call this function before acpi_gbl_permanent_mmap
>> is set ?
>
>
> Hey, you are reading my mind :). Yes, as you said, it also will
> lead to early memory remap leak, any suggestion?
>
>
>>
>>> +       unsigned long madt_end, entry;
>>> +       struct acpi_table_madt *madt;
>>> +       acpi_size tbl_size;
>>> +
>>> +       if (ACPI_FAILURE(acpi_get_table_with_size(ACPI_SIG_MADT, 0,
>>> +                       (struct acpi_table_header **)&madt, &tbl_size)))
>>> +               return -ENODEV;
>>> +
>>> +       entry = (unsigned long)madt;
>>> +       madt_end = entry + madt->header.length;
>>> +
>>> +       /* Parse all entries looking for a match. */
>>> +       entry += sizeof(struct acpi_table_madt);
>>> +       while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>>> +               struct acpi_subtable_header *header =
>>> +                       (struct acpi_subtable_header *)entry;
>>> +
>>> +               if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
>>> +                       struct acpi_madt_generic_interrupt *gicc =
>>> +                               container_of(header,
>>> +                               struct acpi_madt_generic_interrupt,
>>> header);
>>> +
>>> +                       if ((gicc->flags & ACPI_MADT_ENABLED) &&
>>> +                           (gicc->uid == acpi_id)) {
>>> +                               *mpidr = gicc->arm_mpidr;
>>> +                               early_acpi_os_unmap_memory(madt,
>>> tbl_size);
>>> +                               return 0;
>>> +                       }
>>> +               }
>>> +               entry += header->length;
>>> +       }
>>> +
>>> +       early_acpi_os_unmap_memory(madt, tbl_size);
>>> +       return -ENODEV;
>>> +}
>>> +
>>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity
>>> *pa)
>>> +{
>>> +       int pxm, node;
>>> +       u64 mpidr;
>>> +       static int cpus_in_srat;
>>> +
>>> +       if (srat_disabled())
>>> +               return;
>>> +
>>> +       if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>>> +               return;
>>> +
>>> +       if (cpus_in_srat >= NR_CPUS) {
>>> +               pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small,
>>> may not be able to use all cpus\n",
>>> +                            NR_CPUS);
>>> +               return;
>>> +       }
>>> +
>>> +       pxm = pa->proximity_domain;
>>> +       node = setup_node(pxm);
>>> +
>>> +       if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>>> +               pr_err("SRAT: Too many proximity domains %d\n", pxm);
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       if (get_mpidr_in_madt(pa->acpi_processor_uid, &mpidr)) {
>>> +               pr_warn("SRAT: PXM %d with ACPI ID %d has no valid MPIDR
>>> in MADT\n",
>>> +                       pxm, pa->acpi_processor_uid);
>>> +               bad_srat();
>>> +               return;
>>> +       }
>>> +
>>> +       cpu_to_node_map[cpus_in_srat] = node;
>>
>>
>> This looks wrong. cpus_in_srat is a logical index, but I do not see why
>> it has to be sequential. You retrieve the mpidr for a given SRAT entry
>> and with that value you should retrieve the cpu_logical index that
>> corresponds to it (get_logical_index()), or maybe I am missing something
>> from the ACPI specs that enforce a SRAT entries ordering, on which we
>> should not rely upon anyway.
>
>
> No, you are right, there is no enforce of ordering of CPU in SRAT, I
> will update it, good catch.
>
>>
>>> +       node_set(node, numa_nodes_parsed);
>>> +       acpi_numa = 1;
>>
>>
>> What's the point if you are checking for < 0 in srat_disabled() ?
>
>
> acpi_numa will be set as -1 in bad_srat(), if that happens, we
> will not going to parse SRAT anymore, did I understand your
> question?
>
>
>>
>>> +       cpus_in_srat++;
>>> +       pr_info("SRAT: PXM %d -> MPIDR 0x%Lx -> Node %d cpu %d\n",
>>> +                       pxm, mpidr, node, cpus_in_srat);
>>> +}
>>> +
>>> +/* Callback for parsing of the Proximity Domain <-> Memory Area mappings
>>> */
>>> +int __init acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity
>>> *ma)
>>> +{
>>> +       u64 start, end;
>>> +       int node, pxm;
>>> +
>>> +       if (srat_disabled())
>>> +               return -EINVAL;
>>> +
>>> +       if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       start = ma->base_address;
>>> +       end = start + ma->length;
>>> +       pxm = ma->proximity_domain;
>>> +
>>> +       node = setup_node(pxm);
>>> +
>>> +       if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>>> +               pr_err("SRAT: Too many proximity domains.\n");
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
>>> +               node, pxm,
>>> +               (unsigned long long) start, (unsigned long long) end -
>>> 1);
>>> +
>>> +       if (numa_add_memblk(node, start, (end - start)) < 0) {
>>> +               bad_srat();
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>
>>
>> I do not see again any major changes compared to x86, numa_add_memblk()
>> has a different interface (size vs end) but apart from that it would
>> be nice to avoid rewriting the same code time and again.
numa_add_memblk() is not same.

>
>
> Let me see if I can rewrite it in next version.
>
>>
>>> +
>>> +void __init acpi_numa_arch_fixup(void) { }
>>
>>
>> Sigh. How many of these useless callbacks are we forced to define ?
>
>
> Hmm, x86 also use a dummy function for it. I think we can fix it
> in this way:
>
>  - introduce a kconfig named ACPI_HAS_NUMA_ARCH_FIXUP
>
>  - select it on IA64
>
>  - introduce a stub function for x86 and ARM64 when
>    !ACPI_HAS_NUMA_ARCH_FIXUP
>
>  - remove the useless callbacks for x86 and ARM64
>
> what do you think?
>
>
>>
>>> +
>>> +int __init arm64_acpi_numa_init(void)
>>> +{
>>> +       int ret;
>>> +
>>> +       ret = acpi_numa_init();
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       return srat_disabled() ? -EINVAL : 0;
>>> +}
>>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>>> index 209c7a9..c2950fc 100644
>>> --- a/arch/arm64/mm/numa.c
>>> +++ b/arch/arm64/mm/numa.c
>>> @@ -17,6 +17,7 @@
>>>    * along with this program.  If not, see
>>> <http://www.gnu.org/licenses/>.
>>>    */
>>>
>>> +#include <linux/acpi.h>
>>>   #include <linux/bootmem.h>
>>>   #include <linux/ctype.h>
>>>   #include <linux/init.h>
>>> @@ -383,9 +384,13 @@ void __init arm64_numa_init(void)
>>>         int ret = -ENODEV;
>>>
>>>   #ifdef CONFIG_OF_NUMA
>>> -       if (!numa_off)
>>> +       if (!numa_off && acpi_disabled)
>>>                 ret = numa_init(arm64_of_numa_init);
>>>   #endif
>>> +#ifdef CONFIG_ACPI_NUMA
>>> +       if (!numa_off && !acpi_disabled)
>>> +               ret = numa_init(arm64_acpi_numa_init);
>>> +#endif
>>
>>
>> See my comment above on DT/ACPI, this ifdeffery does not look great.
ok,  since both OF_NUMA and ACPI_NUMA are default y for ARM64, we can
remove the ifdef.
we can have like below.

void __init arm64_numa_init(void)
{
        int ret = -ENODEV;

        if (!numa_off && acpi_disabled)
                ret = numa_init(arm64_of_numa_init);
        else if (!numa_off && !acpi_disabled)
                ret = numa_init(arm64_acpi_numa_init);

        if (ret)
                numa_init(dummy_numa_init);
}

>
>
> I will sync with Ganapatrao to see how to combine them together,
> if any problem met, we will back to this discussion.
>
> Thanks
> Hanjun
thanks
Ganapat

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Robert Richter]

On 27.11.15 15:54:55, Shannon Zhao wrote:
> Hi,
> 
> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
> > +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> > +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> > +{
> > +	int pxm, node;
> > +	u64 mpidr;
> > +	static int cpus_in_srat;
> > +
> > +	if (srat_disabled())
> > +		return;
> > +
> > +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> > +		bad_srat();
> > +		return;
> > +	}
> > +
> > +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> > +		return;
> > +
> > +	if (cpus_in_srat >= NR_CPUS) {
> > +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> > +			     NR_CPUS);
> > +		return;
> > +	}
> 
> arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
> arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
> argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
>    pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
> able to use all cpus\n",

Right, should be changed to:

 pr_warn_once("SRAT: cpu_to_node_map[%d] ...

-Robert

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Robert Richter]

On 27.11.15 15:54:55, Shannon Zhao wrote:
> Hi,
> 
> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
> > +/* Callback for Proximity Domain -> ACPI processor UID mapping */
> > +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> > +{
> > +	int pxm, node;
> > +	u64 mpidr;
> > +	static int cpus_in_srat;
> > +
> > +	if (srat_disabled())
> > +		return;
> > +
> > +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
> > +		bad_srat();
> > +		return;
> > +	}
> > +
> > +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
> > +		return;
> > +
> > +	if (cpus_in_srat >= NR_CPUS) {
> > +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
> > +			     NR_CPUS);
> > +		return;
> > +	}
> 
> arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
> arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
> argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
>    pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
> able to use all cpus\n",

Right, should be changed to:

 pr_warn_once("SRAT: cpu_to_node_map[%d] ...

-Robert

Re: [PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Hanjun Guo]

On 2016/1/14 7:04, Robert Richter wrote:
> On 27.11.15 15:54:55, Shannon Zhao wrote:
>> Hi,
>>
>> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
>>> +{
>>> +	int pxm, node;
>>> +	u64 mpidr;
>>> +	static int cpus_in_srat;
>>> +
>>> +	if (srat_disabled())
>>> +		return;
>>> +
>>> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>>> +		bad_srat();
>>> +		return;
>>> +	}
>>> +
>>> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>>> +		return;
>>> +
>>> +	if (cpus_in_srat >= NR_CPUS) {
>>> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
>>> +			     NR_CPUS);
>>> +		return;
>>> +	}
>>
>> arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
>> arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
>> argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
>>     pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
>> able to use all cpus\n",
>
> Right, should be changed to:
>
>   pr_warn_once("SRAT: cpu_to_node_map[%d] ...

Thanks, I will update it in next version.

Hanjun

[PATCH v2 3/4] arm64, acpi, numa: NUMA support based on SRAT and SLIT

[Hanjun Guo]

On 2016/1/14 7:04, Robert Richter wrote:
> On 27.11.15 15:54:55, Shannon Zhao wrote:
>> Hi,
>>
>> On 2015/11/18 2:25, Ganapatrao Kulkarni wrote:
>>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>>> +void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
>>> +{
>>> +	int pxm, node;
>>> +	u64 mpidr;
>>> +	static int cpus_in_srat;
>>> +
>>> +	if (srat_disabled())
>>> +		return;
>>> +
>>> +	if (pa->header.length < sizeof(struct acpi_srat_gicc_affinity)) {
>>> +		bad_srat();
>>> +		return;
>>> +	}
>>> +
>>> +	if (!(pa->flags & ACPI_SRAT_GICC_ENABLED))
>>> +		return;
>>> +
>>> +	if (cpus_in_srat >= NR_CPUS) {
>>> +		pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be able to use all cpus\n",
>>> +			     NR_CPUS);
>>> +		return;
>>> +	}
>>
>> arch/arm64/kernel/acpi_numa.c: In function 'acpi_numa_gicc_affinity_init':
>> arch/arm64/kernel/acpi_numa.c:137:3: warning: format '%ld' expects
>> argument of type 'long int', but argument 2 has type 'int' [-Wformat=]
>>     pr_warn_once("SRAT: cpu_to_node_map[%ld] is too small, may not be
>> able to use all cpus\n",
>
> Right, should be changed to:
>
>   pr_warn_once("SRAT: cpu_to_node_map[%d] ...

Thanks, I will update it in next version.

Hanjun

Re: [PATCH v2 0/4] ACPI based NUMA support for ARM64

[Jon Masters]

On 11/17/15, 1:25 PM, Ganapatrao Kulkarni wrote:
>   -v2 rebased on arm64-numa v7 patches.
>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>
>   -v1 initial patches from Hanjun Guo

Hi Ganapatrao,

Can I confirm that you intend to post an updated version of this patch
series once the merge window is closed? We have a dependency upon this
patch series being upstream and need to do a little planning :)

Jon.

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Jon Masters]

On 11/17/15, 1:25 PM, Ganapatrao Kulkarni wrote:
>   -v2 rebased on arm64-numa v7 patches.
>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>
>   -v1 initial patches from Hanjun Guo

Hi Ganapatrao,

Can I confirm that you intend to post an updated version of this patch
series once the merge window is closed? We have a dependency upon this
patch series being upstream and need to do a little planning :)

Jon.

Re: [PATCH v2 0/4] ACPI based NUMA support for ARM64

[Ganapatrao Kulkarni]

Hi Jon,

On Sun, Jan 17, 2016 at 10:43 AM, Jon Masters <jonathan@jonmasters.org> wrote:
> On 11/17/15, 1:25 PM, Ganapatrao Kulkarni wrote:
>>
>>   -v2 rebased on arm64-numa v7 patches.
>>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>>
>>   -v1 initial patches from Hanjun Guo
>
>
> Hi Ganapatrao,
>
> Can I confirm that you intend to post an updated version of this patch
> series once the merge window is closed? We have a dependency upon this
> patch series being upstream and need to do a little planning :)
We are working on numa-dt-v9 and numa-acpi-v3 patch series.
we will be submitting once merge window is closed.
thanks for your email.
>
> Jon.
>
thanks
Ganapat

[PATCH v2 0/4] ACPI based NUMA support for ARM64

[Ganapatrao Kulkarni]

Hi Jon,

On Sun, Jan 17, 2016 at 10:43 AM, Jon Masters <jonathan@jonmasters.org> wrote:
> On 11/17/15, 1:25 PM, Ganapatrao Kulkarni wrote:
>>
>>   -v2 rebased on arm64-numa v7 patches.
>>   http://www.spinics.net/lists/arm-kernel/msg460813.html
>>
>>   -v1 initial patches from Hanjun Guo
>
>
> Hi Ganapatrao,
>
> Can I confirm that you intend to post an updated version of this patch
> series once the merge window is closed? We have a dependency upon this
> patch series being upstream and need to do a little planning :)
We are working on numa-dt-v9 and numa-acpi-v3 patch series.
we will be submitting once merge window is closed.
thanks for your email.
>
> Jon.
>
thanks
Ganapat