From: Christoph Muellner <cmuellner@linux.com>
The RISC-V base cache management operation (CBO) ISA extension has been
ratified. It defines three extensions: Cache-Block Management, Cache-Block
Prefetch and Cache-Block Zero. More information about the spec can be
found at [1].
Let's start by implementing the Cache-Block Zero extension, Zicboz. It
uses the cbo.zero instruction that, as with all CBO instructions that
will be added later, needs to be implemented in an overlap group with
the LQ instruction due to overlapping patterns.
cbo.zero throws a Illegal Instruction/Virtual Instruction exception
depending on CSR state. This is also the case for the remaining cbo
instructions we're going to add next, so create a check_zicbo_envcfg()
that will be used by all Zicbo[mz] instructions.
[1] https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf
Co-developed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
Signed-off-by: Christoph Muellner <cmuellner@linux.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/cpu.c | 4 ++
target/riscv/cpu.h | 2 +
target/riscv/helper.h | 3 ++
target/riscv/insn32.decode | 10 +++-
target/riscv/insn_trans/trans_rvzicbo.c.inc | 30 +++++++++++
target/riscv/op_helper.c | 55 +++++++++++++++++++++
target/riscv/translate.c | 1 +
7 files changed, 104 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 93b52b826c..7dd37de7f9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -74,6 +74,7 @@ struct isa_ext_data {
static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h),
ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v),
+ ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz),
ISA_EXT_DATA_ENTRY(zicsr, true, PRIV_VERSION_1_10_0, ext_icsr),
ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0, ext_zihintpause),
@@ -1126,6 +1127,9 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
+ DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
+ DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
+
DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
/* Vendor-specific custom extensions */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7128438d8e..6b4c714d3a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -447,6 +447,7 @@ struct RISCVCPUConfig {
bool ext_zkt;
bool ext_ifencei;
bool ext_icsr;
+ bool ext_icboz;
bool ext_zihintpause;
bool ext_smstateen;
bool ext_sstc;
@@ -494,6 +495,7 @@ struct RISCVCPUConfig {
char *vext_spec;
uint16_t vlen;
uint16_t elen;
+ uint16_t cboz_blocksize;
bool mmu;
bool pmp;
bool epmp;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 0497370afd..ce165821b8 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -97,6 +97,9 @@ 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_2(fclass_h, TCG_CALL_NO_RWG_SE, tl, env, i64)
+/* Cache-block operations */
+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 b7e7613ea2..3985bc703f 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -179,7 +179,15 @@ sraw 0100000 ..... ..... 101 ..... 0111011 @r
# *** RV128I Base Instruction Set (in addition to RV64I) ***
ldu ............ ..... 111 ..... 0000011 @i
-lq ............ ..... 010 ..... 0001111 @i
+{
+ [
+ # *** 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..feabc28342
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
@@ -0,0 +1,30 @@
+/*
+ * 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_ZICBOZ(ctx) do { \
+ if (!ctx->cfg_ptr->ext_icboz) { \
+ return false; \
+ } \
+} while (0)
+
+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 48f918b71b..c5053e9446 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,
@@ -123,6 +124,60 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
return int128_getlo(rv);
}
+
+/*
+ * check_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 check_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
+}
+
+void helper_cbo_zero(CPURISCVState *env, target_ulong address)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+ uintptr_t ra = GETPC();
+ uint16_t cbozlen;
+ void *mem;
+
+ check_zicbo_envcfg(env, MENVCFG_CBZE, ra);
+
+ /* Get the size of the cache block for zero instructions. */
+ cbozlen = cpu->cfg.cboz_blocksize;
+
+ /* Mask off low-bits to align-down to the cache-block. */
+ address &= ~(cbozlen - 1);
+
+ mem = tlb_vaddr_to_host(env, address, MMU_DATA_STORE,
+ cpu_mmu_index(env, false));
+
+ if (likely(mem)) {
+ /* 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 772f9d7973..7f687a7e37 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1104,6 +1104,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_rvzawrs.c.inc"
+#include "insn_trans/trans_rvzicbo.c.inc"
#include "insn_trans/trans_rvzfh.c.inc"
#include "insn_trans/trans_rvk.c.inc"
#include "insn_trans/trans_privileged.c.inc"
--
2.39.2
On 2023/2/18 04:34, Daniel Henrique Barboza wrote:
> From: Christoph Muellner <cmuellner@linux.com>
>
> The RISC-V base cache management operation (CBO) ISA extension has been
> ratified. It defines three extensions: Cache-Block Management, Cache-Block
> Prefetch and Cache-Block Zero. More information about the spec can be
> found at [1].
>
> Let's start by implementing the Cache-Block Zero extension, Zicboz. It
> uses the cbo.zero instruction that, as with all CBO instructions that
> will be added later, needs to be implemented in an overlap group with
> the LQ instruction due to overlapping patterns.
>
> cbo.zero throws a Illegal Instruction/Virtual Instruction exception
> depending on CSR state. This is also the case for the remaining cbo
> instructions we're going to add next, so create a check_zicbo_envcfg()
> that will be used by all Zicbo[mz] instructions.
>
> [1] https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf
>
> Co-developed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> Signed-off-by: Christoph Muellner <cmuellner@linux.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu.c | 4 ++
> target/riscv/cpu.h | 2 +
> target/riscv/helper.h | 3 ++
> target/riscv/insn32.decode | 10 +++-
> target/riscv/insn_trans/trans_rvzicbo.c.inc | 30 +++++++++++
> target/riscv/op_helper.c | 55 +++++++++++++++++++++
> target/riscv/translate.c | 1 +
> 7 files changed, 104 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 93b52b826c..7dd37de7f9 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -74,6 +74,7 @@ struct isa_ext_data {
> static const struct isa_ext_data isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h),
> ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v),
> + ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz),
> ISA_EXT_DATA_ENTRY(zicsr, true, PRIV_VERSION_1_10_0, ext_icsr),
> ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
> ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0, ext_zihintpause),
> @@ -1126,6 +1127,9 @@ static Property riscv_cpu_extensions[] = {
> DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
> DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
>
> + DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> + DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
> +
> DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>
> /* Vendor-specific custom extensions */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7128438d8e..6b4c714d3a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -447,6 +447,7 @@ struct RISCVCPUConfig {
> bool ext_zkt;
> bool ext_ifencei;
> bool ext_icsr;
> + bool ext_icboz;
> bool ext_zihintpause;
> bool ext_smstateen;
> bool ext_sstc;
> @@ -494,6 +495,7 @@ struct RISCVCPUConfig {
> char *vext_spec;
> uint16_t vlen;
> uint16_t elen;
> + uint16_t cboz_blocksize;
> bool mmu;
> bool pmp;
> bool epmp;
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 0497370afd..ce165821b8 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -97,6 +97,9 @@ 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_2(fclass_h, TCG_CALL_NO_RWG_SE, tl, env, i64)
>
> +/* Cache-block operations */
> +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 b7e7613ea2..3985bc703f 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -179,7 +179,15 @@ sraw 0100000 ..... ..... 101 ..... 0111011 @r
>
> # *** RV128I Base Instruction Set (in addition to RV64I) ***
> ldu ............ ..... 111 ..... 0000011 @i
> -lq ............ ..... 010 ..... 0001111 @i
> +{
> + [
> + # *** 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..feabc28342
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
> @@ -0,0 +1,30 @@
> +/*
> + * 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_ZICBOZ(ctx) do { \
> + if (!ctx->cfg_ptr->ext_icboz) { \
> + return false; \
> + } \
> +} while (0)
> +
> +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 48f918b71b..c5053e9446 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,
> @@ -123,6 +124,60 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
> return int128_getlo(rv);
> }
>
> +
> +/*
> + * check_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 check_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);
> + }
Check of (env->priv < PRV_M) && !get_field(env->menvcfg, envbits) should
be put before upper check.
Illegal instruction exception should be triggered if (env->priv < PRV_M)
&& !get_field(env->menvcfg, envbits).
However, virtual instruction exception may be triggered if we do upper
check firstly.
Regards,
Weiwei Li
> +
> + 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
> +}
> +
> +void helper_cbo_zero(CPURISCVState *env, target_ulong address)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + uintptr_t ra = GETPC();
> + uint16_t cbozlen;
> + void *mem;
> +
> + check_zicbo_envcfg(env, MENVCFG_CBZE, ra);
> +
> + /* Get the size of the cache block for zero instructions. */
> + cbozlen = cpu->cfg.cboz_blocksize;
> +
> + /* Mask off low-bits to align-down to the cache-block. */
> + address &= ~(cbozlen - 1);
> +
> + mem = tlb_vaddr_to_host(env, address, MMU_DATA_STORE,
> + cpu_mmu_index(env, false));
> +
> + if (likely(mem)) {
> + /* 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 772f9d7973..7f687a7e37 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1104,6 +1104,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_rvzawrs.c.inc"
> +#include "insn_trans/trans_rvzicbo.c.inc"
> #include "insn_trans/trans_rvzfh.c.inc"
> #include "insn_trans/trans_rvk.c.inc"
> #include "insn_trans/trans_privileged.c.inc"
On 2/17/23 10:34, Daniel Henrique Barboza wrote:
> +void helper_cbo_zero(CPURISCVState *env, target_ulong address)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + uintptr_t ra = GETPC();
> + uint16_t cbozlen;
> + void *mem;
> +
> + check_zicbo_envcfg(env, MENVCFG_CBZE, ra);
> +
> + /* Get the size of the cache block for zero instructions. */
> + cbozlen = cpu->cfg.cboz_blocksize;
> +
> + /* Mask off low-bits to align-down to the cache-block. */
> + address &= ~(cbozlen - 1);
> +
> + mem = tlb_vaddr_to_host(env, address, MMU_DATA_STORE,
> + cpu_mmu_index(env, false));
> +
> + if (likely(mem)) {
> + /* Zero the block */
> + memset(mem, 0, cbozlen);
> + }
> +}
Not correct. This fails to zero the block at all under a number of conditions.
Please have a closer look at the feedback on v5.
r~
On 2/18/23 00:44, Richard Henderson wrote:
> On 2/17/23 10:34, Daniel Henrique Barboza wrote:
>> +void helper_cbo_zero(CPURISCVState *env, target_ulong address)
>> +{
>> + RISCVCPU *cpu = env_archcpu(env);
>> + uintptr_t ra = GETPC();
>> + uint16_t cbozlen;
>> + void *mem;
>> +
>> + check_zicbo_envcfg(env, MENVCFG_CBZE, ra);
>> +
>> + /* Get the size of the cache block for zero instructions. */
>> + cbozlen = cpu->cfg.cboz_blocksize;
>> +
>> + /* Mask off low-bits to align-down to the cache-block. */
>> + address &= ~(cbozlen - 1);
>> +
>> + mem = tlb_vaddr_to_host(env, address, MMU_DATA_STORE,
>> + cpu_mmu_index(env, false));
>> +
>> + if (likely(mem)) {
>> + /* Zero the block */
>> + memset(mem, 0, cbozlen);
>> + }
>> +}
>
> Not correct. This fails to zero the block at all under a number of conditions.
Ops. The 'else' conditional in which we would zero the mem in case it's an I/O
region got lost in the middle of rebasing it seems ....
By the way, looking at the lack of any probing in this particular function and at
the probe_writes() that ARM does, I read the spec again. A paragraph in 2.5.2
says:
"A cache-block zero instruction is permitted to access the specified cache block whenever
a store instruction is permitted to access the corresponding physical addresses and when
the PMAs indicate that cache-block zero instructions are a supported access type. If access
to the cache block is not permitted, a cache-block zero instruction raises a store page fault
or store guest-page fault exception if address translation does not permit write access or
raises a store access fault exception otherwise. During address translation, the instruction
also checks the accessed and dirty bits and may either raise an exception or set the bits as
required."
PMA (Physical Memory Access) doesn't seem to be implemented in RISC-V, or at the very least
it's not using the PMA acronym, so let's skip that for now. I'll add a comment mentioning
it in the code mentioning that PMA for I/O should be checked and we're not doing it.
But PMA aside, we have wording and conditionals that resembles the cache-block management
permissions and so on, with the exception that we're not caring about LOAD access this time
around. So I guess we'll also need here something like what check_zicbom_access() is doing
in the next patch.
Thanks,
Daniel
> Please have a closer look at the feedback on v5.
>
>
> r~
On 2/17/23 23:28, Daniel Henrique Barboza wrote:
> "A cache-block zero instruction is permitted to access the specified cache block whenever
> a store instruction is permitted to access the corresponding physical addresses and when
> the PMAs indicate that cache-block zero instructions are a supported access type. If access
> to the cache block is not permitted, a cache-block zero instruction raises a store page fault
> or store guest-page fault exception if address translation does not permit write access or
> raises a store access fault exception otherwise. During address translation, the instruction
> also checks the accessed and dirty bits and may either raise an exception or set the bits as
> required."
By the way, I think the documentation should specify what happens if the page is *not*
accessible. Is badaddr = {rN, aligned(rN), unspecified, but somewhere in the block}?
r~
On 2/18/23 16:35, Richard Henderson wrote:
> On 2/17/23 23:28, Daniel Henrique Barboza wrote:
>> "A cache-block zero instruction is permitted to access the specified cache block whenever
>> a store instruction is permitted to access the corresponding physical addresses and when
>> the PMAs indicate that cache-block zero instructions are a supported access type. If access
>> to the cache block is not permitted, a cache-block zero instruction raises a store page fault
>> or store guest-page fault exception if address translation does not permit write access or
>> raises a store access fault exception otherwise. During address translation, the instruction
>> also checks the accessed and dirty bits and may either raise an exception or set the bits as
>> required."
>
> By the way, I think the documentation should specify what happens if the page is *not* accessible. Is badaddr = {rN, aligned(rN), unspecified, but somewhere in the block}?
>
Do you mean that the doc should tell whether the address to be returned in the store
access fault should be aligned and whatnot?
Yeah, this is not mentioned in the docs. I think you're wondering if we should do like
ARM does, like you're mentioned in the previous version:
=======
While re-reading the ARM code, I remembered that the ARM dc.zva instruction is required
to produce original unmasked address on a page fault, thus the little dance with two
calls to probe_write.
=======
By reading target/riscv code it seems that all store acess faults are being fired via
raise_mmu_exception(), which is only called in riscv_cpu_tlb_fill(), which in turn
doesn't do any thing special with the address before firing the exception. So I guess
we can assume that badddr = aligned?
Daniel
>
> r~
On 2/21/23 23:37, Daniel Henrique Barboza wrote: > Do you mean that the doc should tell whether the address to be returned in the store > access fault should be aligned and whatnot? Yes. > By reading target/riscv code it seems that all store acess faults are being fired via > raise_mmu_exception(), which is only called in riscv_cpu_tlb_fill(), which in turn > doesn't do any thing special with the address before firing the exception. So I guess > we can assume that badddr = aligned? Well that's certainly how the code is written, yes. r~
© 2016 - 2026 Red Hat, Inc.