[PATCH v4 0/2] riscv: Add support for Zicbo[m,z,p] instructions

[PATCH v4 0/2] riscv: Add support for Zicbo[m,z,p] instructions

[Christoph Muellner]

The RISC-V base cache management operation ISA extension has been
ratified [1]. This patchset adds support for the defined instructions.

As the exception behavior of these instructions depend on the PMP
configuration, the first patch introduces a new API to probe the access
of an address range with a specified size with optional nonfaulting
behavior.

The Zicbo[m,z,p] patch should be straight-forward and has been reviewed
in previous versions of this patchset.

The series is rebsed on top of github-alistair23/riscv-to-apply.next plus
the Priv v1.12 series from github-atishp04/priv_1_12_support_v3.

[1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions

v4:
- Add patch to add probe_access_range_flags() interface
- Rename cbozelen -> cboz_blocksize
- Introduce cbom_blocksize
- Remove RISCV_CPU() calls from trans_*()
- Use probe_access_range_flags() to improve exception behavior

v3:
- Enable by default (like zb*)
- Rename flags Zicbo* -> zicbo* (like zb*)
- Rename ext_zicbo* -> ext_icbo* (like ext_icsr)
- Rename trans_zicbo.c.inc -> trans_rvzicbo.c.inc (like all others)
- Simplify prefetch instruction support to a single comment
- Rebase on top of github-alistair23/riscv-to-apply.next plus the
  Priv v1.12 series from github-atishp04/priv_1_12_support_v3

v2:
- Fix overlapping instruction encoding with LQ instructions
- Drop CSR related changes and rebase on Priv 1.12 patchset

Christoph Muellner (2):
  accel/tcg: Add probe_access_range_flags interface
  target/riscv: Enable Zicbo[m,z,p] instructions

 accel/tcg/cputlb.c                          | 17 +++-
 accel/tcg/user-exec.c                       | 15 +++-
 include/exec/exec-all.h                     | 24 +++++
 target/riscv/cpu.c                          |  4 +
 target/riscv/cpu.h                          |  4 +
 target/riscv/helper.h                       |  5 ++
 target/riscv/insn32.decode                  | 16 +++-
 target/riscv/insn_trans/trans_rvzicbo.c.inc | 57 ++++++++++++
 target/riscv/op_helper.c                    | 97 +++++++++++++++++++++
 target/riscv/translate.c                    |  1 +
 10 files changed, 232 insertions(+), 8 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvzicbo.c.inc

-- 
2.35.1

[PATCH v4 1/2] accel/tcg: Add probe_access_range_flags interface

[Christoph Muellner]

The existing probe_access* functions do not allow to specify the
access size and a non-faulting behavior at the same time.

This is resolved by adding a generalization of probe_access_flags()
that takes an additional size parameter.

The semantics is basically the same as probe_access_flags(),
but instead of assuming an access to any byte of the addressed
page, we can restrict to access to a specific area, like
probe_access() allows.

Signed-off-by: Christoph Muellner <cmuellner@linux.com>
---
 accel/tcg/cputlb.c      | 17 +++++++++++++----
 accel/tcg/user-exec.c   | 15 ++++++++++++---
 include/exec/exec-all.h | 24 ++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 5e0d0eebc3..b4f0eb20b0 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1624,13 +1624,14 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
     return flags;
 }
 
