[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: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/1] arm_gic: Update ID registers based on revision |
Date: |
Wed, 20 Jan 2016 23:37:32 +0000 |
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();
}
or something.
thanks
-- PMM