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
在 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
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
>
>
在 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
>
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
>>
>>
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
>
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
© 2016 - 2026 Red Hat, Inc.