RE: [PATCH 2/3] target/riscv: generate misaligned access trap for rvi insn

[Konrad, Frederic]

Hi Richard,

Thanks for the comment.

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: 11 December 2024 22:50
> To: Konrad, Frederic <Frederic.Konrad@amd.com>; qemu-riscv@nongnu.org
> Cc: palmer@dabbelt.com; alistair.francis@wdc.com; bmeng.cn@gmail.com; 
> liwei1518@gmail.com;
> dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com; 
> qemu-devel@nongnu.org; Iglesias, Francisco
> <francisco.iglesias@amd.com>; Michel, Luc <Luc.Michel@amd.com>
> Subject: Re: [PATCH 2/3] target/riscv: generate misaligned access trap for 
> rvi insn
> 
> On 12/11/24 15:19, Frederic Konrad wrote:
> > Now there is an option to enable misaligned accesses traps, check the 
> > alignment
> > during load and store for the RVI instructions.  Do not generate them if the
> > zama16b extension is there.
> >
> > Signed-off-by: Frederic Konrad <fkonrad@amd.com>
> > ---
> >   target/riscv/insn_trans/trans_rvi.c.inc | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> > b/target/riscv/insn_trans/trans_rvi.c.inc
> > index 96c218a9d7..1283207fc7 100644
> > --- a/target/riscv/insn_trans/trans_rvi.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> > @@ -323,6 +323,10 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, 
> > MemOp memop)
> >   {
> >       bool out;
> >
> > +    if (ctx->cfg_ptr->trap_misaligned_access && 
> > !ctx->cfg_ptr->ext_zama16b) {
> > +        memop |= MO_ALIGN;
> > +    }
> > +
> >       if (ctx->cfg_ptr->ext_zama16b) {
> >           memop |= MO_ATOM_WITHIN16;
> >       }
> > @@ -424,6 +428,9 @@ static bool gen_store_i128(DisasContext *ctx, arg_sb 
> > *a, MemOp memop)
> >
> >   static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
> >   {
> > +    if (ctx->cfg_ptr->trap_misaligned_access && 
> > !ctx->cfg_ptr->ext_zama16b) {
> > +        memop |= MO_ALIGN;
> > +    }
> >       if (ctx->cfg_ptr->ext_zama16b) {
> >           memop |= MO_ATOM_WITHIN16;
> >       }
> 
> I would imagine that ext_zama16b would disable or conflict with 
> trap_misaligned_access at
> startup.

Ok I can fix that condition to:
  if (ctx->cfg_ptr->trap_misaligned_access) {

And throw an error in the first patch if both are activated at the same time.

Best Regards,
Fred

> 
> 
> r~