From: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
To: devel@acpica.org
Subject: Re: [Devel] [PATCH 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units
Date: Mon, 12 Jun 2017 11:53:35 +0100 [thread overview]
Message-ID: <20170612105335.GA30594@red-moon> (raw)
In-Reply-To: 1496753787-27735-3-git-send-email-ganapatrao.kulkarni@cavium.com
[-- Attachment #1: Type: text/plain, Size: 8620 bytes --]
On Tue, Jun 06, 2017 at 06:26:27PM +0530, Ganapatrao Kulkarni wrote:
> Add code to parse SRAT ITS Affinity sub table as defined in ACPI 6.2
> Later in driver probe, its devices are mapped to numa node
Nit: Capitalize "its".
> using its id to proximity domain mapping.
Ditto.
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni(a)cavium.com>
> ---
> arch/arm64/include/asm/acpi.h | 2 ++
> arch/arm64/kernel/acpi_numa.c | 59 ++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/numa.c | 32 +++++++++++++++++++++-
> drivers/irqchip/irq-gic-v3-its.c | 3 +-
> include/linux/acpi.h | 3 ++
> 5 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 0e99978..60ea7d1 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -143,9 +143,11 @@ static inline void arch_apei_flush_tlb_one(unsigned long addr)
> #ifdef CONFIG_ACPI_NUMA
> int arm64_acpi_numa_init(void);
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid);
> +int acpi_numa_get_its_nid(u32 its_id);
> #else
> static inline int arm64_acpi_numa_init(void) { return -ENOSYS; }
> static inline int acpi_numa_get_nid(unsigned int cpu, u64 hwid) { return NUMA_NO_NODE; }
> +static inline int acpi_numa_get_its_nid(u32 its_id) { return NUMA_NO_NODE; }
> #endif /* CONFIG_ACPI_NUMA */
>
> #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
> index f01fab6..f7b2ea6 100644
> --- a/arch/arm64/kernel/acpi_numa.c
> +++ b/arch/arm64/kernel/acpi_numa.c
> @@ -29,15 +29,24 @@
> #include <asm/numa.h>
>
> static int cpus_in_srat;
> +static int its_in_srat;
>
> struct __node_cpu_hwid {
> u32 node_id; /* logical node containing this CPU */
> u64 cpu_hwid; /* MPIDR for this CPU */
> };
>
> +struct __node_its_id {
> + u32 node_id; /* numa node id */
> + u32 its_id ; /* GIC ITS ID */
^
Remove space.
> +};
> +
> static struct __node_cpu_hwid early_node_cpu_hwid[NR_CPUS] = {
> [0 ... NR_CPUS - 1] = {NUMA_NO_NODE, PHYS_CPUID_INVALID} };
>
> +static struct __node_its_id early_node_its_id[MAX_NUMNODES] = {
Mind explaining to us where does MAX_NUMNODES as an array size limit
stem from here ?
> +[0 ... MAX_NUMNODES - 1] = {NUMA_NO_NODE, UINT_MAX} };
> +
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> {
> int i;
> @@ -50,6 +59,18 @@ int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> return NUMA_NO_NODE;
> }
>
> +int acpi_numa_get_its_nid(u32 its_id)
> +{
> + int i;
> +
> + for (i = 0; i < its_in_srat; i++) {
> + if (its_id == early_node_its_id[i].its_id)
> + return early_node_its_id[i].node_id;
> + }
> +
> + return NUMA_NO_NODE;
> +}
> +
> /* Callback for Proximity Domain -> ACPI processor UID mapping */
> void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> {
> @@ -100,6 +121,44 @@ void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> pxm, mpidr, node);
> }
>
> +/* Callback for ITS ACPI Proximity Domain mapping */
> +void __init acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa)
> +{
> + int pxm, node;
> +
> + if (srat_disabled())
> + return;
> +
> + if (pa->header.length < sizeof(struct acpi_srat_its_affinity)) {
> + pr_err("SRAT: Invalid SRAT header length: %d\n",
> + pa->header.length);
> + bad_srat();
> + return;
> + }
> +
> + if (its_in_srat >= MAX_NUMNODES) {
I do not understand why maxcount == MAX_NUMNODES in the first place.
> + pr_warn_once("SRAT: its count exceeding max count[%d]\n",
> + MAX_NUMNODES);
> + return;
> + }
> +
> + pxm = pa->proximity_domain;
> + node = acpi_map_pxm_to_node(pxm);
> +
> + if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> + pr_err("SRAT: Too many proximity domains %d\n", pxm);
Wrong error log message or at least inconsistent (yes, GICC code
where you copied this code from needs patching too).
> + bad_srat();
> + return;
> + }
> +
> + early_node_its_id[its_in_srat].node_id = node;
> + early_node_its_id[its_in_srat].its_id = its_in_srat;
^
One space is enough (again - that's through for GICC code too).
> + node_set(node, numa_nodes_parsed);
> + pr_info("SRAT: PXM %d -> ITS %d -> Node %d\n",
> + pxm, its_in_srat, node);
> + its_in_srat++;
> +}
> +
> int __init arm64_acpi_numa_init(void)
> {
> int ret;
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index edb0c79..9c04dba 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -182,6 +182,16 @@ int acpi_map_pxm_to_online_node(int pxm)
> }
> break;
>
> + case ACPI_SRAT_TYPE_GIC_ITS_AFFINITY:
> + {
> + struct acpi_srat_its_affinity *p =
> + (struct acpi_srat_its_affinity *)header;
> + pr_debug("SRAT ITS [0x%04x]) in proximity domain %d\n",
> + p->its_id,
> + p->proximity_domain);
> + }
> + break;
> +
> default:
> pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
> header->type);
> @@ -390,6 +400,24 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
> return 0;
> }
>
> +static int __init
> +acpi_parse_its_affinity(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct acpi_srat_its_affinity *its_affinity;
> +
> + its_affinity = (struct acpi_srat_its_affinity *)header;
> + if (!its_affinity)
> + return -EINVAL;
> +
> + acpi_table_print_srat_entry(header);
> +
> + /* let architecture-dependent part to do it */
> + acpi_numa_its_affinity_init(its_affinity);
> +
> + return 0;
> +}
> +
> static int __initdata parsed_numa_memblks;
>
> static int __init
> @@ -445,7 +473,7 @@ int __init acpi_numa_init(void)
>
> /* SRAT: Static Resource Affinity Table */
> if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
> - struct acpi_subtable_proc srat_proc[3];
> + struct acpi_subtable_proc srat_proc[4];
>
> memset(srat_proc, 0, sizeof(srat_proc));
> srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
> @@ -454,6 +482,8 @@ int __init acpi_numa_init(void)
> srat_proc[1].handler = acpi_parse_x2apic_affinity;
> srat_proc[2].id = ACPI_SRAT_TYPE_GICC_AFFINITY;
> srat_proc[2].handler = acpi_parse_gicc_affinity;
> + srat_proc[3].id = ACPI_SRAT_TYPE_GIC_ITS_AFFINITY;
> + srat_proc[3].handler = acpi_parse_its_affinity;
>
> acpi_table_parse_entries_array(ACPI_SIG_SRAT,
> sizeof(struct acpi_table_srat),
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 45ea1933..84936da 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1861,7 +1861,8 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> goto dom_err;
> }
>
> - err = its_probe_one(&res, dom_handle, NUMA_NO_NODE);
> + err = its_probe_one(&res, dom_handle,
> + acpi_numa_get_its_nid(its_entry->translation_id));
If that's the only usage I wonder whether we really need all arm64
arch code/data, instead of parsing the SRAT in ITS code driver straight
away at probe, retrieve its node and be done with this.
I understand you replicated what x86/GICC does with APIC code, I would
like to understand though if we see a reason why (or better, why we keep
the relevant stashed data in arch/arm64 instead of the ITS driver).
Thanks,
Lorenzo
> if (!err)
> return 0;
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 137e4a3..e5e8ae3 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -260,9 +260,12 @@ int acpi_table_parse_madt(enum acpi_madt_type id,
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> +void acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> +static inline void
> +acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: linux-acpi@vger.kernel.org, devel@acpica.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, lv.zheng@intel.com,
robert.moore@intel.com, catalin.marinas@arm.com,
will.deacon@arm.com, tglx@linutronix.de, jason@lakedaemon.net,
marc.zyngier@arm.com, jnair@caviumnetworks.com,
gpkulkarni@gmail.com
Subject: Re: [PATCH 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units
Date: Mon, 12 Jun 2017 11:53:35 +0100 [thread overview]
Message-ID: <20170612105335.GA30594@red-moon> (raw)
In-Reply-To: <1496753787-27735-3-git-send-email-ganapatrao.kulkarni@cavium.com>
On Tue, Jun 06, 2017 at 06:26:27PM +0530, Ganapatrao Kulkarni wrote:
> Add code to parse SRAT ITS Affinity sub table as defined in ACPI 6.2
> Later in driver probe, its devices are mapped to numa node
Nit: Capitalize "its".
> using its id to proximity domain mapping.
Ditto.
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
> arch/arm64/include/asm/acpi.h | 2 ++
> arch/arm64/kernel/acpi_numa.c | 59 ++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/numa.c | 32 +++++++++++++++++++++-
> drivers/irqchip/irq-gic-v3-its.c | 3 +-
> include/linux/acpi.h | 3 ++
> 5 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 0e99978..60ea7d1 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -143,9 +143,11 @@ static inline void arch_apei_flush_tlb_one(unsigned long addr)
> #ifdef CONFIG_ACPI_NUMA
> int arm64_acpi_numa_init(void);
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid);
> +int acpi_numa_get_its_nid(u32 its_id);
> #else
> static inline int arm64_acpi_numa_init(void) { return -ENOSYS; }
> static inline int acpi_numa_get_nid(unsigned int cpu, u64 hwid) { return NUMA_NO_NODE; }
> +static inline int acpi_numa_get_its_nid(u32 its_id) { return NUMA_NO_NODE; }
> #endif /* CONFIG_ACPI_NUMA */
>
> #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
> index f01fab6..f7b2ea6 100644
> --- a/arch/arm64/kernel/acpi_numa.c
> +++ b/arch/arm64/kernel/acpi_numa.c
> @@ -29,15 +29,24 @@
> #include <asm/numa.h>
>
> static int cpus_in_srat;
> +static int its_in_srat;
>
> struct __node_cpu_hwid {
> u32 node_id; /* logical node containing this CPU */
> u64 cpu_hwid; /* MPIDR for this CPU */
> };
>
> +struct __node_its_id {
> + u32 node_id; /* numa node id */
> + u32 its_id ; /* GIC ITS ID */
^
Remove space.
> +};
> +
> static struct __node_cpu_hwid early_node_cpu_hwid[NR_CPUS] = {
> [0 ... NR_CPUS - 1] = {NUMA_NO_NODE, PHYS_CPUID_INVALID} };
>
> +static struct __node_its_id early_node_its_id[MAX_NUMNODES] = {
Mind explaining to us where does MAX_NUMNODES as an array size limit
stem from here ?
> +[0 ... MAX_NUMNODES - 1] = {NUMA_NO_NODE, UINT_MAX} };
> +
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> {
> int i;
> @@ -50,6 +59,18 @@ int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> return NUMA_NO_NODE;
> }
>
> +int acpi_numa_get_its_nid(u32 its_id)
> +{
> + int i;
> +
> + for (i = 0; i < its_in_srat; i++) {
> + if (its_id == early_node_its_id[i].its_id)
> + return early_node_its_id[i].node_id;
> + }
> +
> + return NUMA_NO_NODE;
> +}
> +
> /* Callback for Proximity Domain -> ACPI processor UID mapping */
> void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> {
> @@ -100,6 +121,44 @@ void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> pxm, mpidr, node);
> }
>
> +/* Callback for ITS ACPI Proximity Domain mapping */
> +void __init acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa)
> +{
> + int pxm, node;
> +
> + if (srat_disabled())
> + return;
> +
> + if (pa->header.length < sizeof(struct acpi_srat_its_affinity)) {
> + pr_err("SRAT: Invalid SRAT header length: %d\n",
> + pa->header.length);
> + bad_srat();
> + return;
> + }
> +
> + if (its_in_srat >= MAX_NUMNODES) {
I do not understand why maxcount == MAX_NUMNODES in the first place.
> + pr_warn_once("SRAT: its count exceeding max count[%d]\n",
> + MAX_NUMNODES);
> + return;
> + }
> +
> + pxm = pa->proximity_domain;
> + node = acpi_map_pxm_to_node(pxm);
> +
> + if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> + pr_err("SRAT: Too many proximity domains %d\n", pxm);
Wrong error log message or at least inconsistent (yes, GICC code
where you copied this code from needs patching too).
> + bad_srat();
> + return;
> + }
> +
> + early_node_its_id[its_in_srat].node_id = node;
> + early_node_its_id[its_in_srat].its_id = its_in_srat;
^
One space is enough (again - that's through for GICC code too).
> + node_set(node, numa_nodes_parsed);
> + pr_info("SRAT: PXM %d -> ITS %d -> Node %d\n",
> + pxm, its_in_srat, node);
> + its_in_srat++;
> +}
> +
> int __init arm64_acpi_numa_init(void)
> {
> int ret;
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index edb0c79..9c04dba 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -182,6 +182,16 @@ int acpi_map_pxm_to_online_node(int pxm)
> }
> break;
>
> + case ACPI_SRAT_TYPE_GIC_ITS_AFFINITY:
> + {
> + struct acpi_srat_its_affinity *p =
> + (struct acpi_srat_its_affinity *)header;
> + pr_debug("SRAT ITS [0x%04x]) in proximity domain %d\n",
> + p->its_id,
> + p->proximity_domain);
> + }
> + break;
> +
> default:
> pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
> header->type);
> @@ -390,6 +400,24 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
> return 0;
> }
>
> +static int __init
> +acpi_parse_its_affinity(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct acpi_srat_its_affinity *its_affinity;
> +
> + its_affinity = (struct acpi_srat_its_affinity *)header;
> + if (!its_affinity)
> + return -EINVAL;
> +
> + acpi_table_print_srat_entry(header);
> +
> + /* let architecture-dependent part to do it */
> + acpi_numa_its_affinity_init(its_affinity);
> +
> + return 0;
> +}
> +
> static int __initdata parsed_numa_memblks;
>
> static int __init
> @@ -445,7 +473,7 @@ int __init acpi_numa_init(void)
>
> /* SRAT: Static Resource Affinity Table */
> if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
> - struct acpi_subtable_proc srat_proc[3];
> + struct acpi_subtable_proc srat_proc[4];
>
> memset(srat_proc, 0, sizeof(srat_proc));
> srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
> @@ -454,6 +482,8 @@ int __init acpi_numa_init(void)
> srat_proc[1].handler = acpi_parse_x2apic_affinity;
> srat_proc[2].id = ACPI_SRAT_TYPE_GICC_AFFINITY;
> srat_proc[2].handler = acpi_parse_gicc_affinity;
> + srat_proc[3].id = ACPI_SRAT_TYPE_GIC_ITS_AFFINITY;
> + srat_proc[3].handler = acpi_parse_its_affinity;
>
> acpi_table_parse_entries_array(ACPI_SIG_SRAT,
> sizeof(struct acpi_table_srat),
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 45ea1933..84936da 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1861,7 +1861,8 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> goto dom_err;
> }
>
> - err = its_probe_one(&res, dom_handle, NUMA_NO_NODE);
> + err = its_probe_one(&res, dom_handle,
> + acpi_numa_get_its_nid(its_entry->translation_id));
If that's the only usage I wonder whether we really need all arm64
arch code/data, instead of parsing the SRAT in ITS code driver straight
away at probe, retrieve its node and be done with this.
I understand you replicated what x86/GICC does with APIC code, I would
like to understand though if we see a reason why (or better, why we keep
the relevant stashed data in arch/arm64 instead of the ITS driver).
Thanks,
Lorenzo
> if (!err)
> return 0;
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 137e4a3..e5e8ae3 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -260,9 +260,12 @@ int acpi_table_parse_madt(enum acpi_madt_type id,
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> +void acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> +static inline void
> +acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units
Date: Mon, 12 Jun 2017 11:53:35 +0100 [thread overview]
Message-ID: <20170612105335.GA30594@red-moon> (raw)
In-Reply-To: <1496753787-27735-3-git-send-email-ganapatrao.kulkarni@cavium.com>
On Tue, Jun 06, 2017 at 06:26:27PM +0530, Ganapatrao Kulkarni wrote:
> Add code to parse SRAT ITS Affinity sub table as defined in ACPI 6.2
> Later in driver probe, its devices are mapped to numa node
Nit: Capitalize "its".
> using its id to proximity domain mapping.
Ditto.
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
> arch/arm64/include/asm/acpi.h | 2 ++
> arch/arm64/kernel/acpi_numa.c | 59 ++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/numa.c | 32 +++++++++++++++++++++-
> drivers/irqchip/irq-gic-v3-its.c | 3 +-
> include/linux/acpi.h | 3 ++
> 5 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 0e99978..60ea7d1 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -143,9 +143,11 @@ static inline void arch_apei_flush_tlb_one(unsigned long addr)
> #ifdef CONFIG_ACPI_NUMA
> int arm64_acpi_numa_init(void);
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid);
> +int acpi_numa_get_its_nid(u32 its_id);
> #else
> static inline int arm64_acpi_numa_init(void) { return -ENOSYS; }
> static inline int acpi_numa_get_nid(unsigned int cpu, u64 hwid) { return NUMA_NO_NODE; }
> +static inline int acpi_numa_get_its_nid(u32 its_id) { return NUMA_NO_NODE; }
> #endif /* CONFIG_ACPI_NUMA */
>
> #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
> index f01fab6..f7b2ea6 100644
> --- a/arch/arm64/kernel/acpi_numa.c
> +++ b/arch/arm64/kernel/acpi_numa.c
> @@ -29,15 +29,24 @@
> #include <asm/numa.h>
>
> static int cpus_in_srat;
> +static int its_in_srat;
>
> struct __node_cpu_hwid {
> u32 node_id; /* logical node containing this CPU */
> u64 cpu_hwid; /* MPIDR for this CPU */
> };
>
> +struct __node_its_id {
> + u32 node_id; /* numa node id */
> + u32 its_id ; /* GIC ITS ID */
^
Remove space.
> +};
> +
> static struct __node_cpu_hwid early_node_cpu_hwid[NR_CPUS] = {
> [0 ... NR_CPUS - 1] = {NUMA_NO_NODE, PHYS_CPUID_INVALID} };
>
> +static struct __node_its_id early_node_its_id[MAX_NUMNODES] = {
Mind explaining to us where does MAX_NUMNODES as an array size limit
stem from here ?
> +[0 ... MAX_NUMNODES - 1] = {NUMA_NO_NODE, UINT_MAX} };
> +
> int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> {
> int i;
> @@ -50,6 +59,18 @@ int acpi_numa_get_nid(unsigned int cpu, u64 hwid)
> return NUMA_NO_NODE;
> }
>
> +int acpi_numa_get_its_nid(u32 its_id)
> +{
> + int i;
> +
> + for (i = 0; i < its_in_srat; i++) {
> + if (its_id == early_node_its_id[i].its_id)
> + return early_node_its_id[i].node_id;
> + }
> +
> + return NUMA_NO_NODE;
> +}
> +
> /* Callback for Proximity Domain -> ACPI processor UID mapping */
> void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> {
> @@ -100,6 +121,44 @@ void __init acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa)
> pxm, mpidr, node);
> }
>
> +/* Callback for ITS ACPI Proximity Domain mapping */
> +void __init acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa)
> +{
> + int pxm, node;
> +
> + if (srat_disabled())
> + return;
> +
> + if (pa->header.length < sizeof(struct acpi_srat_its_affinity)) {
> + pr_err("SRAT: Invalid SRAT header length: %d\n",
> + pa->header.length);
> + bad_srat();
> + return;
> + }
> +
> + if (its_in_srat >= MAX_NUMNODES) {
I do not understand why maxcount == MAX_NUMNODES in the first place.
> + pr_warn_once("SRAT: its count exceeding max count[%d]\n",
> + MAX_NUMNODES);
> + return;
> + }
> +
> + pxm = pa->proximity_domain;
> + node = acpi_map_pxm_to_node(pxm);
> +
> + if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> + pr_err("SRAT: Too many proximity domains %d\n", pxm);
Wrong error log message or at least inconsistent (yes, GICC code
where you copied this code from needs patching too).
> + bad_srat();
> + return;
> + }
> +
> + early_node_its_id[its_in_srat].node_id = node;
> + early_node_its_id[its_in_srat].its_id = its_in_srat;
^
One space is enough (again - that's through for GICC code too).
> + node_set(node, numa_nodes_parsed);
> + pr_info("SRAT: PXM %d -> ITS %d -> Node %d\n",
> + pxm, its_in_srat, node);
> + its_in_srat++;
> +}
> +
> int __init arm64_acpi_numa_init(void)
> {
> int ret;
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index edb0c79..9c04dba 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -182,6 +182,16 @@ int acpi_map_pxm_to_online_node(int pxm)
> }
> break;
>
> + case ACPI_SRAT_TYPE_GIC_ITS_AFFINITY:
> + {
> + struct acpi_srat_its_affinity *p =
> + (struct acpi_srat_its_affinity *)header;
> + pr_debug("SRAT ITS [0x%04x]) in proximity domain %d\n",
> + p->its_id,
> + p->proximity_domain);
> + }
> + break;
> +
> default:
> pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
> header->type);
> @@ -390,6 +400,24 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
> return 0;
> }
>
> +static int __init
> +acpi_parse_its_affinity(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct acpi_srat_its_affinity *its_affinity;
> +
> + its_affinity = (struct acpi_srat_its_affinity *)header;
> + if (!its_affinity)
> + return -EINVAL;
> +
> + acpi_table_print_srat_entry(header);
> +
> + /* let architecture-dependent part to do it */
> + acpi_numa_its_affinity_init(its_affinity);
> +
> + return 0;
> +}
> +
> static int __initdata parsed_numa_memblks;
>
> static int __init
> @@ -445,7 +473,7 @@ int __init acpi_numa_init(void)
>
> /* SRAT: Static Resource Affinity Table */
> if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
> - struct acpi_subtable_proc srat_proc[3];
> + struct acpi_subtable_proc srat_proc[4];
>
> memset(srat_proc, 0, sizeof(srat_proc));
> srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
> @@ -454,6 +482,8 @@ int __init acpi_numa_init(void)
> srat_proc[1].handler = acpi_parse_x2apic_affinity;
> srat_proc[2].id = ACPI_SRAT_TYPE_GICC_AFFINITY;
> srat_proc[2].handler = acpi_parse_gicc_affinity;
> + srat_proc[3].id = ACPI_SRAT_TYPE_GIC_ITS_AFFINITY;
> + srat_proc[3].handler = acpi_parse_its_affinity;
>
> acpi_table_parse_entries_array(ACPI_SIG_SRAT,
> sizeof(struct acpi_table_srat),
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 45ea1933..84936da 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1861,7 +1861,8 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> goto dom_err;
> }
>
> - err = its_probe_one(&res, dom_handle, NUMA_NO_NODE);
> + err = its_probe_one(&res, dom_handle,
> + acpi_numa_get_its_nid(its_entry->translation_id));
If that's the only usage I wonder whether we really need all arm64
arch code/data, instead of parsing the SRAT in ITS code driver straight
away at probe, retrieve its node and be done with this.
I understand you replicated what x86/GICC does with APIC code, I would
like to understand though if we see a reason why (or better, why we keep
the relevant stashed data in arch/arm64 instead of the ITS driver).
Thanks,
Lorenzo
> if (!err)
> return 0;
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 137e4a3..e5e8ae3 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -260,9 +260,12 @@ int acpi_table_parse_madt(enum acpi_madt_type id,
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> +void acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> +static inline void
> +acpi_numa_its_affinity_init(struct acpi_srat_its_affinity *pa) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev reply other threads:[~2017-06-12 10:53 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-06 12:56 [PATCH 0/2] acpi, gicv3-its, numa: Adding numa node mapping for its Ganapatrao Kulkarni
2017-06-06 12:56 ` Ganapatrao Kulkarni
2017-06-06 12:56 ` [PATCH 1/2] ACPICA: ACPI 6.2: Add support for new SRAT subtable Ganapatrao Kulkarni
2017-06-06 12:56 ` Ganapatrao Kulkarni
2017-06-06 12:56 ` [PATCH 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units Ganapatrao Kulkarni
2017-06-06 12:56 ` Ganapatrao Kulkarni
2017-06-12 10:53 ` Lorenzo Pieralisi [this message]
2017-06-12 10:53 ` Lorenzo Pieralisi
2017-06-12 10:53 ` Lorenzo Pieralisi
2017-06-14 9:53 ` [Devel] " Ganapatrao Kulkarni
2017-06-14 9:53 ` Ganapatrao Kulkarni
2017-06-14 9:53 ` Ganapatrao Kulkarni
2017-06-15 10:35 ` [Devel] " Lorenzo Pieralisi
2017-06-15 10:35 ` Lorenzo Pieralisi
2017-06-15 10:35 ` Lorenzo Pieralisi
2017-06-15 10:43 ` Marc Zyngier
2017-06-15 10:43 ` Marc Zyngier
-- strict thread matches above, loose matches on Subject: below --
2017-06-07 3:39 [Devel] " Ganapatrao Kulkarni
2017-06-07 3:39 ` Ganapatrao Kulkarni
2017-06-07 3:39 ` Ganapatrao Kulkarni
2017-06-15 10:55 [Devel] " Ganapatrao Kulkarni
2017-06-15 10:55 ` Ganapatrao Kulkarni
2017-06-15 10:55 ` Ganapatrao Kulkarni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170612105335.GA30594@red-moon \
--to=devel@acpica.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.