-int probe_access_flags(CPUArchState *env, target_ulong addr,
-                       MMUAccessType access_type, int mmu_idx,
-                       bool nonfault, void **phost, uintptr_t retaddr)
+int probe_access_range_flags(CPUArchState *env, target_ulong addr,
+                             int size, MMUAccessType access_type,
+                             int mmu_idx, bool nonfault, void **phost,
+                             uintptr_t retaddr)
 {
     int flags;
 
-    flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
+    flags = probe_access_internal(env, addr, size, access_type, mmu_idx,
                                   nonfault, phost, retaddr);
 
     /* Handle clean RAM pages.  */
@@ -1645,6 +1646,14 @@ int probe_access_flags(CPUArchState *env, target_ulong addr,
     return flags;
 }
 
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool nonfault, void **phost, uintptr_t retaddr)
+{
+    return probe_access_range_flags(env, addr, 0, access_type, mmu_idx,
+                                    nonfault, phost, retaddr);
+}
+
 void *probe_access(CPUArchState *env, target_ulong addr, int size,
                    MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
 {
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 6f5d4933f0..0dbc345e63 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -176,9 +176,10 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
     cpu_loop_exit_sigsegv(env_cpu(env), addr, access_type, maperr, ra);
 }
 
-int probe_access_flags(CPUArchState *env, target_ulong addr,
-                       MMUAccessType access_type, int mmu_idx,
-                       bool nonfault, void **phost, uintptr_t ra)
+int probe_access_range_flags(CPUArchState *env, target_ulong addr,
+                             int size, MMUAccessType access_type,
+                             int mmu_idx, bool nonfault, void **phost,
+                             uintptr_t ra)
 {
     int flags;
 
@@ -187,6 +188,14 @@ int probe_access_flags(CPUArchState *env, target_ulong addr,
     return flags;
 }
 
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool nonfault, void **phost, uintptr_t ra)
+{
+    return probe_access_range_flags(env, addr, 0, access_type, mmu_idx,
+                                    nonfault, phost, ra);
+}
+
 void *probe_access(CPUArchState *env, target_ulong addr, int size,
                    MMUAccessType access_type, int mmu_idx, uintptr_t ra)
 {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 35d8e93976..0d06b45c62 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -441,6 +441,30 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size,
     return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
 }
 
+/**
+ * probe_access_range_flags:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @size: size of the access
+ * @access_type: read, write or execute permission
+ * @mmu_idx: MMU index to use for lookup
+ * @nonfault: suppress the fault
+ * @phost: return value for host address
+ * @retaddr: return address for unwinding
+ *
+ * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
+ * the access range, and storing the host address for RAM in @phost.
+ *
+ * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
+ * Do not handle watchpoints, but include TLB_WATCHPOINT in the returned flags.
+ * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
+ * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
+ */
+int probe_access_range_flags(CPUArchState *env, target_ulong addr,
+                             int size, MMUAccessType access_type,
+                             int mmu_idx, bool nonfault, void **phost,
+                             uintptr_t retaddr);
+
 /**
  * probe_access_flags:
  * @env: CPUArchState
-- 
2.35.1

Re: [PATCH v4 1/2] accel/tcg: Add probe_access_range_flags interface

[Alistair Francis]

On Thu, Feb 17, 2022 at 1:49 AM Christoph Muellner <cmuellner@linux.com> wrote:
>
> The existing probe_access* functions do not allow to specify the
> access size and a non-faulting behavior at the same time.
>
> This is resolved by adding a generalization of probe_access_flags()
> that takes an additional size parameter.
>
> The semantics is basically the same as probe_access_flags(),
> but instead of assuming an access to any byte of the addressed
> page, we can restrict to access to a specific area, like
> probe_access() allows.
>
> Signed-off-by: Christoph Muellner <cmuellner@linux.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c      | 17 +++++++++++++----
>  accel/tcg/user-exec.c   | 15 ++++++++++++---
>  include/exec/exec-all.h | 24 ++++++++++++++++++++++++
>  3 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 5e0d0eebc3..b4f0eb20b0 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1624,13 +1624,14 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
>      return flags;
>  }
>
> -int probe_access_flags(CPUArchState *env, target_ulong addr,
> -                       MMUAccessType access_type, int mmu_idx,
> -                       bool nonfault, void **phost, uintptr_t retaddr)
> +int probe_access_range_flags(CPUArchState *env, target_ulong addr,
> +                             int size, MMUAccessType access_type,
> +                             int mmu_idx, bool nonfault, void **phost,
> +                             uintptr_t retaddr)
>  {
>      int flags;
>
> -    flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
> +    flags = probe_access_internal(env, addr, size, access_type, mmu_idx,
>                                    nonfault, phost, retaddr);
>
>      /* Handle clean RAM pages.  */
> @@ -1645,6 +1646,14 @@ int probe_access_flags(CPUArchState *env, target_ulong addr,
>      return flags;
>  }
>
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> +                       MMUAccessType access_type, int mmu_idx,
> +                       bool nonfault, void **phost, uintptr_t retaddr)
> +{
> +    return probe_access_range_flags(env, addr, 0, access_type, mmu_idx,
> +                                    nonfault, phost, retaddr);
> +}
> +
>  void *probe_access(CPUArchState *env, target_ulong addr, int size,
>                     MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>  {
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 6f5d4933f0..0dbc345e63 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -176,9 +176,10 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
>      cpu_loop_exit_sigsegv(env_cpu(env), addr, access_type, maperr, ra);
>  }
>
> -int probe_access_flags(CPUArchState *env, target_ulong addr,
> -                       MMUAccessType access_type, int mmu_idx,
> -                       bool nonfault, void **phost, uintptr_t ra)
> +int probe_access_range_flags(CPUArchState *env, target_ulong addr,
> +                             int size, MMUAccessType access_type,
> +                             int mmu_idx, bool nonfault, void **phost,
> +                             uintptr_t ra)
>  {
>      int flags;
>
> @@ -187,6 +188,14 @@ int probe_access_flags(CPUArchState *env, target_ulong addr,
>      return flags;
>  }
>
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> +                       MMUAccessType access_type, int mmu_idx,
> +                       bool nonfault, void **phost, uintptr_t ra)
> +{
> +    return probe_access_range_flags(env, addr, 0, access_type, mmu_idx,
> +                                    nonfault, phost, ra);
> +}
> +
>  void *probe_access(CPUArchState *env, target_ulong addr, int size,
>                     MMUAccessType access_type, int mmu_idx, uintptr_t ra)
>  {
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 35d8e93976..0d06b45c62 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -441,6 +441,30 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size,
>      return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
>  }
>
> +/**
> + * probe_access_range_flags:
> + * @env: CPUArchState
> + * @addr: guest virtual address to look up
> + * @size: size of the access
> + * @access_type: read, write or execute permission
> + * @mmu_idx: MMU index to use for lookup
> + * @nonfault: suppress the fault
> + * @phost: return value for host address
> + * @retaddr: return address for unwinding
> + *
> + * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
> + * the access range, and storing the host address for RAM in @phost.
> + *
> + * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
> + * Do not handle watchpoints, but include TLB_WATCHPOINT in the returned flags.
> + * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
> + * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
> + */
> +int probe_access_range_flags(CPUArchState *env, target_ulong addr,
> +                             int size, MMUAccessType access_type,
> +                             int mmu_idx, bool nonfault, void **phost,
> +                             uintptr_t retaddr);
> +
>  /**
>   * probe_access_flags:
>   * @env: CPUArchState
> --
> 2.35.1
>
>

[PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Christoph Muellner]

The RISC-V base cache management operation ISA extension has been
ratified. This patch adds support for the defined instructions.

The cmo.prefetch instructions are nops for QEMU (no emulation of the memory
hierarchy, no illegal instructions, no permission faults, no traps),
therefore there's only a comment where they would be decoded.

The other cbo* instructions are moved into an overlap group to
resolve the overlapping pattern with the LQ instruction.
The cbo* instructions perform permission checks and raise exceptions
according to the specification.

The cache block sizes (for cbom and cboz) are configurable.

Co-developed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
Signed-off-by: Christoph Muellner <cmuellner@linux.com>
---
 target/riscv/cpu.c                          |  4 +
 target/riscv/cpu.h                          |  4 +
 target/riscv/helper.h                       |  5 ++
 target/riscv/insn32.decode                  | 16 +++-
 target/riscv/insn_trans/trans_rvzicbo.c.inc | 57 ++++++++++++
 target/riscv/op_helper.c                    | 97 +++++++++++++++++++++
 target/riscv/translate.c                    |  1 +
 7 files changed, 183 insertions(+), 1 deletion(-)
 create mode 100644 target/riscv/insn_trans/trans_rvzicbo.c.inc

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 39ffb883fc..04500fe352 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
     DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
     DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
+    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
+    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
+    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
+    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
     DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
     DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
     DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fe80caeec0..5fda1fc7be 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -368,6 +368,8 @@ struct RISCVCPUConfig {
     bool ext_counters;
     bool ext_ifencei;
     bool ext_icsr;
+    bool ext_icbom;
+    bool ext_icboz;
     bool ext_zfh;
     bool ext_zfhmin;
     bool ext_zve32f;
@@ -382,6 +384,8 @@ struct RISCVCPUConfig {
     char *vext_spec;
     uint16_t vlen;
     uint16_t elen;
+    uint16_t cbom_blocksize;
+    uint16_t cboz_blocksize;
     bool mmu;
     bool pmp;
     bool epmp;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 72cc2582f4..ef1944da8f 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -92,6 +92,11 @@ DEF_HELPER_FLAGS_2(fcvt_h_l, TCG_CALL_NO_RWG, i64, env, tl)
 DEF_HELPER_FLAGS_2(fcvt_h_lu, TCG_CALL_NO_RWG, i64, env, tl)
 DEF_HELPER_FLAGS_1(fclass_h, TCG_CALL_NO_RWG_SE, tl, i64)
 
+/* Cache-block operations */
+DEF_HELPER_2(cbo_clean_flush, void, env, tl)
+DEF_HELPER_2(cbo_inval, void, env, tl)
+DEF_HELPER_2(cbo_zero, void, env, tl)
+
 /* Special functions */
 DEF_HELPER_2(csrr, tl, env, int)
 DEF_HELPER_3(csrw, void, env, int, tl)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 5bbedc254c..d5f8329970 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -128,6 +128,7 @@ addi     ............     ..... 000 ..... 0010011 @i
 slti     ............     ..... 010 ..... 0010011 @i
 sltiu    ............     ..... 011 ..... 0010011 @i
 xori     ............     ..... 100 ..... 0010011 @i
+# cbo.prefetch_{i,r,m} instructions are ori with rd=x0 and not decoded.
 ori      ............     ..... 110 ..... 0010011 @i
 andi     ............     ..... 111 ..... 0010011 @i
 slli     00000. ......    ..... 001 ..... 0010011 @sh
@@ -168,7 +169,20 @@ sraw     0100000 .....  ..... 101 ..... 0111011 @r
 
 # *** RV128I Base Instruction Set (in addition to RV64I) ***
 ldu      ............   ..... 111 ..... 0000011 @i
-lq       ............   ..... 010 ..... 0001111 @i
+{
+  [
+    # *** RV32 Zicbom Standard Extension ***
+    cbo_clean  0000000 00001 ..... 010 00000 0001111 @sfence_vm
+    cbo_flush  0000000 00010 ..... 010 00000 0001111 @sfence_vm
+    cbo_inval  0000000 00000 ..... 010 00000 0001111 @sfence_vm
+
+    # *** RV32 Zicboz Standard Extension ***
+    cbo_zero   0000000 00100 ..... 010 00000 0001111 @sfence_vm
+  ]
+
+  # *** RVI128 lq ***
+  lq       ............   ..... 010 ..... 0001111 @i
+}
 sq       ............   ..... 100 ..... 0100011 @s
 addid    ............  .....  000 ..... 1011011 @i
 sllid    000000 ......  ..... 001 ..... 1011011 @sh6
diff --git a/target/riscv/insn_trans/trans_rvzicbo.c.inc b/target/riscv/insn_trans/trans_rvzicbo.c.inc
new file mode 100644
index 0000000000..e14754f91d
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
@@ -0,0 +1,57 @@
+/*
+ * RISC-V translation routines for the RISC-V CBO Extension.
+ *
+ * Copyright (c) 2021 Philipp Tomsich, philipp.tomsich@vrull.eu
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define REQUIRE_ZICBOM(ctx) do {	\
+    if (!ctx->cfg_ptr->ext_icbom) {	\
+        return false;			\
+    }					\
+} while (0)
+
+#define REQUIRE_ZICBOZ(ctx) do {	\
+    if (!ctx->cfg_ptr->ext_icboz) {	\
+        return false;			\
+    }					\
+} while (0)
+
+static bool trans_cbo_clean(DisasContext *ctx, arg_cbo_clean *a)
+{
+    REQUIRE_ZICBOM(ctx);
+    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
+    return true;
+}
+
+static bool trans_cbo_flush(DisasContext *ctx, arg_cbo_flush *a)
+{
+    REQUIRE_ZICBOM(ctx);
+    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
+    return true;
+}
+
+static bool trans_cbo_inval(DisasContext *ctx, arg_cbo_inval *a)
+{
+    REQUIRE_ZICBOM(ctx);
+    gen_helper_cbo_inval(cpu_env, cpu_gpr[a->rs1]);
+    return true;
+}
+
+static bool trans_cbo_zero(DisasContext *ctx, arg_cbo_zero *a)
+{
+    REQUIRE_ZICBOZ(ctx);
+    gen_helper_cbo_zero(cpu_env, cpu_gpr[a->rs1]);
+    return true;
+}
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 1a75ba11e6..c207cdf29c 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
  * Copyright (c) 2017-2018 SiFive, Inc.
+ * Copyright (c) 2022      VRULL GmbH
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -114,6 +115,102 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
     return int128_getlo(rv);
 }
 
+
+/* helper_zicbo_envcfg
+ *
+ * Raise virtual exceptions and illegal instruction exceptions for
+ * Zicbo[mz] instructions based on the settings of [mhs]envcfg as
+ * specified in section 2.5.1 of the CMO specification.
+ */
+static void helper_zicbo_envcfg(CPURISCVState *env, target_ulong envbits,
+                                uintptr_t ra)
+{
+#ifndef CONFIG_USER_ONLY
+    /* Check for virtual instruction exceptions first, as we don't see
+     * VU and VS reflected in env->priv (these are just the translated
+     * U and S stated with virtualisation enabled.
+     */
+    if (riscv_cpu_virt_enabled(env) &&
+        (((env->priv < PRV_H) && !get_field(env->henvcfg, envbits)) ||
+         ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits)))) {
+        riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, ra);
+    }
+
+    if (((env->priv < PRV_M) && !get_field(env->menvcfg, envbits)) ||
+        ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits))) {
+        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
+    }
+#endif
+}
+
+/* helper_zicbom_access
+ *
+ * Check access permissions (LOAD, STORE or FETCH as specified in section
+ * 2.5.2 of the CMO specification) for Zicbom, raising either store
+ * page-fault (non-virtualised) or store guest-page fault (virtualised).
+ */
+static void helper_zicbom_access(CPURISCVState *env, target_ulong address,
+                                 uintptr_t ra)
+{
+    int ret;
+    void* phost;
+    int mmu_idx = cpu_mmu_index(env, false);
+
+    /* Get the size of the cache block for management instructions. */
+    RISCVCPU *cpu = env_archcpu(env);
+    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
+
+    /* Mask off low-bits to align-down to the cache-block. */
+    address &= ~(cbomlen - 1);
+
+    /* A cache-block management instruction is permitted to access
+     * the specified cache block whenever a load instruction, store
+     * instruction, or instruction fetch is permitted to access the
+     * corresponding physical addresses.
+     */
+    ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
+                                   mmu_idx, true, &phost, ra);
+    if (ret == TLB_INVALID_MASK)
+        ret = probe_access_range_flags(env, address, cbomlen, MMU_INST_FETCH,
+                                       mmu_idx, true, &phost, ra);
+    if (ret == TLB_INVALID_MASK)
+        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
+                                 mmu_idx, false, &phost, ra);
+}
+
+void helper_cbo_clean_flush(CPURISCVState *env, target_ulong address)
+{
+    uintptr_t ra = GETPC();
+    helper_zicbo_envcfg(env, MENVCFG_CBCFE, ra);
+    helper_zicbom_access(env, address, ra);
+}
+
+void helper_cbo_inval(CPURISCVState *env, target_ulong address)
+{
+    uintptr_t ra = GETPC();
+    helper_zicbo_envcfg(env, MENVCFG_CBIE, ra);
+    helper_zicbom_access(env, address, ra);
+}
+
+void helper_cbo_zero(CPURISCVState *env, target_ulong address)
+{
+    uintptr_t ra = GETPC();
+    helper_zicbo_envcfg(env, MENVCFG_CBZE, ra);
+
+    /* Get the size of the cache block for zero instructions. */
+    RISCVCPU *cpu = env_archcpu(env);
+    uint16_t cbozlen = cpu->cfg.cboz_blocksize;
+
+    /* Mask off low-bits to align-down to the cache-block. */
+    address &= ~(cbozlen - 1);
+
+    void* mem = probe_access(env, address, cbozlen, MMU_DATA_STORE,
+                             cpu_mmu_index(env, false), GETPC());
+
+    /* Zero the block */
+    memset(mem, 0, cbozlen);
+}
+
 #ifndef CONFIG_USER_ONLY
 
 target_ulong helper_sret(CPURISCVState *env)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index eaf5a72c81..0ee2ce85ec 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -861,6 +861,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 #include "insn_trans/trans_rvv.c.inc"
 #include "insn_trans/trans_rvb.c.inc"
 #include "insn_trans/trans_rvzfh.c.inc"
