Re: [Qemu-arm] [RFC PATCH] hw/intc/arm_gic: handle Set-Active/Clear-Active registers

[Peter Maydell]

On 5 September 2016 at 16:45, Alex Bennée <address@hidden> wrote:
>
> Peter Maydell <address@hidden> writes:
>
>> On 5 September 2016 at 15:09, Alex Bennée <address@hidden> wrote:
>>>      } else if (offset < 0x400) {
>>> -        /* Interrupt Active.  */
>>> -        goto bad_reg;
>>> +        /* Interrupt Clear-Active  */
>>> +        irq = (offset - 0x380) * 8 + GIC_BASE_IRQ;
>>> +        if (irq >= s->num_irq)
>>> +            goto bad_reg;
>>
>> This is missing the check against s->revision.
>
> This was deliberate as the GICv1 reference was only mentioned in the
> GICD_ISACTIVERn description. Unfortunately the only documentation for
> GICs on silver.arm.com are for v2 and v3 so I had to guess what v1 had :-/

The GICv2 spec does actually say these registers don't exist
in v1: in the Table 4-1 Distributor register map, the entry
for GICD_ICACTIVERn is marked as having footnote 'e' applying,
which says "GICv2 only".

In the GICv1 spec range 0x380-0x3FC is marked Reserved (ie there
is no clear-active register set at all.) Ditto in the 11MPCore TRM
and the v7M ARM ARM. (The GICv1 register summary also appears in
the A9 MPCore TRM, though that's a bit of an obscure place to
know to look for it.)

As a rule of thumb, if unsure whether something exists in all
configurations, default to leaving it locked down/unimplemented.
We can always widen it later if guest software turns out to use
it, but it's much harder to later know we can remove something
safely once we've let a QEMU into the wild that implemented it.

thanks
-- PMM