Re: [RFC PATCH 4/4] hw/intc: make gicv3_idreg() distinguish between gicv3/gicv4

[Peter Maydell]

On Sun, 24 Jan 2021 at 02:53, Leif Lindholm <leif@nuviainc.com> wrote:
>
> Make gicv3_idreg() able to return either gicv3 or gicv4 data.
> Add a parameter to specify gic version.
>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
>  hw/intc/arm_gicv3_dist.c   |  2 +-
>  hw/intc/arm_gicv3_redist.c |  2 +-
>  hw/intc/gicv3_internal.h   | 12 ++++++++++--
>  3 files changed, 12 insertions(+), 4 deletions(-)

> -static inline uint32_t gicv3_idreg(int regoffset)
> +static inline uint32_t gicv3_idreg(int regoffset, int revision)

I would prefer to pass in the GICv3State* and let the function
look at s->revision.

>  {
>      /* Return the value of the CoreSight ID register at the specified
>       * offset from the first ID register (as found in the distributor
> @@ -331,7 +331,15 @@ static inline uint32_t gicv3_idreg(int regoffset)
>      static const uint8_t gicd_ids[] = {
>          0x44, 0x00, 0x00, 0x00, 0x92, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 
> 0xB1
>      };
> -    return gicd_ids[regoffset / 4];
> +    static const uint8_t gicdv4_ids[] = {
> +        0x44, 0x00, 0x00, 0x00, 0x92, 0xB4, 0x4B, 0x00, 0x0D, 0xF0, 0x05, 
> 0xB1
> +    };
> +
> +    if (revision == 3) {
> +        return gicd_ids[regoffset / 4];
> +    } else {
> +        return gicdv4_ids[regoffset / 4];
> +    }
>  }

Updating the comment "These values indicate an ARM implementation of a GICv3"
to add a note about what the new values are indicating would be nice.

thanks
-- PMM