+#include "insn_trans/trans_rvzicbo.c.inc"
 #include "insn_trans/trans_privileged.c.inc"
 #include "insn_trans/trans_xventanacondops.c.inc"
 
-- 
2.35.1

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Weiwei Li]

在 2022/2/16 下午11:48, Christoph Muellner 写道:
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 39ffb883fc..04500fe352 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
>       DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>       DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>       DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
> +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
Why use two different cache block size here? Is there any new spec 
update for this?
>       DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
>       DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
>       DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> +
> +/* helper_zicbom_access
> + *
> + * Check access permissions (LOAD, STORE or FETCH as specified in section
> + * 2.5.2 of the CMO specification) for Zicbom, raising either store
> + * page-fault (non-virtualised) or store guest-page fault (virtualised).
> + */
> +static void helper_zicbom_access(CPURISCVState *env, target_ulong address,
> +                                 uintptr_t ra)
> +{
> +    int ret;
> +    void* phost;
> +    int mmu_idx = cpu_mmu_index(env, false);
> +
> +    /* Get the size of the cache block for management instructions. */
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
> +
> +    /* Mask off low-bits to align-down to the cache-block. */
> +    address &= ~(cbomlen - 1);
> +
> +    /* A cache-block management instruction is permitted to access
> +     * the specified cache block whenever a load instruction, store
> +     * instruction, or instruction fetch is permitted to access the
> +     * corresponding physical addresses.
> +     */
> +    ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
> +                                   mmu_idx, true, &phost, ra);
> +    if (ret == TLB_INVALID_MASK)
> +        ret = probe_access_range_flags(env, address, cbomlen, MMU_INST_FETCH,
> +                                       mmu_idx, true, &phost, ra);
> +    if (ret == TLB_INVALID_MASK)
> +        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
> +                                 mmu_idx, false, &phost, ra);
> +}
> +


I think it's a little different here. Probe_access_range_flags may 
trigger different execptions for different access_type. For example:

If  the page for the address  is executable and readable but not 
writable,  and the access cannot pass the pmp check for all access_type,

it may trigger access fault for load/fetch access, and  trigger page 
fault for  store access.

I think the final exception should be access fault instead of the page 
fault caused by probe_access_range_flags with MMU_DATA_STORE.

Regards,

Weiwei Li

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Christoph Müllner]

[-- Attachment #1: Type: text/plain, Size: 5112 bytes --]

On Thu, Feb 17, 2022 at 3:15 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:

>
> 在 2022/2/16 下午11:48, Christoph Muellner 写道:
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 39ffb883fc..04500fe352 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
> >       DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> >       DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> >       DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> > +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> > +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize,
> 64),
> > +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize,
> 64),
> Why use two different cache block size here? Is there any new spec
> update for this?
>

No, we are talking about the same specification.

