I agree. Here is the corrected patch. Signed-off-by: Christopher Wrogg <cwr...@umich.edu> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1251 --- target/mips/tcg/octeon.decode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode index 8929ad088e..0c787cb498 100644 --- a/target/mips/tcg/octeon.decode +++ b/target/mips/tcg/octeon.decode @@ -12,7 +12,7 @@ # BBIT132 111110 ..... ..... ................ %bbit_p 28:1 16:5 -BBIT 11 set:1 . 10 rs:5 ..... offset:16 p=%bbit_p +BBIT 11 set:1 . 10 rs:5 ..... offset:s16 p=%bbit_p # Arithmetic # BADDU rd, rs, rt -- 2.30.2 On Thu, Oct 13, 2022 at 2:33 PM Richard Henderson < richard.hender...@linaro.org> wrote: > On 10/13/22 15:08, Christopher Wrogg wrote: > > The Octeon specific BBIT instruction incorrectly computes > > the branch offset. The 16 bit value is not sign extended. > > > > Signed-off-by: Christopher Wrogg <cwr...@umich.edu <mailto: > cwr...@umich.edu>> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1251 > > <https://gitlab.com/qemu-project/qemu/-/issues/1251> > > --- > > target/mips/tcg/octeon_translate.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/target/mips/tcg/octeon_translate.c > b/target/mips/tcg/octeon_translate.c > > index 6a207d2e7e..90f7b105cb 100644 > > --- a/target/mips/tcg/octeon_translate.c > > +++ b/target/mips/tcg/octeon_translate.c > > @@ -38,7 +38,10 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a) > > } > > > > ctx->hflags |= MIPS_HFLAG_BC; > > - ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4; > > + a->offset *= 4; > > + a->offset = (target_long)(int16_t)a->offset; > > + ctx->btarget = ctx->base.pc_next + 4 + a->offset; > > This looks wrong, because you're sign-extending after scaling, which gives > you only 14 > bits of offset instead of 16. > > The correct fix should be earlier in decode: > > - BBIT 11 set:1 . 10 rs:5 ..... offset:16 p=%bbit_p > + BBIT 11 set:1 . 10 rs:5 ..... offset:s16 p=%bbit_p > > to indicate a extract a signed field from the instruction. > > > r~ >
