[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpc

From: LIU Zhiwei
Subject: Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config
Date: Wed, 15 Feb 2023 10:52:06 +0800
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2

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

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 <debug@rivosinc.com>
Signed-off-by: Kip Walker  <kip@rivosinc.com>
  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, 
+    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 
+ 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[] = {
      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
+    /*
+     * 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.


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;

reply via email to

[Prev in Thread] Current Thread [Next in Thread]