| Message ID | 20190605061126.10244-6-david.brenken@efs-auto.org |
|---|---|
| State | New |
| Headers | show |
| Series | tricore: adding new instructions and fixing issues | expand |
Hi, On 6/5/19 8:11 AM, David Brenken wrote: > From: Georg Hofstetter <georg.hofstetter@efs-auto.de> > > Signed-off-by: Andreas Konopik <andreas.konopik@efs-auto.de> > Signed-off-by: David Brenken <david.brenken@efs-auto.de> > Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de> > Signed-off-by: Robert Rasche <robert.rasche@efs-auto.de> > Signed-off-by: Lars Biermanski <lars.biermanski@efs-auto.de> > > --- > target/tricore/translate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/tricore/translate.c b/target/tricore/translate.c > index db09f82c31..cdbc00d654 100644 > --- a/target/tricore/translate.c > +++ b/target/tricore/translate.c > @@ -8811,6 +8811,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) > target_ulong pc_start; > int num_insns = 0; > > + memset(&ctx, 0x00, sizeof(DisasContext)); > pc_start = tb->pc; > ctx.pc = pc_start; > ctx.saved_pc = -1; To me this looks like fixing a symptom instead of the root cause. Which commit did you bisect to? Do you have a reproducer? Cheers, Bastian
Hi Sebastian,
in translate.c:gen_mtcr() code accesses hflags within the structure:
if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) {
/* since we're caching PSW make this a special case */
The code expects the HFLAG set for kernel mode in (i guess) preparation for supporting operation modes.
There is no code modifying these flags.
The flags were introduced in 0aaeb11 and there it looks like it was expected to be zeroed - maybe you know a bit more.
As having a stack variable uninitialized is generally a bad idea, so my proposal was to zero it as a whole, as it would be for bss variables.
Alternatively we could initialize them explicitly to zero or TRICORE_HFLAG_SM.
ctx.hflags = TRICORE_HFLAG_SM;
Not sure though why a cpu state flag is within DisasContext.
Regards,
Georg
-----Ursprüngliche Nachricht-----
Von: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Gesendet: Mittwoch, 5. Juni 2019 11:02
An: David Brenken <david.brenken@efs-auto.org>; qemu-devel@nongnu.org
Cc: Biermanski, Lars (EFS-GH3) <lars.biermanski@efs-auto.de>; Hofstetter, Georg (EFS-GH2) <Georg.Hofstetter@efs-auto.de>; Brenken, David (EFS-GH2) <david.brenken@efs-auto.de>; Rasche, Robert (EFS-GH2) <robert.rasche@efs-auto.de>; Konopik, Andreas (EFS-GH2) <andreas.konopik@efs-auto.de>
Betreff: Re: [Qemu-devel] [PATCH 5/5] tricore: reset DisasContext before generating code
Hi,
On 6/5/19 8:11 AM, David Brenken wrote:
> From: Georg Hofstetter <georg.hofstetter@efs-auto.de>
>
> Signed-off-by: Andreas Konopik <andreas.konopik@efs-auto.de>
> Signed-off-by: David Brenken <david.brenken@efs-auto.de>
> Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
> Signed-off-by: Robert Rasche <robert.rasche@efs-auto.de>
> Signed-off-by: Lars Biermanski <lars.biermanski@efs-auto.de>
>
> ---
> target/tricore/translate.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index db09f82c31..cdbc00d654 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -8811,6 +8811,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> target_ulong pc_start;
> int num_insns = 0;
>
> + memset(&ctx, 0x00, sizeof(DisasContext));
> pc_start = tb->pc;
> ctx.pc = pc_start;
> ctx.saved_pc = -1;
To me this looks like fixing a symptom instead of the root cause. Which commit did you bisect to? Do you have a reproducer?
Cheers,
Bastian
Hi Georg, On 6/6/19 1:44 PM, Hofstetter, Georg (EFS-GH2) wrote: > Hi Sebastian, > > in translate.c:gen_mtcr() code accesses hflags within the structure: > if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) { > /* since we're caching PSW make this a special case */ > > The code expects the HFLAG set for kernel mode in (i guess) preparation for supporting operation modes. > There is no code modifying these flags. > The flags were introduced in 0aaeb11 and there it looks like it was expected to be zeroed - maybe you know a bit more. Yep, the ctx->hflags is supposed to be synced by tb->flags (which is normally synced with CPUTriCoreState via cpu_get_tb_cpu_state()) in gen_intermediate_code(). Somehow I forgot to add the first sync. So, the proper fix is: diff --git a/target/tricore/translate.c b/target/tricore/translate.c index 06c4485e55..44296b3fb1 100644 --- a/target/tricore/translate.c +++ b/target/tricore/translate.c @@ -8804,6 +8804,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) ctx.singlestep_enabled = cs->singlestep_enabled; ctx.bstate = BS_NONE; ctx.mem_idx = cpu_mmu_index(env, false); + ctx.hflags = (uint32_t)tb->flags; tcg_clear_temp_count(); gen_tb_start(tb); Cheers, Bastian
diff --git a/target/tricore/translate.c b/target/tricore/translate.c index db09f82c31..cdbc00d654 100644 --- a/target/tricore/translate.c +++ b/target/tricore/translate.c @@ -8811,6 +8811,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) target_ulong pc_start; int num_insns = 0; + memset(&ctx, 0x00, sizeof(DisasContext)); pc_start = tb->pc; ctx.pc = pc_start; ctx.saved_pc = -1;