Re: [PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1
On Wed, May 20, 2020 at 6:18 PM Bin Meng wrote:
>
> On Fri, May 8, 2020 at 3:22 AM Alistair Francis
> wrote:
> >
> > The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
> > 4.1. It's not commonly used so let's remove support for it.
> >
> > Signed-off-by: Alistair Francis
> > ---
> > target/riscv/cpu.c| 2 -
> > target/riscv/cpu.h| 1 -
> > target/riscv/csr.c| 82 ---
> > .../riscv/insn_trans/trans_privileged.inc.c | 6 --
> > 4 files changed, 17 insertions(+), 74 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 112f2e3a2f..eeb91f8513 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> > **errp)
> > priv_version = PRIV_VERSION_1_11_0;
> > } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
> > priv_version = PRIV_VERSION_1_10_0;
> > -} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
> > -priv_version = PRIV_VERSION_1_09_1;
> > } else {
> > error_setg(errp,
> > "Unsupported privilege spec version '%s'",
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 76b98d7a33..c022539012 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -73,7 +73,6 @@ enum {
> > RISCV_FEATURE_MISA
> > };
> >
> > -#define PRIV_VERSION_1_09_1 0x00010901
> > #define PRIV_VERSION_1_10_0 0x00011000
> > #define PRIV_VERSION_1_11_0 0x00011100
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 11d184cd16..df3498b24f 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
> > #if !defined(CONFIG_USER_ONLY)
> > CPUState *cs = env_cpu(env);
> > RISCVCPU *cpu = RISCV_CPU(cs);
> > -uint32_t ctr_en = ~0u;
> >
> > if (!cpu->cfg.ext_counters) {
> > /* The Counters extensions is not enabled */
> > return -1;
> > }
> > -
> > -/*
> > - * The counters are always enabled at run time on newer priv specs, as
> > the
> > - * CSR has changed from controlling that the counters can be read to
> > - * controlling that the counters increment.
> > - */
> > -if (env->priv_ver > PRIV_VERSION_1_09_1) {
> > -return 0;
> > -}
> > -
> > -if (env->priv < PRV_M) {
> > -ctr_en &= env->mcounteren;
> > -}
> > -if (env->priv < PRV_S) {
> > -ctr_en &= env->scounteren;
> > -}
> > -if (!(ctr_en & (1u << (csrno & 31 {
> > -return -1;
> > -}
> > #endif
> > return 0;
> > }
> > @@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int
> > csrno, target_ulong val)
> > int dirty;
> >
> > /* flush tlb on mstatus fields that affect VM */
> > -if (env->priv_ver <= PRIV_VERSION_1_09_1) {
> > -if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
> > -MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
> > -tlb_flush(env_cpu(env));
> > -}
> > -mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > -MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> > -MSTATUS_MPP | MSTATUS_MXR |
> > -(validate_vm(env, get_field(val, MSTATUS_VM)) ?
> > -MSTATUS_VM : 0);
> > +if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> > +MSTATUS_MPRV | MSTATUS_SUM)) {
> > +tlb_flush(env_cpu(env));
> > }
> > -if (env->priv_ver >= PRIV_VERSION_1_10_0) {
> > -if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> > -MSTATUS_MPRV | MSTATUS_SUM)) {
> > -tlb_flush(env_cpu(env));
> > -}
> > -mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > -MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> > -MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> > -MSTATUS_TW;
> > +mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > +MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> > +MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> > +MSTATUS_TW;
> > #if defined(TARGET_RISCV64)
> > -/*
> > - * RV32: MPV and MTL are not in mstatus. The current plan is to
> > - * add them to mstatush. For now, we just don't support it.
> > - */
> > -mask |= MSTATUS_MTL | MSTATUS_MPV;
> > +/*
> > + * RV32: MPV and MTL are not in mstatus. The current plan is to
> > + * add them to mstatush. For now, we just don't support it.
> > + */
> > +mask |= MSTATUS_MTL | MSTATUS_MPV;
>
> The indentation level is wrong
Good catch.
>
> > #endif
> > -}
Re: [PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1
On Fri, May 8, 2020 at 3:22 AM Alistair Francis
wrote:
>
> The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
> 4.1. It's not commonly used so let's remove support for it.
>
> Signed-off-by: Alistair Francis
> ---
> target/riscv/cpu.c| 2 -
> target/riscv/cpu.h| 1 -
> target/riscv/csr.c| 82 ---
> .../riscv/insn_trans/trans_privileged.inc.c | 6 --
> 4 files changed, 17 insertions(+), 74 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 112f2e3a2f..eeb91f8513 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> priv_version = PRIV_VERSION_1_11_0;
> } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
> priv_version = PRIV_VERSION_1_10_0;
> -} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
> -priv_version = PRIV_VERSION_1_09_1;
> } else {
> error_setg(errp,
> "Unsupported privilege spec version '%s'",
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 76b98d7a33..c022539012 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -73,7 +73,6 @@ enum {
> RISCV_FEATURE_MISA
> };
>
> -#define PRIV_VERSION_1_09_1 0x00010901
> #define PRIV_VERSION_1_10_0 0x00011000
> #define PRIV_VERSION_1_11_0 0x00011100
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 11d184cd16..df3498b24f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
> #if !defined(CONFIG_USER_ONLY)
> CPUState *cs = env_cpu(env);
> RISCVCPU *cpu = RISCV_CPU(cs);
> -uint32_t ctr_en = ~0u;
>
> if (!cpu->cfg.ext_counters) {
> /* The Counters extensions is not enabled */
> return -1;
> }
> -
> -/*
> - * The counters are always enabled at run time on newer priv specs, as
> the
> - * CSR has changed from controlling that the counters can be read to
> - * controlling that the counters increment.
> - */
> -if (env->priv_ver > PRIV_VERSION_1_09_1) {
> -return 0;
> -}
> -
> -if (env->priv < PRV_M) {
> -ctr_en &= env->mcounteren;
> -}
> -if (env->priv < PRV_S) {
> -ctr_en &= env->scounteren;
> -}
> -if (!(ctr_en & (1u << (csrno & 31 {
> -return -1;
> -}
> #endif
> return 0;
> }
> @@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int csrno,
> target_ulong val)
> int dirty;
>
> /* flush tlb on mstatus fields that affect VM */
> -if (env->priv_ver <= PRIV_VERSION_1_09_1) {
> -if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
> -MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
> -tlb_flush(env_cpu(env));
> -}
> -mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> -MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> -MSTATUS_MPP | MSTATUS_MXR |
> -(validate_vm(env, get_field(val, MSTATUS_VM)) ?
> -MSTATUS_VM : 0);
> +if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> +MSTATUS_MPRV | MSTATUS_SUM)) {
> +tlb_flush(env_cpu(env));
> }
> -if (env->priv_ver >= PRIV_VERSION_1_10_0) {
> -if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> -MSTATUS_MPRV | MSTATUS_SUM)) {
> -tlb_flush(env_cpu(env));
> -}
> -mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> -MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> -MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> -MSTATUS_TW;
> +mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> +MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> +MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> +MSTATUS_TW;
> #if defined(TARGET_RISCV64)
> -/*
> - * RV32: MPV and MTL are not in mstatus. The current plan is to
> - * add them to mstatush. For now, we just don't support it.
> - */
> -mask |= MSTATUS_MTL | MSTATUS_MPV;
> +/*
> + * RV32: MPV and MTL are not in mstatus. The current plan is to
> + * add them to mstatush. For now, we just don't support it.
> + */
> +mask |= MSTATUS_MTL | MSTATUS_MPV;
The indentation level is wrong
> #endif
> -}
>
> mstatus = (mstatus & ~mask) | (val & mask);
>
> @@ -553,8 +520,7 @@ static int write_mcounteren(CPURISCVState *env, int
> csrno, target_ulong val)
> /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
> static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
> {
> -
[PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1
The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
4.1. It's not commonly used so let's remove support for it.
Signed-off-by: Alistair Francis
---
target/riscv/cpu.c| 2 -
target/riscv/cpu.h| 1 -
target/riscv/csr.c| 82 ---
.../riscv/insn_trans/trans_privileged.inc.c | 6 --
4 files changed, 17 insertions(+), 74 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 112f2e3a2f..eeb91f8513 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
priv_version = PRIV_VERSION_1_11_0;
} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
priv_version = PRIV_VERSION_1_10_0;
-} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
-priv_version = PRIV_VERSION_1_09_1;
} else {
error_setg(errp,
"Unsupported privilege spec version '%s'",
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 76b98d7a33..c022539012 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -73,7 +73,6 @@ enum {
RISCV_FEATURE_MISA
};
-#define PRIV_VERSION_1_09_1 0x00010901
#define PRIV_VERSION_1_10_0 0x00011000
#define PRIV_VERSION_1_11_0 0x00011100
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 11d184cd16..df3498b24f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
#if !defined(CONFIG_USER_ONLY)
CPUState *cs = env_cpu(env);
RISCVCPU *cpu = RISCV_CPU(cs);
-uint32_t ctr_en = ~0u;
if (!cpu->cfg.ext_counters) {
/* The Counters extensions is not enabled */
return -1;
}
-
-/*
- * The counters are always enabled at run time on newer priv specs, as the
- * CSR has changed from controlling that the counters can be read to
- * controlling that the counters increment.
- */
-if (env->priv_ver > PRIV_VERSION_1_09_1) {
-return 0;
-}
-
-if (env->priv < PRV_M) {
-ctr_en &= env->mcounteren;
-}
-if (env->priv < PRV_S) {
-ctr_en &= env->scounteren;
-}
-if (!(ctr_en & (1u << (csrno & 31 {
-return -1;
-}
#endif
return 0;
}
@@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int csrno,
target_ulong val)
int dirty;
/* flush tlb on mstatus fields that affect VM */
-if (env->priv_ver <= PRIV_VERSION_1_09_1) {
-if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
-MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
-tlb_flush(env_cpu(env));
-}
-mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
-MSTATUS_MPP | MSTATUS_MXR |
-(validate_vm(env, get_field(val, MSTATUS_VM)) ?
-MSTATUS_VM : 0);
+if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
+MSTATUS_MPRV | MSTATUS_SUM)) {
+tlb_flush(env_cpu(env));
}
-if (env->priv_ver >= PRIV_VERSION_1_10_0) {
-if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
-MSTATUS_MPRV | MSTATUS_SUM)) {
-tlb_flush(env_cpu(env));
-}
-mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
-MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
-MSTATUS_TW;
+mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
+MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
+MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
+MSTATUS_TW;
#if defined(TARGET_RISCV64)
-/*
- * RV32: MPV and MTL are not in mstatus. The current plan is to
- * add them to mstatush. For now, we just don't support it.
- */
-mask |= MSTATUS_MTL | MSTATUS_MPV;
+/*
+ * RV32: MPV and MTL are not in mstatus. The current plan is to
+ * add them to mstatush. For now, we just don't support it.
+ */
+mask |= MSTATUS_MTL | MSTATUS_MPV;
#endif
-}
mstatus = (mstatus & ~mask) | (val & mask);
@@ -553,8 +520,7 @@ static int write_mcounteren(CPURISCVState *env, int csrno,
target_ulong val)
/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
{
-if (env->priv_ver > PRIV_VERSION_1_09_1
-&& env->priv_ver < PRIV_VERSION_1_11_0) {
+if (env->priv_ver < PRIV_VERSION_1_11_0) {
return -1;
}
*val = env->mcounteren;
@@ -564,8 +530,7 @@ static int read_mscounteren(CPURISCVState *env, int csrno,
target_ulong *val)
/* This regiser is replaced with
