Re: [Qemu-devel] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the ICPIDR2 register

[Alistair Francis]

On Mon, Jan 18, 2016 at 2:13 PM, Peter Maydell <address@hidden> wrote:
> On 18 January 2016 at 21:41, Alistair Francis
> <address@hidden> wrote:
>> On Mon, Jan 11, 2016 at 5:58 AM, Peter Maydell <address@hidden> wrote:
>>> The current implementation of the ID registers seems to be
>>> simply "like the 11MPCore interrupt controller". I think we
>>> should get them right more generally if we're going to fix them:
>>>
>>>                        fd0 .. fdc     fe0 .. fec      ff0 ... ffc
>>> 11MPCore               reserved       90 13 04 00     0d f0 05 b1
>>> ARM GICv1 (eg A9)      04 00 00 00    90 b3 1b 00     0d f0 05 b1
>>> ARM GICv2 (eg A15)     04 00 00 00    90 b4 2b 00     0d f0 05 b1
>>>
>>> Your patch doesn't account for ICPIDR1 also having a field that
>>> changes between GICv1 and GICv2 (for ARM implementations), and
>>> we're missing the fd0..fdc bytes.
>>>
>>> I think this is probably simplest modelled with several
>>> gic_id arrays and using the appropriate one for 11MPCore/GICv1/GICv2,
>>> rather than trying to OR extra values into the 11MPCore ID values.
>>
>> Ok, just to make sure I am reading this right. You think I should just
>> create three arrays and then if() the revision to determine which one
>> to use?
>
> Yes, something like that. The fd0..fdc being reserved for 11MPCore
> is documented to mean RAZ/WI, so you can just make those array elements
> zeroes.

Ok, sounds good to me. I'm about to send a patch out. As basically the
whole patch and title changed it is back to V1.

Thanks,

Alistair

>
> thanks
> -- PMM
>