Section 2.7 states the following:
"""
The initial set of CMO extensions requires the following information to be
discovered by software:
* The size of the cache block for management and prefetch instructions
* The size of the cache block for zero instructions
* CBIE support at each privilege level
"""

So at least the spec authors did differentiate between the two block sizes
as well.


> >       DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> >       DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> >       DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > +
> > +/* helper_zicbom_access
> > + *
> > + * Check access permissions (LOAD, STORE or FETCH as specified in
> section
> > + * 2.5.2 of the CMO specification) for Zicbom, raising either store
> > + * page-fault (non-virtualised) or store guest-page fault (virtualised).
> > + */
> > +static void helper_zicbom_access(CPURISCVState *env, target_ulong
> address,
> > +                                 uintptr_t ra)
> > +{
> > +    int ret;
> > +    void* phost;
> > +    int mmu_idx = cpu_mmu_index(env, false);
> > +
> > +    /* Get the size of the cache block for management instructions. */
> > +    RISCVCPU *cpu = env_archcpu(env);
> > +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
> > +
> > +    /* Mask off low-bits to align-down to the cache-block. */
> > +    address &= ~(cbomlen - 1);
> > +
> > +    /* A cache-block management instruction is permitted to access
> > +     * the specified cache block whenever a load instruction, store
> > +     * instruction, or instruction fetch is permitted to access the
> > +     * corresponding physical addresses.
> > +     */
> > +    ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
> > +                                   mmu_idx, true, &phost, ra);
> > +    if (ret == TLB_INVALID_MASK)
> > +        ret = probe_access_range_flags(env, address, cbomlen,
> MMU_INST_FETCH,
> > +                                       mmu_idx, true, &phost, ra);
> > +    if (ret == TLB_INVALID_MASK)
> > +        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
> > +                                 mmu_idx, false, &phost, ra);
> > +}
> > +
>
>
> I think it's a little different here. Probe_access_range_flags may
> trigger different execptions for different access_type. For example:
>
> If  the page for the address  is executable and readable but not
> writable,  and the access cannot pass the pmp check for all access_type,
>
> it may trigger access fault for load/fetch access, and  trigger page
> fault for  store access.
>

Just to be clear:
The patch does not trigger any fault for LOAD or FETCH because nonfault is
set
to true (6th argument of probe_access_range_flags()).
Only the last call to probe_access_range_flags() raises an exception.

Section 2.5.2 states the following:
"""
If access to the cache block is not permitted, a cache-block management
instruction raises a store page fault or store guest-page fault exception
if address translation does not permit any
access or raises a store access fault exception otherwise.
"""

In your scenario we have (1...allowed; 0...not allowed):
* read: perm:1, pmp:0
* fetch: perm:1: pmp:0
* write: perm:0, pmp:0

Address translation would allow read and fetch access, but PMP blocks that.
So the "does not permit any"-part is wrong, therefore we should raise a
store page fault.

In fact, I can't predict what will happen, because the code in
target/riscv/cpu_helper.c does
not really prioritize page faults or PMP faults. it returns one of them,
once they are encountered.

In order to model this properly, we would have to refactor cpu_helper.c to
separate page permissions
from PMP. However, that seems a bit out of scope for a Zicbo* support
patchset.



>
> I think the final exception should be access fault instead of the page
> fault caused by probe_access_range_flags with MMU_DATA_STORE.
>
> Regards,
>
> Weiwei Li
>
>

[-- Attachment #2: Type: text/html, Size: 6710 bytes --]

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Weiwei Li]

[-- Attachment #1: Type: text/plain, Size: 6121 bytes --]


在 2022/2/17 上午11:59, Christoph Müllner 写道:
>
>
> On Thu, Feb 17, 2022 at 3:15 AM Weiwei Li <liweiwei@iscas.ac.cn 
> <mailto:liweiwei@iscas.ac.cn>> wrote:
>
>
>     在 2022/2/16 下午11:48, Christoph Muellner 写道:
>     > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>     > index 39ffb883fc..04500fe352 100644
>     > --- a/target/riscv/cpu.c
>     > +++ b/target/riscv/cpu.c
>     > @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
>     >       DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters,
>     true),
>     >       DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>     >       DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>     > +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
>     > +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
>     > +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU,
>     cfg.cbom_blocksize, 64),
>     > +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU,
>     cfg.cboz_blocksize, 64),
>     Why use two different cache block size here? Is there any new spec
>     update for this?
>
>
> No, we are talking about the same specification.
>
> Section 2.7 states the following:
> """
> The initial set of CMO extensions requires the following information 
> to be discovered by software:
> * The size of the cache block for management and prefetch instructions
> * The size of the cache block for zero instructions
> * CBIE support at each privilege level
> """
>
> So at least the spec authors did differentiate between the two block 
> sizes as well.
>
OK. This seems a little unreasonable from personal point.

>     >       DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
>     >       DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
>     >       DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
>     > +
>     > +/* helper_zicbom_access
>     > + *
>     > + * Check access permissions (LOAD, STORE or FETCH as specified
>     in section
>     > + * 2.5.2 of the CMO specification) for Zicbom, raising either store
>     > + * page-fault (non-virtualised) or store guest-page fault
>     (virtualised).
>     > + */
>     > +static void helper_zicbom_access(CPURISCVState *env,
>     target_ulong address,
>     > +                                 uintptr_t ra)
>     > +{
>     > +    int ret;
>     > +    void* phost;
>     > +    int mmu_idx = cpu_mmu_index(env, false);
>     > +
>     > +    /* Get the size of the cache block for management
>     instructions. */
>     > +    RISCVCPU *cpu = env_archcpu(env);
>     > +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
>     > +
>     > +    /* Mask off low-bits to align-down to the cache-block. */
>     > +    address &= ~(cbomlen - 1);
>     > +
>     > +    /* A cache-block management instruction is permitted to access
>     > +     * the specified cache block whenever a load instruction, store
>     > +     * instruction, or instruction fetch is permitted to access the
>     > +     * corresponding physical addresses.
>     > +     */
>     > +    ret = probe_access_range_flags(env, address, cbomlen,
>     MMU_DATA_LOAD,
>     > +                                   mmu_idx, true, &phost, ra);
>     > +    if (ret == TLB_INVALID_MASK)
>     > +        ret = probe_access_range_flags(env, address, cbomlen,
>     MMU_INST_FETCH,
>     > +                                       mmu_idx, true, &phost, ra);
>     > +    if (ret == TLB_INVALID_MASK)
>     > +        probe_access_range_flags(env, address, cbomlen,
>     MMU_DATA_STORE,
>     > +                                 mmu_idx, false, &phost, ra);
>     > +}
>     > +
>
>
>     I think it's a little different here. Probe_access_range_flags may
>     trigger different execptions for different access_type. For example:
>
>     If  the page for the address  is executable and readable but not
>     writable,  and the access cannot pass the pmp check for all
>     access_type,
>
>     it may trigger access fault for load/fetch access, and trigger page
>     fault for  store access.
>
>
> Just to be clear:
> The patch does not trigger any fault for LOAD or FETCH because 
> nonfault is set
> to true (6th argument of probe_access_range_flags()).
> Only the last call to probe_access_range_flags() raises an exception.
>
> Section 2.5.2 states the following:
> """
> If access to the cache block is not permitted, a cache-block management
> instruction raises a store page fault or store guest-page fault 
> exception if address translation does not permit any
> access or raises a store access fault exception otherwise.
> """
>
> In your scenario we have (1...allowed; 0...not allowed):
> * read: perm:1, pmp:0
> * fetch: perm:1: pmp:0
> * write: perm:0, pmp:0
>
> Address translation would allow read and fetch access, but PMP blocks 
> that.
> So the "does not permit any"-part is wrong, therefore we should raise 
> a store page fault.

There is debate between us here. I think the opposite of "any" here is 
"permit one of access type" not "permit all access types".

  And from your above code,  it also will ignore check for fetch and 
write, if read access is permitted(the only difference with my example 
is that read also pass PMP check).

So if Address translation  allow read and fetch access, page fault 
shouldn't be triggered.

