Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-15 Thread Richard Henderson

On 2/15/23 15:46, LIU Zhiwei wrote:

We can rearrange the patch set as follows:

1. Implement the zimop extension.

2. Implement the forward cfi only for system mode.

3. Implement the backward cfi only for system mode.

4. Carefully make the forward cfi can work on user mode.

5. Carefully make the backward cfi work for user mode.

I don't think we can easily make cfi  work on user mode. So we can also ignore the 4 or 5, 
or both.


This is a good ordering.  Similar to how we implemented CFI for AArch64.

I strongly suspect that you will need to defer 5 until the Linux uabi is defined.  It will 
require some mmap bit (MAP_* or PROT_*) which libc will use to define shadow stacks for 
new threads.


But having the system support in means that you can work on the corresponding 
kernel bits.


r~



Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-15 Thread LIU Zhiwei



On 2023/2/16 4:47, Deepak Gupta wrote:

On Tue, Feb 14, 2023 at 6:52 PM LIU Zhiwei  wrote:


On 2023/2/9 14:23, Deepak Gupta wrote:

Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.

Does  the zimops has a independent specification? If so, you should give
a link to this
specification.

Actual formal documentation is still a work in progress.
I am hoping to provide a reference to it in my next iteration.


This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.

You should don't add two extensions in one patch. I think you should add
them one by one.
And add the zimop first.  In my opinion, you should implement the whole
zimop extension before
adding any patch for zisslpcfi, including the implementation of mop.rr
and mop.r.

Noted will make sure of that and will send two different patch series then.


We can rearrange the patch set as follows:

1. Implement the zimop extension.

2. Implement the forward cfi only for system mode.

3. Implement the backward cfi only for system mode.

4. Carefully make the forward cfi can work on user mode.

5. Carefully make the backward cfi work for user mode.

I don't think we can easily make cfi  work on user mode. So we can also 
ignore the 4 or 5, or both.


Thus, we don't have to implement a patch like the patch 8, which is too 
big to review.



Zhiwei




Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
   target/riscv/cpu.c | 13 +
   target/riscv/cpu.h |  2 ++
   2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
   ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
   ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
   ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),

Add them one by one.

   };

   static bool isa_ext_is_enabled(RISCVCPU *cpu,
@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
   return;
   }

+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+

Seems reasonable for me.

   /* Set the ISA extensions, checks should have happened above */
   if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
   cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
   #ifndef CONFIG_USER_ONLY
   DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
   #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),

Default value should be false.

Yes, I have to fix this.


Zhiwei


   DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
