* [PATCH v4] target/riscv: fix H extension TVM trap
@ 2023-03-12 12:05 Yi Chen
2023-03-17 8:45 ` LIU Zhiwei
2023-04-06 1:56 ` Alistair Francis
0 siblings, 2 replies; 4+ messages in thread
From: Yi Chen @ 2023-03-12 12:05 UTC (permalink / raw)
To: qemu-devel
Cc: Yi Chen, Weiwei Li, Palmer Dabbelt, Alistair Francis, Bin Meng,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
- Trap satp/hgatp accesses from HS-mode when MSTATUS.TVM is enabled.
- Trap satp accesses from VS-mode when HSTATUS.VTVM is enabled.
- Raise RISCV_EXCP_ILLEGAL_INST when U-mode executes SFENCE.VMA/SINVAL.VMA.
- Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
SFENCE.VMA/SINVAL.VMA or VS-mode executes SFENCE.VMA/SINVAL.VMA with
HSTATUS.VTVM enabled.
- Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
HFENCE.GVMA/HFENCE.VVMA/HINVAL.GVMA/HINVAL.VVMA.
Signed-off-by: Yi Chen <chenyi2000@zju.edu.cn>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
---
Add reviewed-by
Replace "env->priv <= PRV_S && riscv_cpu_virt_enabled(env)" with "riscv_cpu_virt_enabled(env)"
target/riscv/csr.c | 56 +++++++++++++++++++++++++---------------
target/riscv/op_helper.c | 12 ++++-----
2 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..26a02e57bd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -443,6 +443,30 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
return sstc(env, csrno);
}
+static RISCVException satp(CPURISCVState *env, int csrno)
+{
+ if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
+ get_field(env->mstatus, MSTATUS_TVM)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+ if (env->priv == PRV_S && riscv_cpu_virt_enabled(env) &&
+ get_field(env->hstatus, HSTATUS_VTVM)) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
+ return smode(env, csrno);
+}
+
+static RISCVException hgatp(CPURISCVState *env, int csrno)
+{
+ if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
+ get_field(env->mstatus, MSTATUS_TVM)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return hmode(env, csrno);
+}
+
/* Checks if PointerMasking registers could be accessed */
static RISCVException pointer_masking(CPURISCVState *env, int csrno)
{
@@ -2655,13 +2679,7 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
*val = 0;
return RISCV_EXCP_NONE;
}
-
- if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
- return RISCV_EXCP_ILLEGAL_INST;
- } else {
- *val = env->satp;
- }
-
+ *val = env->satp;
return RISCV_EXCP_NONE;
}
@@ -2684,18 +2702,14 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
}
if (vm && mask) {
- if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
- return RISCV_EXCP_ILLEGAL_INST;
- } else {
- /*
- * The ISA defines SATP.MODE=Bare as "no translation", but we still
- * pass these through QEMU's TLB emulation as it improves
- * performance. Flushing the TLB on SATP writes with paging
- * enabled avoids leaking those invalid cached mappings.
- */
- tlb_flush(env_cpu(env));
- env->satp = val;
- }
+ /*
+ * The ISA defines SATP.MODE=Bare as "no translation", but we still
+ * pass these through QEMU's TLB emulation as it improves
+ * performance. Flushing the TLB on SATP writes with paging
+ * enabled avoids leaking those invalid cached mappings.
+ */
+ tlb_flush(env_cpu(env));
+ env->satp = val;
}
return RISCV_EXCP_NONE;
}
@@ -4180,7 +4194,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
.min_priv_ver = PRIV_VERSION_1_12_0 },
/* Supervisor Protection and Translation */
- [CSR_SATP] = { "satp", smode, read_satp, write_satp },
+ [CSR_SATP] = { "satp", satp, read_satp, write_satp },
/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
[CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
@@ -4217,7 +4231,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
.min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_HGEIP] = { "hgeip", hmode, read_hgeip,
.min_priv_ver = PRIV_VERSION_1_12_0 },
- [CSR_HGATP] = { "hgatp", hmode, read_hgatp, write_hgatp,
+ [CSR_HGATP] = { "hgatp", hgatp, read_hgatp, write_hgatp,
.min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_HTIMEDELTA] = { "htimedelta", hmode, read_htimedelta,
write_htimedelta,
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 84ee018f7d..8e16020f8d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -381,12 +381,12 @@ void helper_wfi(CPURISCVState *env)
void helper_tlb_flush(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
- if (!(env->priv >= PRV_S) ||
- (env->priv == PRV_S &&
- get_field(env->mstatus, MSTATUS_TVM))) {
+ if (!riscv_cpu_virt_enabled(env) &&
+ (env->priv == PRV_U ||
+ (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)))) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
- } else if (riscv_has_ext(env, RVH) && riscv_cpu_virt_enabled(env) &&
- get_field(env->hstatus, HSTATUS_VTVM)) {
+ } else if (riscv_cpu_virt_enabled(env) &&
+ (env->priv == PRV_U || get_field(env->hstatus, HSTATUS_VTVM))) {
riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
} else {
tlb_flush(cs);
@@ -403,7 +403,7 @@ void helper_hyp_tlb_flush(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
- if (env->priv == PRV_S && riscv_cpu_virt_enabled(env)) {
+ if (riscv_cpu_virt_enabled(env)) {
riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
}
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] target/riscv: fix H extension TVM trap
2023-03-12 12:05 [PATCH v4] target/riscv: fix H extension TVM trap Yi Chen
@ 2023-03-17 8:45 ` LIU Zhiwei
2023-04-06 1:56 ` Alistair Francis
1 sibling, 0 replies; 4+ messages in thread
From: LIU Zhiwei @ 2023-03-17 8:45 UTC (permalink / raw)
To: Yi Chen
Cc: Weiwei Li, Palmer Dabbelt, Alistair Francis, Bin Meng,
Daniel Henrique Barboza, open list:RISC-V TCG CPUs,
qemu-devel@nongnu.org Developers
On 2023/3/12 20:05, Yi Chen wrote:
> - Trap satp/hgatp accesses from HS-mode when MSTATUS.TVM is enabled.
> - Trap satp accesses from VS-mode when HSTATUS.VTVM is enabled.
> - Raise RISCV_EXCP_ILLEGAL_INST when U-mode executes SFENCE.VMA/SINVAL.VMA.
> - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> SFENCE.VMA/SINVAL.VMA or VS-mode executes SFENCE.VMA/SINVAL.VMA with
> HSTATUS.VTVM enabled.
> - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> HFENCE.GVMA/HFENCE.VVMA/HINVAL.GVMA/HINVAL.VVMA.
>
> Signed-off-by: Yi Chen <chenyi2000@zju.edu.cn>
> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
> ---
> Add reviewed-by
> Replace "env->priv <= PRV_S && riscv_cpu_virt_enabled(env)" with "riscv_cpu_virt_enabled(env)"
> target/riscv/csr.c | 56 +++++++++++++++++++++++++---------------
> target/riscv/op_helper.c | 12 ++++-----
> 2 files changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d522efc0b6..26a02e57bd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -443,6 +443,30 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
> return sstc(env, csrno);
> }
>
> +static RISCVException satp(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> + get_field(env->mstatus, MSTATUS_TVM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + if (env->priv == PRV_S && riscv_cpu_virt_enabled(env) &&
> + get_field(env->hstatus, HSTATUS_VTVM)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> +
> + return smode(env, csrno);
> +}
> +
> +static RISCVException hgatp(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> + get_field(env->mstatus, MSTATUS_TVM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return hmode(env, csrno);
> +}
> +
> /* Checks if PointerMasking registers could be accessed */
> static RISCVException pointer_masking(CPURISCVState *env, int csrno)
> {
> @@ -2655,13 +2679,7 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
> *val = 0;
> return RISCV_EXCP_NONE;
> }
> -
> - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - } else {
> - *val = env->satp;
> - }
> -
> + *val = env->satp;
> return RISCV_EXCP_NONE;
> }
>
> @@ -2684,18 +2702,14 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
> }
>
> if (vm && mask) {
> - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - } else {
> - /*
> - * The ISA defines SATP.MODE=Bare as "no translation", but we still
> - * pass these through QEMU's TLB emulation as it improves
> - * performance. Flushing the TLB on SATP writes with paging
> - * enabled avoids leaking those invalid cached mappings.
> - */
> - tlb_flush(env_cpu(env));
> - env->satp = val;
> - }
> + /*
> + * The ISA defines SATP.MODE=Bare as "no translation", but we still
> + * pass these through QEMU's TLB emulation as it improves
> + * performance. Flushing the TLB on SATP writes with paging
> + * enabled avoids leaking those invalid cached mappings.
> + */
> + tlb_flush(env_cpu(env));
> + env->satp = val;
> }
> return RISCV_EXCP_NONE;
> }
> @@ -4180,7 +4194,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> .min_priv_ver = PRIV_VERSION_1_12_0 },
>
> /* Supervisor Protection and Translation */
> - [CSR_SATP] = { "satp", smode, read_satp, write_satp },
> + [CSR_SATP] = { "satp", satp, read_satp, write_satp },
>
> /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> [CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
> @@ -4217,7 +4231,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_HGEIP] = { "hgeip", hmode, read_hgeip,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> - [CSR_HGATP] = { "hgatp", hmode, read_hgatp, write_hgatp,
> + [CSR_HGATP] = { "hgatp", hgatp, read_hgatp, write_hgatp,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_HTIMEDELTA] = { "htimedelta", hmode, read_htimedelta,
> write_htimedelta,
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 84ee018f7d..8e16020f8d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -381,12 +381,12 @@ void helper_wfi(CPURISCVState *env)
> void helper_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
> - if (!(env->priv >= PRV_S) ||
> - (env->priv == PRV_S &&
> - get_field(env->mstatus, MSTATUS_TVM))) {
> + if (!riscv_cpu_virt_enabled(env) &&
> + (env->priv == PRV_U ||
> + (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)))) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> - } else if (riscv_has_ext(env, RVH) && riscv_cpu_virt_enabled(env) &&
> - get_field(env->hstatus, HSTATUS_VTVM)) {
> + } else if (riscv_cpu_virt_enabled(env) &&
> + (env->priv == PRV_U || get_field(env->hstatus, HSTATUS_VTVM))) {
> riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> } else {
> tlb_flush(cs);
> @@ -403,7 +403,7 @@ void helper_hyp_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
>
> - if (env->priv == PRV_S && riscv_cpu_virt_enabled(env)) {
> + if (riscv_cpu_virt_enabled(env)) {
> riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> }
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] target/riscv: fix H extension TVM trap
2023-03-12 12:05 [PATCH v4] target/riscv: fix H extension TVM trap Yi Chen
2023-03-17 8:45 ` LIU Zhiwei
@ 2023-04-06 1:56 ` Alistair Francis
2023-04-06 10:24 ` CHEN Yi
1 sibling, 1 reply; 4+ messages in thread
From: Alistair Francis @ 2023-04-06 1:56 UTC (permalink / raw)
To: Yi Chen
Cc: qemu-devel, Weiwei Li, Palmer Dabbelt, Alistair Francis, Bin Meng,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
On Sun, Mar 12, 2023 at 10:07 PM Yi Chen <chenyi2000@zju.edu.cn> wrote:
>
> - Trap satp/hgatp accesses from HS-mode when MSTATUS.TVM is enabled.
> - Trap satp accesses from VS-mode when HSTATUS.VTVM is enabled.
> - Raise RISCV_EXCP_ILLEGAL_INST when U-mode executes SFENCE.VMA/SINVAL.VMA.
> - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> SFENCE.VMA/SINVAL.VMA or VS-mode executes SFENCE.VMA/SINVAL.VMA with
> HSTATUS.VTVM enabled.
> - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> HFENCE.GVMA/HFENCE.VVMA/HINVAL.GVMA/HINVAL.VVMA.
Thanks for the patch!
It looks like this patch needs to be rebased. Do you mind rebasing it
on https://github.com/alistair23/qemu/tree/riscv-to-apply.next and
then re-sending?
Also, when you are fixing a range of issues it's best to split the
fixes into patches that fix each individual issue (where that is
possible). This makes it easier to review but also makes it easier to
track changes and regressions if any problems arise.
In this case you don't need to split them up for a v5, but in future
it's something to keep in mind
The changes look good otherwise though :)
Alistair
>
> Signed-off-by: Yi Chen <chenyi2000@zju.edu.cn>
> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
> ---
> Add reviewed-by
> Replace "env->priv <= PRV_S && riscv_cpu_virt_enabled(env)" with "riscv_cpu_virt_enabled(env)"
> target/riscv/csr.c | 56 +++++++++++++++++++++++++---------------
> target/riscv/op_helper.c | 12 ++++-----
> 2 files changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d522efc0b6..26a02e57bd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -443,6 +443,30 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
> return sstc(env, csrno);
> }
>
> +static RISCVException satp(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> + get_field(env->mstatus, MSTATUS_TVM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + if (env->priv == PRV_S && riscv_cpu_virt_enabled(env) &&
> + get_field(env->hstatus, HSTATUS_VTVM)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> +
> + return smode(env, csrno);
> +}
> +
> +static RISCVException hgatp(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> + get_field(env->mstatus, MSTATUS_TVM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return hmode(env, csrno);
> +}
> +
> /* Checks if PointerMasking registers could be accessed */
> static RISCVException pointer_masking(CPURISCVState *env, int csrno)
> {
> @@ -2655,13 +2679,7 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
> *val = 0;
> return RISCV_EXCP_NONE;
> }
> -
> - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - } else {
> - *val = env->satp;
> - }
> -
> + *val = env->satp;
> return RISCV_EXCP_NONE;
> }
>
> @@ -2684,18 +2702,14 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
> }
>
> if (vm && mask) {
> - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - } else {
> - /*
> - * The ISA defines SATP.MODE=Bare as "no translation", but we still
> - * pass these through QEMU's TLB emulation as it improves
> - * performance. Flushing the TLB on SATP writes with paging
> - * enabled avoids leaking those invalid cached mappings.
> - */
> - tlb_flush(env_cpu(env));
> - env->satp = val;
> - }
> + /*
> + * The ISA defines SATP.MODE=Bare as "no translation", but we still
> + * pass these through QEMU's TLB emulation as it improves
> + * performance. Flushing the TLB on SATP writes with paging
> + * enabled avoids leaking those invalid cached mappings.
> + */
> + tlb_flush(env_cpu(env));
> + env->satp = val;
> }
> return RISCV_EXCP_NONE;
> }
> @@ -4180,7 +4194,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> .min_priv_ver = PRIV_VERSION_1_12_0 },
>
> /* Supervisor Protection and Translation */
> - [CSR_SATP] = { "satp", smode, read_satp, write_satp },
> + [CSR_SATP] = { "satp", satp, read_satp, write_satp },
>
> /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> [CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
> @@ -4217,7 +4231,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_HGEIP] = { "hgeip", hmode, read_hgeip,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> - [CSR_HGATP] = { "hgatp", hmode, read_hgatp, write_hgatp,
> + [CSR_HGATP] = { "hgatp", hgatp, read_hgatp, write_hgatp,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_HTIMEDELTA] = { "htimedelta", hmode, read_htimedelta,
> write_htimedelta,
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 84ee018f7d..8e16020f8d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -381,12 +381,12 @@ void helper_wfi(CPURISCVState *env)
> void helper_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
> - if (!(env->priv >= PRV_S) ||
> - (env->priv == PRV_S &&
> - get_field(env->mstatus, MSTATUS_TVM))) {
> + if (!riscv_cpu_virt_enabled(env) &&
> + (env->priv == PRV_U ||
> + (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)))) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> - } else if (riscv_has_ext(env, RVH) && riscv_cpu_virt_enabled(env) &&
> - get_field(env->hstatus, HSTATUS_VTVM)) {
> + } else if (riscv_cpu_virt_enabled(env) &&
> + (env->priv == PRV_U || get_field(env->hstatus, HSTATUS_VTVM))) {
> riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> } else {
> tlb_flush(cs);
> @@ -403,7 +403,7 @@ void helper_hyp_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
>
> - if (env->priv == PRV_S && riscv_cpu_virt_enabled(env)) {
> + if (riscv_cpu_virt_enabled(env)) {
> riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> }
>
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH v4] target/riscv: fix H extension TVM trap
2023-04-06 1:56 ` Alistair Francis
@ 2023-04-06 10:24 ` CHEN Yi
0 siblings, 0 replies; 4+ messages in thread
From: CHEN Yi @ 2023-04-06 10:24 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, Weiwei Li, Palmer Dabbelt, Alistair Francis, Bin Meng,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
> -----Original Messages-----
> From: "Alistair Francis" <alistair23@gmail.com>
> Sent Time: 2023-04-06 09:56:58 (Thursday)
> To: "Yi Chen" <chenyi2000@zju.edu.cn>
> Cc: qemu-devel@nongnu.org, "Weiwei Li" <liweiwei@iscas.ac.cn>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Alistair Francis" <alistair.francis@wdc.com>, "Bin Meng" <bin.meng@windriver.com>, "Daniel Henrique Barboza" <dbarboza@ventanamicro.com>, "Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>, "open list:RISC-V TCG CPUs" <qemu-riscv@nongnu.org>
> Subject: Re: [PATCH v4] target/riscv: fix H extension TVM trap
>
> On Sun, Mar 12, 2023 at 10:07 PM Yi Chen <chenyi2000@zju.edu.cn> wrote:
> >
> > - Trap satp/hgatp accesses from HS-mode when MSTATUS.TVM is enabled.
> > - Trap satp accesses from VS-mode when HSTATUS.VTVM is enabled.
> > - Raise RISCV_EXCP_ILLEGAL_INST when U-mode executes SFENCE.VMA/SINVAL.VMA.
> > - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> > SFENCE.VMA/SINVAL.VMA or VS-mode executes SFENCE.VMA/SINVAL.VMA with
> > HSTATUS.VTVM enabled.
> > - Raise RISCV_EXCP_VIRT_INSTRUCTION_FAULT when VU-mode executes
> > HFENCE.GVMA/HFENCE.VVMA/HINVAL.GVMA/HINVAL.VVMA.
>
> Thanks for the patch!
>
> It looks like this patch needs to be rebased. Do you mind rebasing it
> on https://github.com/alistair23/qemu/tree/riscv-to-apply.next and
> then re-sending?
Sure. I sent it just now.
> Also, when you are fixing a range of issues it's best to split the
> fixes into patches that fix each individual issue (where that is
> possible). This makes it easier to review but also makes it easier to
> track changes and regressions if any problems arise.
>
> In this case you don't need to split them up for a v5, but in future
> it's something to keep in mind
I see. I will keep that in mind in the future. Thanks for your kind note.
Best,
Yi
> The changes look good otherwise though :)
>
> Alistair
>
> >
> > Signed-off-by: Yi Chen <chenyi2000@zju.edu.cn>
> > Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > ---
> > Add reviewed-by
> > Replace "env->priv <= PRV_S && riscv_cpu_virt_enabled(env)" with "riscv_cpu_virt_enabled(env)"
> > target/riscv/csr.c | 56 +++++++++++++++++++++++++---------------
> > target/riscv/op_helper.c | 12 ++++-----
> > 2 files changed, 41 insertions(+), 27 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index d522efc0b6..26a02e57bd 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -443,6 +443,30 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
> > return sstc(env, csrno);
> > }
> >
> > +static RISCVException satp(CPURISCVState *env, int csrno)
> > +{
> > + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> > + get_field(env->mstatus, MSTATUS_TVM)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + if (env->priv == PRV_S && riscv_cpu_virt_enabled(env) &&
> > + get_field(env->hstatus, HSTATUS_VTVM)) {
> > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > + }
> > +
> > + return smode(env, csrno);
> > +}
> > +
> > +static RISCVException hgatp(CPURISCVState *env, int csrno)
> > +{
> > + if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
> > + get_field(env->mstatus, MSTATUS_TVM)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + return hmode(env, csrno);
> > +}
> > +
> > /* Checks if PointerMasking registers could be accessed */
> > static RISCVException pointer_masking(CPURISCVState *env, int csrno)
> > {
> > @@ -2655,13 +2679,7 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
> > *val = 0;
> > return RISCV_EXCP_NONE;
> > }
> > -
> > - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> > - return RISCV_EXCP_ILLEGAL_INST;
> > - } else {
> > - *val = env->satp;
> > - }
> > -
> > + *val = env->satp;
> > return RISCV_EXCP_NONE;
> > }
> >
> > @@ -2684,18 +2702,14 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
> > }
> >
> > if (vm && mask) {
> > - if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> > - return RISCV_EXCP_ILLEGAL_INST;
> > - } else {
> > - /*
> > - * The ISA defines SATP.MODE=Bare as "no translation", but we still
> > - * pass these through QEMU's TLB emulation as it improves
> > - * performance. Flushing the TLB on SATP writes with paging
> > - * enabled avoids leaking those invalid cached mappings.
> > - */
> > - tlb_flush(env_cpu(env));
> > - env->satp = val;
> > - }
> > + /*
> > + * The ISA defines SATP.MODE=Bare as "no translation", but we still
> > + * pass these through QEMU's TLB emulation as it improves
> > + * performance. Flushing the TLB on SATP writes with paging
> > + * enabled avoids leaking those invalid cached mappings.
> > + */
> > + tlb_flush(env_cpu(env));
> > + env->satp = val;
> > }
> > return RISCV_EXCP_NONE;
> > }
> > @@ -4180,7 +4194,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > .min_priv_ver = PRIV_VERSION_1_12_0 },
> >
> > /* Supervisor Protection and Translation */
> > - [CSR_SATP] = { "satp", smode, read_satp, write_satp },
> > + [CSR_SATP] = { "satp", satp, read_satp, write_satp },
> >
> > /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> > [CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
> > @@ -4217,7 +4231,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > .min_priv_ver = PRIV_VERSION_1_12_0 },
> > [CSR_HGEIP] = { "hgeip", hmode, read_hgeip,
> > .min_priv_ver = PRIV_VERSION_1_12_0 },
> > - [CSR_HGATP] = { "hgatp", hmode, read_hgatp, write_hgatp,
> > + [CSR_HGATP] = { "hgatp", hgatp, read_hgatp, write_hgatp,
> > .min_priv_ver = PRIV_VERSION_1_12_0 },
> > [CSR_HTIMEDELTA] = { "htimedelta", hmode, read_htimedelta,
> > write_htimedelta,
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index 84ee018f7d..8e16020f8d 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -381,12 +381,12 @@ void helper_wfi(CPURISCVState *env)
> > void helper_tlb_flush(CPURISCVState *env)
> > {
> > CPUState *cs = env_cpu(env);
> > - if (!(env->priv >= PRV_S) ||
> > - (env->priv == PRV_S &&
> > - get_field(env->mstatus, MSTATUS_TVM))) {
> > + if (!riscv_cpu_virt_enabled(env) &&
> > + (env->priv == PRV_U ||
> > + (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)))) {
> > riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> > - } else if (riscv_has_ext(env, RVH) && riscv_cpu_virt_enabled(env) &&
> > - get_field(env->hstatus, HSTATUS_VTVM)) {
> > + } else if (riscv_cpu_virt_enabled(env) &&
> > + (env->priv == PRV_U || get_field(env->hstatus, HSTATUS_VTVM))) {
> > riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> > } else {
> > tlb_flush(cs);
> > @@ -403,7 +403,7 @@ void helper_hyp_tlb_flush(CPURISCVState *env)
> > {
> > CPUState *cs = env_cpu(env);
> >
> > - if (env->priv == PRV_S && riscv_cpu_virt_enabled(env)) {
> > + if (riscv_cpu_virt_enabled(env)) {
> > riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> > }
> >
> > --
> > 2.39.2
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-06 10:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-12 12:05 [PATCH v4] target/riscv: fix H extension TVM trap Yi Chen
2023-03-17 8:45 ` LIU Zhiwei
2023-04-06 1:56 ` Alistair Francis
2023-04-06 10:24 ` CHEN Yi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).