target/arm: Report IL=0 for Thumb 16-bit BKPT insn

[PATCH] target/arm: Report IL=0 for Thumb 16-bit BKPT insn

Posted by Peter Maydell 3 weeks, 4 days ago

The Thumb BKPT insn is 16-bit, and the ESR_ELx syndrome register
definition requires that we set the IL bit to 0 for this, and 1 for
the 32-bit A32 and A64 BKPT/BRK.

We used to do this correctly, but accidentally lost it in the
conversion to decodetree, because we converted the A32 BKPT first,
and then when we converted the T16 BKPT we forgot that trans_BKPT()
was unconditionally setting IL=1.

Pass the right value for syn_aa32_bkpt()'s is_16bit argument.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3474
Fixes: 43f7e42c7d515f ("target/arm: Convert T16, Miscellaneous 16-bit instructions")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index ce427c5a3c..7f76a57653 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -3580,7 +3580,7 @@ static bool trans_BKPT(DisasContext *s, arg_BKPT *a)
         (a->imm == 0xab)) {
         gen_exception_internal_insn(s, EXCP_SEMIHOST);
     } else {
-        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, false));
+        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, s->thumb));
     }
     return true;
 }
-- 
2.43.0

Re: [PATCH] target/arm: Report IL=0 for Thumb 16-bit BKPT insn

Posted by Richard Henderson 3 weeks, 3 days ago

On 5/5/26 05:37, Peter Maydell wrote:
> The Thumb BKPT insn is 16-bit, and the ESR_ELx syndrome register
> definition requires that we set the IL bit to 0 for this, and 1 for
> the 32-bit A32 and A64 BKPT/BRK.
> 
> We used to do this correctly, but accidentally lost it in the
> conversion to decodetree, because we converted the A32 BKPT first,
> and then when we converted the T16 BKPT we forgot that trans_BKPT()
> was unconditionally setting IL=1.
> 
> Pass the right value for syn_aa32_bkpt()'s is_16bit argument.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3474
> Fixes: 43f7e42c7d515f ("target/arm: Convert T16, Miscellaneous 16-bit instructions")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> index ce427c5a3c..7f76a57653 100644
> --- a/target/arm/tcg/translate.c
> +++ b/target/arm/tcg/translate.c
> @@ -3580,7 +3580,7 @@ static bool trans_BKPT(DisasContext *s, arg_BKPT *a)
>           (a->imm == 0xab)) {
>           gen_exception_internal_insn(s, EXCP_SEMIHOST);
>       } else {
> -        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, false));
> +        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, s->thumb));
>       }
>       return true;
>   }

While correct, perhaps clearer to use curr_insn_len.

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] target/arm: Report IL=0 for Thumb 16-bit BKPT insn

Posted by Peter Maydell 3 weeks, 2 days ago

On Wed, 6 May 2026 at 13:48, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/5/26 05:37, Peter Maydell wrote:
> > The Thumb BKPT insn is 16-bit, and the ESR_ELx syndrome register
> > definition requires that we set the IL bit to 0 for this, and 1 for
> > the 32-bit A32 and A64 BKPT/BRK.
> >
> > We used to do this correctly, but accidentally lost it in the
> > conversion to decodetree, because we converted the A32 BKPT first,
> > and then when we converted the T16 BKPT we forgot that trans_BKPT()
> > was unconditionally setting IL=1.
> >
> > Pass the right value for syn_aa32_bkpt()'s is_16bit argument.
> >
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3474
> > Fixes: 43f7e42c7d515f ("target/arm: Convert T16, Miscellaneous 16-bit instructions")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   target/arm/tcg/translate.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> > index ce427c5a3c..7f76a57653 100644
> > --- a/target/arm/tcg/translate.c
> > +++ b/target/arm/tcg/translate.c
> > @@ -3580,7 +3580,7 @@ static bool trans_BKPT(DisasContext *s, arg_BKPT *a)
> >           (a->imm == 0xab)) {
> >           gen_exception_internal_insn(s, EXCP_SEMIHOST);
> >       } else {
> > -        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, false));
> > +        gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, s->thumb));
> >       }
> >       return true;
> >   }
>
> While correct, perhaps clearer to use curr_insn_len.

Ah, I didn't notice we had that; I'll use it. (I was following
the use of s->thumb in the syn_aa32_svc() call.)

-- PMM

Re: [PATCH] target/arm: Report IL=0 for Thumb 16-bit BKPT insn

Posted by Alex Bennée 3 weeks, 4 days ago

Peter Maydell <peter.maydell@linaro.org> writes:

> The Thumb BKPT insn is 16-bit, and the ESR_ELx syndrome register
> definition requires that we set the IL bit to 0 for this, and 1 for
> the 32-bit A32 and A64 BKPT/BRK.
>
> We used to do this correctly, but accidentally lost it in the
> conversion to decodetree, because we converted the A32 BKPT first,
> and then when we converted the T16 BKPT we forgot that trans_BKPT()
> was unconditionally setting IL=1.
>
> Pass the right value for syn_aa32_bkpt()'s is_16bit argument.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3474
> Fixes: 43f7e42c7d515f ("target/arm: Convert T16, Miscellaneous 16-bit instructions")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] target/arm: Report IL=0 for Thumb 16-bit BKPT insn

Posted by Philippe Mathieu-Daudé 3 weeks, 4 days ago

On 5/5/26 12:37, Peter Maydell wrote:
> The Thumb BKPT insn is 16-bit, and the ESR_ELx syndrome register
> definition requires that we set the IL bit to 0 for this, and 1 for
> the 32-bit A32 and A64 BKPT/BRK.
> 
> We used to do this correctly, but accidentally lost it in the
> conversion to decodetree, because we converted the A32 BKPT first,
> and then when we converted the T16 BKPT we forgot that trans_BKPT()
> was unconditionally setting IL=1.
> 
> Pass the right value for syn_aa32_bkpt()'s is_16bit argument.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3474
> Fixes: 43f7e42c7d515f ("target/arm: Convert T16, Miscellaneous 16-bit instructions")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>