Re: [Qemu-devel] [PATCH 4/9] arm: add dummy gic security registers

[Peter Maydell]

On 20 December 2011 21:06, Mark Langsdorf <address@hidden> wrote:
> On 12/20/2011 01:58 PM, Peter Maydell wrote:
>> On 20 December 2011 19:11, Mark Langsdorf<address@hidden>  wrote:
>>> From: Rob Herring<address@hidden>
>>>
>>> Signed-off-by: Rob Herring<address@hidden>
>>> Signed-off-by: Mark Langsdorf<address@hidden>
>>> ---
>>>   hw/arm_gic.c |   10 ++++++++--
>>>   1 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
>>> index 9b52119..5974c2f 100644
>>> --- a/hw/arm_gic.c
>>> +++ b/hw/arm_gic.c
>>> @@ -274,7 +274,7 @@ static uint32_t gic_dist_readb(void *opaque,
>>> target_phys_addr_t offset)
>>>
>>>      cpu = gic_get_current_cpu();
>>>      cm = 1<<  cpu;
>>> -    if (offset<  0x100) {
>>> +    if (offset<  0x80) {
>>>   #ifndef NVIC
>>>          if (offset == 0)
>>>              return s->enabled;
>>> @@ -284,6 +284,9 @@ static uint32_t gic_dist_readb(void *opaque,
>>> target_phys_addr_t offset)
>>>              return 0;
>>>   #endif
>>>          goto bad_reg;
>>> +    } else if (offset<  0x100) {
>>> +        /* Interrupt Security */
>>> +        return 0;
>>
>> This won't actually break anything, but really the handling of 0x80..0xff
>> would be inside the first if() clause, because of the way we piggyback
>> the v7M NVIC off these functions. (We should clean that up, really).
>> Anyway, the v7M NVIC doesn't own 0x0..0xff, which is what the first
>> if () clause is marking off. Ditto in the write function.
>>
>> It would also be nice to have the comment explicitly say that these
>> registers are defined to RAZ/WI in GIC implementations that don't
>> implement the security extensions.
>>
> I'm not sure what changes you want made here. I can resubmit with
> the comment. But I don't know how qemu piggybacks the v7M NVIC so I
> don't understand what you want done.

I just mean that you want something like

    if (offset < 0x100) {
#ifndef NVIC
        if (offset == 0)
            return s->enabled;
        if (offset == 4)
            return ((GIC_NIRQ / 32) - 1) | ((NUM_CPU(s) - 1) << 5);
        if (offset < 0x08)
            return 0;
        if (offset >= 0x80 && offset < 0x100) {
            /* Interrupt security, RAZ/WI */
            return 0;
        }
#endif
        goto bad_reg;
    } else if (offset < 0x200) {

to keep this GIC-only register inside the not-NVIC ifdef.

(I really want to clean this up but I can't do that until we can
properly have memory regions which don't start at a page boundary
and which pass the right offset through.)

-- PMM