From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier Subject: Re: [PATCH v3 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units Date: Wed, 21 Jun 2017 09:58:21 +0100 Message-ID: <89b8b97d-1f4c-4938-9932-b2d637797e14@arm.com> References: <1498025743-6340-1-git-send-email-ganapatrao.kulkarni@cavium.com> <1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from foss.arm.com ([217.140.101.70]:48884 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752310AbdFUI6Z (ORCPT ); Wed, 21 Jun 2017 04:58:25 -0400 In-Reply-To: <1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Ganapatrao Kulkarni , linux-acpi@vger.kernel.org, devel@acpica.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: lv.zheng@intel.com, robert.moore@intel.com, catalin.marinas@arm.com, will.deacon@arm.com, lorenzo.pieralisi@arm.com, hanjun.guo@linaro.org, tglx@linutronix.de, jason@lakedaemon.net, jnair@caviumnetworks.com, gpkulkarni@gmail.com On 21/06/17 07:15, 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} }; > + > +static int its_in_srat __initdata; > + > +static int __init > +acpi_get_its_numa_node(u32 its_id) Please keep these on the same line. > +{ > + 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) Same remark. > +{ > + 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)) { Same thing. > + 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; > + } So if you find an entry that doesn't match the current kernel configuration, you drop all the subsequent entries? That doesn't feel right. > + > + 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); If you don't check the return value, there is no point returning it. > +} > +#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); > } > Thanks, M. -- Jazz is not dead. It just smells funny... From mboxrd@z Thu Jan 1 00:00:00 1970 From: marc.zyngier@arm.com (Marc Zyngier) Date: Wed, 21 Jun 2017 09:58:21 +0100 Subject: [PATCH v3 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units In-Reply-To: <1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com> References: <1498025743-6340-1-git-send-email-ganapatrao.kulkarni@cavium.com> <1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com> Message-ID: <89b8b97d-1f4c-4938-9932-b2d637797e14@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 21/06/17 07:15, 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} }; > + > +static int its_in_srat __initdata; > + > +static int __init > +acpi_get_its_numa_node(u32 its_id) Please keep these on the same line. > +{ > + 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) Same remark. > +{ > + 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)) { Same thing. > + 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; > + } So if you find an entry that doesn't match the current kernel configuration, you drop all the subsequent entries? That doesn't feel right. > + > + 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); If you don't check the return value, there is no point returning it. > +} > +#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); > } > Thanks, M. -- Jazz is not dead. It just smells funny... From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============1868681623842318576==" MIME-Version: 1.0 From: Marc Zyngier Subject: Re: [Devel] [PATCH v3 2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units Date: Wed, 21 Jun 2017 09:58:21 +0100 Message-ID: <89b8b97d-1f4c-4938-9932-b2d637797e14@arm.com> In-Reply-To: 1498025743-6340-3-git-send-email-ganapatrao.kulkarni@cavium.com List-ID: To: devel@acpica.org --===============1868681623842318576== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On 21/06/17 07:15, 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-v= 3-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 =3D { > + [0 ... MAX_NUMNODES - 1] =3D {NUMA_NO_NODE, UINT_MAX} }; > + > +static int its_in_srat __initdata; > + > +static int __init > +acpi_get_its_numa_node(u32 its_id) Please keep these on the same line. > +{ > + int i; > + > + for (i =3D 0; i < its_in_srat; i++) { > + if (its_id =3D=3D 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) Same remark. > +{ > + int pxm, node; > + struct acpi_srat_its_affinity *its_affinity; > + > + its_affinity =3D (struct acpi_srat_its_affinity *)header; > + if (!its_affinity) > + return -EINVAL; > + > + if (its_affinity->header.length < > + sizeof(struct acpi_srat_its_affinity)) { Same thing. > + pr_err("SRAT:ITS: Invalid SRAT header length: %d\n", > + its_affinity->header.length); > + return -EINVAL; > + } > + > + if (its_in_srat >=3D MAX_NUMNODES) { > + pr_err("SRAT:ITS: ITS devices exceeding max count[%d]\n", > + MAX_NUMNODES); > + return -EINVAL; > + } > + > + pxm =3D its_affinity->proximity_domain; > + node =3D acpi_map_pxm_to_node(pxm); > + > + if (node =3D=3D NUMA_NO_NODE || node >=3D MAX_NUMNODES) { > + pr_err("SRAT:ITS: Invalid numa node %d\n", node); > + return -EINVAL; > + } So if you find an entry that doesn't match the current kernel configuration, you drop all the subsequent entries? That doesn't feel right. > + > + its_srat_maps[its_in_srat].numa_node =3D node; > + its_srat_maps[its_in_srat].its_id =3D 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); If you don't check the return value, there is no point returning it. > +} > +#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 *h= eader, > const unsigned long end) > { > @@ -1861,7 +1937,8 @@ static int __init gic_acpi_parse_madt_its(struct ac= pi_subtable_header *header, > goto dom_err; > } > = > - err =3D its_probe_one(&res, dom_handle, NUMA_NO_NODE); > + err =3D 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 ac= pi_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); > } > = Thanks, M. -- = Jazz is not dead. It just smells funny... --===============1868681623842318576==--