LinuxLists.cc - [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX

2017-07-22 04:01:23

Subject: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

From: Hanjun Guo <[email protected]>

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. Of course,
its_srat_maps will be freed after ITS probing because
we don't need that after boot.

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 <[email protected]>
Cc: Ganapatrao Kulkarni <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Marc Zyngier <[email protected]>
---

v1->v2:
- Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
- Free the its_srat_maps after ITS probing.

drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 3ccdf76..1d692aa 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1847,13 +1847,16 @@ 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)
{
int i;

+ if (!its_srat_maps)
+ return NUMA_NO_NODE;
+
for (i = 0; i < its_in_srat; i++) {
if (its_id == its_srat_maps[i].its_id)
return its_srat_maps[i].numa_node;
@@ -1861,6 +1864,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 +1886,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,14 +1904,35 @@ 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,
gic_acpi_parse_srat_its, 0);
}
+
+/* free the its_srat_maps after ITS probing */
+static void __init acpi_its_srat_maps_free(void)
+{
+ kfree(its_srat_maps);
+}
#else
static void __init acpi_table_parse_srat_its(void) { }
static int __init acpi_get_its_numa_node(u32 its_id) { return NUMA_NO_NODE; }
+static void __init acpi_its_srat_maps_free(void) { }
#endif

static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
@@ -1955,6 +1979,7 @@ 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);
+ acpi_its_srat_maps_free();
}
#else
static void __init its_acpi_probe(void) { }
--
1.7.12.4

2017-07-25 10:30:30

by Marc Zyngier

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 22/07/17 04:54, Hanjun Guo wrote:
> From: Hanjun Guo <[email protected]>
>
> 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. Of course,
> its_srat_maps will be freed after ITS probing because
> we don't need that after boot.
>
> 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 <[email protected]>
> Cc: Ganapatrao Kulkarni <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> ---
>
> v1->v2:
> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
> - Free the its_srat_maps after ITS probing.
>
> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 3ccdf76..1d692aa 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1847,13 +1847,16 @@ 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)
> {
> int i;
>
> + if (!its_srat_maps)
> + return NUMA_NO_NODE;
> +
> for (i = 0; i < its_in_srat; i++) {
> if (its_id == its_srat_maps[i].its_id)
> return its_srat_maps[i].numa_node;
> @@ -1861,6 +1864,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 +1886,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;
> - }
> -

So you're getting rid of that message when overflowing the array...

> node = acpi_map_pxm_to_node(its_affinity->proximity_domain);
>
> if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
> @@ -1901,14 +1904,35 @@ 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;

and here silently returning when failing to allocate the memory. I think
it'd be worth having a bit of a warning.

> +
> acpi_table_parse_entries(ACPI_SIG_SRAT,
> sizeof(struct acpi_table_srat),
> ACPI_SRAT_TYPE_GIC_ITS_AFFINITY,
> gic_acpi_parse_srat_its, 0);
> }
> +
> +/* free the its_srat_maps after ITS probing */
> +static void __init acpi_its_srat_maps_free(void)
> +{
> + kfree(its_srat_maps);
> +}
> #else
> static void __init acpi_table_parse_srat_its(void) { }
> static int __init acpi_get_its_numa_node(u32 its_id) { return NUMA_NO_NODE; }
> +static void __init acpi_its_srat_maps_free(void) { }
> #endif
>
> static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> @@ -1955,6 +1979,7 @@ 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);
> + acpi_its_srat_maps_free();
> }
> #else
> static void __init its_acpi_probe(void) { }
>

Otherwise looks good.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2017-07-25 10:47:02

by Lorenzo Pieralisi

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On Sat, Jul 22, 2017 at 11:54:12AM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <[email protected]>
>
> When running 4.13-rc1 on top of D05, I got the boot log:

