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

[Peter Maydell]

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.

>          } else {
> -            res = gic_id[(offset - 0xfe0) >> 2];
> +            switch (s->revision) {
> +            case REV_11MPCORE:
> +                res = gic_id_11mpcore[(offset - 0xfd0) >> 2];
> +                break;
> +            case 1:
> +                res = gic_id_gicv1[(offset - 0xfd0) >> 2];
> +                break;
> +            case 2:
> +                res = gic_id_gicv2[(offset - 0xfd0) >> 2];
> +                break;
> +            case REV_NVIC:
> +                /* Shouldn't be able to get here */
> +                abort();
> +            default:
> +                res = 0;
> +            }
>          }
>      }
>      return res;
> --
> 2.5.0
>

thanks
-- PMM