Re: [Qemu-devel] [PATCH 4/9] arm: add dummy gic security registers

[Peter Maydell]

On 20 December 2011 19:11, Mark Langsdorf <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Signed-off-by: Rob Herring <address@hidden>
> Signed-off-by: Mark Langsdorf <address@hidden>
> ---
>  hw/arm_gic.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> index 9b52119..5974c2f 100644
> --- a/hw/arm_gic.c
> +++ b/hw/arm_gic.c
> @@ -274,7 +274,7 @@ static uint32_t gic_dist_readb(void *opaque,
> target_phys_addr_t offset)
>
>     cpu = gic_get_current_cpu();
>     cm = 1 << cpu;
> -    if (offset < 0x100) {
> +    if (offset < 0x80) {
>  #ifndef NVIC
>         if (offset == 0)
>             return s->enabled;
> @@ -284,6 +284,9 @@ static uint32_t gic_dist_readb(void *opaque,
> target_phys_addr_t offset)
>             return 0;
>  #endif
>         goto bad_reg;
> +    } else if (offset < 0x100) {
> +        /* Interrupt Security */
> +        return 0;

This won't actually break anything, but really the handling of 0x80..0xff
would be inside the first if() clause, because of the way we piggyback
the v7M NVIC off these functions. (We should clean that up, really).
Anyway, the v7M NVIC doesn't own 0x0..0xff, which is what the first
if () clause is marking off. Ditto in the write function.

It would also be nice to have the comment explicitly say that these
registers are defined to RAZ/WI in GIC implementations that don't
implement the security extensions.

(Strictly speaking, in the 11MPcore GIC these locations are reserved
and probably don't RAZ/WI, but I don't think we need to worry about
that wrinkle now.)

>     } else if (offset < 0x200) {
>         /* Interrupt Set/Clear Enable.  */
>         if (offset < 0x180)
> @@ -404,7 +407,7 @@ static void gic_dist_writeb(void *opaque,
> target_phys_addr_t offset,
>     int cpu;
>
>     cpu = gic_get_current_cpu();
> -    if (offset < 0x100) {
> +    if (offset < 0x80) {
>  #ifdef NVIC
>         goto bad_reg;
>  #else
> @@ -417,6 +420,9 @@ static void gic_dist_writeb(void *opaque,
> target_phys_addr_t offset,
>             goto bad_reg;
>         }
>  #endif
> +    } else if (offset < 0x100) {
> +        /* Interrupt Security Registers */
> +        /* ignore */
>     } else if (offset < 0x180) {
>         /* Interrupt Set Enable.  */
>         irq = (offset - 0x100) * 8 + GIC_BASE_IRQ;
> --
> 1.7.5.4
>

-- PMM