Nit: You should stick to what the problem is and why you need to solve
it, "Fixes:" tag gives the commit history you need, the rest (eg "When
running 4.13-rc1") does not belong in the commit 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. Of course,
> its_srat_maps will be freed after ITS probing because
> we don't need that after boot.
>
> 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

Question (unrelated): how are PCI devices (or better PCI host bridges)
mapped to ITSs ? I ask because in IORT we currently ignore the notion
of ITS groups - so it is just out of curiosity (I suspect you have
a static 1:1 mapping PCI-host-bridge->ITS).

> Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")
> Signed-off-by: Hanjun Guo <[email protected]>
> Cc: Ganapatrao Kulkarni <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> ---
>
> v1->v2:
> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
> - Free the its_srat_maps after ITS probing.
>
> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------

Reviewed-by: Lorenzo Pieralisi <[email protected]>

> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 3ccdf76..1d692aa 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1847,13 +1847,16 @@ 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)
> {
> int i;
>
> + if (!its_srat_maps)
> + return NUMA_NO_NODE;
> +
> for (i = 0; i < its_in_srat; i++) {
> if (its_id == its_srat_maps[i].its_id)
> return its_srat_maps[i].numa_node;
> @@ -1861,6 +1864,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 +1886,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,14 +1904,35 @@ 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,
> gic_acpi_parse_srat_its, 0);
> }
> +
> +/* free the its_srat_maps after ITS probing */
> +static void __init acpi_its_srat_maps_free(void)
> +{
> + kfree(its_srat_maps);
> +}
> #else
> static void __init acpi_table_parse_srat_its(void) { }
> static int __init acpi_get_its_numa_node(u32 its_id) { return NUMA_NO_NODE; }
> +static void __init acpi_its_srat_maps_free(void) { }
> #endif
>
> static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> @@ -1955,6 +1979,7 @@ 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);
> + acpi_its_srat_maps_free();
> }
> #else
> static void __init its_acpi_probe(void) { }
> --
> 1.7.12.4
>

2017-07-25 11:02:47

by John Garry

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 22/07/2017 04:54, Hanjun Guo wrote:
> From: Hanjun Guo <[email protected]>
>
> 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. Of course,
> its_srat_maps will be freed after ITS probing because
> we don't need that after boot.
>
> 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 <[email protected]>
> Cc: Ganapatrao Kulkarni <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> ---
>
> v1->v2:
> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
> - Free the its_srat_maps after ITS probing.
>
> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 3ccdf76..1d692aa 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1847,13 +1847,16 @@ 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)
> {
> int i;
>
> + if (!its_srat_maps)
> + return NUMA_NO_NODE;

Question: Does !its_srat_maps always imply its_in_srat == 0, so we could
just fall through the for loop and return NUMA_NO_NODE without this check?

Or should we be safe/explicit/or falling through loops is a bad coding
style?

Cheers,
John

> +
> for (i = 0; i < its_in_srat; i++) {
> if (its_id == its_srat_maps[i].its_id)
> return its_srat_maps[i].numa_node;
> @@ -1861,6 +1864,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 +1886,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,14 +1904,35 @@ 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,
> gic_acpi_parse_srat_its, 0);
> }
> +
> +/* free the its_srat_maps after ITS probing */
> +static void __init acpi_its_srat_maps_free(void)
> +{
> + kfree(its_srat_maps);
> +}
> #else
> static void __init acpi_table_parse_srat_its(void) { }
> static int __init acpi_get_its_numa_node(u32 its_id) { return NUMA_NO_NODE; }
> +static void __init acpi_its_srat_maps_free(void) { }
> #endif
>
> static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> @@ -1955,6 +1979,7 @@ 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);
> + acpi_its_srat_maps_free();
> }
> #else
> static void __init its_acpi_probe(void) { }
>

