Patchwork hw/omap_gpio: Fix infinite recursion when doing 8/16 bit reads

login
register
mail settings
Submitter Peter Maydell
Date Nov. 7, 2011, 1:25 p.m.
Message ID <1320672345-5776-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/124073/
State New
Headers show

Comments

Peter Maydell - Nov. 7, 2011, 1:25 p.m.
Fix a long-standing bug which meant that any attempt to do an
8 or 16 bit read from the OMAP GPIO module would cause qemu to
crash due to an infinite recursion.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This has actually been in the code since the original OMAP2 support
was added in 2008; we've never noticed before because the kernel happened
to always do 32 bit accesses...
Long term we should fix this by conversion to MemoryRegion; this is
the minimally invasive fix for 1.0.

 hw/omap_gpio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Peter Maydell - Nov. 12, 2011, 12:38 a.m.
Anthony -- you've committed a bunch of later patches but not this one:
did you miss it?

thanks
-- PMM

On 7 November 2011 13:25, Peter Maydell <peter.maydell@linaro.org> wrote:
> Fix a long-standing bug which meant that any attempt to do an
> 8 or 16 bit read from the OMAP GPIO module would cause qemu to
> crash due to an infinite recursion.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This has actually been in the code since the original OMAP2 support
> was added in 2008; we've never noticed before because the kernel happened
> to always do 32 bit accesses...
> Long term we should fix this by conversion to MemoryRegion; this is
> the minimally invasive fix for 1.0.
>
>  hw/omap_gpio.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
> index d775df6..d630748 100644
> --- a/hw/omap_gpio.c
> +++ b/hw/omap_gpio.c
> @@ -510,7 +510,7 @@ static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
>
>  static uint32_t omap2_gpio_module_readp(void *opaque, target_phys_addr_t addr)
>  {
> -    return omap2_gpio_module_readp(opaque, addr) >> ((addr & 3) << 3);
> +    return omap2_gpio_module_read(opaque, addr & ~3) >> ((addr & 3) << 3);
>  }
>
>  static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,
> --
> 1.7.4.1
Anthony Liguori - Nov. 12, 2011, 2:57 a.m.
On 11/11/2011 06:38 PM, Peter Maydell wrote:
> Anthony -- you've committed a bunch of later patches but not this one:
> did you miss it?

Most likely an over zealous delete on my part.  I'll requeue.

Regards,

Anthony Liguori

>
> thanks
> -- PMM
>
> On 7 November 2011 13:25, Peter Maydell<peter.maydell@linaro.org>  wrote:
>> Fix a long-standing bug which meant that any attempt to do an
>> 8 or 16 bit read from the OMAP GPIO module would cause qemu to
>> crash due to an infinite recursion.
>>
>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
>> ---
>> This has actually been in the code since the original OMAP2 support
>> was added in 2008; we've never noticed before because the kernel happened
>> to always do 32 bit accesses...
>> Long term we should fix this by conversion to MemoryRegion; this is
>> the minimally invasive fix for 1.0.
>>
>>   hw/omap_gpio.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
>> index d775df6..d630748 100644
>> --- a/hw/omap_gpio.c
>> +++ b/hw/omap_gpio.c
>> @@ -510,7 +510,7 @@ static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
>>
>>   static uint32_t omap2_gpio_module_readp(void *opaque, target_phys_addr_t addr)
>>   {
>> -    return omap2_gpio_module_readp(opaque, addr)>>  ((addr&  3)<<  3);
>> +    return omap2_gpio_module_read(opaque, addr&  ~3)>>  ((addr&  3)<<  3);
>>   }
>>
>>   static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,
>> --
>> 1.7.4.1
>
>
Anthony Liguori - Nov. 15, 2011, 3:04 p.m.
On 11/07/2011 07:25 AM, Peter Maydell wrote:
> Fix a long-standing bug which meant that any attempt to do an
> 8 or 16 bit read from the OMAP GPIO module would cause qemu to
> crash due to an infinite recursion.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
> This has actually been in the code since the original OMAP2 support
> was added in 2008; we've never noticed before because the kernel happened
> to always do 32 bit accesses...
> Long term we should fix this by conversion to MemoryRegion; this is
> the minimally invasive fix for 1.0.
>
>   hw/omap_gpio.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
> index d775df6..d630748 100644
> --- a/hw/omap_gpio.c
> +++ b/hw/omap_gpio.c
> @@ -510,7 +510,7 @@ static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
>
>   static uint32_t omap2_gpio_module_readp(void *opaque, target_phys_addr_t addr)
>   {
> -    return omap2_gpio_module_readp(opaque, addr)>>  ((addr&  3)<<  3);
> +    return omap2_gpio_module_read(opaque, addr&  ~3)>>  ((addr&  3)<<  3);
>   }
>
>   static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,

Patch

diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
index d775df6..d630748 100644
--- a/hw/omap_gpio.c
+++ b/hw/omap_gpio.c
@@ -510,7 +510,7 @@  static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
 
 static uint32_t omap2_gpio_module_readp(void *opaque, target_phys_addr_t addr)
 {
-    return omap2_gpio_module_readp(opaque, addr) >> ((addr & 3) << 3);
+    return omap2_gpio_module_read(opaque, addr & ~3) >> ((addr & 3) << 3);
 }
 
 static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,