* [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags
2023-03-24 5:59 [PATCH 0/4] Fix tb flags use LIU Zhiwei
@ 2023-03-24 5:59 ` LIU Zhiwei
2023-03-24 12:46 ` liweiwei
2023-03-24 17:00 ` Richard Henderson
2023-03-24 5:59 ` [PATCH 2/4] target/riscv: Add a general status enum for extensions LIU Zhiwei
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: LIU Zhiwei @ 2023-03-24 5:59 UTC (permalink / raw)
To: qemu-devel
Cc: richard.henderson, Alistair.Francis, palmer, bin.meng, liweiwei,
dbarboza, qemu-riscv, LIU Zhiwei
Virt enabled state is not a constant. So we should put it into tb flags.
Thus we can use it like a constant condition at translation phase.
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
target/riscv/cpu.h | 2 ++
target/riscv/cpu_helper.c | 2 ++
target/riscv/translate.c | 10 +---------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..12fe8d8546 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -650,6 +650,8 @@ FIELD(TB_FLAGS, VTA, 24, 1)
FIELD(TB_FLAGS, VMA, 25, 1)
/* Native debug itrigger */
FIELD(TB_FLAGS, ITRIGGER, 26, 1)
+/* Virtual mode enabled */
+FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
#ifdef TARGET_RISCV32
#define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..9d50e7bbb6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -104,6 +104,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
get_field(env->mstatus_hs, MSTATUS_VS));
+ flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED,
+ get_field(env->virt, VIRT_ONOFF));
}
if (cpu->cfg.debug && !icount_enabled()) {
flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..880f6318aa 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1156,15 +1156,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
ctx->priv_ver = env->priv_ver;
-#if !defined(CONFIG_USER_ONLY)
- if (riscv_has_ext(env, RVH)) {
- ctx->virt_enabled = riscv_cpu_virt_enabled(env);
- } else {
- ctx->virt_enabled = false;
- }
-#else
- ctx->virt_enabled = false;
-#endif
+ ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
ctx->misa_ext = env->misa_ext;
ctx->frm = -1; /* unknown rounding mode */
ctx->cfg_ptr = &(cpu->cfg);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags
2023-03-24 5:59 ` [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags LIU Zhiwei
@ 2023-03-24 12:46 ` liweiwei
2023-03-24 17:00 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: liweiwei @ 2023-03-24 12:46 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: liweiwei, richard.henderson, Alistair.Francis, palmer, bin.meng,
dbarboza, qemu-riscv
On 2023/3/24 13:59, LIU Zhiwei wrote:
> Virt enabled state is not a constant. So we should put it into tb flags.
> Thus we can use it like a constant condition at translation phase.
>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Weiwei L
> target/riscv/cpu.h | 2 ++
> target/riscv/cpu_helper.c | 2 ++
> target/riscv/translate.c | 10 +---------
> 3 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 638e47c75a..12fe8d8546 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -650,6 +650,8 @@ FIELD(TB_FLAGS, VTA, 24, 1)
> FIELD(TB_FLAGS, VMA, 25, 1)
> /* Native debug itrigger */
> FIELD(TB_FLAGS, ITRIGGER, 26, 1)
> +/* Virtual mode enabled */
> +FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
>
> #ifdef TARGET_RISCV32
> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..9d50e7bbb6 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -104,6 +104,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>
> flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
> get_field(env->mstatus_hs, MSTATUS_VS));
> + flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED,
> + get_field(env->virt, VIRT_ONOFF));
> }
> if (cpu->cfg.debug && !icount_enabled()) {
> flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0ee8ee147d..880f6318aa 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1156,15 +1156,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
> ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
> ctx->priv_ver = env->priv_ver;
> -#if !defined(CONFIG_USER_ONLY)
> - if (riscv_has_ext(env, RVH)) {
> - ctx->virt_enabled = riscv_cpu_virt_enabled(env);
> - } else {
> - ctx->virt_enabled = false;
> - }
> -#else
> - ctx->virt_enabled = false;
> -#endif
> + ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
> ctx->misa_ext = env->misa_ext;
> ctx->frm = -1; /* unknown rounding mode */
> ctx->cfg_ptr = &(cpu->cfg);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags
2023-03-24 5:59 ` [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags LIU Zhiwei
2023-03-24 12:46 ` liweiwei
@ 2023-03-24 17:00 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2023-03-24 17:00 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: Alistair.Francis, palmer, bin.meng, liweiwei, dbarboza, qemu-riscv
On 3/23/23 22:59, LIU Zhiwei wrote:
> Virt enabled state is not a constant. So we should put it into tb flags.
> Thus we can use it like a constant condition at translation phase.
>
> Reported-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/cpu_helper.c | 2 ++
> target/riscv/translate.c | 10 +---------
> 3 files changed, 5 insertions(+), 9 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] target/riscv: Add a general status enum for extensions
2023-03-24 5:59 [PATCH 0/4] Fix tb flags use LIU Zhiwei
2023-03-24 5:59 ` [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags LIU Zhiwei
@ 2023-03-24 5:59 ` LIU Zhiwei
2023-03-24 12:53 ` liweiwei
2023-03-24 5:59 ` [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags LIU Zhiwei
2023-03-24 5:59 ` [PATCH 4/4] target/riscv: Add a tb flags field for vstart LIU Zhiwei
3 siblings, 1 reply; 13+ messages in thread
From: LIU Zhiwei @ 2023-03-24 5:59 UTC (permalink / raw)
To: qemu-devel
Cc: richard.henderson, Alistair.Francis, palmer, bin.meng, liweiwei,
dbarboza, qemu-riscv, LIU Zhiwei
The pointer masking is the only extension that directly use status.
The vector or float extension uses the status in an indirect way.
Replace the pointer masking extension special status fields with
the general status.
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 9 +++++++++
target/riscv/cpu_bits.h | 6 ------
target/riscv/csr.c | 14 +++++++-------
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af2..1135106b3e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj)
i++;
}
/* mmte is supposed to have pm.current hardwired to 1 */
- env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
+ env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
#endif
env->xl = riscv_cpu_mxl(env);
riscv_cpu_update_mask(env);
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 12fe8d8546..5049e21518 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -99,6 +99,15 @@ enum {
TRANSLATE_G_STAGE_FAIL
};
+/* Extension Context Status */
+enum {
+ EXT_STATUS_DISABLED = 0,
+ EXT_STATUS_INITIAL,
+ EXT_STATUS_CLEAN,
+ EXT_STATUS_DIRTY,
+ EXT_STATUS_MASK,
+};
+
#define MMU_USER_IDX 3
#define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..5280bd41c2 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -736,12 +736,6 @@ typedef enum RISCVException {
#define PM_INSN 0x00000004ULL
#define PM_XS_MASK 0x00000003ULL
-/* PointerMasking XS bits values */
-#define PM_EXT_DISABLE 0x00000000ULL
-#define PM_EXT_INITIAL 0x00000001ULL
-#define PM_EXT_CLEAN 0x00000002ULL
-#define PM_EXT_DIRTY 0x00000003ULL
-
/* Execution enviornment configuration bits */
#define MENVCFG_FIOM BIT(0)
#define MENVCFG_CBIE (3UL << 4)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..abea7b749e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState *env, int csrno,
/* hardwiring pm.instruction bit to 0, since it's not supported yet */
wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
- env->mmte = wpri_val | PM_EXT_DIRTY;
+ env->mmte = wpri_val | EXT_STATUS_DIRTY;
riscv_cpu_update_mask(env);
/* Set XS and SD bits, since PM CSRs are dirty */
@@ -3593,7 +3593,7 @@ static RISCVException write_mpmmask(CPURISCVState *env, int csrno,
if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
env->cur_pmmask = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
@@ -3621,7 +3621,7 @@ static RISCVException write_spmmask(CPURISCVState *env, int csrno,
if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
env->cur_pmmask = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
@@ -3649,7 +3649,7 @@ static RISCVException write_upmmask(CPURISCVState *env, int csrno,
if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
env->cur_pmmask = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
@@ -3673,7 +3673,7 @@ static RISCVException write_mpmbase(CPURISCVState *env, int csrno,
if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
env->cur_pmbase = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
@@ -3701,7 +3701,7 @@ static RISCVException write_spmbase(CPURISCVState *env, int csrno,
if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
env->cur_pmbase = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
@@ -3729,7 +3729,7 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
env->cur_pmbase = val;
}
- env->mmte |= PM_EXT_DIRTY;
+ env->mmte |= EXT_STATUS_DIRTY;
/* Set XS and SD bits, since PM CSRs are dirty */
mstatus = env->mstatus | MSTATUS_XS;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] target/riscv: Add a general status enum for extensions
2023-03-24 5:59 ` [PATCH 2/4] target/riscv: Add a general status enum for extensions LIU Zhiwei
@ 2023-03-24 12:53 ` liweiwei
2023-03-24 13:47 ` LIU Zhiwei
0 siblings, 1 reply; 13+ messages in thread
From: liweiwei @ 2023-03-24 12:53 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: liweiwei, richard.henderson, Alistair.Francis, palmer, bin.meng,
dbarboza, qemu-riscv
On 2023/3/24 13:59, LIU Zhiwei wrote:
> The pointer masking is the only extension that directly use status.
> The vector or float extension uses the status in an indirect way.
>
> Replace the pointer masking extension special status fields with
> the general status.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
> target/riscv/cpu.c | 2 +-
> target/riscv/cpu.h | 9 +++++++++
> target/riscv/cpu_bits.h | 6 ------
> target/riscv/csr.c | 14 +++++++-------
> 4 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1e97473af2..1135106b3e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj)
> i++;
> }
> /* mmte is supposed to have pm.current hardwired to 1 */
> - env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
> + env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
> #endif
> env->xl = riscv_cpu_mxl(env);
> riscv_cpu_update_mask(env);
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 12fe8d8546..5049e21518 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -99,6 +99,15 @@ enum {
> TRANSLATE_G_STAGE_FAIL
> };
>
> +/* Extension Context Status */
> +enum {
> + EXT_STATUS_DISABLED = 0,
> + EXT_STATUS_INITIAL,
> + EXT_STATUS_CLEAN,
> + EXT_STATUS_DIRTY,
> + EXT_STATUS_MASK,
I think the right value for EXT_STATUS_MASK should be 3 here.
And it can replace the following PM_XS_MASK.
Regards,
Weiwei Li
> +};
> +
> #define MMU_USER_IDX 3
>
> #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index fca7ef0cef..5280bd41c2 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -736,12 +736,6 @@ typedef enum RISCVException {
> #define PM_INSN 0x00000004ULL
> #define PM_XS_MASK 0x00000003ULL
>
> -/* PointerMasking XS bits values */
> -#define PM_EXT_DISABLE 0x00000000ULL
> -#define PM_EXT_INITIAL 0x00000001ULL
> -#define PM_EXT_CLEAN 0x00000002ULL
> -#define PM_EXT_DIRTY 0x00000003ULL
> -
> /* Execution enviornment configuration bits */
> #define MENVCFG_FIOM BIT(0)
> #define MENVCFG_CBIE (3UL << 4)
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d522efc0b6..abea7b749e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState *env, int csrno,
>
> /* hardwiring pm.instruction bit to 0, since it's not supported yet */
> wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
> - env->mmte = wpri_val | PM_EXT_DIRTY;
> + env->mmte = wpri_val | EXT_STATUS_DIRTY;
> riscv_cpu_update_mask(env);
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> @@ -3593,7 +3593,7 @@ static RISCVException write_mpmmask(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
> env->cur_pmmask = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
> @@ -3621,7 +3621,7 @@ static RISCVException write_spmmask(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
> env->cur_pmmask = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
> @@ -3649,7 +3649,7 @@ static RISCVException write_upmmask(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
> env->cur_pmmask = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
> @@ -3673,7 +3673,7 @@ static RISCVException write_mpmbase(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
> env->cur_pmbase = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
> @@ -3701,7 +3701,7 @@ static RISCVException write_spmbase(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
> env->cur_pmbase = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
> @@ -3729,7 +3729,7 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
> env->cur_pmbase = val;
> }
> - env->mmte |= PM_EXT_DIRTY;
> + env->mmte |= EXT_STATUS_DIRTY;
>
> /* Set XS and SD bits, since PM CSRs are dirty */
> mstatus = env->mstatus | MSTATUS_XS;
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] target/riscv: Add a general status enum for extensions
2023-03-24 12:53 ` liweiwei
@ 2023-03-24 13:47 ` LIU Zhiwei
0 siblings, 0 replies; 13+ messages in thread
From: LIU Zhiwei @ 2023-03-24 13:47 UTC (permalink / raw)
To: liweiwei, qemu-devel
Cc: richard.henderson, Alistair.Francis, palmer, bin.meng, dbarboza,
qemu-riscv
On 2023/3/24 20:53, liweiwei wrote:
>
> On 2023/3/24 13:59, LIU Zhiwei wrote:
>> The pointer masking is the only extension that directly use status.
>> The vector or float extension uses the status in an indirect way.
>>
>> Replace the pointer masking extension special status fields with
>> the general status.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>> target/riscv/cpu.c | 2 +-
>> target/riscv/cpu.h | 9 +++++++++
>> target/riscv/cpu_bits.h | 6 ------
>> target/riscv/csr.c | 14 +++++++-------
>> 4 files changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 1e97473af2..1135106b3e 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj)
>> i++;
>> }
>> /* mmte is supposed to have pm.current hardwired to 1 */
>> - env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
>> + env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
>> #endif
>> env->xl = riscv_cpu_mxl(env);
>> riscv_cpu_update_mask(env);
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 12fe8d8546..5049e21518 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -99,6 +99,15 @@ enum {
>> TRANSLATE_G_STAGE_FAIL
>> };
>> +/* Extension Context Status */
>> +enum {
>> + EXT_STATUS_DISABLED = 0,
>> + EXT_STATUS_INITIAL,
>> + EXT_STATUS_CLEAN,
>> + EXT_STATUS_DIRTY,
>> + EXT_STATUS_MASK,
>
> I think the right value for EXT_STATUS_MASK should be 3 here.
Yes, it is.
>
> And it can replace the following PM_XS_MASK.
Once I wanted to replace PM_XS_MASK with EXT_STATUS_MASK here.
But PM_XS_MASK has a ULL type which is needed for a 64-bit register.
So I want to drop the definition of EXT_STATUS_MASK from here. And define a
EXT_STATUS_MASK macro in cpu_bits.h. It will replace the PM_XS_MASK.
Zhiwei
>
> Regards,
>
> Weiwei Li
>
>> +};
>> +
>> #define MMU_USER_IDX 3
>> #define MAX_RISCV_PMPS (16)
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index fca7ef0cef..5280bd41c2 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -736,12 +736,6 @@ typedef enum RISCVException {
>> #define PM_INSN 0x00000004ULL
>> #define PM_XS_MASK 0x00000003ULL
>> -/* PointerMasking XS bits values */
>> -#define PM_EXT_DISABLE 0x00000000ULL
>> -#define PM_EXT_INITIAL 0x00000001ULL
>> -#define PM_EXT_CLEAN 0x00000002ULL
>> -#define PM_EXT_DIRTY 0x00000003ULL
>> -
>> /* Execution enviornment configuration bits */
>> #define MENVCFG_FIOM BIT(0)
>> #define MENVCFG_CBIE (3UL << 4)
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index d522efc0b6..abea7b749e 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState
>> *env, int csrno,
>> /* hardwiring pm.instruction bit to 0, since it's not
>> supported yet */
>> wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
>> - env->mmte = wpri_val | PM_EXT_DIRTY;
>> + env->mmte = wpri_val | EXT_STATUS_DIRTY;
>> riscv_cpu_update_mask(env);
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> @@ -3593,7 +3593,7 @@ static RISCVException
>> write_mpmmask(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
>> env->cur_pmmask = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3621,7 +3621,7 @@ static RISCVException
>> write_spmmask(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
>> env->cur_pmmask = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3649,7 +3649,7 @@ static RISCVException
>> write_upmmask(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
>> env->cur_pmmask = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3673,7 +3673,7 @@ static RISCVException
>> write_mpmbase(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
>> env->cur_pmbase = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3701,7 +3701,7 @@ static RISCVException
>> write_spmbase(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
>> env->cur_pmbase = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3729,7 +3729,7 @@ static RISCVException
>> write_upmbase(CPURISCVState *env, int csrno,
>> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
>> env->cur_pmbase = val;
>> }
>> - env->mmte |= PM_EXT_DIRTY;
>> + env->mmte |= EXT_STATUS_DIRTY;
>> /* Set XS and SD bits, since PM CSRs are dirty */
>> mstatus = env->mstatus | MSTATUS_XS;
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags
2023-03-24 5:59 [PATCH 0/4] Fix tb flags use LIU Zhiwei
2023-03-24 5:59 ` [PATCH 1/4] target/riscv: Extract virt enabled state from tb flags LIU Zhiwei
2023-03-24 5:59 ` [PATCH 2/4] target/riscv: Add a general status enum for extensions LIU Zhiwei
@ 2023-03-24 5:59 ` LIU Zhiwei
2023-03-24 12:58 ` liweiwei
2023-03-24 17:03 ` Richard Henderson
2023-03-24 5:59 ` [PATCH 4/4] target/riscv: Add a tb flags field for vstart LIU Zhiwei
3 siblings, 2 replies; 13+ messages in thread
From: LIU Zhiwei @ 2023-03-24 5:59 UTC (permalink / raw)
To: qemu-devel
Cc: richard.henderson, Alistair.Francis, palmer, bin.meng, liweiwei,
dbarboza, qemu-riscv, LIU Zhiwei
Reuse the MSTATUS_FS and MSTATUS_VS for the tb flags positions is not a normal
way.
It will make us change the tb flags layout difficult. And even worse, if we
want to keep tb flags for a same extension togather without a hole.
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
target/riscv/cpu.h | 15 +++++++--------
target/riscv/cpu_helper.c | 11 ++++++-----
target/riscv/insn_trans/trans_rvv.c.inc | 8 ++++----
target/riscv/translate.c | 20 ++++++++++----------
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5049e21518..41f7aef666 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -634,18 +634,17 @@ void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
#define TB_FLAGS_PRIV_MMU_MASK 3
#define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 2)
-#define TB_FLAGS_MSTATUS_FS MSTATUS_FS
-#define TB_FLAGS_MSTATUS_VS MSTATUS_VS
#include "exec/cpu-all.h"
FIELD(TB_FLAGS, MEM_IDX, 0, 3)
-FIELD(TB_FLAGS, LMUL, 3, 3)
-FIELD(TB_FLAGS, SEW, 6, 3)
-/* Skip MSTATUS_VS (0x600) bits */
-FIELD(TB_FLAGS, VL_EQ_VLMAX, 11, 1)
-FIELD(TB_FLAGS, VILL, 12, 1)
-/* Skip MSTATUS_FS (0x6000) bits */
+FIELD(TB_FLAGS, FS, 3, 2)
+/* Vector flags */
+FIELD(TB_FLAGS, VS, 5, 2)
+FIELD(TB_FLAGS, LMUL, 7, 3)
+FIELD(TB_FLAGS, SEW, 10, 3)
+FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
+FIELD(TB_FLAGS, VILL, 14, 1)
/* Is a Hypervisor instruction load/store allowed? */
FIELD(TB_FLAGS, HLSX, 15, 1)
FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9d50e7bbb6..87c6effcc2 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -79,16 +79,17 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
}
#ifdef CONFIG_USER_ONLY
- flags |= TB_FLAGS_MSTATUS_FS;
- flags |= TB_FLAGS_MSTATUS_VS;
+ flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
+ flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
#else
flags |= cpu_mmu_index(env, 0);
if (riscv_cpu_fp_enabled(env)) {
- flags |= env->mstatus & MSTATUS_FS;
+ flags = FIELD_DP32(flags, TB_FLAGS, FS,
+ get_field(env->mstatus, MSTATUS_FS));
}
-
if (riscv_cpu_vector_enabled(env)) {
- flags |= env->mstatus & MSTATUS_VS;
+ flags = FIELD_DP32(flags, TB_FLAGS, VS,
+ get_field(env->mstatus, MSTATUS_VS));
}
if (riscv_has_ext(env, RVH)) {
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index f2e3d38515..6297c3b50d 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -29,12 +29,12 @@ static inline bool is_overlapped(const int8_t astart, int8_t asize,
static bool require_rvv(DisasContext *s)
{
- return s->mstatus_vs != 0;
+ return s->mstatus_vs != EXT_STATUS_DISABLED;
}
static bool require_rvf(DisasContext *s)
{
- if (s->mstatus_fs == 0) {
+ if (s->mstatus_fs == EXT_STATUS_DISABLED) {
return false;
}
@@ -52,7 +52,7 @@ static bool require_rvf(DisasContext *s)
static bool require_scale_rvf(DisasContext *s)
{
- if (s->mstatus_fs == 0) {
+ if (s->mstatus_fs == EXT_STATUS_DISABLED) {
return false;
}
@@ -70,7 +70,7 @@ static bool require_scale_rvf(DisasContext *s)
static bool require_scale_rvfmin(DisasContext *s)
{
- if (s->mstatus_fs == 0) {
+ if (s->mstatus_fs == EXT_STATUS_DISABLED) {
return false;
}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 880f6318aa..85ca3ba202 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -611,9 +611,9 @@ static void mark_fs_dirty(DisasContext *ctx)
return;
}
- if (ctx->mstatus_fs != MSTATUS_FS) {
+ if (ctx->mstatus_fs != EXT_STATUS_DIRTY) {
/* Remember the state change for the rest of the TB. */
- ctx->mstatus_fs = MSTATUS_FS;
+ ctx->mstatus_fs = EXT_STATUS_DIRTY;
tmp = tcg_temp_new();
tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
@@ -621,9 +621,9 @@ static void mark_fs_dirty(DisasContext *ctx)
tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
}
- if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
+ if (ctx->virt_enabled && ctx->mstatus_hs_fs != EXT_STATUS_DIRTY) {
/* Remember the stage change for the rest of the TB. */
- ctx->mstatus_hs_fs = MSTATUS_FS;
+ ctx->mstatus_hs_fs = EXT_STATUS_DIRTY;
tmp = tcg_temp_new();
tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
@@ -645,9 +645,9 @@ static void mark_vs_dirty(DisasContext *ctx)
{
TCGv tmp;
- if (ctx->mstatus_vs != MSTATUS_VS) {
+ if (ctx->mstatus_vs != EXT_STATUS_DIRTY) {
/* Remember the state change for the rest of the TB. */
- ctx->mstatus_vs = MSTATUS_VS;
+ ctx->mstatus_vs = EXT_STATUS_DIRTY;
tmp = tcg_temp_new();
tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
@@ -655,9 +655,9 @@ static void mark_vs_dirty(DisasContext *ctx)
tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
}
- if (ctx->virt_enabled && ctx->mstatus_hs_vs != MSTATUS_VS) {
+ if (ctx->virt_enabled && ctx->mstatus_hs_vs != EXT_STATUS_DIRTY) {
/* Remember the stage change for the rest of the TB. */
- ctx->mstatus_hs_vs = MSTATUS_VS;
+ ctx->mstatus_hs_vs = EXT_STATUS_DIRTY;
tmp = tcg_temp_new();
tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
@@ -1153,8 +1153,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->pc_succ_insn = ctx->base.pc_first;
ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
- ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
- ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
+ ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+ ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
ctx->priv_ver = env->priv_ver;
ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
ctx->misa_ext = env->misa_ext;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags
2023-03-24 5:59 ` [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags LIU Zhiwei
@ 2023-03-24 12:58 ` liweiwei
2023-03-24 17:03 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: liweiwei @ 2023-03-24 12:58 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: liweiwei, richard.henderson, Alistair.Francis, palmer, bin.meng,
dbarboza, qemu-riscv
On 2023/3/24 13:59, LIU Zhiwei wrote:
> Reuse the MSTATUS_FS and MSTATUS_VS for the tb flags positions is not a normal
> way.
>
> It will make us change the tb flags layout difficult. And even worse, if we
> want to keep tb flags for a same extension togather without a hole.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Weiwei Li
> target/riscv/cpu.h | 15 +++++++--------
> target/riscv/cpu_helper.c | 11 ++++++-----
> target/riscv/insn_trans/trans_rvv.c.inc | 8 ++++----
> target/riscv/translate.c | 20 ++++++++++----------
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 5049e21518..41f7aef666 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -634,18 +634,17 @@ void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
>
> #define TB_FLAGS_PRIV_MMU_MASK 3
> #define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 2)
> -#define TB_FLAGS_MSTATUS_FS MSTATUS_FS
> -#define TB_FLAGS_MSTATUS_VS MSTATUS_VS
>
> #include "exec/cpu-all.h"
>
> FIELD(TB_FLAGS, MEM_IDX, 0, 3)
> -FIELD(TB_FLAGS, LMUL, 3, 3)
> -FIELD(TB_FLAGS, SEW, 6, 3)
> -/* Skip MSTATUS_VS (0x600) bits */
> -FIELD(TB_FLAGS, VL_EQ_VLMAX, 11, 1)
> -FIELD(TB_FLAGS, VILL, 12, 1)
> -/* Skip MSTATUS_FS (0x6000) bits */
> +FIELD(TB_FLAGS, FS, 3, 2)
> +/* Vector flags */
> +FIELD(TB_FLAGS, VS, 5, 2)
> +FIELD(TB_FLAGS, LMUL, 7, 3)
> +FIELD(TB_FLAGS, SEW, 10, 3)
> +FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
> +FIELD(TB_FLAGS, VILL, 14, 1)
> /* Is a Hypervisor instruction load/store allowed? */
> FIELD(TB_FLAGS, HLSX, 15, 1)
> FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9d50e7bbb6..87c6effcc2 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -79,16 +79,17 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> }
>
> #ifdef CONFIG_USER_ONLY
> - flags |= TB_FLAGS_MSTATUS_FS;
> - flags |= TB_FLAGS_MSTATUS_VS;
> + flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
> + flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
> #else
> flags |= cpu_mmu_index(env, 0);
> if (riscv_cpu_fp_enabled(env)) {
> - flags |= env->mstatus & MSTATUS_FS;
> + flags = FIELD_DP32(flags, TB_FLAGS, FS,
> + get_field(env->mstatus, MSTATUS_FS));
> }
> -
> if (riscv_cpu_vector_enabled(env)) {
> - flags |= env->mstatus & MSTATUS_VS;
> + flags = FIELD_DP32(flags, TB_FLAGS, VS,
> + get_field(env->mstatus, MSTATUS_VS));
> }
>
> if (riscv_has_ext(env, RVH)) {
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index f2e3d38515..6297c3b50d 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -29,12 +29,12 @@ static inline bool is_overlapped(const int8_t astart, int8_t asize,
>
> static bool require_rvv(DisasContext *s)
> {
> - return s->mstatus_vs != 0;
> + return s->mstatus_vs != EXT_STATUS_DISABLED;
> }
>
> static bool require_rvf(DisasContext *s)
> {
> - if (s->mstatus_fs == 0) {
> + if (s->mstatus_fs == EXT_STATUS_DISABLED) {
> return false;
> }
>
> @@ -52,7 +52,7 @@ static bool require_rvf(DisasContext *s)
>
> static bool require_scale_rvf(DisasContext *s)
> {
> - if (s->mstatus_fs == 0) {
> + if (s->mstatus_fs == EXT_STATUS_DISABLED) {
> return false;
> }
>
> @@ -70,7 +70,7 @@ static bool require_scale_rvf(DisasContext *s)
>
> static bool require_scale_rvfmin(DisasContext *s)
> {
> - if (s->mstatus_fs == 0) {
> + if (s->mstatus_fs == EXT_STATUS_DISABLED) {
> return false;
> }
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 880f6318aa..85ca3ba202 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -611,9 +611,9 @@ static void mark_fs_dirty(DisasContext *ctx)
> return;
> }
>
> - if (ctx->mstatus_fs != MSTATUS_FS) {
> + if (ctx->mstatus_fs != EXT_STATUS_DIRTY) {
> /* Remember the state change for the rest of the TB. */
> - ctx->mstatus_fs = MSTATUS_FS;
> + ctx->mstatus_fs = EXT_STATUS_DIRTY;
>
> tmp = tcg_temp_new();
> tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> @@ -621,9 +621,9 @@ static void mark_fs_dirty(DisasContext *ctx)
> tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> }
>
> - if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
> + if (ctx->virt_enabled && ctx->mstatus_hs_fs != EXT_STATUS_DIRTY) {
> /* Remember the stage change for the rest of the TB. */
> - ctx->mstatus_hs_fs = MSTATUS_FS;
> + ctx->mstatus_hs_fs = EXT_STATUS_DIRTY;
>
> tmp = tcg_temp_new();
> tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
> @@ -645,9 +645,9 @@ static void mark_vs_dirty(DisasContext *ctx)
> {
> TCGv tmp;
>
> - if (ctx->mstatus_vs != MSTATUS_VS) {
> + if (ctx->mstatus_vs != EXT_STATUS_DIRTY) {
> /* Remember the state change for the rest of the TB. */
> - ctx->mstatus_vs = MSTATUS_VS;
> + ctx->mstatus_vs = EXT_STATUS_DIRTY;
>
> tmp = tcg_temp_new();
> tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> @@ -655,9 +655,9 @@ static void mark_vs_dirty(DisasContext *ctx)
> tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> }
>
> - if (ctx->virt_enabled && ctx->mstatus_hs_vs != MSTATUS_VS) {
> + if (ctx->virt_enabled && ctx->mstatus_hs_vs != EXT_STATUS_DIRTY) {
> /* Remember the stage change for the rest of the TB. */
> - ctx->mstatus_hs_vs = MSTATUS_VS;
> + ctx->mstatus_hs_vs = EXT_STATUS_DIRTY;
>
> tmp = tcg_temp_new();
> tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
> @@ -1153,8 +1153,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>
> ctx->pc_succ_insn = ctx->base.pc_first;
> ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
> - ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
> - ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
> + ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> + ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
> ctx->priv_ver = env->priv_ver;
> ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
> ctx->misa_ext = env->misa_ext;
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags
2023-03-24 5:59 ` [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags LIU Zhiwei
2023-03-24 12:58 ` liweiwei
@ 2023-03-24 17:03 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2023-03-24 17:03 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: Alistair.Francis, palmer, bin.meng, liweiwei, dbarboza, qemu-riscv
On 3/23/23 22:59, LIU Zhiwei wrote:
> Reuse the MSTATUS_FS and MSTATUS_VS for the tb flags positions is not a normal
> way.
>
> It will make us change the tb flags layout difficult. And even worse, if we
> want to keep tb flags for a same extension togather without a hole.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> + flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
> + flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
...
> + flags = FIELD_DP32(flags, TB_FLAGS, FS,
...
> + flags = FIELD_DP32(flags, TB_FLAGS, VS,
Extra space after =.
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] target/riscv: Add a tb flags field for vstart
2023-03-24 5:59 [PATCH 0/4] Fix tb flags use LIU Zhiwei
` (2 preceding siblings ...)
2023-03-24 5:59 ` [PATCH 3/4] target/riscv: Encode the FS and VS on a normal way for tb flags LIU Zhiwei
@ 2023-03-24 5:59 ` LIU Zhiwei
2023-03-24 13:02 ` liweiwei
2023-03-24 17:05 ` Richard Henderson
3 siblings, 2 replies; 13+ messages in thread
From: LIU Zhiwei @ 2023-03-24 5:59 UTC (permalink / raw)
To: qemu-devel
Cc: richard.henderson, Alistair.Francis, palmer, bin.meng, liweiwei,
dbarboza, qemu-riscv, LIU Zhiwei
Once we mistook the vstart directly from the env->vstart. As env->vstart is not
a constant, we should record it in the tb flags if we want to use
it in translation.
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
target/riscv/cpu.h | 21 +++++++++++----------
target/riscv/cpu_helper.c | 1 +
target/riscv/insn_trans/trans_rvv.c.inc | 14 +++++++-------
target/riscv/translate.c | 4 ++--
4 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 41f7aef666..623288e6f9 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -645,21 +645,22 @@ FIELD(TB_FLAGS, LMUL, 7, 3)
FIELD(TB_FLAGS, SEW, 10, 3)
FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
FIELD(TB_FLAGS, VILL, 14, 1)
+FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
/* Is a Hypervisor instruction load/store allowed? */
-FIELD(TB_FLAGS, HLSX, 15, 1)
-FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
-FIELD(TB_FLAGS, MSTATUS_HS_VS, 18, 2)
+FIELD(TB_FLAGS, HLSX, 16, 1)
+FIELD(TB_FLAGS, MSTATUS_HS_FS, 17, 2)
+FIELD(TB_FLAGS, MSTATUS_HS_VS, 19, 2)
/* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
-FIELD(TB_FLAGS, XL, 20, 2)
+FIELD(TB_FLAGS, XL, 21, 2)
/* If PointerMasking should be applied */
-FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
-FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
-FIELD(TB_FLAGS, VTA, 24, 1)
-FIELD(TB_FLAGS, VMA, 25, 1)
+FIELD(TB_FLAGS, PM_MASK_ENABLED, 23, 1)
+FIELD(TB_FLAGS, PM_BASE_ENABLED, 24, 1)
+FIELD(TB_FLAGS, VTA, 25, 1)
+FIELD(TB_FLAGS, VMA, 26, 1)
/* Native debug itrigger */
-FIELD(TB_FLAGS, ITRIGGER, 26, 1)
+FIELD(TB_FLAGS, ITRIGGER, 27, 1)
/* Virtual mode enabled */
-FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
+FIELD(TB_FLAGS, VIRT_ENABLED, 28, 1)
#ifdef TARGET_RISCV32
#define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 87c6effcc2..f80d069884 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -74,6 +74,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
FIELD_EX64(env->vtype, VTYPE, VTA));
flags = FIELD_DP32(flags, TB_FLAGS, VMA,
FIELD_EX64(env->vtype, VTYPE, VMA));
+ flags = FIELD_DP32(flags, TB_FLAGS, VSTART_EQ_ZERO, env->vstart == 0);
} else {
flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
}
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 6297c3b50d..32b3b9a8e5 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -547,7 +547,7 @@ static bool vext_check_sds(DisasContext *s, int vd, int vs1, int vs2, int vm)
*/
static bool vext_check_reduction(DisasContext *s, int vs2)
{
- return require_align(vs2, s->lmul) && (s->vstart == 0);
+ return require_align(vs2, s->lmul) && s->vstart_eq_zero;
}
/*
@@ -3083,7 +3083,7 @@ static bool trans_vcpop_m(DisasContext *s, arg_rmr *a)
{
if (require_rvv(s) &&
vext_check_isa_ill(s) &&
- s->vstart == 0) {
+ s->vstart_eq_zero) {
TCGv_ptr src2, mask;
TCGv dst;
TCGv_i32 desc;
@@ -3112,7 +3112,7 @@ static bool trans_vfirst_m(DisasContext *s, arg_rmr *a)
{
if (require_rvv(s) &&
vext_check_isa_ill(s) &&
- s->vstart == 0) {
+ s->vstart_eq_zero) {
TCGv_ptr src2, mask;
TCGv dst;
TCGv_i32 desc;
@@ -3146,7 +3146,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) \
vext_check_isa_ill(s) && \
require_vm(a->vm, a->rd) && \
(a->rd != a->rs2) && \
- (s->vstart == 0)) { \
+ s->vstart_eq_zero) { \
uint32_t data = 0; \
gen_helper_gvec_3_ptr *fn = gen_helper_##NAME; \
TCGLabel *over = gen_new_label(); \
@@ -3187,7 +3187,7 @@ static bool trans_viota_m(DisasContext *s, arg_viota_m *a)
!is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs2, 1) &&
require_vm(a->vm, a->rd) &&
require_align(a->rd, s->lmul) &&
- (s->vstart == 0)) {
+ s->vstart_eq_zero) {
uint32_t data = 0;
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
@@ -3636,7 +3636,7 @@ static bool vcompress_vm_check(DisasContext *s, arg_r *a)
require_align(a->rs2, s->lmul) &&
(a->rd != a->rs2) &&
!is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs1, 1) &&
- (s->vstart == 0);
+ s->vstart_eq_zero;
}
static bool trans_vcompress_vm(DisasContext *s, arg_r *a)
@@ -3675,7 +3675,7 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * a) \
QEMU_IS_ALIGNED(a->rd, LEN) && \
QEMU_IS_ALIGNED(a->rs2, LEN)) { \
uint32_t maxsz = (s->cfg_ptr->vlen >> 3) * LEN; \
- if (s->vstart == 0) { \
+ if (s->vstart_eq_zero) { \
/* EEW = 8 */ \
tcg_gen_gvec_mov(MO_8, vreg_ofs(s, a->rd), \
vreg_ofs(s, a->rs2), maxsz, maxsz); \
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 85ca3ba202..e8bac1b470 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -99,7 +99,7 @@ typedef struct DisasContext {
uint8_t vta;
uint8_t vma;
bool cfg_vta_all_1s;
- target_ulong vstart;
+ bool vstart_eq_zero;
bool vl_eq_vlmax;
CPUState *cs;
TCGv zero;
@@ -1169,7 +1169,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) && cpu->cfg.rvv_ta_all_1s;
ctx->vma = FIELD_EX32(tb_flags, TB_FLAGS, VMA) && cpu->cfg.rvv_ma_all_1s;
ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
- ctx->vstart = env->vstart;
+ ctx->vstart_eq_zero = FIELD_EX32(tb_flags, TB_FLAGS, VSTART_EQ_ZERO);
ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
ctx->misa_mxl_max = env->misa_mxl_max;
ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] target/riscv: Add a tb flags field for vstart
2023-03-24 5:59 ` [PATCH 4/4] target/riscv: Add a tb flags field for vstart LIU Zhiwei
@ 2023-03-24 13:02 ` liweiwei
2023-03-24 17:05 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: liweiwei @ 2023-03-24 13:02 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: liweiwei, richard.henderson, Alistair.Francis, palmer, bin.meng,
dbarboza, qemu-riscv
On 2023/3/24 13:59, LIU Zhiwei wrote:
> Once we mistook the vstart directly from the env->vstart. As env->vstart is not
> a constant, we should record it in the tb flags if we want to use
> it in translation.
>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Weiwei Li
> target/riscv/cpu.h | 21 +++++++++++----------
> target/riscv/cpu_helper.c | 1 +
> target/riscv/insn_trans/trans_rvv.c.inc | 14 +++++++-------
> target/riscv/translate.c | 4 ++--
> 4 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 41f7aef666..623288e6f9 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -645,21 +645,22 @@ FIELD(TB_FLAGS, LMUL, 7, 3)
> FIELD(TB_FLAGS, SEW, 10, 3)
> FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
> FIELD(TB_FLAGS, VILL, 14, 1)
> +FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
> /* Is a Hypervisor instruction load/store allowed? */
> -FIELD(TB_FLAGS, HLSX, 15, 1)
> -FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
> -FIELD(TB_FLAGS, MSTATUS_HS_VS, 18, 2)
> +FIELD(TB_FLAGS, HLSX, 16, 1)
> +FIELD(TB_FLAGS, MSTATUS_HS_FS, 17, 2)
> +FIELD(TB_FLAGS, MSTATUS_HS_VS, 19, 2)
> /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
> -FIELD(TB_FLAGS, XL, 20, 2)
> +FIELD(TB_FLAGS, XL, 21, 2)
> /* If PointerMasking should be applied */
> -FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
> -FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
> -FIELD(TB_FLAGS, VTA, 24, 1)
> -FIELD(TB_FLAGS, VMA, 25, 1)
> +FIELD(TB_FLAGS, PM_MASK_ENABLED, 23, 1)
> +FIELD(TB_FLAGS, PM_BASE_ENABLED, 24, 1)
> +FIELD(TB_FLAGS, VTA, 25, 1)
> +FIELD(TB_FLAGS, VMA, 26, 1)
> /* Native debug itrigger */
> -FIELD(TB_FLAGS, ITRIGGER, 26, 1)
> +FIELD(TB_FLAGS, ITRIGGER, 27, 1)
> /* Virtual mode enabled */
> -FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
> +FIELD(TB_FLAGS, VIRT_ENABLED, 28, 1)
>
> #ifdef TARGET_RISCV32
> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 87c6effcc2..f80d069884 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -74,6 +74,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> FIELD_EX64(env->vtype, VTYPE, VTA));
> flags = FIELD_DP32(flags, TB_FLAGS, VMA,
> FIELD_EX64(env->vtype, VTYPE, VMA));
> + flags = FIELD_DP32(flags, TB_FLAGS, VSTART_EQ_ZERO, env->vstart == 0);
> } else {
> flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
> }
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 6297c3b50d..32b3b9a8e5 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -547,7 +547,7 @@ static bool vext_check_sds(DisasContext *s, int vd, int vs1, int vs2, int vm)
> */
> static bool vext_check_reduction(DisasContext *s, int vs2)
> {
> - return require_align(vs2, s->lmul) && (s->vstart == 0);
> + return require_align(vs2, s->lmul) && s->vstart_eq_zero;
> }
>
> /*
> @@ -3083,7 +3083,7 @@ static bool trans_vcpop_m(DisasContext *s, arg_rmr *a)
> {
> if (require_rvv(s) &&
> vext_check_isa_ill(s) &&
> - s->vstart == 0) {
> + s->vstart_eq_zero) {
> TCGv_ptr src2, mask;
> TCGv dst;
> TCGv_i32 desc;
> @@ -3112,7 +3112,7 @@ static bool trans_vfirst_m(DisasContext *s, arg_rmr *a)
> {
> if (require_rvv(s) &&
> vext_check_isa_ill(s) &&
> - s->vstart == 0) {
> + s->vstart_eq_zero) {
> TCGv_ptr src2, mask;
> TCGv dst;
> TCGv_i32 desc;
> @@ -3146,7 +3146,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) \
> vext_check_isa_ill(s) && \
> require_vm(a->vm, a->rd) && \
> (a->rd != a->rs2) && \
> - (s->vstart == 0)) { \
> + s->vstart_eq_zero) { \
> uint32_t data = 0; \
> gen_helper_gvec_3_ptr *fn = gen_helper_##NAME; \
> TCGLabel *over = gen_new_label(); \
> @@ -3187,7 +3187,7 @@ static bool trans_viota_m(DisasContext *s, arg_viota_m *a)
> !is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs2, 1) &&
> require_vm(a->vm, a->rd) &&
> require_align(a->rd, s->lmul) &&
> - (s->vstart == 0)) {
> + s->vstart_eq_zero) {
> uint32_t data = 0;
> TCGLabel *over = gen_new_label();
> tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
> @@ -3636,7 +3636,7 @@ static bool vcompress_vm_check(DisasContext *s, arg_r *a)
> require_align(a->rs2, s->lmul) &&
> (a->rd != a->rs2) &&
> !is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs1, 1) &&
> - (s->vstart == 0);
> + s->vstart_eq_zero;
> }
>
> static bool trans_vcompress_vm(DisasContext *s, arg_r *a)
> @@ -3675,7 +3675,7 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * a) \
> QEMU_IS_ALIGNED(a->rd, LEN) && \
> QEMU_IS_ALIGNED(a->rs2, LEN)) { \
> uint32_t maxsz = (s->cfg_ptr->vlen >> 3) * LEN; \
> - if (s->vstart == 0) { \
> + if (s->vstart_eq_zero) { \
> /* EEW = 8 */ \
> tcg_gen_gvec_mov(MO_8, vreg_ofs(s, a->rd), \
> vreg_ofs(s, a->rs2), maxsz, maxsz); \
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 85ca3ba202..e8bac1b470 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -99,7 +99,7 @@ typedef struct DisasContext {
> uint8_t vta;
> uint8_t vma;
> bool cfg_vta_all_1s;
> - target_ulong vstart;
> + bool vstart_eq_zero;
> bool vl_eq_vlmax;
> CPUState *cs;
> TCGv zero;
> @@ -1169,7 +1169,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) && cpu->cfg.rvv_ta_all_1s;
> ctx->vma = FIELD_EX32(tb_flags, TB_FLAGS, VMA) && cpu->cfg.rvv_ma_all_1s;
> ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
> - ctx->vstart = env->vstart;
> + ctx->vstart_eq_zero = FIELD_EX32(tb_flags, TB_FLAGS, VSTART_EQ_ZERO);
> ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
> ctx->misa_mxl_max = env->misa_mxl_max;
> ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] target/riscv: Add a tb flags field for vstart
2023-03-24 5:59 ` [PATCH 4/4] target/riscv: Add a tb flags field for vstart LIU Zhiwei
2023-03-24 13:02 ` liweiwei
@ 2023-03-24 17:05 ` Richard Henderson
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2023-03-24 17:05 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: Alistair.Francis, palmer, bin.meng, liweiwei, dbarboza, qemu-riscv
On 3/23/23 22:59, LIU Zhiwei wrote:
> Once we mistook the vstart directly from the env->vstart. As env->vstart is not
> a constant, we should record it in the tb flags if we want to use
> it in translation.
>
> Reported-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
> ---
> target/riscv/cpu.h | 21 +++++++++++----------
> target/riscv/cpu_helper.c | 1 +
> target/riscv/insn_trans/trans_rvv.c.inc | 14 +++++++-------
> target/riscv/translate.c | 4 ++--
> 4 files changed, 21 insertions(+), 19 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 13+ messages in thread