Regards,

Weiwei Li

>
> In fact, I can't predict what will happen, because the code in 
> target/riscv/cpu_helper.c does
> not really prioritize page faults or PMP faults. it returns one of 
> them, once they are encountered.
> In order to model this properly, we would have to refactor 
> cpu_helper.c to separate page permissions
> from PMP. However, that seems a bit out of scope for a Zicbo* support 
> patchset.
>
>
>     I think the final exception should be access fault instead of the
>     page
>     fault caused by probe_access_range_flags with MMU_DATA_STORE.
>
>     Regards,
>
>     Weiwei Li
>

[-- Attachment #2: Type: text/html, Size: 10405 bytes --]

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Frank Chang]

[-- Attachment #1: Type: text/plain, Size: 6071 bytes --]

Christoph Müllner <cmuellner@linux.com> 於 2022年2月17日 週四 下午12:00寫道：

>
>
> On Thu, Feb 17, 2022 at 3:15 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>>
>> 在 2022/2/16 下午11:48, Christoph Muellner 写道:
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 39ffb883fc..04500fe352 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
>> >       DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>> >       DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>> >       DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>> > +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
>> > +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
>> > +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize,
>> 64),
>> > +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize,
>> 64),
>> Why use two different cache block size here? Is there any new spec
>> update for this?
>>
>
> No, we are talking about the same specification.
>
> Section 2.7 states the following:
> """
> The initial set of CMO extensions requires the following information to be
> discovered by software:
> * The size of the cache block for management and prefetch instructions
> * The size of the cache block for zero instructions
> * CBIE support at each privilege level
> """
>
> So at least the spec authors did differentiate between the two block sizes
> as well.
>
>
>> >       DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
>> >       DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
>> >       DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
>> > +
>> > +/* helper_zicbom_access
>> > + *
>> > + * Check access permissions (LOAD, STORE or FETCH as specified in
>> section
>> > + * 2.5.2 of the CMO specification) for Zicbom, raising either store
>> > + * page-fault (non-virtualised) or store guest-page fault
>> (virtualised).
>> > + */
>> > +static void helper_zicbom_access(CPURISCVState *env, target_ulong
>> address,
>> > +                                 uintptr_t ra)
>> > +{
>> > +    int ret;
>> > +    void* phost;
>> > +    int mmu_idx = cpu_mmu_index(env, false);
>> > +
>> > +    /* Get the size of the cache block for management instructions. */
>> > +    RISCVCPU *cpu = env_archcpu(env);
>> > +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
>> > +
>> > +    /* Mask off low-bits to align-down to the cache-block. */
>> > +    address &= ~(cbomlen - 1);
>> > +
>> > +    /* A cache-block management instruction is permitted to access
>> > +     * the specified cache block whenever a load instruction, store
>> > +     * instruction, or instruction fetch is permitted to access the
>> > +     * corresponding physical addresses.
>> > +     */
>> > +    ret = probe_access_range_flags(env, address, cbomlen,
>> MMU_DATA_LOAD,
>> > +                                   mmu_idx, true, &phost, ra);
>> > +    if (ret == TLB_INVALID_MASK)
>> > +        ret = probe_access_range_flags(env, address, cbomlen,
>> MMU_INST_FETCH,
>> > +                                       mmu_idx, true, &phost, ra);
>> > +    if (ret == TLB_INVALID_MASK)
>> > +        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
>> > +                                 mmu_idx, false, &phost, ra);
>> > +}
>> > +
>>
>>
>> I think it's a little different here. Probe_access_range_flags may
>> trigger different execptions for different access_type. For example:
>>
>> If  the page for the address  is executable and readable but not
>> writable,  and the access cannot pass the pmp check for all access_type,
>>
>> it may trigger access fault for load/fetch access, and  trigger page
>> fault for  store access.
>>
>
> Just to be clear:
> The patch does not trigger any fault for LOAD or FETCH because nonfault is
> set
> to true (6th argument of probe_access_range_flags()).
> Only the last call to probe_access_range_flags() raises an exception.
>
> Section 2.5.2 states the following:
> """
> If access to the cache block is not permitted, a cache-block management
> instruction raises a store page fault or store guest-page fault exception
> if address translation does not permit any
> access or raises a store access fault exception otherwise.
> """
>
> In your scenario we have (1...allowed; 0...not allowed):
> * read: perm:1, pmp:0
> * fetch: perm:1: pmp:0
> * write: perm:0, pmp:0
>
> Address translation would allow read and fetch access, but PMP blocks that.
> So the "does not permit any"-part is wrong, therefore we should raise a
> store page fault.
>
> In fact, I can't predict what will happen, because the code in
> target/riscv/cpu_helper.c does
> not really prioritize page faults or PMP faults. it returns one of them,
> once they are encountered.
>

Hi Christoph,

May I ask what does "page faults or PMP faults are not prioritized" here
mean?

In target/riscv/cpu_helper.c, if "pmp_violation" flag is not set to true,
page fault will be picked.
So as long as the TRANSLATE_PMP_FAIL is returned, it will be indicated as a
PMP fault.
(The only exception I can see is that TRANSLATE_PMP_FAIL may be converted
to TRANSLATE_G_STAGE_FAIL
  if it's the second stage translation and PMP fault on PTE entry's PA.)
As the "final PA" is checked only after the page table is walked,
shouldn't the "pmp_violation" flag only be set after all the translation
accesses are checked and granted?

Regards,
Frank Chang


>
> In order to model this properly, we would have to refactor cpu_helper.c to
> separate page permissions
> from PMP. However, that seems a bit out of scope for a Zicbo* support
> patchset.
>
>
>
>>
>> I think the final exception should be access fault instead of the page
>> fault caused by probe_access_range_flags with MMU_DATA_STORE.
>>
>> Regards,
>>
>> Weiwei Li
>>
>>

[-- Attachment #2: Type: text/html, Size: 8139 bytes --]

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Anup Patel]

On Wed, Feb 16, 2022 at 9:18 PM Christoph Muellner <cmuellner@linux.com> wrote:
>
> The RISC-V base cache management operation ISA extension has been
> ratified. This patch adds support for the defined instructions.
>
> The cmo.prefetch instructions are nops for QEMU (no emulation of the memory
> hierarchy, no illegal instructions, no permission faults, no traps),
> therefore there's only a comment where they would be decoded.
>
> The other cbo* instructions are moved into an overlap group to
> resolve the overlapping pattern with the LQ instruction.
> The cbo* instructions perform permission checks and raise exceptions
> according to the specification.
>
> The cache block sizes (for cbom and cboz) are configurable.
>
> Co-developed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> Signed-off-by: Christoph Muellner <cmuellner@linux.com>

Can you rebase this series upon Atish's "target/riscv: Add isa extenstion
strings to the device tree" patch ?

Also, please add cmo extensions in the generated ISA string.

Regards,
Anup

