qemu-arm
[Top][All Lists]
Advanced

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

Re: [Qemu-arm] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the IC


From: Peter Maydell
Subject: Re: [Qemu-arm] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the ICPIDR2 register
Date: Mon, 11 Jan 2016 13:58:15 +0000

On 8 January 2016 at 18:57, Alistair Francis
<address@hidden> wrote:
> The ARM GIC documentation (page 4-119) describes that bits
> 7 to 4 of the ICPIDR2 register should include the GIC architecture
> version. This patche ORs the version into the existing return value.
>
> Signed-off-by: Alistair Francis <address@hidden>
> Tested-by: Sören Brinkmann <address@hidden>
> ---
>
>  hw/intc/arm_gic.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 13e297d..f47d606 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -688,6 +688,10 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr 
> offset, MemTxAttrs attrs)
>      } else /* offset >= 0xfe0 */ {
>          if (offset & 3) {
>              res = 0;
> +        } else if (offset == 0xfe8 && s->revision != REV_11MPCORE &&
> +                                      s->revision != REV_NVIC) {
> +            /* ICPIDR2 includes the GICv1 or GICv2 version information */
> +            res = gic_id[(offset - 0xfe0) >> 2] | (s->revision << 4);
>          } else {
>              res = gic_id[(offset - 0xfe0) >> 2];
>          }

The current implementation of the ID registers seems to be
simply "like the 11MPCore interrupt controller". I think we
should get them right more generally if we're going to fix them:

                       fd0 .. fdc     fe0 .. fec      ff0 ... ffc
11MPCore               reserved       90 13 04 00     0d f0 05 b1
ARM GICv1 (eg A9)      04 00 00 00    90 b3 1b 00     0d f0 05 b1
ARM GICv2 (eg A15)     04 00 00 00    90 b4 2b 00     0d f0 05 b1

Your patch doesn't account for ICPIDR1 also having a field that
changes between GICv1 and GICv2 (for ARM implementations), and
we're missing the fd0..fdc bytes.

I think this is probably simplest modelled with several
gic_id arrays and using the appropriate one for 11MPCore/GICv1/GICv2,
rather than trying to OR extra values into the 11MPCore ID values.

For the NVIC these registers don't exist at all, and the
construction of the memory regions in armv7m_nvic.c ensures
we can't reach this bit of the function, so we don't need
to specially handle it. (Also there's a rewrite of the NVIC
to disentangle it from the GIC which hopefully will land
some time before 2.6.)

thanks
-- PMM



reply via email to

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