| Message ID | 20210226065758.547824-2-andrew@aj.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: LPC peripheral controller devices | expand |
On 2/26/21 7:57 AM, Andrew Jeffery wrote: > This appears to be a requirement of the GIC model. If so this should be adjusted in the GIC or a15mp_priv_realize(), not in each caller, isn't it? > The AST2600 allocates > 197 GIC IRQs, which we will adjust shortly. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > hw/arm/aspeed_ast2600.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-)
On Fri, 26 Feb 2021, at 19:26, Philippe Mathieu-Daudé wrote: > On 2/26/21 7:57 AM, Andrew Jeffery wrote: > > This appears to be a requirement of the GIC model. > > If so this should be adjusted in the GIC or a15mp_priv_realize(), > not in each caller, isn't it? > Maybe, let me look into it. I'll clean it up in v2 if it makes sense. Cheers, Andrew
On Mon, 1 Mar 2021, at 09:37, Andrew Jeffery wrote: > > > On Fri, 26 Feb 2021, at 19:26, Philippe Mathieu-Daudé wrote: > > On 2/26/21 7:57 AM, Andrew Jeffery wrote: > > > This appears to be a requirement of the GIC model. > > > > If so this should be adjusted in the GIC or a15mp_priv_realize(), > > not in each caller, isn't it? > > > > Maybe, let me look into it. I'll clean it up in v2 if it makes sense. So the current behaviour has been around since 2009, originating in 41c1e2f54e6f ("arm: make sure that number of irqs can be represented in GICD_TYPER."). The GIC architecture specification says: "The GICD_TYPER.ITLinesNumber field identifies the number of implemented GICD_ISENABLERns, and therefore the maximum number of SPIs that might be supported." While the code says: /* ITLinesNumber is represented as (N / 32) - 1 (see * gic_dist_readb) so this is an implementation imposed * restriction, not an architectural one: */ if (s->num_irq < 32 || (s->num_irq % 32)) { error_setg(errp, "%d interrupt lines unsupported: not divisible by 32", num_irq); return; } My feeling is that it's better to be explicit in the models that are affected (i.e. leave the ROUND_UP() as I have it in this patch). This way if the implementation restriction is ever lifted, we know which models we can clean up. I won't be reworking the GIC to remove the restriction in this series, so unless you have a particularly strong preference/justification for the implicit ROUND_UP(), I plan to leave it as is. Cheers, Andrew
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index bf31ca351feb..bc0eeb058b24 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -65,7 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { #define ASPEED_A7MPCORE_ADDR 0x40460000 -#define ASPEED_SOC_AST2600_MAX_IRQ 128 +#define AST2600_MAX_IRQ 128 /* Shared Peripheral Interrupt values below are offset by -32 from datasheet */ static const int aspeed_soc_ast2600_irqmap[] = { @@ -267,7 +267,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) object_property_set_int(OBJECT(&s->a7mpcore), "num-cpu", sc->num_cpus, &error_abort); object_property_set_int(OBJECT(&s->a7mpcore), "num-irq", - ASPEED_SOC_AST2600_MAX_IRQ + GIC_INTERNAL, + ROUND_UP(AST2600_MAX_IRQ + GIC_INTERNAL, 32), &error_abort); sysbus_realize(SYS_BUS_DEVICE(&s->a7mpcore), &error_abort);
This appears to be a requirement of the GIC model. The AST2600 allocates 197 GIC IRQs, which we will adjust shortly. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- hw/arm/aspeed_ast2600.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)