[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/riscv: Convert epmp from feature to ratified smepmp e
|
From: |
Daniel Henrique Barboza |
|
Subject: |
Re: [PATCH] target/riscv: Convert epmp from feature to ratified smepmp extension |
|
Date: |
Tue, 28 Feb 2023 12:53:26 -0300 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
Hey. Sorry for the delay, this patch went through the cracks :(
(CCing the reviewers)
(CCing qemu-devel as well)
On 2/3/23 01:24, Himanshu Chauhan wrote:
Smepmp is a ratified extension. This patch converts the existing epmp
support as a feature to an extension. With this, "smepmp" string will
be concatenated to riscv,isa in generated FDT.
Since the official name of the extension is Smepmp, the references
to epmp have also been changed to smepmp.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 16 ++++++----------
target/riscv/cpu.h | 3 +--
target/riscv/csr.c | 8 +++++---
target/riscv/pmp.c | 13 +++++++++----
4 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 14a7027095..d9bba04226 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -103,6 +103,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zhinx, true, PRIV_VERSION_1_12_0, ext_zhinx),
ISA_EXT_DATA_ENTRY(zhinxmin, true, PRIV_VERSION_1_12_0, ext_zhinxmin),
ISA_EXT_DATA_ENTRY(smaia, true, PRIV_VERSION_1_12_0, ext_smaia),
+ ISA_EXT_DATA_ENTRY(smepmp, true, PRIV_VERSION_1_12_0, ext_smepmp),
ISA_EXT_DATA_ENTRY(ssaia, true, PRIV_VERSION_1_12_0, ext_ssaia),
ISA_EXT_DATA_ENTRY(sscofpmf, true, PRIV_VERSION_1_12_0, ext_sscofpmf),
ISA_EXT_DATA_ENTRY(sstc, true, PRIV_VERSION_1_12_0, ext_sstc),
@@ -325,7 +326,7 @@ static void rv32_ibex_cpu_init(Object *obj)
register_cpu_props(DEVICE(obj));
set_priv_version(env, PRIV_VERSION_1_11_0);
cpu->cfg.mmu = false;
- cpu->cfg.epmp = true;
+ cpu->cfg.ext_smepmp = true;
}
static void rv32_imafcu_nommu_cpu_init(Object *obj)
@@ -884,14 +885,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
if (cpu->cfg.pmp) {
riscv_set_feature(env, RISCV_FEATURE_PMP);
-
- /*
- * Enhanced PMP should only be available
- * on harts with PMP support
- */
- if (cpu->cfg.epmp) {
- riscv_set_feature(env, RISCV_FEATURE_EPMP);
- }
+ } else {
+ /* smepmp requires pmp support */
+ cpu->cfg.ext_smepmp = false;
I believe this and other chunk like it will collide with the work done in:
"[PATCH v7 00/10] make write_misa a no-op and FEATURE_* cleanups"
That series is already acked, so it would make the maintainer's life easier to
base this patch on top of that. You can add a:
Based-on: 20230222185205.355361-1-dbarboza@ventanamicro.com
("[PATCH v7 00/10] make write_misa a no-op and FEATURE_* cleanups")
at the start of the cover-letter to indicate the dependency.
}
if (cpu->cfg.debug) {
@@ -1051,6 +1047,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false),
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+ DEFINE_PROP_BOOL("smepmp", RISCVCPU, cfg.ext_smepmp, false),
DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true),
DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
@@ -1093,7 +1090,6 @@ static Property riscv_cpu_extensions[] = {
/* These are experimental so mark with 'x-' */
DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
/* ePMP 0.9.3 */
- DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
DEFINE_PROP_BOOL("x-ssaia", RISCVCPU, cfg.ext_ssaia, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bcf0826753..30fd29e578 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -87,7 +87,6 @@
enum {
RISCV_FEATURE_MMU,
RISCV_FEATURE_PMP,
- RISCV_FEATURE_EPMP,
RISCV_FEATURE_MISA,
RISCV_FEATURE_DEBUG
};
@@ -465,6 +464,7 @@ struct RISCVCPUConfig {
bool ext_smaia;
bool ext_ssaia;
bool ext_sscofpmf;
+ bool ext_smepmp;
bool rvv_ta_all_1s;
bool rvv_ma_all_1s;
@@ -484,7 +484,6 @@ struct RISCVCPUConfig {
uint16_t elen;
bool mmu;
bool pmp;
- bool epmp;
bool debug;
bool short_isa_string;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 62e6c4acbd..f70f6d9aad 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -426,9 +426,11 @@ static RISCVException pmp(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
-static RISCVException epmp(CPURISCVState *env, int csrno)
+static RISCVException smepmp(CPURISCVState *env, int csrno)
{
- if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) {
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (env->priv == PRV_M && cpu->cfg.ext_smepmp) {
I did a recent cleanup in these code patterns by using a new riscv_cpu_cfg()
API.
If you rebase this patch on top of that series (which has the API) this code
would
become:
- if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) {
+ if (env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smepmp) {
return RISCV_EXCP_NONE;
}
@@ -4259,7 +4261,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_VSIPH] = { "vsiph", aia_hmode32, NULL, NULL, rmw_vsiph },
/* Physical Memory Protection */
- [CSR_MSECCFG] = { "mseccfg", epmp, read_mseccfg, write_mseccfg,
+ [CSR_MSECCFG] = { "mseccfg", smepmp, read_mseccfg, write_mseccfg,
.min_priv_ver = PRIV_VERSION_1_11_0 },
[CSR_PMPCFG0] = { "pmpcfg0", pmp, read_pmpcfg, write_pmpcfg },
[CSR_PMPCFG1] = { "pmpcfg1", pmp, read_pmpcfg, write_pmpcfg },
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index d1126a6066..d85ad07caa 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -58,6 +58,11 @@ static inline int pmp_is_locked(CPURISCVState *env, uint32_t
pmp_index)
return 0;
}
+static inline int smepmp_is_enabled(CPURISCVState *env)
+{
+ return (RISCV_CPU(env_cpu(env))->cfg.ext_smepmp);
+}
+
I believe you can just read "riscv_cpu_cfg(env)->ext_smepmp" directly wherever
you need
instead of creating a new function
/*
* Count the number of active rules.
*/
@@ -88,7 +93,7 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t
pmp_index, uint8_t val)
if (pmp_index < MAX_RISCV_PMPS) {
bool locked = true;
- if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
+ if (smepmp_is_enabled(env)) {
/* mseccfg.RLB is set */
if (MSECCFG_RLB_ISSET(env)) {
locked = false;
@@ -239,7 +244,7 @@ static bool pmp_hart_has_privs_default(CPURISCVState *env,
target_ulong addr,
{
bool ret;
- if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
+ if (smepmp_is_enabled(env)) {
if (MSECCFG_MMWP_ISSET(env)) {
/*
* The Machine Mode Whitelist Policy (mseccfg.MMWP) is set
@@ -265,7 +270,7 @@ static bool pmp_hart_has_privs_default(CPURISCVState *env,
target_ulong addr,
}
}
- if ((!riscv_feature(env, RISCV_FEATURE_PMP)) || (mode == PRV_M)) {
+ if (!smepmp_is_enabled(env) || (mode == PRV_M)) {
/*
* Privileged spec v1.10 states if HW doesn't implement any PMP entry
* or no PMP entry matches an M-Mode access, the access succeeds.
@@ -348,7 +353,7 @@ int pmp_hart_has_privs(CPURISCVState *env, target_ulong
addr,
/*
* Convert the PMP permissions to match the truth table in the
- * ePMP spec.
+ * Smepmp spec.
*/
const uint8_t epmp_operation =
((env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) >> 4) |
- Re: [PATCH] target/riscv: Convert epmp from feature to ratified smepmp extension,
Daniel Henrique Barboza <=