false),

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
   uint32_t mvendorid;
   uint64_t marchid;
   uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;

   /* Vendor-specific custom extensions */
   bool ext_XVentanaCondOps;




Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-15 Thread Deepak Gupta
On Tue, Feb 14, 2023 at 6:52 PM LIU Zhiwei  wrote:
>
>
> On 2023/2/9 14:23, Deepak Gupta wrote:
> > Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
> > extension provides hardware assistance to riscv hart to enable control
> > flow integrity (CFI) for software.
> >
> > `zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
> > for "unprivileged integer maybe operations". `zimops` carve out certain
> > reserved opcodes encodings from integer spec to "may be operations"
> > encodings. `zimops` opcode encodings simply move 0 to rd.
> > `zisslpcfi` claims some of the `zimops` encodings and use them for shadow
> > stack management or indirect branch tracking. Any future extension can
> > also claim `zimops` encodings.
>
> Does  the zimops has a independent specification? If so, you should give
> a link to this
> specification.

Actual formal documentation is still a work in progress.
I am hoping to provide a reference to it in my next iteration.

>
> >
> > This patch also adds a dependency check for `zimops` to be enabled if
> > `zisslpcfi` is enabled on the hart.
>
> You should don't add two extensions in one patch. I think you should add
> them one by one.
> And add the zimop first.  In my opinion, you should implement the whole
> zimop extension before
> adding any patch for zisslpcfi, including the implementation of mop.rr
> and mop.r.

Noted will make sure of that and will send two different patch series then.

>
> >
> > Signed-off-by: Deepak Gupta 
> > Signed-off-by: Kip Walker  
> > ---
> >   target/riscv/cpu.c | 13 +
> >   target/riscv/cpu.h |  2 ++
> >   2 files changed, 15 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index cc75ca7667..6b4e90eb91 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
> >   ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
> >   ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> >   ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
> > ext_XVentanaCondOps),
> > +ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
> > +ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),
> Add them one by one.
> >   };
> >
> >   static bool isa_ext_is_enabled(RISCVCPU *cpu,
> > @@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >   return;
> >   }
> >
> > +if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
> > +error_setg(errp, "Zisslpcfi extension requires Zimops 
> > extension");
> > +return;
> > +}
> > +
> Seems reasonable for me.
> >   /* Set the ISA extensions, checks should have happened above */
> >   if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> >   cpu->cfg.ext_zhinxmin) {
> > @@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
> >   #ifndef CONFIG_USER_ONLY
> >   DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, 
> > DEFAULT_RSTVEC),
> >   #endif
> > +/*
> > + * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
> > + * implemented
> > + */
> > +DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
> > +DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
>
> Default value should be false.

Yes, I have to fix this.

>
> Zhiwei
>
> >
> >   DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
> > false),
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index f5609b62a2..9a923760b2 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -471,6 +471,8 @@ struct RISCVCPUConfig {
> >   uint32_t mvendorid;
> >   uint64_t marchid;
> >   uint64_t mimpid;
> > +bool ext_zimops;
> > +bool ext_cfi;
> >
> >   /* Vendor-specific custom extensions */
> >   bool ext_XVentanaCondOps;



Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-14 Thread LIU Zhiwei



On 2023/2/9 14:23, Deepak Gupta wrote:

Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.


Does  the zimops has a independent specification? If so, you should give 
a link to this

specification.



This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.


You should don't add two extensions in one patch. I think you should add 
them one by one.
And add the zimop first.  In my opinion, you should implement the whole 
zimop extension before
adding any patch for zisslpcfi, including the implementation of mop.rr 
and mop.r.




Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
  target/riscv/cpu.c | 13 +
  target/riscv/cpu.h |  2 ++
  2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
  ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
  ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
  ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),

Add them one by one.

  };
  
  static bool isa_ext_is_enabled(RISCVCPU *cpu,

@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  return;
  }
  
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {

+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+

Seems reasonable for me.

  /* Set the ISA extensions, checks should have happened above */
  if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
  cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
  #ifndef CONFIG_USER_ONLY
  DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
  #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),


Default value should be false.

Zhiwei

  
  DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
  
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h

index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
  uint32_t mvendorid;
  uint64_t marchid;
  uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
  
  /* Vendor-specific custom extensions */

  bool ext_XVentanaCondOps;




Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-12 Thread Deepak Gupta

On Sat, Feb 11, 2023 at 11:19:11AM +0800, weiwei wrote:


On 2023/2/9 14:23, Deepak Gupta wrote:

Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.

This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.

Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
 target/riscv/cpu.c | 13 +
 target/riscv/cpu.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
 ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
 ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),


By convention, it  should be ext_zisslpcfi  .


Noted. Will fix it.




 };
 static bool isa_ext_is_enabled(RISCVCPU *cpu,
@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+


If  Zisslpcfi implicitly means Zimops is implemented as commented in 
following code, I think we should just enable zimops  when zisslpcfi 
is enabled.



Hmm... That's a good idea (at-least for qemu implementation)
Only catch is this
   - Since zimops does move 0 to rd. That's still an operation that's happening on 
 destination. If none of the extensions are implemented, it might be good to have

 just zimops enabled *just* to make sure binary is not breaking anything 
(by moving
 0 to destination)

 /* Set the ISA extensions, checks should have happened above */
 if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
 cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
 #ifndef CONFIG_USER_ONLY
 DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
 #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),


These properties can not expose to users before all its functions are 
ready. And it need add 'x-' prefix as experimental extensions 
currently.


Noted will revise it.



Regards,

Weiwei Li


 DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
 uint32_t mvendorid;
 uint64_t marchid;
 uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
 /* Vendor-specific custom extensions */
 bool ext_XVentanaCondOps;






Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-10 Thread weiwei



On 2023/2/9 14:23, Deepak Gupta wrote:

Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.

This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.

Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
  target/riscv/cpu.c | 13 +
  target/riscv/cpu.h |  2 ++
  2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
  ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
  ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
  ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),


By convention, it  should be ext_zisslpcfi  .


  };
  
  static bool isa_ext_is_enabled(RISCVCPU *cpu,

@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  return;
  }
  
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {

+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+


If  Zisslpcfi implicitly means Zimops is implemented as commented in 
following code, I think we should just enable zimops  when zisslpcfi is 
enabled.



  /* Set the ISA extensions, checks should have happened above */
  if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
  cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
  #ifndef CONFIG_USER_ONLY
  DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
  #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),


These properties can not expose to users before all its functions are 
ready. And it need add 'x-' prefix as experimental extensions currently.


Regards,

Weiwei Li

  
  DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
  
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h

index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
  uint32_t mvendorid;
  uint64_t marchid;
  uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
  
  /* Vendor-specific custom extensions */

  bool ext_XVentanaCondOps;





[PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-08 Thread Deepak Gupta
Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.

This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.

Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
 target/riscv/cpu.c | 13 +
 target/riscv/cpu.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
 ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
 ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),
 };
 
 static bool isa_ext_is_enabled(RISCVCPU *cpu,
@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+
 /* Set the ISA extensions, checks should have happened above */
 if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
 cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
 #ifndef CONFIG_USER_ONLY
 DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
 #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
 
 DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
false),
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
 uint32_t mvendorid;
 uint64_t marchid;
 uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
 
 /* Vendor-specific custom extensions */
 bool ext_XVentanaCondOps;
-- 
2.25.1




[PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config

2023-02-08 Thread Deepak Gupta
Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.

`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.

This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.

Signed-off-by: Deepak Gupta 
Signed-off-by: Kip Walker  
---
 target/riscv/cpu.c | 13 +
 target/riscv/cpu.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
 ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
 ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),
 };
 
 static bool isa_ext_is_enabled(RISCVCPU *cpu,
@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+
 /* Set the ISA extensions, checks should have happened above */
 if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
 cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
 #ifndef CONFIG_USER_ONLY
 DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
 #endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
 
 DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
false),
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
 uint32_t mvendorid;
 uint64_t marchid;
 uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
 
 /* Vendor-specific custom extensions */
 bool ext_XVentanaCondOps;
-- 
2.25.1