Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752665AbdFUKGY (ORCPT ); Wed, 21 Jun 2017 06:06:24 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:35341 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751971AbdFUKGW (ORCPT ); Wed, 21 Jun 2017 06:06:22 -0400 MIME-Version: 1.0 In-Reply-To: <20170621092853.GA31599@red-moon> References: <1498025743-6340-1-git-send-email-ganapatrao.kulkarni@cavium.com> <1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com> <20170621092853.GA31599@red-moon> From: Ganapatrao Kulkarni Date: Wed, 21 Jun 2017 15:36:21 +0530 Message-ID: Subject: Re: [PATCH v3 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units To: Lorenzo Pieralisi Cc: Ganapatrao Kulkarni , linux-acpi@vger.kernel.org, devel@acpica.org, "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Lv Zheng , Robert Moore , Marc Zyngier , Catalin Marinas , Will Deacon , Hanjun Guo , "tglx@linutronix.de" , Jason Cooper , Jayachandran C Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4806 Lines: 140 On Wed, Jun 21, 2017 at 2:58 PM, Lorenzo Pieralisi wrote: > On Wed, Jun 21, 2017 at 11:45:43AM +0530, Ganapatrao Kulkarni wrote: >> Add code to parse SRAT ITS Affinity sub table as defined in ACPI 6.2 >> Later in per device probe, ITS devices are mapped to >> numa node using ITS id to proximity domain mapping. >> >> Signed-off-by: Ganapatrao Kulkarni >> --- >> drivers/irqchip/irq-gic-v3-its.c | 80 +++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 79 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 45ea1933..88cfb32 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -1833,6 +1833,82 @@ static int __init its_of_probe(struct device_node *node) >> >> #define ACPI_GICV3_ITS_MEM_SIZE (SZ_128K) >> >> +#ifdef CONFIG_ACPI_NUMA >> +struct its_srat_map { >> + u32 numa_node; /* numa node id */ >> + u32 its_id; /* GIC ITS ID */ >> +}; >> + >> +static struct its_srat_map its_srat_maps[MAX_NUMNODES] __initdata = { >> + [0 ... MAX_NUMNODES - 1] = {NUMA_NO_NODE, UINT_MAX} }; > > Nit: UINT_MAX is as valid as 0 as its_id (but the intent here is to make > it look like an invalid value), given that the its_in_srat counter > defines what entries are valid I do not necessarily see the point in > initializing with something that is a valid value != 0. ok, this array may not need any init, > >> +static int its_in_srat __initdata; >> + >> +static int __init >> +acpi_get_its_numa_node(u32 its_id) >> +{ >> + int i; >> + >> + for (i = 0; i < its_in_srat; i++) { >> + if (its_id == its_srat_maps[i].its_id) >> + return its_srat_maps[i].numa_node; >> + } >> + return NUMA_NO_NODE; >> +} >> + >> +static int __init >> +gic_acpi_parse_srat_its(struct acpi_subtable_header *header, >> + const unsigned long end) >> +{ >> + int pxm, node; >> + struct acpi_srat_its_affinity *its_affinity; >> + >> + its_affinity = (struct acpi_srat_its_affinity *)header; >> + if (!its_affinity) >> + return -EINVAL; >> + >> + if (its_affinity->header.length < >> + sizeof(struct acpi_srat_its_affinity)) { >> + pr_err("SRAT:ITS: Invalid SRAT header length: %d\n", >> + its_affinity->header.length); >> + return -EINVAL; >> + } >> + >> + if (its_in_srat >= MAX_NUMNODES) { >> + pr_err("SRAT:ITS: ITS devices exceeding max count[%d]\n", >> + MAX_NUMNODES); >> + return -EINVAL; >> + } >> + >> + pxm = its_affinity->proximity_domain; >> + node = acpi_map_pxm_to_node(pxm); >> + >> + if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) { >> + pr_err("SRAT:ITS: Invalid numa node %d\n", node); >> + return -EINVAL; > > As Marc mentioned you should bail out here but continue parsing entries, > aka "return 0"; agreed. > > Thanks, > Lorenzo > >> + } >> + >> + its_srat_maps[its_in_srat].numa_node = node; >> + its_srat_maps[its_in_srat].its_id = its_affinity->its_id; >> + its_in_srat++; >> + pr_info("ACPI: NUMA: SRAT: ITS: PXM %d -> ITS_ID %d -> NODE %d\n", >> + pxm, its_affinity->its_id, node); >> + >> + return 0; >> +} >> + >> +static int __init acpi_table_parse_srat_its(void) >> +{ >> + return acpi_table_parse_entries(ACPI_SIG_SRAT, >> + sizeof(struct acpi_table_srat), >> + ACPI_SRAT_TYPE_GIC_ITS_AFFINITY, >> + gic_acpi_parse_srat_its, 0); >> +} >> +#else >> +#define acpi_table_parse_srat_its() do { } while (0) >> +#define acpi_get_its_numa_node(its_id) NUMA_NO_NODE >> +#endif >> + >> static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header, >> const unsigned long end) >> { >> @@ -1861,7 +1937,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_get_its_numa_node(its_entry->translation_id)); >> if (!err) >> return 0; >> >> @@ -1873,6 +1950,7 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header, >> >> static void __init its_acpi_probe(void) >> { >> + acpi_table_parse_srat_its(); >> acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR, >> gic_acpi_parse_madt_its, 0); >> } >> -- >> 1.8.1.4 >> thanks Ganapat