On Thu, Aug 29, 2024 at 10:07:15AM +1000, Alistair Francis wrote:
On Thu, Aug 29, 2024 at 10:06 AM Deepak Gupta <de...@rivosinc.com> wrote:

On Thu, Aug 29, 2024 at 10:01:30AM +1000, Alistair Francis wrote:
>On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <de...@rivosinc.com> wrote:
>>
>> zicfiss has following instructions
>>  - sspopchk: pops a value from shadow stack and compares with x1/x5.
>>    If they dont match, reports a sw check exception with tval = 3.
>>  - sspush: pushes value in x1/x5 on shadow stack
>>  - ssrdp: reads current shadow stack
>>  - ssamoswap: swaps contents of shadow stack atomically
>>
>> sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0
>>
>> If SSE=0, ssamoswap is illegal instruction exception.
>>
>> This patch implements shadow stack operations for qemu-user and shadow
>> stack is not protected.
>>
>> Signed-off-by: Deepak Gupta <de...@rivosinc.com>
>> Co-developed-by: Jim Shu <jim....@sifive.com>
>> Co-developed-by: Andy Chiu <andy.c...@sifive.com>
>> ---
>>  target/riscv/cpu_bits.h                       |  2 +
>>  target/riscv/insn32.decode                    | 21 +++++-
>>  target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
>>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
>>  target/riscv/translate.c                      |  5 ++
>>  5 files changed, 140 insertions(+), 2 deletions(-)
>>  create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>
>>  # *** Zabhb Standard Extension ***
>>  amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
>> diff --git a/target/riscv/insn_trans/trans_rva.c.inc 
b/target/riscv/insn_trans/trans_rva.c.inc
>> index 9cf3ae8019..a2119393a6 100644
>> --- a/target/riscv/insn_trans/trans_rva.c.inc
>> +++ b/target/riscv/insn_trans/trans_rva.c.inc
>> @@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, 
arg_amoswap_w *a)
>>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
>>  }
>>
>> +static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>> +{
>> +    REQUIRE_A_OR_ZAAMO(ctx);
>> +    if (!ctx->bcfi_enabled) {
>> +        return false;
>> +    }
>> +
>> +    TCGv dest = dest_gpr(ctx, a->rd);
>> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> +
>> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> +    src1 = get_address(ctx, a->rs1, 0);
>> +
>> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> +                           (MO_ALIGN | MO_TESL));
>> +    gen_set_gpr(ctx, a->rd, dest);
>> +    return true;
>> +}
>> +
>>  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
>>  {
>>      REQUIRE_A_OR_ZAAMO(ctx);
>> @@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, 
arg_amoswap_d *a)
>>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
>>  }
>>
>> +static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
>> +{
>> +    REQUIRE_64BIT(ctx);
>> +    REQUIRE_A_OR_ZAAMO(ctx);
>> +    if (!ctx->bcfi_enabled) {
>> +        return false;
>> +    }
>> +
>> +    TCGv dest = dest_gpr(ctx, a->rd);
>> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> +
>> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> +    src1 = get_address(ctx, a->rs1, 0);
>> +
>> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> +                           (MO_ALIGN | MO_TESQ));
>> +    gen_set_gpr(ctx, a->rd, dest);
>> +    return true;
>> +}
>
>Why aren't these in the rvzicfiss file?

`ssamoswap` encodings are coming from (reserved) AMO encodings (and not zimop)
That's why kept it in trans_rva

But the instructions are defined in the rvzicfiss extension, so I feel
it makes sense to include them there

Ok noted.
I'll wait for a day or two. If there are not objections from anyone else, I'll 
move
them to rvzicfiss.


Alistair


>
>Otherwise:
>
>Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
>
>Alistair
>
>> +
>>  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
>>  {

Reply via email to