[Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Leon Alrae]

BadVAddr is supposed to capture the most recent address that caused
the exception. Currently this is not happening as translation is not stopped
and BadVAddr is updated with subsequent addresses.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index f33c10c..635192c 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -18438,6 +18438,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
     if (ctx->pc & 0x3) {
         env->CP0_BadVAddr = ctx->pc;
         generate_exception_err(ctx, EXCP_AdEL, EXCP_INST_NOTAVAIL);
+        ctx->bstate = BS_STOP;
         return;
     }
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Maciej W. Rozycki]

On Mon, 26 Jan 2015, Leon Alrae wrote:

> BadVAddr is supposed to capture the most recent address that caused
> the exception. Currently this is not happening as translation is not stopped
> and BadVAddr is updated with subsequent addresses.
> 
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---

 I think this deserves a better description as it is about the specific 
case of an unaligned standard MIPS instruction fetch.  Address Error 
exceptions can also happen for other reasons: unaligned data accesses or 
any accesses outside memory segments the current execution mode is allowed 
to reach.

 While at it I think it may be worth double-checking if the other places 
that trigger this exception get it right.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Leon Alrae]

On 28/01/2015 00:39, Maciej W. Rozycki wrote:
> On Mon, 26 Jan 2015, Leon Alrae wrote:
> 
>> BadVAddr is supposed to capture the most recent address that caused
>> the exception. Currently this is not happening as translation is not stopped
>> and BadVAddr is updated with subsequent addresses.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
> 
>  I think this deserves a better description as it is about the specific 
> case of an unaligned standard MIPS instruction fetch.  Address Error 
> exceptions can also happen for other reasons: unaligned data accesses or 
> any accesses outside memory segments the current execution mode is allowed 
> to reach.

I believe that the one line change in the patch makes that clear. I
agree however that the description itself could be more precise.

>  While at it I think it may be worth double-checking if the other places 
> that trigger this exception get it right.

Other places seem to look fine. Even decode_micromips_opc handles it
correctly whereas decode_opc -– which obviously was implemented before
microMIPS -- never got fixed.

Regards,
Leon

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Maciej W. Rozycki]

On Thu, 19 Feb 2015, Leon Alrae wrote:

> >  I think this deserves a better description as it is about the specific 
> > case of an unaligned standard MIPS instruction fetch.  Address Error 
> > exceptions can also happen for other reasons: unaligned data accesses or 
> > any accesses outside memory segments the current execution mode is allowed 
> > to reach.
> 
> I believe that the one line change in the patch makes that clear. I
> agree however that the description itself could be more precise.

 I agree the patch itself is clear, but someone reading the commit might 
infer this is the only place the Address Error exception happens.  Also 
the patch itself does not show up in some places, e.g. `git log' where -p 
hasn't been also supplied, to complement information provided in the log.

> >  While at it I think it may be worth double-checking if the other places 
> > that trigger this exception get it right.
> 
> Other places seem to look fine. Even decode_micromips_opc handles it
> correctly whereas decode_opc -– which obviously was implemented before
> microMIPS -- never got fixed.

 Great that you checked, thanks!

  Maciej