| Message ID | 20200709141327.14631-10-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | misc rc0 fixes (docs, plugins, docker) | expand |
On 7/9/20 4:13 PM, Alex Bennée wrote: > The translator_ld* functions very much expect us to be decoding one > instruction at a time. Otherwise we will see weirdness such as: > > qemu-sh4: warning: plugin_disas: 6 bytes left over > > when we use the disas functions. For what SH4 is doing here (scanning > ahead in the instruction stream) this is the right function to use. > > Reported-by: Claudio Fontana <cfontana@suse.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > target/sh4/translate.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/target/sh4/translate.c b/target/sh4/translate.c > index 6192d83e8c66..919da72a0c98 100644 > --- a/target/sh4/translate.c > +++ b/target/sh4/translate.c > @@ -1915,9 +1915,13 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State *env) > goto fail; > } > > - /* Read all of the insns for the region. */ > + /* > + * Read all of the insns for the region. We do this directly with > + * cpu_lduw_code to avoid confusing the plugins by decoding > + * multiple instructions. > + */ > for (i = 0; i < max_insns; ++i) { > - insns[i] = translator_lduw(env, pc + i * 2); > + insns[i] = cpu_lduw_code(env, pc + i * 2); Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > } > > ld_adr = ld_dst = ld_mop = -1; >
On 7/9/20 7:13 AM, Alex Bennée wrote: > The translator_ld* functions very much expect us to be decoding one > instruction at a time. Otherwise we will see weirdness such as: > > qemu-sh4: warning: plugin_disas: 6 bytes left over > > when we use the disas functions. For what SH4 is doing here (scanning > ahead in the instruction stream) this is the right function to use. It is not just scanning ahead. It does that, but after having done so it may also commit to translating them all at once. In the case to which you refer, above, we have translated 4 insns into one operation. Just having plugin_disas see the first one is not really correct either. r~ > > Reported-by: Claudio Fontana <cfontana@suse.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > target/sh4/translate.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/target/sh4/translate.c b/target/sh4/translate.c > index 6192d83e8c66..919da72a0c98 100644 > --- a/target/sh4/translate.c > +++ b/target/sh4/translate.c > @@ -1915,9 +1915,13 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State *env) > goto fail; > } > > - /* Read all of the insns for the region. */ > + /* > + * Read all of the insns for the region. We do this directly with > + * cpu_lduw_code to avoid confusing the plugins by decoding > + * multiple instructions. > + */ > for (i = 0; i < max_insns; ++i) { > - insns[i] = translator_lduw(env, pc + i * 2); > + insns[i] = cpu_lduw_code(env, pc + i * 2); > } > > ld_adr = ld_dst = ld_mop = -1; >
diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 6192d83e8c66..919da72a0c98 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -1915,9 +1915,13 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State *env) goto fail; } - /* Read all of the insns for the region. */ + /* + * Read all of the insns for the region. We do this directly with + * cpu_lduw_code to avoid confusing the plugins by decoding + * multiple instructions. + */ for (i = 0; i < max_insns; ++i) { - insns[i] = translator_lduw(env, pc + i * 2); + insns[i] = cpu_lduw_code(env, pc + i * 2); } ld_adr = ld_dst = ld_mop = -1;
The translator_ld* functions very much expect us to be decoding one instruction at a time. Otherwise we will see weirdness such as: qemu-sh4: warning: plugin_disas: 6 bytes left over when we use the disas functions. For what SH4 is doing here (scanning ahead in the instruction stream) this is the right function to use. Reported-by: Claudio Fontana <cfontana@suse.de> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/sh4/translate.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- 2.20.1