target/riscv/cpu.c | 1 + target/riscv/cpu.h | 4 ++++ target/riscv/cpu_helper.c | 21 ++++++++------------- 3 files changed, 13 insertions(+), 13 deletions(-)
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>
---
Unfortunately, I tested the previous version of the patch only against
the RISC-V softmmu target, not against the linux-user target.
I modified the patch, so that the two_stage_lookup is neither present
nor updated for the linux-user target.
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 4 ++++
target/riscv/cpu_helper.c | 21 ++++++++-------------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddea8fbeeb..e8c4455525 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -356,6 +356,7 @@ static void riscv_cpu_reset(DeviceState *dev)
env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
env->mcause = 0;
env->pc = env->resetvec;
+ env->two_stage_lookup = false;
#endif
cs->exception_index = EXCP_NONE;
env->load_res = -1;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0edb2826a2..0a33d387ba 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -213,6 +213,10 @@ struct CPURISCVState {
target_ulong satp_hs;
uint64_t mstatus_hs;
+ /* Signals whether the current exception occurred with two-stage address
+ translation active. */
+ bool two_stage_lookup;
+
target_ulong scounteren;
target_ulong mcounteren;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2f43939fb6..af00728829 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;
}
}
@@ -1016,6 +1010,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
* RISC-V ISA Specification.
*/
+ env->two_stage_lookup = false;
#endif
cs->exception_index = EXCP_NONE; /* mark handled to qemu */
}
--
2.31.0
On Fri, Mar 19, 2021 at 10:16 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 > --- > Unfortunately, I tested the previous version of the patch only against > the RISC-V softmmu target, not against the linux-user target. > I modified the patch, so that the two_stage_lookup is neither present > nor updated for the linux-user target. > > target/riscv/cpu.c | 1 + > target/riscv/cpu.h | 4 ++++ > target/riscv/cpu_helper.c | 21 ++++++++------------- > 3 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ddea8fbeeb..e8c4455525 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -356,6 +356,7 @@ static void riscv_cpu_reset(DeviceState *dev) > env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV); > env->mcause = 0; > env->pc = env->resetvec; > + env->two_stage_lookup = false; > #endif > cs->exception_index = EXCP_NONE; > env->load_res = -1; > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 0edb2826a2..0a33d387ba 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -213,6 +213,10 @@ struct CPURISCVState { > target_ulong satp_hs; > uint64_t mstatus_hs; > > + /* Signals whether the current exception occurred with two-stage address > + translation active. */ > + bool two_stage_lookup; > + > target_ulong scounteren; > target_ulong mcounteren; > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 2f43939fb6..af00728829 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; > } > } > @@ -1016,6 +1010,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > * RISC-V ISA Specification. > */ > > + env->two_stage_lookup = false; > #endif > cs->exception_index = EXCP_NONE; /* mark handled to qemu */ > } > -- > 2.31.0 > >
On Fri, Mar 19, 2021 at 10:16 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 > --- > Unfortunately, I tested the previous version of the patch only against > the RISC-V softmmu target, not against the linux-user target. > I modified the patch, so that the two_stage_lookup is neither present > nor updated for the linux-user target. > > target/riscv/cpu.c | 1 + > target/riscv/cpu.h | 4 ++++ > target/riscv/cpu_helper.c | 21 ++++++++------------- > 3 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ddea8fbeeb..e8c4455525 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -356,6 +356,7 @@ static void riscv_cpu_reset(DeviceState *dev) > env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV); > env->mcause = 0; > env->pc = env->resetvec; > + env->two_stage_lookup = false; > #endif > cs->exception_index = EXCP_NONE; > env->load_res = -1; > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 0edb2826a2..0a33d387ba 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -213,6 +213,10 @@ struct CPURISCVState { > target_ulong satp_hs; > uint64_t mstatus_hs; > > + /* Signals whether the current exception occurred with two-stage address > + translation active. */ > + bool two_stage_lookup; > + > target_ulong scounteren; > target_ulong mcounteren; > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 2f43939fb6..af00728829 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; > } > } > @@ -1016,6 +1010,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > * RISC-V ISA Specification. > */ > > + env->two_stage_lookup = false; > #endif > cs->exception_index = EXCP_NONE; /* mark handled to qemu */ > } > -- > 2.31.0 > >
© 2016 - 2024 Red Hat, Inc.