> ---
>  target/riscv/cpu.c                          |  4 +
>  target/riscv/cpu.h                          |  4 +
>  target/riscv/helper.h                       |  5 ++
>  target/riscv/insn32.decode                  | 16 +++-
>  target/riscv/insn_trans/trans_rvzicbo.c.inc | 57 ++++++++++++
>  target/riscv/op_helper.c                    | 97 +++++++++++++++++++++
>  target/riscv/translate.c                    |  1 +
>  7 files changed, 183 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/insn_trans/trans_rvzicbo.c.inc
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 39ffb883fc..04500fe352 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>      DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>      DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
> +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
>      DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
>      DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
>      DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe80caeec0..5fda1fc7be 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -368,6 +368,8 @@ struct RISCVCPUConfig {
>      bool ext_counters;
>      bool ext_ifencei;
>      bool ext_icsr;
> +    bool ext_icbom;
> +    bool ext_icboz;
>      bool ext_zfh;
>      bool ext_zfhmin;
>      bool ext_zve32f;
> @@ -382,6 +384,8 @@ struct RISCVCPUConfig {
>      char *vext_spec;
>      uint16_t vlen;
>      uint16_t elen;
> +    uint16_t cbom_blocksize;
> +    uint16_t cboz_blocksize;
>      bool mmu;
>      bool pmp;
>      bool epmp;
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 72cc2582f4..ef1944da8f 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -92,6 +92,11 @@ DEF_HELPER_FLAGS_2(fcvt_h_l, TCG_CALL_NO_RWG, i64, env, tl)
>  DEF_HELPER_FLAGS_2(fcvt_h_lu, TCG_CALL_NO_RWG, i64, env, tl)
>  DEF_HELPER_FLAGS_1(fclass_h, TCG_CALL_NO_RWG_SE, tl, i64)
>
> +/* Cache-block operations */
> +DEF_HELPER_2(cbo_clean_flush, void, env, tl)
> +DEF_HELPER_2(cbo_inval, void, env, tl)
> +DEF_HELPER_2(cbo_zero, void, env, tl)
> +
>  /* Special functions */
>  DEF_HELPER_2(csrr, tl, env, int)
>  DEF_HELPER_3(csrw, void, env, int, tl)
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 5bbedc254c..d5f8329970 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -128,6 +128,7 @@ addi     ............     ..... 000 ..... 0010011 @i
>  slti     ............     ..... 010 ..... 0010011 @i
>  sltiu    ............     ..... 011 ..... 0010011 @i
>  xori     ............     ..... 100 ..... 0010011 @i
> +# cbo.prefetch_{i,r,m} instructions are ori with rd=x0 and not decoded.
>  ori      ............     ..... 110 ..... 0010011 @i
>  andi     ............     ..... 111 ..... 0010011 @i
>  slli     00000. ......    ..... 001 ..... 0010011 @sh
> @@ -168,7 +169,20 @@ sraw     0100000 .....  ..... 101 ..... 0111011 @r
>
>  # *** RV128I Base Instruction Set (in addition to RV64I) ***
>  ldu      ............   ..... 111 ..... 0000011 @i
> -lq       ............   ..... 010 ..... 0001111 @i
> +{
> +  [
> +    # *** RV32 Zicbom Standard Extension ***
> +    cbo_clean  0000000 00001 ..... 010 00000 0001111 @sfence_vm
> +    cbo_flush  0000000 00010 ..... 010 00000 0001111 @sfence_vm
> +    cbo_inval  0000000 00000 ..... 010 00000 0001111 @sfence_vm
> +
> +    # *** RV32 Zicboz Standard Extension ***
> +    cbo_zero   0000000 00100 ..... 010 00000 0001111 @sfence_vm
> +  ]
> +
> +  # *** RVI128 lq ***
> +  lq       ............   ..... 010 ..... 0001111 @i
> +}
>  sq       ............   ..... 100 ..... 0100011 @s
>  addid    ............  .....  000 ..... 1011011 @i
>  sllid    000000 ......  ..... 001 ..... 1011011 @sh6
> diff --git a/target/riscv/insn_trans/trans_rvzicbo.c.inc b/target/riscv/insn_trans/trans_rvzicbo.c.inc
> new file mode 100644
> index 0000000000..e14754f91d
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
> @@ -0,0 +1,57 @@
> +/*
> + * RISC-V translation routines for the RISC-V CBO Extension.
> + *
> + * Copyright (c) 2021 Philipp Tomsich, philipp.tomsich@vrull.eu
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define REQUIRE_ZICBOM(ctx) do {       \
> +    if (!ctx->cfg_ptr->ext_icbom) {    \
> +        return false;                  \
> +    }                                  \
> +} while (0)
> +
> +#define REQUIRE_ZICBOZ(ctx) do {       \
> +    if (!ctx->cfg_ptr->ext_icboz) {    \
> +        return false;                  \
> +    }                                  \
> +} while (0)
> +
> +static bool trans_cbo_clean(DisasContext *ctx, arg_cbo_clean *a)
> +{
> +    REQUIRE_ZICBOM(ctx);
> +    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
> +    return true;
> +}
> +
> +static bool trans_cbo_flush(DisasContext *ctx, arg_cbo_flush *a)
> +{
> +    REQUIRE_ZICBOM(ctx);
> +    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
> +    return true;
> +}
> +
> +static bool trans_cbo_inval(DisasContext *ctx, arg_cbo_inval *a)
> +{
> +    REQUIRE_ZICBOM(ctx);
> +    gen_helper_cbo_inval(cpu_env, cpu_gpr[a->rs1]);
> +    return true;
> +}
> +
> +static bool trans_cbo_zero(DisasContext *ctx, arg_cbo_zero *a)
> +{
> +    REQUIRE_ZICBOZ(ctx);
> +    gen_helper_cbo_zero(cpu_env, cpu_gpr[a->rs1]);
> +    return true;
> +}
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 1a75ba11e6..c207cdf29c 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -3,6 +3,7 @@
>   *
>   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
>   * Copyright (c) 2017-2018 SiFive, Inc.
> + * Copyright (c) 2022      VRULL GmbH
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -114,6 +115,102 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
>      return int128_getlo(rv);
>  }
>
> +
> +/* helper_zicbo_envcfg
> + *
> + * Raise virtual exceptions and illegal instruction exceptions for
> + * Zicbo[mz] instructions based on the settings of [mhs]envcfg as
> + * specified in section 2.5.1 of the CMO specification.
> + */
> +static void helper_zicbo_envcfg(CPURISCVState *env, target_ulong envbits,
> +                                uintptr_t ra)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    /* Check for virtual instruction exceptions first, as we don't see
> +     * VU and VS reflected in env->priv (these are just the translated
> +     * U and S stated with virtualisation enabled.
> +     */
> +    if (riscv_cpu_virt_enabled(env) &&
> +        (((env->priv < PRV_H) && !get_field(env->henvcfg, envbits)) ||
> +         ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits)))) {
> +        riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, ra);
> +    }
> +
> +    if (((env->priv < PRV_M) && !get_field(env->menvcfg, envbits)) ||
> +        ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits))) {
> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
> +    }
> +#endif
> +}
> +
> +/* helper_zicbom_access
> + *
> + * Check access permissions (LOAD, STORE or FETCH as specified in section
> + * 2.5.2 of the CMO specification) for Zicbom, raising either store
> + * page-fault (non-virtualised) or store guest-page fault (virtualised).
> + */
> +static void helper_zicbom_access(CPURISCVState *env, target_ulong address,
> +                                 uintptr_t ra)
> +{
> +    int ret;
> +    void* phost;
> +    int mmu_idx = cpu_mmu_index(env, false);
> +
> +    /* Get the size of the cache block for management instructions. */
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
> +
> +    /* Mask off low-bits to align-down to the cache-block. */
> +    address &= ~(cbomlen - 1);
> +
> +    /* A cache-block management instruction is permitted to access
> +     * the specified cache block whenever a load instruction, store
> +     * instruction, or instruction fetch is permitted to access the
> +     * corresponding physical addresses.
> +     */
> +    ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
> +                                   mmu_idx, true, &phost, ra);
> +    if (ret == TLB_INVALID_MASK)
> +        ret = probe_access_range_flags(env, address, cbomlen, MMU_INST_FETCH,
> +                                       mmu_idx, true, &phost, ra);
> +    if (ret == TLB_INVALID_MASK)
> +        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
> +                                 mmu_idx, false, &phost, ra);
> +}
> +
> +void helper_cbo_clean_flush(CPURISCVState *env, target_ulong address)
> +{
> +    uintptr_t ra = GETPC();
> +    helper_zicbo_envcfg(env, MENVCFG_CBCFE, ra);
> +    helper_zicbom_access(env, address, ra);
> +}
> +
> +void helper_cbo_inval(CPURISCVState *env, target_ulong address)
> +{
> +    uintptr_t ra = GETPC();
> +    helper_zicbo_envcfg(env, MENVCFG_CBIE, ra);
> +    helper_zicbom_access(env, address, ra);
> +}
> +
> +void helper_cbo_zero(CPURISCVState *env, target_ulong address)
> +{
> +    uintptr_t ra = GETPC();
> +    helper_zicbo_envcfg(env, MENVCFG_CBZE, ra);
> +
> +    /* Get the size of the cache block for zero instructions. */
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint16_t cbozlen = cpu->cfg.cboz_blocksize;
> +
> +    /* Mask off low-bits to align-down to the cache-block. */
> +    address &= ~(cbozlen - 1);
> +
> +    void* mem = probe_access(env, address, cbozlen, MMU_DATA_STORE,
> +                             cpu_mmu_index(env, false), GETPC());
> +
> +    /* Zero the block */
> +    memset(mem, 0, cbozlen);
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>
>  target_ulong helper_sret(CPURISCVState *env)
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index eaf5a72c81..0ee2ce85ec 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -861,6 +861,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>  #include "insn_trans/trans_rvv.c.inc"
>  #include "insn_trans/trans_rvb.c.inc"
>  #include "insn_trans/trans_rvzfh.c.inc"
> +#include "insn_trans/trans_rvzicbo.c.inc"
>  #include "insn_trans/trans_privileged.c.inc"
>  #include "insn_trans/trans_xventanacondops.c.inc"
>
> --
> 2.35.1
>

Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions

[Atish Patra]

On Wed, Mar 16, 2022 at 1:01 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Wed, Feb 16, 2022 at 9:18 PM Christoph Muellner <cmuellner@linux.com> wrote:
> >
> > The RISC-V base cache management operation ISA extension has been
> > ratified. This patch adds support for the defined instructions.
> >
> > The cmo.prefetch instructions are nops for QEMU (no emulation of the memory
> > hierarchy, no illegal instructions, no permission faults, no traps),
> > therefore there's only a comment where they would be decoded.
> >
> > The other cbo* instructions are moved into an overlap group to
> > resolve the overlapping pattern with the LQ instruction.
> > The cbo* instructions perform permission checks and raise exceptions
> > according to the specification.
> >
> > The cache block sizes (for cbom and cboz) are configurable.
> >
> > Co-developed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > Signed-off-by: Christoph Muellner <cmuellner@linux.com>
>
> Can you rebase this series upon Atish's "target/riscv: Add isa extenstion
> strings to the device tree" patch ?
>
> Also, please add cmo extensions in the generated ISA string.
>

I couldn't find a v5. Is this the latest version or did I miss something ?

> Regards,
> Anup
>
> > ---
> >  target/riscv/cpu.c                          |  4 +
> >  target/riscv/cpu.h                          |  4 +
> >  target/riscv/helper.h                       |  5 ++
> >  target/riscv/insn32.decode                  | 16 +++-
> >  target/riscv/insn_trans/trans_rvzicbo.c.inc | 57 ++++++++++++
> >  target/riscv/op_helper.c                    | 97 +++++++++++++++++++++
> >  target/riscv/translate.c                    |  1 +
> >  7 files changed, 183 insertions(+), 1 deletion(-)
> >  create mode 100644 target/riscv/insn_trans/trans_rvzicbo.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 39ffb883fc..04500fe352 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
> >      DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> >      DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> >      DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > +    DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> > +    DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> > +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
> > +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
> >      DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> >      DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> >      DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index fe80caeec0..5fda1fc7be 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -368,6 +368,8 @@ struct RISCVCPUConfig {
> >      bool ext_counters;
> >      bool ext_ifencei;
> >      bool ext_icsr;
> > +    bool ext_icbom;
> > +    bool ext_icboz;
> >      bool ext_zfh;
> >      bool ext_zfhmin;
> >      bool ext_zve32f;
> > @@ -382,6 +384,8 @@ struct RISCVCPUConfig {
> >      char *vext_spec;
> >      uint16_t vlen;
> >      uint16_t elen;
> > +    uint16_t cbom_blocksize;
> > +    uint16_t cboz_blocksize;
> >      bool mmu;
> >      bool pmp;
> >      bool epmp;
> > diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> > index 72cc2582f4..ef1944da8f 100644
> > --- a/target/riscv/helper.h
> > +++ b/target/riscv/helper.h
> > @@ -92,6 +92,11 @@ DEF_HELPER_FLAGS_2(fcvt_h_l, TCG_CALL_NO_RWG, i64, env, tl)
> >  DEF_HELPER_FLAGS_2(fcvt_h_lu, TCG_CALL_NO_RWG, i64, env, tl)
> >  DEF_HELPER_FLAGS_1(fclass_h, TCG_CALL_NO_RWG_SE, tl, i64)
> >
> > +/* Cache-block operations */
> > +DEF_HELPER_2(cbo_clean_flush, void, env, tl)
> > +DEF_HELPER_2(cbo_inval, void, env, tl)
> > +DEF_HELPER_2(cbo_zero, void, env, tl)
> > +
> >  /* Special functions */
> >  DEF_HELPER_2(csrr, tl, env, int)
> >  DEF_HELPER_3(csrw, void, env, int, tl)
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 5bbedc254c..d5f8329970 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -128,6 +128,7 @@ addi     ............     ..... 000 ..... 0010011 @i
> >  slti     ............     ..... 010 ..... 0010011 @i
> >  sltiu    ............     ..... 011 ..... 0010011 @i
> >  xori     ............     ..... 100 ..... 0010011 @i
> > +# cbo.prefetch_{i,r,m} instructions are ori with rd=x0 and not decoded.
> >  ori      ............     ..... 110 ..... 0010011 @i
> >  andi     ............     ..... 111 ..... 0010011 @i
> >  slli     00000. ......    ..... 001 ..... 0010011 @sh
> > @@ -168,7 +169,20 @@ sraw     0100000 .....  ..... 101 ..... 0111011 @r
> >
> >  # *** RV128I Base Instruction Set (in addition to RV64I) ***
> >  ldu      ............   ..... 111 ..... 0000011 @i
> > -lq       ............   ..... 010 ..... 0001111 @i
> > +{
> > +  [
> > +    # *** RV32 Zicbom Standard Extension ***
> > +    cbo_clean  0000000 00001 ..... 010 00000 0001111 @sfence_vm
> > +    cbo_flush  0000000 00010 ..... 010 00000 0001111 @sfence_vm
> > +    cbo_inval  0000000 00000 ..... 010 00000 0001111 @sfence_vm
> > +
> > +    # *** RV32 Zicboz Standard Extension ***
> > +    cbo_zero   0000000 00100 ..... 010 00000 0001111 @sfence_vm
> > +  ]
> > +
> > +  # *** RVI128 lq ***
> > +  lq       ............   ..... 010 ..... 0001111 @i
> > +}
> >  sq       ............   ..... 100 ..... 0100011 @s
> >  addid    ............  .....  000 ..... 1011011 @i
> >  sllid    000000 ......  ..... 001 ..... 1011011 @sh6
> > diff --git a/target/riscv/insn_trans/trans_rvzicbo.c.inc b/target/riscv/insn_trans/trans_rvzicbo.c.inc
> > new file mode 100644
> > index 0000000000..e14754f91d
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
> > @@ -0,0 +1,57 @@
> > +/*
> > + * RISC-V translation routines for the RISC-V CBO Extension.
> > + *
> > + * Copyright (c) 2021 Philipp Tomsich, philipp.tomsich@vrull.eu
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#define REQUIRE_ZICBOM(ctx) do {       \
> > +    if (!ctx->cfg_ptr->ext_icbom) {    \
> > +        return false;                  \
> > +    }                                  \
> > +} while (0)
> > +
> > +#define REQUIRE_ZICBOZ(ctx) do {       \
> > +    if (!ctx->cfg_ptr->ext_icboz) {    \
> > +        return false;                  \
> > +    }                                  \
> > +} while (0)
> > +
> > +static bool trans_cbo_clean(DisasContext *ctx, arg_cbo_clean *a)
> > +{
> > +    REQUIRE_ZICBOM(ctx);
> > +    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
> > +    return true;
> > +}
> > +
> > +static bool trans_cbo_flush(DisasContext *ctx, arg_cbo_flush *a)
> > +{
> > +    REQUIRE_ZICBOM(ctx);
> > +    gen_helper_cbo_clean_flush(cpu_env, cpu_gpr[a->rs1]);
> > +    return true;
> > +}
> > +
> > +static bool trans_cbo_inval(DisasContext *ctx, arg_cbo_inval *a)
> > +{
> > +    REQUIRE_ZICBOM(ctx);
> > +    gen_helper_cbo_inval(cpu_env, cpu_gpr[a->rs1]);
> > +    return true;
> > +}
> > +
> > +static bool trans_cbo_zero(DisasContext *ctx, arg_cbo_zero *a)
> > +{
> > +    REQUIRE_ZICBOZ(ctx);
> > +    gen_helper_cbo_zero(cpu_env, cpu_gpr[a->rs1]);
> > +    return true;
> > +}
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index 1a75ba11e6..c207cdf29c 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -3,6 +3,7 @@
> >   *
> >   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
> >   * Copyright (c) 2017-2018 SiFive, Inc.
> > + * Copyright (c) 2022      VRULL GmbH
> >   *
> >   * This program is free software; you can redistribute it and/or modify it
> >   * under the terms and conditions of the GNU General Public License,
> > @@ -114,6 +115,102 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
> >      return int128_getlo(rv);
> >  }
> >
> > +
> > +/* helper_zicbo_envcfg
> > + *
> > + * Raise virtual exceptions and illegal instruction exceptions for
> > + * Zicbo[mz] instructions based on the settings of [mhs]envcfg as
> > + * specified in section 2.5.1 of the CMO specification.
> > + */
> > +static void helper_zicbo_envcfg(CPURISCVState *env, target_ulong envbits,
> > +                                uintptr_t ra)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > +    /* Check for virtual instruction exceptions first, as we don't see
> > +     * VU and VS reflected in env->priv (these are just the translated
> > +     * U and S stated with virtualisation enabled.
> > +     */
> > +    if (riscv_cpu_virt_enabled(env) &&
> > +        (((env->priv < PRV_H) && !get_field(env->henvcfg, envbits)) ||
> > +         ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits)))) {
> > +        riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, ra);
> > +    }
> > +
> > +    if (((env->priv < PRV_M) && !get_field(env->menvcfg, envbits)) ||
> > +        ((env->priv < PRV_S) && !get_field(env->senvcfg, envbits))) {
> > +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
> > +    }
> > +#endif
> > +}
> > +
> > +/* helper_zicbom_access
> > + *
> > + * Check access permissions (LOAD, STORE or FETCH as specified in section
> > + * 2.5.2 of the CMO specification) for Zicbom, raising either store
> > + * page-fault (non-virtualised) or store guest-page fault (virtualised).
> > + */
> > +static void helper_zicbom_access(CPURISCVState *env, target_ulong address,
> > +                                 uintptr_t ra)
> > +{
> > +    int ret;
> > +    void* phost;
> > +    int mmu_idx = cpu_mmu_index(env, false);
> > +
> > +    /* Get the size of the cache block for management instructions. */
> > +    RISCVCPU *cpu = env_archcpu(env);
> > +    uint16_t cbomlen = cpu->cfg.cbom_blocksize;
> > +
> > +    /* Mask off low-bits to align-down to the cache-block. */
> > +    address &= ~(cbomlen - 1);
> > +
> > +    /* A cache-block management instruction is permitted to access
> > +     * the specified cache block whenever a load instruction, store
> > +     * instruction, or instruction fetch is permitted to access the
> > +     * corresponding physical addresses.
> > +     */
> > +    ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
> > +                                   mmu_idx, true, &phost, ra);
> > +    if (ret == TLB_INVALID_MASK)
> > +        ret = probe_access_range_flags(env, address, cbomlen, MMU_INST_FETCH,
> > +                                       mmu_idx, true, &phost, ra);
> > +    if (ret == TLB_INVALID_MASK)
> > +        probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
> > +                                 mmu_idx, false, &phost, ra);
> > +}
> > +
> > +void helper_cbo_clean_flush(CPURISCVState *env, target_ulong address)
> > +{
> > +    uintptr_t ra = GETPC();
> > +    helper_zicbo_envcfg(env, MENVCFG_CBCFE, ra);
> > +    helper_zicbom_access(env, address, ra);
> > +}
> > +
> > +void helper_cbo_inval(CPURISCVState *env, target_ulong address)
> > +{
> > +    uintptr_t ra = GETPC();
> > +    helper_zicbo_envcfg(env, MENVCFG_CBIE, ra);
> > +    helper_zicbom_access(env, address, ra);
> > +}
> > +
> > +void helper_cbo_zero(CPURISCVState *env, target_ulong address)
> > +{
> > +    uintptr_t ra = GETPC();
> > +    helper_zicbo_envcfg(env, MENVCFG_CBZE, ra);
> > +
> > +    /* Get the size of the cache block for zero instructions. */
> > +    RISCVCPU *cpu = env_archcpu(env);
> > +    uint16_t cbozlen = cpu->cfg.cboz_blocksize;
> > +
> > +    /* Mask off low-bits to align-down to the cache-block. */
> > +    address &= ~(cbozlen - 1);
> > +
> > +    void* mem = probe_access(env, address, cbozlen, MMU_DATA_STORE,
> > +                             cpu_mmu_index(env, false), GETPC());
> > +
> > +    /* Zero the block */
> > +    memset(mem, 0, cbozlen);
> > +}
> > +
> >  #ifndef CONFIG_USER_ONLY
> >
> >  target_ulong helper_sret(CPURISCVState *env)
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index eaf5a72c81..0ee2ce85ec 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -861,6 +861,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> >  #include "insn_trans/trans_rvv.c.inc"
> >  #include "insn_trans/trans_rvb.c.inc"
> >  #include "insn_trans/trans_rvzfh.c.inc"
> > +#include "insn_trans/trans_rvzicbo.c.inc"
> >  #include "insn_trans/trans_privileged.c.inc"
> >  #include "insn_trans/trans_xventanacondops.c.inc"
> >
> > --
> > 2.35.1
> >
>


