Re: [PATCH] target/mips: Advance pc after semihosting exception

[Philippe Mathieu-Daudé]

On Tue, Aug 2, 2022 at 4:11 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 8/1/22 23:48, Philippe Mathieu-Daudé wrote:
> > Hi Richard,
> >
> > On 30/7/22 04:18, Richard Henderson wrote:
> >> Delay generating the exception until after we know the
> >> insn length, and record that length in env->error_code.
> >>
> >> Fixes: 8ec7e3c53d4 ("target/mips: Use an exception for semihosting")
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1126
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   target/mips/tcg/translate.h               |  4 ++++
> >>   target/mips/tcg/sysemu/tlb_helper.c       |  1 +
> >>   target/mips/tcg/translate.c               | 10 +++++-----
> >>   target/mips/tcg/micromips_translate.c.inc |  6 +++---
> >>   target/mips/tcg/mips16e_translate.c.inc   |  2 +-
> >>   target/mips/tcg/nanomips_translate.c.inc  |  4 ++--
> >>   6 files changed, 16 insertions(+), 11 deletions(-)

> >> @@ -16098,6 +16095,9 @@ static void 
> >> mips_tr_translate_insn(DisasContextBase *dcbase,
> >> CPUState *cs)
> >>       if (is_slot) {
> >>           gen_branch(ctx, insn_bytes);
> >>       }
> >> +    if (ctx->base.is_jmp == DISAS_SEMIHOST) {
> >> +        generate_exception_err(ctx, EXCP_SEMIHOST, insn_bytes);
> >
> > Hmm this API takes an error_code argument:
> >
> >    int error_code;
> >    #define EXCP_TLB_NOMATCH   0x1
> >    #define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr 
> > */
> >
> > Now we pass bytes. What about adding an extra helper?
> >
> >    void generate_exception_err_displace(DisasContext *ctx,
> >                                         int excp, int err,
> >                                         target_long displacement);
>
> These error codes are specific to the matching EXCP.
> We don't need to introduce extra storage, I don't think.

OK, fine then.

> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>