2017-07-26 07:52:40

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 2017/7/25 18:30, Marc Zyngier wrote:
> On 22/07/17 04:54, Hanjun Guo wrote:
>> From: Hanjun Guo <[email protected]>
>>
>> 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. Of course,
>> its_srat_maps will be freed after ITS probing because
>> we don't need that after boot.
>>
>> 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 <[email protected]>
>> Cc: Ganapatrao Kulkarni <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Marc Zyngier <[email protected]>
>> ---
>>
>> v1->v2:
>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>> - Free the its_srat_maps after ITS probing.
>>
>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 3ccdf76..1d692aa 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -1847,13 +1847,16 @@ 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)
>> {
>> int i;
>>
>> + if (!its_srat_maps)
>> + return NUMA_NO_NODE;
>> +
>> for (i = 0; i < its_in_srat; i++) {
>> if (its_id == its_srat_maps[i].its_id)
>> return its_srat_maps[i].numa_node;
>> @@ -1861,6 +1864,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 +1886,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;
>> - }
>> -
>
> So you're getting rid of that message when overflowing the array...

This overflowing will not happen, because I scan the SRAT
to count the entry(ies) of ITS affinity first to alloc the
array, and then parse the same SRAT again to setup the mapping
of NUMA node to ITS, so is it fine for us to just remove the
check here?

>
>> node = acpi_map_pxm_to_node(its_affinity->proximity_domain);
>>
>> if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>> @@ -1901,14 +1904,35 @@ 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;
>
> and here silently returning when failing to allocate the memory. I think
> it'd be worth having a bit of a warning.

Will do.

>
>> +
>> acpi_table_parse_entries(ACPI_SIG_SRAT,
>> sizeof(struct acpi_table_srat),
>> ACPI_SRAT_TYPE_GIC_ITS_AFFINITY,
>> gic_acpi_parse_srat_its, 0);
>> }
>> +
>> +/* free the its_srat_maps after ITS probing */
>> +static void __init acpi_its_srat_maps_free(void)
>> +{
>> + kfree(its_srat_maps);
>> +}
>> #else
>> static void __init acpi_table_parse_srat_its(void) { }
>> static int __init acpi_get_its_numa_node(u32 its_id) { return NUMA_NO_NODE; }
>> +static void __init acpi_its_srat_maps_free(void) { }
>> #endif
>>
>> static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
>> @@ -1955,6 +1979,7 @@ 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);
>> + acpi_its_srat_maps_free();
>> }
>> #else
>> static void __init its_acpi_probe(void) { }
>>
>
> Otherwise looks good.

Thanks
Hanjun

2017-07-26 08:00:45

