diff mbox

[v2,2/2] acpi, gicv3-its, numa: Adding numa node mapping for gic-its units

Message ID 1497942437-1390-3-git-send-email-ganapatrao.kulkarni@cavium.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kulkarni, Ganapatrao June 20, 2017, 7:07 a.m. UTC
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 <ganapatrao.kulkarni@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 76 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

Comments

Lorenzo Pieralisi June 20, 2017, 3:17 p.m. UTC | #1
Hi Ganapatrao,

On Tue, Jun 20, 2017 at 12:37:17PM +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 <ganapatrao.kulkarni@cavium.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 76 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 45ea1933..5865a75 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1833,6 +1833,78 @@ 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 */
> +	struct list_head	entry;
> +};
> +
> +static LIST_HEAD(its_srat_maps);
> +
> +static int acpi_get_its_numa_node(u32 its_id)
> +{
> +	struct its_srat_map *srat_map;
> +
> +	list_for_each_entry(srat_map, &its_srat_maps, entry) {
> +		if (its_id == srat_map->its_id)
> +			return srat_map->numa_node;
> +	}
> +	return NUMA_NO_NODE;
> +}

I do not want to come across as pedantic and I understand that the API
to parse static ACPI tables sucks (ie in theory you could avoid stashing
nodes altogether and just use a global variable to parse an ITS ID at
a time but it sucks to go through the SRAT multiple times) but would not
it be easier to just declare an __initdata static array and counter and
be done with this ? We do not need this stuff after boot so I do not see
a point in keeping it in memory and in allocating the nodes dynamically.

Apologies if I was not clear in my comments to the previous set, I did
not like stashing ITS PXM values in arch/arm64 code but I understand you
need an array (__initdata) to avoid reading the SRAT multiple times.

Thanks,
Lorenzo

> +
> +static int __init
> +gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
> +			 const unsigned long end)
> +{
> +	int pxm, node;
> +	struct its_srat_map *srat_map;
> +	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;
> +	}
> +
> +	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;
> +	}
> +
> +	srat_map = kzalloc(sizeof(*srat_map), GFP_KERNEL);
> +	if (!srat_map)
> +		return -ENOMEM;
> +
> +	srat_map->numa_node = node;
> +	srat_map->its_id = its_affinity->its_id;
> +	list_add(&srat_map->entry, &its_srat_maps);
> +	pr_info("ACPI: NUMA: SRAT: ITS: PXM %d -> ITS_ID %d -> NODE %d\n",
> +		srat_map->its_id, pxm, 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(void) { }
> +#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 +1933,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 +1946,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
>
Ganapatrao Kulkarni June 20, 2017, 5:17 p.m. UTC | #2
Hi Lorenzo,

On Tue, Jun 20, 2017 at 8:47 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Hi Ganapatrao,
>
> On Tue, Jun 20, 2017 at 12:37:17PM +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 <ganapatrao.kulkarni@cavium.com>
>> ---
>>  drivers/irqchip/irq-gic-v3-its.c | 76 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 75 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 45ea1933..5865a75 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -1833,6 +1833,78 @@ 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 */
>> +     struct list_head        entry;
>> +};
>> +
>> +static LIST_HEAD(its_srat_maps);
>> +
>> +static int acpi_get_its_numa_node(u32 its_id)
>> +{
>> +     struct its_srat_map *srat_map;
>> +
>> +     list_for_each_entry(srat_map, &its_srat_maps, entry) {
>> +             if (its_id == srat_map->its_id)
>> +                     return srat_map->numa_node;
>> +     }
>> +     return NUMA_NO_NODE;
>> +}
>
> I do not want to come across as pedantic and I understand that the API
> to parse static ACPI tables sucks (ie in theory you could avoid stashing
> nodes altogether and just use a global variable to parse an ITS ID at
> a time but it sucks to go through the SRAT multiple times) but would not
> it be easier to just declare an __initdata static array and counter and
> be done with this ? We do not need this stuff after boot so I do not see
> a point in keeping it in memory and in allocating the nodes dynamically.

yes, it was my initial thought (as done v1), and i have replaced to
dynamic array in this series. I might have misunderstood your comments.
if you have no objection for the static array of size MAX_NUMNODES
as done v1 [1], i can send v3 with that change?

[1] https://patchwork.kernel.org/patch/9768869/

>
> Apologies if I was not clear in my comments to the previous set, I did
> not like stashing ITS PXM values in arch/arm64 code but I understand you
> need an array (__initdata) to avoid reading the SRAT multiple times.

yes, having static array to hold parsed table is required to avoid
multiple iterations
of parsing functions.
This array can go in __initdata.

