Subject: [PATCH] hw/intc/arm_gicv3: Fix GICD_TYPER ITLinesNumber advertisement

Luke Starrett posted 1 patch 2 weeks, 2 days ago
Failed in applying to current master (apply log)
hw/intc/arm_gicv3_dist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Subject: [PATCH] hw/intc/arm_gicv3: Fix GICD_TYPER ITLinesNumber advertisement
Posted by Luke Starrett 2 weeks, 2 days ago
The ARM GICv3 TRM describes that the ITLinesNumber field of GICD_TYPER
register:

"indicates the maximum SPI INTID that the GIC implementation supports"

As SPI #0 is absolute IRQ #32, the max SPI INTID should have accounted
for the internal 16x SGI's and 16x PPI's.  However, the original GICv3
model subtracted off the SGI/PPI.  Cosmetically this can be seen at OS
boot (Linux) showing 32 shy of what should be there, i.e.:

     [    0.000000] GICv3: 224 SPIs implemented

Though in hw/arm/virt.c, the machine is configured for 256 SPI's. ARM
virt machine likely doesn't have a problem with this because the upper
32 IRQ's don't actually have anything meaningful wired. But, this does
become a functional issue on a custom use case which wants to make use
of these IRQ's.  Additionally, boot code (i.e. TF-A) will only init up
to the number (blocks of 32) that it believes to actually be there.

Signed-off-by: Luke Starrett <lukes@xsightlabs.com>
---
  hw/intc/arm_gicv3_dist.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index eea0368118..d599fefcbc 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -390,9 +390,9 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
           * MBIS == 0 (message-based SPIs not supported)
           * SecurityExtn == 1 if security extns supported
           * CPUNumber == 0 since for us ARE is always 1
-         * ITLinesNumber == (num external irqs / 32) - 1
+         * ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
           */
-        int itlinesnumber = ((s->num_irq - GIC_INTERNAL) / 32) - 1;
+        int itlinesnumber = (s->num_irq / 32) - 1;
          /*
           * SecurityExtn must be RAZ if GICD_CTLR.DS == 1, and
           * "security extensions not supported" always implies DS == 1,
-- 
2.27.0



Re: Subject: [PATCH] hw/intc/arm_gicv3: Fix GICD_TYPER ITLinesNumber advertisement
Posted by Luke Starrett 2 weeks, 2 days ago
Please disregard this one.  Sent in error.

On 11/22/2022 2:18 PM, Luke Starrett wrote:
> The ARM GICv3 TRM describes that the ITLinesNumber field of GICD_TYPER
> register:
>
> "indicates the maximum SPI INTID that the GIC implementation supports"
>
> As SPI #0 is absolute IRQ #32, the max SPI INTID should have accounted
> for the internal 16x SGI's and 16x PPI's.  However, the original GICv3
> model subtracted off the SGI/PPI.  Cosmetically this can be seen at OS
> boot (Linux) showing 32 shy of what should be there, i.e.:
>
>     [    0.000000] GICv3: 224 SPIs implemented
>
> Though in hw/arm/virt.c, the machine is configured for 256 SPI's. ARM
> virt machine likely doesn't have a problem with this because the upper
> 32 IRQ's don't actually have anything meaningful wired. But, this does
> become a functional issue on a custom use case which wants to make use
> of these IRQ's.  Additionally, boot code (i.e. TF-A) will only init up
> to the number (blocks of 32) that it believes to actually be there.
>
> Signed-off-by: Luke Starrett <lukes@xsightlabs.com>
> ---
>  hw/intc/arm_gicv3_dist.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
> index eea0368118..d599fefcbc 100644
> --- a/hw/intc/arm_gicv3_dist.c
> +++ b/hw/intc/arm_gicv3_dist.c
> @@ -390,9 +390,9 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
>           * MBIS == 0 (message-based SPIs not supported)
>           * SecurityExtn == 1 if security extns supported
>           * CPUNumber == 0 since for us ARE is always 1
> -         * ITLinesNumber == (num external irqs / 32) - 1
> +         * ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
>           */
> -        int itlinesnumber = ((s->num_irq - GIC_INTERNAL) / 32) - 1;
> +        int itlinesnumber = (s->num_irq / 32) - 1;
>          /*
>           * SecurityExtn must be RAZ if GICD_CTLR.DS == 1, and
>           * "security extensions not supported" always implies DS == 1,