diff mbox

irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

Message ID 1500630715-2156-1-git-send-email-guohanjun@huawei.com
State New
Headers show

Commit Message

Hanjun Guo July 21, 2017, 9:51 a.m. UTC 
From: Hanjun Guo <hanjun.guo@linaro.org>


When running 4.13-rc1 on top of D05, I got the boot log:

[    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0
[    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0
[    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0
[    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1
[    0.000000] SRAT: ITS affinity exceeding max count[4]

This is wrong on D05 as we have 8 ITSes with 4 NUMA nodes.

So dynamically alloc the memory needed instead of using
its_srat_maps[MAX_NUMNODES], which count the number of
ITS entry(ies) in SRAT and alloc its_srat_maps as needed,
then build the mapping of numa node to ITS ID.

After doing this, I got what I wanted:

[    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0
[    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0
[    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0
[    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1
[    0.000000] SRAT: PXM 2 -> ITS 4 -> Node 2
[    0.000000] SRAT: PXM 2 -> ITS 5 -> Node 2
[    0.000000] SRAT: PXM 2 -> ITS 6 -> Node 2
[    0.000000] SRAT: PXM 3 -> ITS 7 -> Node 3

Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

-- 
1.7.12.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

Comments

Hanjun Guo July 21, 2017, 10:06 a.m. UTC | #1 
On 2017/7/21 17:51, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>

>

> When running 4.13-rc1 on top of D05, I got the boot log:

>

> [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

> [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

> [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

> [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

> [    0.000000] SRAT: ITS affinity exceeding max count[4]

>

> This is wrong on D05 as we have 8 ITSes with 4 NUMA nodes.

>

> So dynamically alloc the memory needed instead of using

> its_srat_maps[MAX_NUMNODES], which count the number of

> ITS entry(ies) in SRAT and alloc its_srat_maps as needed,

> then build the mapping of numa node to ITS ID.

>

> After doing this, I got what I wanted:

>

> [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

> [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

> [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

> [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

> [    0.000000] SRAT: PXM 2 -> ITS 4 -> Node 2

> [    0.000000] SRAT: PXM 2 -> ITS 5 -> Node 2

> [    0.000000] SRAT: PXM 2 -> ITS 6 -> Node 2

> [    0.000000] SRAT: PXM 3 -> ITS 7 -> Node 3

>

> Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

> Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>

> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> Cc: Marc Zyngier <marc.zyngier@arm.com>

> ---

>  drivers/irqchip/irq-gic-v3-its.c | 28 +++++++++++++++++++++-------

>  1 file changed, 21 insertions(+), 7 deletions(-)

>

> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c

> index 3ccdf76..fb1c090 100644

> --- a/drivers/irqchip/irq-gic-v3-its.c

> +++ b/drivers/irqchip/irq-gic-v3-its.c

> @@ -1847,7 +1847,7 @@ struct its_srat_map {

>  	u32	its_id;

>  };

>  

> -static struct its_srat_map its_srat_maps[MAX_NUMNODES] __initdata;

> +static struct its_srat_map *its_srat_maps __initdata;

>  static int its_in_srat __initdata;

>  

>  static int __init acpi_get_its_numa_node(u32 its_id)

> @@ -1861,6 +1861,12 @@ static int __init acpi_get_its_numa_node(u32 its_id)

>  	return NUMA_NO_NODE;

>  }


Oops, need to check if the its_srat_maps valid or not here, please
comment on what else I'm missing or wrong, then I will prepare another
version.

Thanks
Hanjun

--
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
Hanjun Guo July 21, 2017, 10:50 a.m. UTC | #2 
On 21 July 2017 at 18:06, Hanjun Guo <guohanjun@huawei.com> wrote:
>

> On 2017/7/21 17:51, Hanjun Guo wrote:

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

> >

> > When running 4.13-rc1 on top of D05, I got the boot log:

> >

> > [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

> > [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

> > [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

> > [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

> > [    0.000000] SRAT: ITS affinity exceeding max count[4]

> >

> > This is wrong on D05 as we have 8 ITSes with 4 NUMA nodes.

> >

> > So dynamically alloc the memory needed instead of using

> > its_srat_maps[MAX_NUMNODES], which count the number of

> > ITS entry(ies) in SRAT and alloc its_srat_maps as needed,

> > then build the mapping of numa node to ITS ID.

> >

> > After doing this, I got what I wanted:

> >

> > [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

> > [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

> > [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

> > [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

> > [    0.000000] SRAT: PXM 2 -> ITS 4 -> Node 2

> > [    0.000000] SRAT: PXM 2 -> ITS 5 -> Node 2

> > [    0.000000] SRAT: PXM 2 -> ITS 6 -> Node 2

> > [    0.000000] SRAT: PXM 3 -> ITS 7 -> Node 3

> >

> > Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")

> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

> > Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>

> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> > Cc: Marc Zyngier <marc.zyngier@arm.com>

> > ---

> >  drivers/irqchip/irq-gic-v3-its.c | 28 +++++++++++++++++++++-------

> >  1 file changed, 21 insertions(+), 7 deletions(-)

> >

> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c

> > index 3ccdf76..fb1c090 100644

> > --- a/drivers/irqchip/irq-gic-v3-its.c

> > +++ b/drivers/irqchip/irq-gic-v3-its.c

> > @@ -1847,7 +1847,7 @@ struct its_srat_map {

> >       u32     its_id;

> >  };

> >

> > -static struct its_srat_map its_srat_maps[MAX_NUMNODES] __initdata;

> > +static struct its_srat_map *its_srat_maps __initdata;

> >  static int its_in_srat __initdata;

> >

> >  static int __init acpi_get_its_numa_node(u32 its_id)

> > @@ -1861,6 +1861,12 @@ static int __init acpi_get_its_numa_node(u32 its_id)

> >       return NUMA_NO_NODE;

> >  }

>

> Oops, need to check if the its_srat_maps valid or not here, please

> comment on what else I'm missing or wrong, then I will prepare another


                                                                   ^^^
And need to free the memory.

Thanks
Hanjun
--
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
Hanjun Guo July 22, 2017, 3:44 a.m. UTC | #3 
On 2017/7/21 19:20, Marc Zyngier wrote:
> On 21/07/17 11:06, Hanjun Guo wrote:

>> On 2017/7/21 17:51, Hanjun Guo wrote:

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

>>>

>>> When running 4.13-rc1 on top of D05, I got the boot log:

>>>

>>> [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

>>> [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

>>> [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

>>> [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

>>> [    0.000000] SRAT: ITS affinity exceeding max count[4]

>>>

>>> This is wrong on D05 as we have 8 ITSes with 4 NUMA nodes.

> I'm not seeing this on the D05 I have access to. Actually, I'm not

> seeing any data related to SRAT and the ITS. Is that a different firmware?


Yes, actually it's just a updated version in this week, that's why
the bug wasn't be spotted when Ganapat's patch was merged.

>

>>> So dynamically alloc the memory needed instead of using

>>> its_srat_maps[MAX_NUMNODES], which count the number of

>>> ITS entry(ies) in SRAT and alloc its_srat_maps as needed,

>>> then build the mapping of numa node to ITS ID.

>>>

>>> After doing this, I got what I wanted:

>>>

>>> [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

>>> [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

>>> [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

>>> [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

>>> [    0.000000] SRAT: PXM 2 -> ITS 4 -> Node 2

>>> [    0.000000] SRAT: PXM 2 -> ITS 5 -> Node 2

>>> [    0.000000] SRAT: PXM 2 -> ITS 6 -> Node 2

>>> [    0.000000] SRAT: PXM 3 -> ITS 7 -> Node 3

>>>

>>> Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")

>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

>>> Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>

>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

>>> Cc: Marc Zyngier <marc.zyngier@arm.com>

>>> ---

>>>  drivers/irqchip/irq-gic-v3-its.c | 28 +++++++++++++++++++++-------

>>>  1 file changed, 21 insertions(+), 7 deletions(-)

>>>

>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c

>>> index 3ccdf76..fb1c090 100644

>>> --- a/drivers/irqchip/irq-gic-v3-its.c

>>> +++ b/drivers/irqchip/irq-gic-v3-its.c

>>> @@ -1847,7 +1847,7 @@ struct its_srat_map {

>>>  	u32	its_id;

>>>  };

>>>  

>>> -static struct its_srat_map its_srat_maps[MAX_NUMNODES] __initdata;

>>> +static struct its_srat_map *its_srat_maps __initdata;

>>>  static int its_in_srat __initdata;

>>>  

>>>  static int __init acpi_get_its_numa_node(u32 its_id)

>>> @@ -1861,6 +1861,12 @@ static int __init acpi_get_its_numa_node(u32 its_id)

>>>  	return NUMA_NO_NODE;

>>>  }

>> Oops, need to check if the its_srat_maps valid or not here, please

>> comment on what else I'm missing or wrong, then I will prepare another

>> version.

> Please post a patch that has all of the fixes, and I'll review that.


Will do that later.

Thanks
Hanjun

--
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
Hanjun Guo July 22, 2017, 3:52 a.m. UTC | #4 
On 2017/7/21 19:42, Ganapatrao Kulkarni wrote:
> Hi Hanjun,

>

>

> On Fri, Jul 21, 2017 at 4:50 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

>> On 21/07/17 11:06, Hanjun Guo wrote:

>>> On 2017/7/21 17:51, Hanjun Guo wrote:

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

>>>>

>>>> When running 4.13-rc1 on top of D05, I got the boot log:

>>>>

>>>> [    0.000000] SRAT: PXM 0 -> ITS 0 -> Node 0

>>>> [    0.000000] SRAT: PXM 0 -> ITS 1 -> Node 0

>>>> [    0.000000] SRAT: PXM 0 -> ITS 2 -> Node 0

>>>> [    0.000000] SRAT: PXM 1 -> ITS 3 -> Node 1

>>>> [    0.000000] SRAT: ITS affinity exceeding max count[4]

>>>>

>>>> This is wrong on D05 as we have 8 ITSes with 4 NUMA nodes.

> Used static array to keep it simple.  we can have either dynamic array

> or increase static array size (which is in init data) to a larger

> number.

> We may have to go for dynamic array to be more sane. You can refer my

> v2 patch [1], which was doing dynamic allocation and avoids

> 2 calls to acpi parser as done in this patch.

>

> [1] https://patchwork.kernel.org/patch/9798659/


Hmm, although scanning the SRAT twice is bad, it will keep the
code simple, and no need to alloc the memory every time when we
find a ITS affinity entry, you need to free that memory when
ITS probing is done.

I will post a patch later, please take a look if it's ok to you :)

Thanks
Hanjun

--
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
diff mbox

Patch

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 3ccdf76..fb1c090 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1847,7 +1847,7 @@  struct its_srat_map {
 	u32	its_id;
 };
 
-static struct its_srat_map its_srat_maps[MAX_NUMNODES] __initdata;
+static struct its_srat_map *its_srat_maps __initdata;
 static int its_in_srat __initdata;
 
 static int __init acpi_get_its_numa_node(u32 its_id)
@@ -1861,6 +1861,12 @@  static int __init acpi_get_its_numa_node(u32 its_id)
 	return NUMA_NO_NODE;
 }
 
+static int __init gic_acpi_match_srat_its(struct acpi_subtable_header *header,
+					  const unsigned long end)
+{
+	return 0;
+}
+
 static int __init gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
 			 const unsigned long end)
 {
@@ -1877,12 +1883,6 @@  static int __init gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
 		return -EINVAL;
 	}
 
-	if (its_in_srat >= MAX_NUMNODES) {
-		pr_err("SRAT: ITS affinity exceeding max count[%d]\n",
-				MAX_NUMNODES);
-		return -EINVAL;
-	}
-
 	node = acpi_map_pxm_to_node(its_affinity->proximity_domain);
 
 	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
@@ -1901,6 +1901,20 @@  static int __init gic_acpi_parse_srat_its(struct acpi_subtable_header *header,
 
 static void __init acpi_table_parse_srat_its(void)
 {
+	int count;
+
+	count = acpi_table_parse_entries(ACPI_SIG_SRAT,
+			sizeof(struct acpi_table_srat),
+			ACPI_SRAT_TYPE_GIC_ITS_AFFINITY,
+			gic_acpi_match_srat_its, 0);
+	if (count <= 0)
+		return;
+
+	its_srat_maps = kmalloc(count * sizeof(struct its_srat_map),
+				GFP_KERNEL);
+	if (!its_srat_maps)
+		return;
+
 	acpi_table_parse_entries(ACPI_SIG_SRAT,
 			sizeof(struct acpi_table_srat),
 			ACPI_SRAT_TYPE_GIC_ITS_AFFINITY,