>
> Thanks,
> Lorenzo
>
>> +
>> +static int __init
>> +gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
>> +                      const unsigned long end)
>> +{
>> +     int pxm, node;
>> +     struct its_srat_map *srat_map;
>> +     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;
>> +     }
>> +
>> +     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;
>> +     }
>> +
>> +     srat_map = kzalloc(sizeof(*srat_map), GFP_KERNEL);
>> +     if (!srat_map)
>> +             return -ENOMEM;
>> +
>> +     srat_map->numa_node = node;
>> +     srat_map->its_id = its_affinity->its_id;
>> +     list_add(&srat_map->entry, &its_srat_maps);
>> +     pr_info("ACPI: NUMA: SRAT: ITS: PXM %d -> ITS_ID %d -> NODE %d\n",
>> +             srat_map->its_id, pxm, 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(void) { }
>> +#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 +1933,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 +1946,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
Lorenzo Pieralisi June 20, 2017, 5:30 p.m. UTC | #3
On Tue, Jun 20, 2017 at 10:47:24PM +0530, Ganapatrao Kulkarni wrote:
> Hi Lorenzo,
> 
> On Tue, Jun 20, 2017 at 8:47 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > Hi Ganapatrao,
> >
> > On Tue, Jun 20, 2017 at 12:37:17PM +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 <ganapatrao.kulkarni@cavium.com>
> >> ---
> >>  drivers/irqchip/irq-gic-v3-its.c | 76 +++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 75 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> >> index 45ea1933..5865a75 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -1833,6 +1833,78 @@ 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 */
> >> +     struct list_head        entry;
> >> +};
> >> +
> >> +static LIST_HEAD(its_srat_maps);
> >> +
> >> +static int acpi_get_its_numa_node(u32 its_id)
> >> +{
> >> +     struct its_srat_map *srat_map;
> >> +
> >> +     list_for_each_entry(srat_map, &its_srat_maps, entry) {
> >> +             if (its_id == srat_map->its_id)
> >> +                     return srat_map->numa_node;
> >> +     }
> >> +     return NUMA_NO_NODE;
> >> +}
> >
> > I do not want to come across as pedantic and I understand that the API
> > to parse static ACPI tables sucks (ie in theory you could avoid stashing
> > nodes altogether and just use a global variable to parse an ITS ID at
> > a time but it sucks to go through the SRAT multiple times) but would not
> > it be easier to just declare an __initdata static array and counter and
> > be done with this ? We do not need this stuff after boot so I do not see
> > a point in keeping it in memory and in allocating the nodes dynamically.
> 
> yes, it was my initial thought (as done v1), and i have replaced to
> dynamic array in this series. I might have misunderstood your comments.
> if you have no objection for the static array of size MAX_NUMNODES
> as done v1 [1], i can send v3 with that change?
> 
> [1] https://patchwork.kernel.org/patch/9768869/
>
It is ok by me sorry for delaying you further.

> > Apologies if I was not clear in my comments to the previous set, I did
> > not like stashing ITS PXM values in arch/arm64 code but I understand you
> > need an array (__initdata) to avoid reading the SRAT multiple times.
> 
> yes, having static array to hold parsed table is required to avoid
> multiple iterations
> of parsing functions.
> This array can go in __initdata.

Agreed, thanks.

Lorenzo

> 
> >
> > Thanks,
> > Lorenzo
> >
> >> +
> >> +static int __init
> >> +gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
> >> +                      const unsigned long end)
> >> +{
> >> +     int pxm, node;
> >> +     struct its_srat_map *srat_map;
> >> +     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;
> >> +     }
> >> +
> >> +     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;
> >> +     }
> >> +
> >> +     srat_map = kzalloc(sizeof(*srat_map), GFP_KERNEL);
> >> +     if (!srat_map)
> >> +             return -ENOMEM;
> >> +
> >> +     srat_map->numa_node = node;
> >> +     srat_map->its_id = its_affinity->its_id;
> >> +     list_add(&srat_map->entry, &its_srat_maps);
> >> +     pr_info("ACPI: NUMA: SRAT: ITS: PXM %d -> ITS_ID %d -> NODE %d\n",
> >> +             srat_map->its_id, pxm, 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(void) { }
> >> +#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 +1933,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 +1946,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
diff mbox

Patch

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 45ea1933..5865a75 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1833,6 +1833,78 @@  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 */
+	struct list_head	entry;
+};
+
+static LIST_HEAD(its_srat_maps);
+
+static int acpi_get_its_numa_node(u32 its_id)
+{
+	struct its_srat_map *srat_map;
+
+	list_for_each_entry(srat_map, &its_srat_maps, entry) {
+		if (its_id == srat_map->its_id)
+			return srat_map->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 its_srat_map *srat_map;
+	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;
+	}
+
+	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;
+	}
+
+	srat_map = kzalloc(sizeof(*srat_map), GFP_KERNEL);
+	if (!srat_map)
+		return -ENOMEM;
+
+	srat_map->numa_node = node;
+	srat_map->its_id = its_affinity->its_id;
+	list_add(&srat_map->entry, &its_srat_maps);
+	pr_info("ACPI: NUMA: SRAT: ITS: PXM %d -> ITS_ID %d -> NODE %d\n",
+		srat_map->its_id, pxm, 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(void) { }
+#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 +1933,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 +1946,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);
 }