by Marc Zyngier

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 26/07/17 08:52, Hanjun Guo wrote:
> On 2017/7/25 18:30, Marc Zyngier wrote:
>> On 22/07/17 04:54, Hanjun Guo wrote:
>>> From: Hanjun Guo <[email protected]>
>>>
>>> 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. Of course,
>>> its_srat_maps will be freed after ITS probing because
>>> we don't need that after boot.
>>>
>>> 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 <[email protected]>
>>> Cc: Ganapatrao Kulkarni <[email protected]>
>>> Cc: Lorenzo Pieralisi <[email protected]>
>>> Cc: Marc Zyngier <[email protected]>
>>> ---
>>>
>>> v1->v2:
>>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>>> - Free the its_srat_maps after ITS probing.
>>>
>>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>> index 3ccdf76..1d692aa 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>> @@ -1847,13 +1847,16 @@ 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)
>>> {
>>> int i;
>>>
>>> + if (!its_srat_maps)
>>> + return NUMA_NO_NODE;
>>> +
>>> for (i = 0; i < its_in_srat; i++) {
>>> if (its_id == its_srat_maps[i].its_id)
>>> return its_srat_maps[i].numa_node;
>>> @@ -1861,6 +1864,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 +1886,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;
>>> - }
>>> -
>>
>> So you're getting rid of that message when overflowing the array...
>
> This overflowing will not happen, because I scan the SRAT
> to count the entry(ies) of ITS affinity first to alloc the
> array, and then parse the same SRAT again to setup the mapping
> of NUMA node to ITS, so is it fine for us to just remove the
> check here?

Removing that check is fine, as long as you make sure the allocation
hasn't failed.

>>
>>> node = acpi_map_pxm_to_node(its_affinity->proximity_domain);
>>>
>>> if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
>>> @@ -1901,14 +1904,35 @@ 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;
>>
>> and here silently returning when failing to allocate the memory. I think
>> it'd be worth having a bit of a warning.
>
> Will do.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2017-07-26 08:11:21

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 2017/7/25 18:47, Lorenzo Pieralisi wrote:
> On Sat, Jul 22, 2017 at 11:54:12AM +0800, Hanjun Guo wrote:
>> From: Hanjun Guo <[email protected]>
>>
>> When running 4.13-rc1 on top of D05, I got the boot log:
>
> Nit: You should stick to what the problem is and why you need to solve
> it, "Fixes:" tag gives the commit history you need, the rest (eg "When
> running 4.13-rc1") does not belong in the commit log.

Updated as "After enabling the ITS NUMA support on 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. Of course,
>> its_srat_maps will be freed after ITS probing because
>> we don't need that after boot.
>>
>> 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
>
> Question (unrelated): how are PCI devices (or better PCI host bridges)
> mapped to ITSs ? I ask because in IORT we currently ignore the notion
> of ITS groups - so it is just out of curiosity (I suspect you have
> a static 1:1 mapping PCI-host-bridge->ITS).

Yes, on D05 we enabled 8 ITSs, and also have 8 PCI hostbridges, here is
the IORT for D05:

https://github.com/hisilicon/OpenPlatformPkg/blob/bb17676e6c529732af8adf438fc2c8ceeb9b3271/Chips/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl

>
>> Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")
>> Signed-off-by: Hanjun Guo <[email protected]>
>> Cc: Ganapatrao Kulkarni <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Marc Zyngier <[email protected]>
>> ---
>>
>> v1->v2:
>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>> - Free the its_srat_maps after ITS probing.
>>
>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>
> Reviewed-by: Lorenzo Pieralisi <[email protected]>

Thanks!

Hanjun

2017-07-26 09:47:49

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 2017/7/25 19:02, John Garry wrote:
> On 22/07/2017 04:54, Hanjun Guo wrote:
>> From: Hanjun Guo <[email protected]>
>>
>> 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. Of course,
>> its_srat_maps will be freed after ITS probing because
>> we don't need that after boot.
>>
>> 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 <[email protected]>
>> Cc: Ganapatrao Kulkarni <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Marc Zyngier <[email protected]>
>> ---
>>
>> v1->v2:
>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>> - Free the its_srat_maps after ITS probing.
>>
>> drivers/irqchip/irq-gic-v3-its.c | 39
>> ++++++++++++++++++++++++++++++++-------
>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 3ccdf76..1d692aa 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -1847,13 +1847,16 @@ 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)
>> {
>> int i;
>>
>> + if (!its_srat_maps)
>> + return NUMA_NO_NODE;
>
> Question: Does !its_srat_maps always imply its_in_srat == 0, so we could
> just fall through the for loop and return NUMA_NO_NODE without this check?
>
> Or should we be safe/explicit/or falling through loops is a bad coding
> style?

Hmm, you are right, I missed that point, its_in_srat will
always be 0 if its_srat_maps be NULL, removed the NULL
check and tested it on D03 (without ITS NUMA) and D03 boots
OK, will remove the check in the new version.

Thanks
Hanjun

2017-07-26 09:48:13

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

2017-07-26 09:55:35

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 2017/7/26 16:00, Marc Zyngier wrote:
> On 26/07/17 08:52, Hanjun Guo wrote:
>> On 2017/7/25 18:30, Marc Zyngier wrote:
>>> On 22/07/17 04:54, Hanjun Guo wrote:
>>>> From: Hanjun Guo <[email protected]>
>>>>
>>>> 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. Of course,
>>>> its_srat_maps will be freed after ITS probing because
>>>> we don't need that after boot.
>>>>
>>>> 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 <[email protected]>
>>>> Cc: Ganapatrao Kulkarni <[email protected]>
>>>> Cc: Lorenzo Pieralisi <[email protected]>
>>>> Cc: Marc Zyngier <[email protected]>
>>>> ---
>>>>
>>>> v1->v2:
>>>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>>>> - Free the its_srat_maps after ITS probing.
>>>>
>>>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>>> index 3ccdf76..1d692aa 100644
>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>> @@ -1847,13 +1847,16 @@ 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)
>>>> {
>>>> int i;
>>>>
>>>> + if (!its_srat_maps)
>>>> + return NUMA_NO_NODE;
>>>> +
>>>> for (i = 0; i < its_in_srat; i++) {
>>>> if (its_id == its_srat_maps[i].its_id)
>>>> return its_srat_maps[i].numa_node;
>>>> @@ -1861,6 +1864,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 +1886,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;
>>>> - }
>>>> -
>>>
>>> So you're getting rid of that message when overflowing the array...
>>
>> This overflowing will not happen, because I scan the SRAT
>> to count the entry(ies) of ITS affinity first to alloc the
>> array, and then parse the same SRAT again to setup the mapping
>> of NUMA node to ITS, so is it fine for us to just remove the
>> check here?
>
> Removing that check is fine, as long as you make sure the allocation
> hasn't failed.

Sorry, just want to make sure I understand correctly. This function will
not be called if allocation failure, so do you mean we can keep the code
as it is?

Thanks
Hanjun

2017-07-26 10:01:13

by Marc Zyngier

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 26/07/17 10:55, Hanjun Guo wrote:
> On 2017/7/26 16:00, Marc Zyngier wrote:
>> On 26/07/17 08:52, Hanjun Guo wrote:
>>> On 2017/7/25 18:30, Marc Zyngier wrote:
>>>> On 22/07/17 04:54, Hanjun Guo wrote:
>>>>> From: Hanjun Guo <[email protected]>
>>>>>
>>>>> 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. Of course,
>>>>> its_srat_maps will be freed after ITS probing because
>>>>> we don't need that after boot.
>>>>>
>>>>> 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 <[email protected]>
>>>>> Cc: Ganapatrao Kulkarni <[email protected]>
>>>>> Cc: Lorenzo Pieralisi <[email protected]>
>>>>> Cc: Marc Zyngier <[email protected]>
>>>>> ---
>>>>>
>>>>> v1->v2:
>>>>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>>>>> - Free the its_srat_maps after ITS probing.
>>>>>
>>>>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>>>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>>>> index 3ccdf76..1d692aa 100644
>>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>>> @@ -1847,13 +1847,16 @@ 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)
>>>>> {
>>>>> int i;
>>>>>
>>>>> + if (!its_srat_maps)
>>>>> + return NUMA_NO_NODE;
>>>>> +
>>>>> for (i = 0; i < its_in_srat; i++) {
>>>>> if (its_id == its_srat_maps[i].its_id)
>>>>> return its_srat_maps[i].numa_node;
>>>>> @@ -1861,6 +1864,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 +1886,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;
>>>>> - }
>>>>> -
>>>>
>>>> So you're getting rid of that message when overflowing the array...
>>>
>>> This overflowing will not happen, because I scan the SRAT
>>> to count the entry(ies) of ITS affinity first to alloc the
>>> array, and then parse the same SRAT again to setup the mapping
>>> of NUMA node to ITS, so is it fine for us to just remove the
>>> check here?
>>
>> Removing that check is fine, as long as you make sure the allocation
>> hasn't failed.
>
> Sorry, just want to make sure I understand correctly. This function will
> not be called if allocation failure, so do you mean we can keep the code
> as it is?

No. I mean adding this warning when the allocation fails, so that we
know that our NUMA topology is screwed.

M.
--
Jazz is not dead. It just smells funny...

2017-07-26 10:03:46

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 2017/7/26 18:01, Marc Zyngier wrote:
> On 26/07/17 10:55, Hanjun Guo wrote:
>> On 2017/7/26 16:00, Marc Zyngier wrote:
>>> On 26/07/17 08:52, Hanjun Guo wrote:
>>>> On 2017/7/25 18:30, Marc Zyngier wrote:
>>>>> On 22/07/17 04:54, Hanjun Guo wrote:
>>>>>> From: Hanjun Guo <[email protected]>
>>>>>>
>>>>>> 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. Of course,
>>>>>> its_srat_maps will be freed after ITS probing because
>>>>>> we don't need that after boot.
>>>>>>
>>>>>> 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 <[email protected]>
>>>>>> Cc: Ganapatrao Kulkarni <[email protected]>
>>>>>> Cc: Lorenzo Pieralisi <[email protected]>
>>>>>> Cc: Marc Zyngier <[email protected]>
>>>>>> ---
>>>>>>
>>>>>> v1->v2:
>>>>>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity case;
>>>>>> - Free the its_srat_maps after ITS probing.
>>>>>>
>>>>>> drivers/irqchip/irq-gic-v3-its.c | 39 ++++++++++++++++++++++++++++++++-------
>>>>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>>>>> index 3ccdf76..1d692aa 100644
>>>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>>>> @@ -1847,13 +1847,16 @@ 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)
>>>>>> {
>>>>>> int i;
>>>>>>
>>>>>> + if (!its_srat_maps)
>>>>>> + return NUMA_NO_NODE;
>>>>>> +
>>>>>> for (i = 0; i < its_in_srat; i++) {
>>>>>> if (its_id == its_srat_maps[i].its_id)
>>>>>> return its_srat_maps[i].numa_node;
>>>>>> @@ -1861,6 +1864,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 +1886,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;
>>>>>> - }
>>>>>> -
>>>>>
>>>>> So you're getting rid of that message when overflowing the array...
>>>>
>>>> This overflowing will not happen, because I scan the SRAT
>>>> to count the entry(ies) of ITS affinity first to alloc the
>>>> array, and then parse the same SRAT again to setup the mapping
>>>> of NUMA node to ITS, so is it fine for us to just remove the
>>>> check here?
>>>
>>> Removing that check is fine, as long as you make sure the allocation
>>> hasn't failed.
>>
>> Sorry, just want to make sure I understand correctly. This function will
>> not be called if allocation failure, so do you mean we can keep the code
>> as it is?
>
> No. I mean adding this warning when the allocation fails, so that we
> know that our NUMA topology is screwed.

OK, thanks!

Hanjun

2017-07-26 11:05:54

by Robin Murphy

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

On 26/07/17 09:11, Hanjun Guo wrote:
> On 2017/7/25 18:47, Lorenzo Pieralisi wrote:
>> On Sat, Jul 22, 2017 at 11:54:12AM +0800, Hanjun Guo wrote:
>>> From: Hanjun Guo <[email protected]>
>>>
>>> When running 4.13-rc1 on top of D05, I got the boot log:
>>
>> Nit: You should stick to what the problem is and why you need to solve
>> it, "Fixes:" tag gives the commit history you need, the rest (eg "When
>> running 4.13-rc1") does not belong in the commit log.
>
> Updated as "After enabling the ITS NUMA support on 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. Of course,
>>> its_srat_maps will be freed after ITS probing because
>>> we don't need that after boot.
>>>
>>> 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
>>
>> Question (unrelated): how are PCI devices (or better PCI host bridges)
>> mapped to ITSs ? I ask because in IORT we currently ignore the notion
>> of ITS groups - so it is just out of curiosity (I suspect you have
>> a static 1:1 mapping PCI-host-bridge->ITS).
>
> Yes, on D05 we enabled 8 ITSs, and also have 8 PCI hostbridges, here is
> the IORT for D05:
>
> https://github.com/hisilicon/OpenPlatformPkg/blob/bb17676e6c529732af8adf438fc2c8ceeb9b3271/Chips/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl

On a further side note, do the SMMUs really have no interrupts, or is
that just a hack to avoid MBIgen-related problems? Now that we have
working probe-deferral for IOMMU masters, it ought to be possible to
address any dependency by deferring the SMMU itself until the IRQs are
available. At a glance I guess ACPI doesn't make it as easy as
of_irq_get() does, but it might be worth looking into.

Robin.

>>> Fixes: dbd2b8267233 ("irqchip/gic-v3-its: Add ACPI NUMA node mapping")
>>> Signed-off-by: Hanjun Guo <[email protected]>
>>> Cc: Ganapatrao Kulkarni <[email protected]>
>>> Cc: Lorenzo Pieralisi <[email protected]>
>>> Cc: Marc Zyngier <[email protected]>
>>> ---
>>>
>>> v1->v2:
>>> - Add NULL check in acpi_get_its_numa_node() for no ITS affinity
>>> case;
>>> - Free the its_srat_maps after ITS probing.
>>>
>>> drivers/irqchip/irq-gic-v3-its.c | 39
>>> ++++++++++++++++++++++++++++++++-------
>>
>> Reviewed-by: Lorenzo Pieralisi <[email protected]>
>
> Thanks!
>
> Hanjun
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2017-07-27 07:36:32

by Hanjun Guo

[permalink] [raw]

Subject: Re: [PATCH v2] irqchip/gic-v3-its: Allow GIC ITS number more than MAX_NUMNODES

Hi Robin,

On 2017/7/26 19:05, Robin Murphy wrote:
> On 26/07/17 09:11, Hanjun Guo wrote:
>> On 2017/7/25 18:47, Lorenzo Pieralisi wrote:
>>> On Sat, Jul 22, 2017 at 11:54:12AM +0800, Hanjun Guo wrote:
>>>> From: Hanjun Guo <[email protected]>
>>>>
>>>> When running 4.13-rc1 on top of D05, I got the boot log:
>>>
>>> Nit: You should stick to what the problem is and why you need to solve
>>> it, "Fixes:" tag gives the commit history you need, the rest (eg "When
>>> running 4.13-rc1") does not belong in the commit log.
>>
>> Updated as "After enabling the ITS NUMA support on 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. Of course,
>>>> its_srat_maps will be freed after ITS probing because
>>>> we don't need that after boot.
>>>>
>>>> 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
>>>
>>> Question (unrelated): how are PCI devices (or better PCI host bridges)
>>> mapped to ITSs ? I ask because in IORT we currently ignore the notion
>>> of ITS groups - so it is just out of curiosity (I suspect you have
>>> a static 1:1 mapping PCI-host-bridge->ITS).
>>
>> Yes, on D05 we enabled 8 ITSs, and also have 8 PCI hostbridges, here is
>> the IORT for D05:
>>
>> https://github.com/hisilicon/OpenPlatformPkg/blob/bb17676e6c529732af8adf438fc2c8ceeb9b3271/Chips/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl
>
> On a further side note, do the SMMUs really have no interrupts, or is
> that just a hack to avoid MBIgen-related problems? Now that we have
> working probe-deferral for IOMMU masters, it ought to be possible to
> address any dependency by deferring the SMMU itself until the IRQs are
> available. At a glance I guess ACPI doesn't make it as easy as
> of_irq_get() does, but it might be worth looking into.

SMMUs on D05 have interrupts as SMMU spec defined, but as you said they
are routed to MBIgen.

For now, IORT can support two types of interrupt for SMMU, one is SPI
which connect to GICD, the other one is using MSI which describing the
mapping of smmu dev id to ITS.

So interrupts connecting to MBIgen or other interrupt controller are not
supported, but I investigated that for a while and I think we can update
the IORT table to support other interrupt controllers by introducing
"ACPI device object full path name" in the IORT that the SMMU interrupts
are referring.

Thanks
Hanjun