* [PATCH] target/riscv: Add proper two-stage lookup exception detection
@ 2021-03-11 10:29 Georg Kotheimer
2021-03-16 20:14 ` Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Georg Kotheimer @ 2021-03-11 10:29 UTC (permalink / raw)
To: qemu-devel, qemu-riscv; +Cc: Alistair Francis, Georg Kotheimer
The current two-stage lookup detection in riscv_cpu_do_interrupt falls
short of its purpose, as all it checks is whether two-stage address
translation either via the hypervisor-load store instructions or the
MPRV feature would be allowed.
What we really need instead is whether two-stage address translation was
active when the exception was raised. However, in riscv_cpu_do_interrupt
we do not have the information to reliably detect this. Therefore, when
we raise a memory fault exception we have to record whether two-stage
address translation is active.
Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 3 +++
target/riscv/cpu_helper.c | 21 ++++++++-------------
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddea8fbeeb..865caddb06 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -358,6 +358,7 @@ static void riscv_cpu_reset(DeviceState *dev)
env->pc = env->resetvec;
#endif
cs->exception_index = EXCP_NONE;
+ env->two_stage_lookup = false;
env->load_res = -1;
set_default_nan_mode(1, &env->fp_status);
}
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0edb2826a2..1288ff5981 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -131,6 +131,9 @@ struct CPURISCVState {
target_ulong badaddr;
target_ulong guest_phys_fault_addr;
+ /* Signals whether the current exception occurred with two-stage address
+ translation active. */
+ bool two_stage_lookup;
target_ulong priv_ver;
target_ulong vext_ver;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2f43939fb6..2de870d3a3 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -605,6 +605,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
g_assert_not_reached();
}
env->badaddr = address;
+ env->two_stage_lookup = two_stage;
}
hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
@@ -646,6 +647,8 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
}
env->badaddr = addr;
+ env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
+ riscv_cpu_two_stage_lookup(mmu_idx);
riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
}
@@ -669,6 +672,8 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
g_assert_not_reached();
}
env->badaddr = addr;
+ env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
+ riscv_cpu_two_stage_lookup(mmu_idx);
riscv_raise_exception(env, cs->exception_index, retaddr);
}
#endif /* !CONFIG_USER_ONLY */
@@ -910,16 +915,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
/* handle the trap in S-mode */
if (riscv_has_ext(env, RVH)) {
target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
- bool two_stage_lookup = false;
- if (env->priv == PRV_M ||
- (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
- (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
- get_field(env->hstatus, HSTATUS_HU))) {
- two_stage_lookup = true;
- }
-
- if ((riscv_cpu_virt_enabled(env) || two_stage_lookup) && write_tval) {
+ if (env->two_stage_lookup && write_tval) {
/*
* If we are writing a guest virtual address to stval, set
* this to 1. If we are trapping to VS we will set this to 0
@@ -957,10 +954,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
riscv_cpu_set_force_hs_excep(env, 0);
} else {
/* Trap into HS mode */
- if (!two_stage_lookup) {
- env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
- riscv_cpu_virt_enabled(env));
- }
+ env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
htval = env->guest_phys_fault_addr;
}
}
@@ -1018,4 +1012,5 @@ void riscv_cpu_do_interrupt(CPUState *cs)
#endif
cs->exception_index = EXCP_NONE; /* mark handled to qemu */
+ env->two_stage_lookup = false;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: Add proper two-stage lookup exception detection
2021-03-11 10:29 [PATCH] target/riscv: Add proper two-stage lookup exception detection Georg Kotheimer
@ 2021-03-16 20:14 ` Alistair Francis
2021-03-17 18:24 ` Alistair Francis
2021-03-19 12:49 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2021-03-16 20:14 UTC (permalink / raw)
To: Georg Kotheimer
Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers
On Thu, Mar 11, 2021 at 5:29 AM Georg Kotheimer
<georg.kotheimer@kernkonzept.com> wrote:
>
> The current two-stage lookup detection in riscv_cpu_do_interrupt falls
> short of its purpose, as all it checks is whether two-stage address
> translation either via the hypervisor-load store instructions or the
> MPRV feature would be allowed.
>
> What we really need instead is whether two-stage address translation was
> active when the exception was raised. However, in riscv_cpu_do_interrupt
> we do not have the information to reliably detect this. Therefore, when
> we raise a memory fault exception we have to record whether two-stage
> address translation is active.
>
> Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 1 +
> target/riscv/cpu.h | 3 +++
> target/riscv/cpu_helper.c | 21 ++++++++-------------
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddea8fbeeb..865caddb06 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -358,6 +358,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->pc = env->resetvec;
> #endif
> cs->exception_index = EXCP_NONE;
> + env->two_stage_lookup = false;
> env->load_res = -1;
> set_default_nan_mode(1, &env->fp_status);
> }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb2826a2..1288ff5981 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -131,6 +131,9 @@ struct CPURISCVState {
>
> target_ulong badaddr;
> target_ulong guest_phys_fault_addr;
> + /* Signals whether the current exception occurred with two-stage address
> + translation active. */
> + bool two_stage_lookup;
>
> target_ulong priv_ver;
> target_ulong vext_ver;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2f43939fb6..2de870d3a3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -605,6 +605,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> g_assert_not_reached();
> }
> env->badaddr = address;
> + env->two_stage_lookup = two_stage;
> }
>
> hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> @@ -646,6 +647,8 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> }
>
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
> }
>
> @@ -669,6 +672,8 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> g_assert_not_reached();
> }
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(env, cs->exception_index, retaddr);
> }
> #endif /* !CONFIG_USER_ONLY */
> @@ -910,16 +915,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> /* handle the trap in S-mode */
> if (riscv_has_ext(env, RVH)) {
> target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
> - bool two_stage_lookup = false;
>
> - if (env->priv == PRV_M ||
> - (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
> - (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
> - get_field(env->hstatus, HSTATUS_HU))) {
> - two_stage_lookup = true;
> - }
> -
> - if ((riscv_cpu_virt_enabled(env) || two_stage_lookup) && write_tval) {
> + if (env->two_stage_lookup && write_tval) {
> /*
> * If we are writing a guest virtual address to stval, set
> * this to 1. If we are trapping to VS we will set this to 0
> @@ -957,10 +954,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> riscv_cpu_set_force_hs_excep(env, 0);
> } else {
> /* Trap into HS mode */
> - if (!two_stage_lookup) {
> - env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
> - riscv_cpu_virt_enabled(env));
> - }
> + env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> htval = env->guest_phys_fault_addr;
> }
> }
> @@ -1018,4 +1012,5 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>
> #endif
> cs->exception_index = EXCP_NONE; /* mark handled to qemu */
> + env->two_stage_lookup = false;
> }
> --
> 2.30.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: Add proper two-stage lookup exception detection
2021-03-11 10:29 [PATCH] target/riscv: Add proper two-stage lookup exception detection Georg Kotheimer
2021-03-16 20:14 ` Alistair Francis
@ 2021-03-17 18:24 ` Alistair Francis
2021-03-19 12:49 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2021-03-17 18:24 UTC (permalink / raw)
To: Georg Kotheimer
Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers
On Thu, Mar 11, 2021 at 5:29 AM Georg Kotheimer
<georg.kotheimer@kernkonzept.com> wrote:
>
> The current two-stage lookup detection in riscv_cpu_do_interrupt falls
> short of its purpose, as all it checks is whether two-stage address
> translation either via the hypervisor-load store instructions or the
> MPRV feature would be allowed.
>
> What we really need instead is whether two-stage address translation was
> active when the exception was raised. However, in riscv_cpu_do_interrupt
> we do not have the information to reliably detect this. Therefore, when
> we raise a memory fault exception we have to record whether two-stage
> address translation is active.
>
> Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/cpu.c | 1 +
> target/riscv/cpu.h | 3 +++
> target/riscv/cpu_helper.c | 21 ++++++++-------------
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddea8fbeeb..865caddb06 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -358,6 +358,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->pc = env->resetvec;
> #endif
> cs->exception_index = EXCP_NONE;
> + env->two_stage_lookup = false;
> env->load_res = -1;
> set_default_nan_mode(1, &env->fp_status);
> }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb2826a2..1288ff5981 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -131,6 +131,9 @@ struct CPURISCVState {
>
> target_ulong badaddr;
> target_ulong guest_phys_fault_addr;
> + /* Signals whether the current exception occurred with two-stage address
> + translation active. */
> + bool two_stage_lookup;
>
> target_ulong priv_ver;
> target_ulong vext_ver;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2f43939fb6..2de870d3a3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -605,6 +605,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> g_assert_not_reached();
> }
> env->badaddr = address;
> + env->two_stage_lookup = two_stage;
> }
>
> hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> @@ -646,6 +647,8 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> }
>
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
> }
>
> @@ -669,6 +672,8 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> g_assert_not_reached();
> }
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(env, cs->exception_index, retaddr);
> }
> #endif /* !CONFIG_USER_ONLY */
> @@ -910,16 +915,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> /* handle the trap in S-mode */
> if (riscv_has_ext(env, RVH)) {
> target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
> - bool two_stage_lookup = false;
>
> - if (env->priv == PRV_M ||
> - (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
> - (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
> - get_field(env->hstatus, HSTATUS_HU))) {
> - two_stage_lookup = true;
> - }
> -
> - if ((riscv_cpu_virt_enabled(env) || two_stage_lookup) && write_tval) {
> + if (env->two_stage_lookup && write_tval) {
> /*
> * If we are writing a guest virtual address to stval, set
> * this to 1. If we are trapping to VS we will set this to 0
> @@ -957,10 +954,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> riscv_cpu_set_force_hs_excep(env, 0);
> } else {
> /* Trap into HS mode */
> - if (!two_stage_lookup) {
> - env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
> - riscv_cpu_virt_enabled(env));
> - }
> + env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> htval = env->guest_phys_fault_addr;
> }
> }
> @@ -1018,4 +1012,5 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>
> #endif
> cs->exception_index = EXCP_NONE; /* mark handled to qemu */
> + env->two_stage_lookup = false;
> }
> --
> 2.30.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: Add proper two-stage lookup exception detection
2021-03-11 10:29 [PATCH] target/riscv: Add proper two-stage lookup exception detection Georg Kotheimer
2021-03-16 20:14 ` Alistair Francis
2021-03-17 18:24 ` Alistair Francis
@ 2021-03-19 12:49 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2021-03-19 12:49 UTC (permalink / raw)
To: Georg Kotheimer
Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers
On Thu, Mar 11, 2021 at 5:29 AM Georg Kotheimer
<georg.kotheimer@kernkonzept.com> wrote:
>
> The current two-stage lookup detection in riscv_cpu_do_interrupt falls
> short of its purpose, as all it checks is whether two-stage address
> translation either via the hypervisor-load store instructions or the
> MPRV feature would be allowed.
>
> What we really need instead is whether two-stage address translation was
> active when the exception was raised. However, in riscv_cpu_do_interrupt
> we do not have the information to reliably detect this. Therefore, when
> we raise a memory fault exception we have to record whether two-stage
> address translation is active.
>
> Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
> ---
> target/riscv/cpu.c | 1 +
> target/riscv/cpu.h | 3 +++
> target/riscv/cpu_helper.c | 21 ++++++++-------------
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddea8fbeeb..865caddb06 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -358,6 +358,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->pc = env->resetvec;
> #endif
> cs->exception_index = EXCP_NONE;
> + env->two_stage_lookup = false;
> env->load_res = -1;
> set_default_nan_mode(1, &env->fp_status);
> }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb2826a2..1288ff5981 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -131,6 +131,9 @@ struct CPURISCVState {
>
> target_ulong badaddr;
> target_ulong guest_phys_fault_addr;
> + /* Signals whether the current exception occurred with two-stage address
> + translation active. */
> + bool two_stage_lookup;
>
> target_ulong priv_ver;
> target_ulong vext_ver;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2f43939fb6..2de870d3a3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -605,6 +605,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> g_assert_not_reached();
> }
> env->badaddr = address;
> + env->two_stage_lookup = two_stage;
> }
>
> hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> @@ -646,6 +647,8 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> }
>
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
> }
>
> @@ -669,6 +672,8 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> g_assert_not_reached();
> }
> env->badaddr = addr;
> + env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> + riscv_cpu_two_stage_lookup(mmu_idx);
> riscv_raise_exception(env, cs->exception_index, retaddr);
> }
> #endif /* !CONFIG_USER_ONLY */
> @@ -910,16 +915,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> /* handle the trap in S-mode */
> if (riscv_has_ext(env, RVH)) {
> target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
> - bool two_stage_lookup = false;
>
> - if (env->priv == PRV_M ||
> - (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
> - (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
> - get_field(env->hstatus, HSTATUS_HU))) {
> - two_stage_lookup = true;
> - }
> -
> - if ((riscv_cpu_virt_enabled(env) || two_stage_lookup) && write_tval) {
> + if (env->two_stage_lookup && write_tval) {
> /*
> * If we are writing a guest virtual address to stval, set
> * this to 1. If we are trapping to VS we will set this to 0
> @@ -957,10 +954,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> riscv_cpu_set_force_hs_excep(env, 0);
> } else {
> /* Trap into HS mode */
> - if (!two_stage_lookup) {
> - env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
> - riscv_cpu_virt_enabled(env));
> - }
> + env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> htval = env->guest_phys_fault_addr;
> }
> }
> @@ -1018,4 +1012,5 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>
> #endif
> cs->exception_index = EXCP_NONE; /* mark handled to qemu */
> + env->two_stage_lookup = false;
Hello,
Thanks for the patch.
There is no env declared here and this fails to compile.
Can you please send a new patch after testing that it compiles?
Alistair
> }
> --
> 2.30.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-19 12:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 10:29 [PATCH] target/riscv: Add proper two-stage lookup exception detection Georg Kotheimer
2021-03-16 20:14 ` Alistair Francis
2021-03-17 18:24 ` Alistair Francis
2021-03-19 12:49 ` Alistair Francis
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).