[v4] riscv support for control flow integrity extensions

[PATCH v4 13/16] target/riscv: compressed encodings for sspush and sspopchk

Posted by Deepak Gupta 3 months, 1 week ago

sspush/sspopchk have compressed encodings carved out of zcmops.
compressed sspush is designated as c.mop.1 while compressed sspopchk
is designated as c.mop.5.

Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
c.sspopchk x5 exists while c.sspopchk x1 doesn't.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
---
 target/riscv/insn16.decode                    |  2 ++
 target/riscv/insn_trans/trans_rvzicfiss.c.inc | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 3953bcf82d..3b84a36233 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -140,6 +140,8 @@ sw                110  ... ... .. ... 00 @cs_w
 addi              000 .  .....  ..... 01 @ci
 addi              010 .  .....  ..... 01 @c_li
 {
+  c_sspush        011 0  00001  00000 01 rs2=1 rs1=0 # c.sspush x1 carving out of zcmops
+  c_sspopchk      011 0  00101  00000 01 rs1=5 rd=0 # c.sspopchk x5 carving out of zcmops
   c_mop_n         011 0 0 n:3 1 00000 01
   illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
   addi            011 .  00010  ..... 01 @c_addi16sp
diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
index 05d439c1f6..67f5c7804a 100644
--- a/target/riscv/insn_trans/trans_rvzicfiss.c.inc
+++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
@@ -109,3 +109,15 @@ static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
 {
     return gen_sspush(ctx, a->rs2);
 }
+
+static bool trans_c_sspopchk(DisasContext *ctx, arg_c_sspopchk *a)
+{
+    assert(a->rs1 == 5);
+    return gen_sspopchk(ctx, a->rs1);
+}
+
+static bool trans_c_sspush(DisasContext *ctx, arg_c_sspush *a)
+{
+    assert(a->rs2 == 1);
+    return gen_sspush(ctx, a->rs2);
+}
-- 
2.44.0

Re: [PATCH v4 13/16] target/riscv: compressed encodings for sspush and sspopchk

Posted by Richard Henderson 3 months, 1 week ago

On 8/16/24 11:07, Deepak Gupta wrote:
> sspush/sspopchk have compressed encodings carved out of zcmops.
> compressed sspush is designated as c.mop.1 while compressed sspopchk
> is designated as c.mop.5.
> 
> Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
> c.sspopchk x5 exists while c.sspopchk x1 doesn't.
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Co-developed-by: Jim Shu <jim.shu@sifive.com>
> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>   target/riscv/insn16.decode                    |  2 ++
>   target/riscv/insn_trans/trans_rvzicfiss.c.inc | 12 ++++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
> index 3953bcf82d..3b84a36233 100644
> --- a/target/riscv/insn16.decode
> +++ b/target/riscv/insn16.decode
> @@ -140,6 +140,8 @@ sw                110  ... ... .. ... 00 @cs_w
>   addi              000 .  .....  ..... 01 @ci
>   addi              010 .  .....  ..... 01 @c_li
>   {
> +  c_sspush        011 0  00001  00000 01 rs2=1 rs1=0 # c.sspush x1 carving out of zcmops
> +  c_sspopchk      011 0  00101  00000 01 rs1=5 rd=0 # c.sspopchk x5 carving out of zcmops
>     c_mop_n         011 0 0 n:3 1 00000 01
>     illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
>     addi            011 .  00010  ..... 01 @c_addi16sp
> diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
> index 05d439c1f6..67f5c7804a 100644
> --- a/target/riscv/insn_trans/trans_rvzicfiss.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
> @@ -109,3 +109,15 @@ static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
>   {
>       return gen_sspush(ctx, a->rs2);
>   }
> +
> +static bool trans_c_sspopchk(DisasContext *ctx, arg_c_sspopchk *a)
> +{
> +    assert(a->rs1 == 5);
> +    return gen_sspopchk(ctx, a->rs1);
> +}
> +
> +static bool trans_c_sspush(DisasContext *ctx, arg_c_sspush *a)
> +{
> +    assert(a->rs2 == 1);
> +    return gen_sspush(ctx, a->rs2);
> +}

This indirection is pointless.  Have the decoder invoke the proper insn in the first 
place.  Identically with how we're treating 'addi', for instance.


r~

Re: [PATCH v4 13/16] target/riscv: compressed encodings for sspush and sspopchk

Posted by Deepak Gupta 3 months, 1 week ago

On Fri, Aug 16, 2024 at 03:09:10PM +1000, Richard Henderson wrote:
>On 8/16/24 11:07, Deepak Gupta wrote:
>>sspush/sspopchk have compressed encodings carved out of zcmops.
>>compressed sspush is designated as c.mop.1 while compressed sspopchk
>>is designated as c.mop.5.
>>
>>Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
>>c.sspopchk x5 exists while c.sspopchk x1 doesn't.
>>
>>Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>>Co-developed-by: Jim Shu <jim.shu@sifive.com>
>>Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
>>---
>>  target/riscv/insn16.decode                    |  2 ++
>>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 12 ++++++++++++
>>  2 files changed, 14 insertions(+)
>>
>>diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
>>index 3953bcf82d..3b84a36233 100644
>>--- a/target/riscv/insn16.decode
>>+++ b/target/riscv/insn16.decode
>>@@ -140,6 +140,8 @@ sw                110  ... ... .. ... 00 @cs_w
>>  addi              000 .  .....  ..... 01 @ci
>>  addi              010 .  .....  ..... 01 @c_li
>>  {
>>+  c_sspush        011 0  00001  00000 01 rs2=1 rs1=0 # c.sspush x1 carving out of zcmops
>>+  c_sspopchk      011 0  00101  00000 01 rs1=5 rd=0 # c.sspopchk x5 carving out of zcmops
>>    c_mop_n         011 0 0 n:3 1 00000 01
>>    illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
>>    addi            011 .  00010  ..... 01 @c_addi16sp
>>diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>index 05d439c1f6..67f5c7804a 100644
>>--- a/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>+++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>@@ -109,3 +109,15 @@ static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
>>  {
>>      return gen_sspush(ctx, a->rs2);
>>  }
>>+
>>+static bool trans_c_sspopchk(DisasContext *ctx, arg_c_sspopchk *a)
>>+{
>>+    assert(a->rs1 == 5);
>>+    return gen_sspopchk(ctx, a->rs1);
>>+}
>>+
>>+static bool trans_c_sspush(DisasContext *ctx, arg_c_sspush *a)
>>+{
>>+    assert(a->rs2 == 1);
>>+    return gen_sspush(ctx, a->rs2);
>>+}
>
>This indirection is pointless.  Have the decoder invoke the proper 
>insn in the first place.  Identically with how we're treating 'addi', 
>for instance.
>

I was getting compilation error. How to reconcile with arugment sets between
insn32.decode and insn16.decode. Earlier I was doing that and didn't need it.
But after removing indirection in arguments and using single use format, type for
structs instruction arguments ends up conflicting and compiler complains.


>
>r~
>

Re: [PATCH v4 13/16] target/riscv: compressed encodings for sspush and sspopchk

Posted by Richard Henderson 3 months, 1 week ago

On 8/16/24 16:56, Deepak Gupta wrote:
>>> +  c_sspush        011 0  00001  00000 01 rs2=1 rs1=0 # c.sspush x1 carving out of zcmops
>>> +  c_sspopchk      011 0  00101  00000 01 rs1=5 rd=0 # c.sspopchk x5 carving out of zcmops
...
>> This indirection is pointless.  Have the decoder invoke the proper insn in the first 
>> place.  Identically with how we're treating 'addi', for instance.
>>
> 
> I was getting compilation error. How to reconcile with arugment sets between
> insn32.decode and insn16.decode. Earlier I was doing that and didn't need it.
> But after removing indirection in arguments and using single use format, type for
> structs instruction arguments ends up conflicting and compiler complains.

Ah, for that you need to use named argument sets.  In this case,

sspush     .... &r2_s rs2=1 rs1=0
sspopchk   .... &r2   rs2=5 rd=0

in both insn32.decode and insn16.decode


r~