-- 
Regards,
Atish

Re: [PATCH v4 0/2] riscv: Add support for Zicbo[m,z,p] instructions

[Sudip Mukherjee]

Hi Christoph,

On Wed, Feb 16, 2022 at 04:48:37PM +0100, Christoph Muellner wrote:
> The RISC-V base cache management operation ISA extension has been
> ratified [1]. This patchset adds support for the defined instructions.
> 
> As the exception behavior of these instructions depend on the PMP
> configuration, the first patch introduces a new API to probe the access
> of an address range with a specified size with optional nonfaulting
> behavior.
> 
> The Zicbo[m,z,p] patch should be straight-forward and has been reviewed
> in previous versions of this patchset.

I have not seen any v5 yet, unless I have missed. Are you planning to
send one?
fwiw, I rebased them on top of v7.2.0 and tested that it works.

-- 
Regards
Sudip

Re: [PATCH v4 0/2] riscv: Add support for Zicbo[m,z,p] instructions

[Daniel Henrique Barboza]

Hi,

FYI I grabbed these patches, rebased using Alistair's riscv-to-apply.next, and I
plan to re-send them as v5 in the next few days (maintaining Christoph's authorship,
of course).

I'll see if I can implement some of the suggestions made in the v4 one year ago as
well. To keep the original patches intact I'll do that in separated patches.


Thanks,


Daniel

On 1/24/23 15:04, Sudip Mukherjee wrote:
> Hi Christoph,
> 
> On Wed, Feb 16, 2022 at 04:48:37PM +0100, Christoph Muellner wrote:
>> The RISC-V base cache management operation ISA extension has been
>> ratified [1]. This patchset adds support for the defined instructions.
>>
>> As the exception behavior of these instructions depend on the PMP
>> configuration, the first patch introduces a new API to probe the access
>> of an address range with a specified size with optional nonfaulting
>> behavior.
>>
>> The Zicbo[m,z,p] patch should be straight-forward and has been reviewed
>> in previous versions of this patchset.
> 
> I have not seen any v5 yet, unless I have missed. Are you planning to
> send one?
> fwiw, I rebased them on top of v7.2.0 and tested that it works.
>