Patchwork sh7750: Fix crash when accessing PVR/PRR/CVR

login
register
mail settings
Submitter Peter Maydell
Date March 26, 2013, 12:20 p.m.
Message ID <1364300401-9340-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/231184/
State New
Headers show

Comments

Peter Maydell - March 26, 2013, 12:20 p.m.
Commit b350ab75 causes segfaults on accesses to PVR/PRR/CVR because
it tries to call SUPERH_CPU_GET_CLASS() on a pointer that isn't a
QOM object. Fix this by getting the actual QOM CPU object first.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Tested with the r2d image/instructions from
https://oss.renesas.com/modules/document/?Getting%20Started%20with%20SH4%20and%20QEMU

 hw/sh4/sh7750.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Andreas Färber - March 26, 2013, 2:35 p.m.
Am 26.03.2013 13:20, schrieb Peter Maydell:
> Commit b350ab75 causes segfaults on accesses to PVR/PRR/CVR because
> it tries to call SUPERH_CPU_GET_CLASS() on a pointer that isn't a
> QOM object. Fix this by getting the actual QOM CPU object first.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Tested with the r2d image/instructions from
> https://oss.renesas.com/modules/document/?Getting%20Started%20with%20SH4%20and%20QEMU

Oops, reproduces with the test image from the QEMU Wiki as well. Seems
to be a result of cherry-picking this commit before the full SH7750
QOM'ification.

Is this blocking any work of yours? Otherwise I would try rebasing my
SH7750 patches so that s->cpu becomes a SuperHCPU as expected here.
(Not sure if simply reverting my offending patch still works?)

Andreas

> 
>  hw/sh4/sh7750.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
> index e4d37ad..3580c87 100644
> --- a/hw/sh4/sh7750.c
> +++ b/hw/sh4/sh7750.c
> @@ -289,13 +289,13 @@ static uint32_t sh7750_mem_readl(void *opaque, hwaddr addr)
>      case SH7750_CCR_A7:
>  	return s->ccr;
>      case 0x1f000030:		/* Processor version */
> -        scc = SUPERH_CPU_GET_CLASS(s->cpu);
> +        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
>          return scc->pvr;
>      case 0x1f000040:		/* Cache version */
> -        scc = SUPERH_CPU_GET_CLASS(s->cpu);
> +        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
>          return scc->cvr;
>      case 0x1f000044:		/* Processor revision */
> -        scc = SUPERH_CPU_GET_CLASS(s->cpu);
> +        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
>          return scc->prr;
>      default:
>  	error_access("long read", addr);
>
Peter Maydell - March 26, 2013, 2:44 p.m.
On 26 March 2013 14:35, Andreas Färber <afaerber@suse.de> wrote:
> Am 26.03.2013 13:20, schrieb Peter Maydell:
>> Commit b350ab75 causes segfaults on accesses to PVR/PRR/CVR because
>> it tries to call SUPERH_CPU_GET_CLASS() on a pointer that isn't a
>> QOM object. Fix this by getting the actual QOM CPU object first.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> Tested with the r2d image/instructions from
>> https://oss.renesas.com/modules/document/?Getting%20Started%20with%20SH4%20and%20QEMU
>
> Oops, reproduces with the test image from the QEMU Wiki as well. Seems
> to be a result of cherry-picking this commit before the full SH7750
> QOM'ification.
>
> Is this blocking any work of yours? Otherwise I would try rebasing my
> SH7750 patches so that s->cpu becomes a SuperHCPU as expected here.
> (Not sure if simply reverting my offending patch still works?)

Well, I have a workaround so it doesn't affect me now :-)
I wasn't really doing anything much with the sh4 board, I was just
looking at whether we could get rid of its use of taddr properties.
(Oddly it uses qdev_prop_set_taddr() to set a property which isn't
defined as a TADDR property, but this works anyway...)

-- PMM
Peter Maydell - April 8, 2013, 12:52 p.m.
On 26 March 2013 14:35, Andreas Färber <afaerber@suse.de> wrote:
> Am 26.03.2013 13:20, schrieb Peter Maydell:
>> Commit b350ab75 causes segfaults on accesses to PVR/PRR/CVR because
>> it tries to call SUPERH_CPU_GET_CLASS() on a pointer that isn't a
>> QOM object. Fix this by getting the actual QOM CPU object first.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> Tested with the r2d image/instructions from
>> https://oss.renesas.com/modules/document/?Getting%20Started%20with%20SH4%20and%20QEMU
>
> Oops, reproduces with the test image from the QEMU Wiki as well. Seems
> to be a result of cherry-picking this commit before the full SH7750
> QOM'ification.
>
> Is this blocking any work of yours? Otherwise I would try rebasing my
> SH7750 patches so that s->cpu becomes a SuperHCPU as expected here.
> (Not sure if simply reverting my offending patch still works?)

Ping -- were you planning to submit these patches soon or should
we just commit this patch to fix the breakage for now?

thanks
-- PMM
Andreas Färber - April 9, 2013, 2:52 p.m.
Am 08.04.2013 14:52, schrieb Peter Maydell:
> On 26 March 2013 14:35, Andreas Färber <afaerber@suse.de> wrote:
>> Am 26.03.2013 13:20, schrieb Peter Maydell:
>>> Commit b350ab75 causes segfaults on accesses to PVR/PRR/CVR because
>>> it tries to call SUPERH_CPU_GET_CLASS() on a pointer that isn't a
>>> QOM object. Fix this by getting the actual QOM CPU object first.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> Tested with the r2d image/instructions from
>>> https://oss.renesas.com/modules/document/?Getting%20Started%20with%20SH4%20and%20QEMU
>>
>> Oops, reproduces with the test image from the QEMU Wiki as well. Seems
>> to be a result of cherry-picking this commit before the full SH7750
>> QOM'ification.
>>
>> Is this blocking any work of yours? Otherwise I would try rebasing my
>> SH7750 patches so that s->cpu becomes a SuperHCPU as expected here.
>> (Not sure if simply reverting my offending patch still works?)
> 
> Ping -- were you planning to submit these patches soon or should
> we just commit this patch to fix the breakage for now?

Submitted now, sorry for the delay.

Andreas

Patch

diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index e4d37ad..3580c87 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -289,13 +289,13 @@  static uint32_t sh7750_mem_readl(void *opaque, hwaddr addr)
     case SH7750_CCR_A7:
 	return s->ccr;
     case 0x1f000030:		/* Processor version */
-        scc = SUPERH_CPU_GET_CLASS(s->cpu);
+        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
         return scc->pvr;
     case 0x1f000040:		/* Cache version */
-        scc = SUPERH_CPU_GET_CLASS(s->cpu);
+        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
         return scc->cvr;
     case 0x1f000044:		/* Processor revision */
-        scc = SUPERH_CPU_GET_CLASS(s->cpu);
+        scc = SUPERH_CPU_GET_CLASS(ENV_GET_CPU(s->cpu));
         return scc->prr;
     default:
 	error_access("long read", addr);