qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 1/1] arm_gic: Update ID registers based on re


From: Alistair Francis
Subject: Re: [Qemu-devel] [PATCH v2 1/1] arm_gic: Update ID registers based on revision
Date: Wed, 20 Jan 2016 15:49:04 -0800

On Wed, Jan 20, 2016 at 3:37 PM, Peter Maydell <address@hidden> wrote:
> On 20 January 2016 at 23:08, Alistair Francis
> <address@hidden> wrote:
>> On Wed, Jan 20, 2016 at 2:48 PM, Peter Maydell <address@hidden> wrote:
>>> On 20 January 2016 at 22:42, Alistair Francis
>>> <address@hidden> wrote:
>>>> Update the GIC ID registers (registers above 0xfe0) based on the GIC
>>>> revision instead of using the sames values for all GIC implementations.
>>>>
>>>> Signed-off-by: Alistair Francis <address@hidden>
>>>> Tested-by: Sören Brinkmann <address@hidden>
>>>> ---
>>>> V2:
>>>>  - Update array indexing to match new values
>>>>  - Check the maximum value of offset as well
>>>>
>>>>  hw/intc/arm_gic.c | 35 ++++++++++++++++++++++++++++++-----
>>>>  1 file changed, 30 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>>>> index 13e297d..8eb3a2d 100644
>>>> --- a/hw/intc/arm_gic.c
>>>> +++ b/hw/intc/arm_gic.c
>>>> @@ -31,8 +31,16 @@ do { fprintf(stderr, "arm_gic: " fmt , ## __VA_ARGS__); 
>>>> } while (0)
>>>>  #define DPRINTF(fmt, ...) do {} while(0)
>>>>  #endif
>>>>
>>>> -static const uint8_t gic_id[] = {
>>>> -    0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
>>>> +static const uint8_t gic_id_11mpcore[] = {
>>>> +    0x00, 0x00, 0x00, 0x00, 0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
>>>> +};
>>>> +
>>>> +static const uint8_t gic_id_gicv1[] = {
>>>> +    0x04, 0x00, 0x00, 0x00, 0x90, 0xb3, 0x1b, 0x00, 0x0d, 0xf0, 0x05, 0xb1
>>>> +};
>>>> +
>>>> +static const uint8_t gic_id_gicv2[] = {
>>>> +    0x04, 0x00, 0x00, 0x00, 0x90, 0xb4, 0x2b, 0x00, 0x0d, 0xf0, 0x05, 0xb1
>>>>  };
>>>>
>>>>  static inline int gic_get_current_cpu(GICState *s)
>>>> @@ -683,13 +691,30 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr 
>>>> offset, MemTxAttrs attrs)
>>>>          }
>>>>
>>>>          res = s->sgi_pending[irq][cpu];
>>>> -    } else if (offset < 0xfe0) {
>>>> +    } else if (offset < 0xfd0) {
>>>>          goto bad_reg;
>>>> -    } else /* offset >= 0xfe0 */ {
>>>> +    } else /* offset >= 0xfd0 */ {
>>>>          if (offset & 3) {
>>>>              res = 0;
>>>> +        } else if (offset > 0xfdf) {
>>>> +            goto bad_reg;
>>>
>>> Where did this condition come from? It will mean all
>>> the ID regs fe0 and above end up treated as bad registers
>>> rather than taking the code below to return the ID value.
>>
>> Woops, I copied the wrong value. I mean that to be 0xffc
>>
>> Do you still want a maximum check (otherwise it will walk off the array)?
>
> FFD..FFF should read as zeroes (like the other non-multiple-of-4
> offset addresses in this region), and you can't get more than 0xfff
> because the size of the memory region is 0x1000. That's not
> entirely obvious though, so if you want a max check you can
> put one in. I think that having it match the style of the rest
> of the if..else if ladder would be better though:
>
>   ...
>  else if (offset < 0x1000) {
>      array code here;
>  else {
>      g_assert_not_reached();
>  }
>

Ok, I like that.

Thanks,

Alistair

> or something.
>
> thanks
> -- PMM
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]