irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

PATCH New Stuart Yoder 2 participants 6 messages May 14, 2015 – May 18, 2015

Subsystems (2)

irqchip drivers Arm generic interrupt controller drivers

lore

Share & tools

lore

Stuart Yoder

author

2015-05-14 23:02 UTC #0 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.

Signed-off-by Stuart Yoder <[email protected]>

---

think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC

 drivers/irqchip/irq-gic-v3-its.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/irqchip/irq-gic-v3-its.c +1 -1

index 9687f8a..58a6612 100644
@@ -800,7 +800,7 @@ static int its_alloc_tables(struct its_node *its)
 {
 	int err;
 	int i;
-	int psz = SZ_64K;
+	int psz = PAGE_SIZE;
 	u64 shr = GITS_BASER_InnerShareable;
 	u64 cache = GITS_BASER_WaWb;

Signature · 1 line

2.3.3

Marc Zyngier

2015-05-18 13:23 UTC #1 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

Hi Stuart,

Quoted 9 lines · Stuart Yoder

its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.
Signed-off-by Stuart Yoder <[email protected]>
---
think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC

What you are suggesting here is a effectively a revert of commit 790b57a, which would break other implementations.

Can you please explain the actual issue? I'm failing to see how you end up in an infinite loop here (the system page size and the ITS base granule should be completely unrelated...).

Or has it anything to do with Minghuan Lian's patch
(https://lkml.org/lkml/2015/4/16/36), which looks more correct (if still massively under-documented)?

Thanks,

M.

Quoted 18 lines

 drivers/irqchip/irq-gic-v3-its.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/irqchip/irq-gic-v3-its.c +1 -1

index 9687f8a..58a6612 100644
@@ -800,7 +800,7 @@ static int its_alloc_tables(struct its_node *its)
 {
 	int err;
 	int i;
-	int psz = SZ_64K;
+	int psz = PAGE_SIZE;
 	u64 shr = GITS_BASER_InnerShareable;
 	u64 cache = GITS_BASER_WaWb;

Signature · 1 line

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

Stuart Yoder

author

2015-05-18 13:38 UTC #2 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

Quoted 22 lines

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 8:23 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Hi Stuart,
Quoted 9 lines · Stuart Yoder
its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.
Signed-off-by Stuart Yoder <[email protected]>
---
think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC

Quoted 6 lines

What you are suggesting here is a effectively a revert of commit 790b57a, which would break other implementations.
Can you please explain the actual issue? I'm failing to see how you end up in an infinite loop here (the system page size and the ITS base granule should be completely unrelated...).

Here is the problem line:

      val |= (alloc_size / psz) - 1;

In our case:
alloc_size=16K
psz=64K

...so (alloc_size / psz) = 0, and thus val becomes -1, and everything is screwed up. We get stuck in a loop to retry_baser:

Thanks,
Stuart

Marc Zyngier

2015-05-18 14:09 UTC #3 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

Quoted 31 lines · Stuart Yoder

Quoted 22 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 8:23 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Hi Stuart,
Quoted 9 lines · Stuart Yoder
its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.
Signed-off-by Stuart Yoder <[email protected]>
---
think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC
Quoted 6 lines
What you are suggesting here is a effectively a revert of commit 790b57a, which would break other implementations.
Can you please explain the actual issue? I'm failing to see how you end up in an infinite loop here (the system page size and the ITS base granule should be completely unrelated...).
Here is the problem line:

Quoted 8 lines

      val |= (alloc_size / psz) - 1;
In our case:
   alloc_size=16K
   psz=64K
...so (alloc_size / psz) = 0, and thus val becomes -1, and everything is screwed up. We get stuck in a loop to retry_baser:

If alloc_size is 16k, you have an order of 2, and I have to assume this is an allocation for a device table (otherwise order would be 4). So things fail because we've computed an alloc_size smaller than what we want to allocate as a minimum.

Isn't that exactly what Minghuan's patch fixes?

Thanks,

M.

Signature · 1 line

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

Stuart Yoder

author

2015-05-18 15:33 UTC #4 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

Quoted 50 lines

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 9:09 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Quoted 31 lines · Stuart Yoder
Quoted 22 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 8:23 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Hi Stuart,
Quoted 9 lines · Stuart Yoder
its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.
Signed-off-by Stuart Yoder <[email protected]>
---
think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC
Quoted 6 lines
What you are suggesting here is a effectively a revert of commit 790b57a, which would break other implementations.
Can you please explain the actual issue? I'm failing to see how you end up in an infinite loop here (the system page size and the ITS base granule should be completely unrelated...).
Here is the problem line:
Quoted 8 lines
      val |= (alloc_size / psz) - 1;
In our case:
   alloc_size=16K
   psz=64K
...so (alloc_size / psz) = 0, and thus val becomes -1, and everything is screwed up. We get stuck in a loop to retry_baser:

Quoted 6 lines

If alloc_size is 16k, you have an order of 2, and I have to assume this is an allocation for a device table (otherwise order would be 4). So things fail because we've computed an alloc_size smaller than what we want to allocate as a minimum.
Isn't that exactly what Minghuan's patch fixes?

Yes. I'll get with Minghuan and between the 2 of us we'll get a properly commented version of his patch sent out.

Thanks,
Stuart

Marc Zyngier

2015-05-18 15:36 UTC #5 lore raw

PATCH 1/1 irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables

Quoted 55 lines · Stuart Yoder

Quoted 55 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 9:09 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Quoted 31 lines · Stuart Yoder
Quoted 22 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Monday, May 18, 2015 8:23 AM
To: Yoder Stuart-B08248; tglx at linutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] irqchip: GICv3: ITS: don't assume 64K page size in its_alloc_tables
Hi Stuart,
Quoted 9 lines · Stuart Yoder
its_alloc_tables() needs to account for page sizes other than 64KB. Without this change, when PAGE_SIZE=4KB its_alloc_tables() gets stuck in an infinite loop.
Signed-off-by Stuart Yoder <[email protected]>
---
think this should go into 4.1 if at all possible...without it I am unable to boot a 4.1 kernel on the LS2085 SoC
Quoted 6 lines
What you are suggesting here is a effectively a revert of commit 790b57a, which would break other implementations.
Can you please explain the actual issue? I'm failing to see how you end up in an infinite loop here (the system page size and the ITS base granule should be completely unrelated...).
Here is the problem line:
Quoted 8 lines
      val |= (alloc_size / psz) - 1;
In our case:
   alloc_size=16K
   psz=64K
...so (alloc_size / psz) = 0, and thus val becomes -1, and everything is screwed up. We get stuck in a loop to retry_baser:
If alloc_size is 16k, you have an order of 2, and I have to assume this is an allocation for a device table (otherwise order would be 4). So things fail because we've computed an alloc_size smaller than what we want to allocate as a minimum.

Quoted 4 lines

Quoted 1 line
Isn't that exactly what Minghuan's patch fixes?
Yes. I'll get with Minghuan and between the 2 of us we'll get a properly commented version of his patch sent out.

Thanks Stuart.

M.

Signature · 1 line

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