Re: [PATCH 1/4] arm: ast2600: Force a multiple of 32 of IRQs for the GIC

[Andrew Jeffery]

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