Re: [PATCH: fix for virt instr exception] target/riscv: fix for virtual instr exception

[Alistair Francis <alistair23@gmail.com>]

On Sat, Jan 28, 2023 at 6:37 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> Please dis-regard this.
> I've sent the patch to qemu-riscv@nongnu.org

That's not entirely correct either.

You can run the `./scripts/get_maintainer.pl` script on your patch to
get the email addresses to send this patch to. qemu-devel is the main
one, but it's also a good idea to CC the qemu-riscv list as well as
maintainers.

>
> On Sat, Jan 28, 2023 at 12:48 AM Deepak Gupta <debug@rivosinc.com> wrote:
> >
> > commit fb3f3730e4 added mechanism to generate virtual instruction
> > exception during instruction decode when virt is enabled.
> >
> > However in some situations, illegal instruction exception can be raised
> > due to state of CPU. One such situation is implementing branch tracking.
> > [1] An indirect branch if doesn't land on a landing pad instruction, then
> > cpu must raise an illegal instruction exception.
> > Implementation would raise such expcetion due to missing landing pad inst
> > and not due to decode. Thus DisasContext must have `virt_inst_excp`

Isn't a landing pad instruction just an instruction that needs to be
decoded? Or are you doing the check as part of the jump instruction?

Overall the change looks ok, but I'm not sure if it's required. Do you
have an implementation that needs this?

Alistair

> > initialized to false during DisasContxt initialization for TB.
> >
> > [1] - https://github.com/riscv/riscv-cfi
> >
> > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > ---
> >  target/riscv/translate.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index df38db7553..76f61a39d3 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -1167,6 +1167,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> >      ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
> >      ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
> >      ctx->zero = tcg_constant_tl(0);
> > +    ctx->virt_inst_excp = false;
> >  }
> >
> >  static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> > --
> > 2.25.1
> >
>