[Qemu-devel] [PATCH] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Qemu-devel] [PATCH] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Meador Inge]

When setting up the NVIC memory regions the memory range
0x100..0xcff is aliased to an IO memory region that belongs
to the ARM GIC.  This aliased region should be added to the
NVIC memory container, but the actual GIC IO memory region
was being added instead.  This mixup was causing the wrong
IO memory access functions to be called when accessing parts
of the NVIC memory.

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 hw/armv7m_nvic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index 6a0832e..5c09116 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -489,7 +489,8 @@ static int armv7m_nvic_init(SysBusDevice *dev)
      */
     memory_region_init_alias(&s->gic_iomem_alias, "nvic-gic", &s->gic.iomem,
                              0x100, 0xc00);
-    memory_region_add_subregion_overlap(&s->container, 0x100, &s->gic.iomem, 1);
+    memory_region_add_subregion_overlap(&s->container, 0x100,
+                                        &s->gic_iomem_alias, 1);
     /* Map the whole thing into system memory at the location required
      * by the v7M architecture.
      */
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Peter Maydell]

On 28 August 2012 00:41, Meador Inge <meadori@codesourcery.com> wrote:
> When setting up the NVIC memory regions the memory range
> 0x100..0xcff is aliased to an IO memory region that belongs
> to the ARM GIC.  This aliased region should be added to the
> NVIC memory container, but the actual GIC IO memory region
> was being added instead.  This mixup was causing the wrong
> IO memory access functions to be called when accessing parts
> of the NVIC memory.
>
> Signed-off-by: Meador Inge <meadori@codesourcery.com>

Whoops.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH for-1.2] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Peter Maydell]

Hi Anthony; I realised somewhat belatedly that this bugfix
should probably go into 1.2 if possible. Can we still
squeeze it in or is it too late?

Patchwork url: http://patchwork.ozlabs.org/patch/180315/

thanks
-- PMM

On 28 August 2012 00:41, Meador Inge <meadori@codesourcery.com> wrote:
> When setting up the NVIC memory regions the memory range
> 0x100..0xcff is aliased to an IO memory region that belongs
> to the ARM GIC.  This aliased region should be added to the
> NVIC memory container, but the actual GIC IO memory region
> was being added instead.  This mixup was causing the wrong
> IO memory access functions to be called when accessing parts
> of the NVIC memory.
>
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
>  hw/armv7m_nvic.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
> index 6a0832e..5c09116 100644
> --- a/hw/armv7m_nvic.c
> +++ b/hw/armv7m_nvic.c
> @@ -489,7 +489,8 @@ static int armv7m_nvic_init(SysBusDevice *dev)
>       */
>      memory_region_init_alias(&s->gic_iomem_alias, "nvic-gic", &s->gic.iomem,
>                               0x100, 0xc00);
> -    memory_region_add_subregion_overlap(&s->container, 0x100, &s->gic.iomem, 1);
> +    memory_region_add_subregion_overlap(&s->container, 0x100,
> +                                        &s->gic_iomem_alias, 1);
>      /* Map the whole thing into system memory at the location required
>       * by the v7M architecture.
>       */
> --
> 1.7.7.6
>

Re: [Qemu-devel] [PATCH for-1.2] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> Hi Anthony; I realised somewhat belatedly that this bugfix
> should probably go into 1.2 if possible. Can we still
> squeeze it in or is it too late?
>
> Patchwork url: http://patchwork.ozlabs.org/patch/180315/

It's too late for GA.  Can go in via stable though.

Regards,

Anthony Liguori

>
> thanks
> -- PMM
>
> On 28 August 2012 00:41, Meador Inge <meadori@codesourcery.com> wrote:
>> When setting up the NVIC memory regions the memory range
>> 0x100..0xcff is aliased to an IO memory region that belongs
>> to the ARM GIC.  This aliased region should be added to the
>> NVIC memory container, but the actual GIC IO memory region
>> was being added instead.  This mixup was causing the wrong
>> IO memory access functions to be called when accessing parts
>> of the NVIC memory.
>>
>> Signed-off-by: Meador Inge <meadori@codesourcery.com>
>> ---
>>  hw/armv7m_nvic.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
>> index 6a0832e..5c09116 100644
>> --- a/hw/armv7m_nvic.c
>> +++ b/hw/armv7m_nvic.c
>> @@ -489,7 +489,8 @@ static int armv7m_nvic_init(SysBusDevice *dev)
>>       */
>>      memory_region_init_alias(&s->gic_iomem_alias, "nvic-gic", &s->gic.iomem,
>>                               0x100, 0xc00);
>> -    memory_region_add_subregion_overlap(&s->container, 0x100, &s->gic.iomem, 1);
>> +    memory_region_add_subregion_overlap(&s->container, 0x100,
>> +                                        &s->gic_iomem_alias, 1);
>>      /* Map the whole thing into system memory at the location required
>>       * by the v7M architecture.
>>       */
>> --
>> 1.7.7.6
>>

Re: [Qemu-devel] [PATCH for-1.2] hw/armv7m_nvic: Correctly register GIC region when setting up NVIC

[Peter Maydell]

On 31 August 2012 19:16, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> Hi Anthony; I realised somewhat belatedly that this bugfix
>> should probably go into 1.2 if possible. Can we still
>> squeeze it in or is it too late?
>>
>> Patchwork url: http://patchwork.ozlabs.org/patch/180315/
>
> It's too late for GA.  Can go in via stable though.

Oh well, never mind. (I don't generally bother to feed
ARM patches to stable; I figure people can wait til the
next mainline release.)

-- PMM