target/riscv/cpu.h | 10 +++++++--- target/riscv/cpu_helper.c | 35 ++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 12 deletions(-)
VS-stage translation at get_physical_address needs to translate pte
address by G-stage translation. But the G-stage translation error
can not be distinguished from VS-stage translation error in
riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
and this G-stage translation error must be handled by HS-mode. So
introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
distinguish and raise it to HS-mode.
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
---
target/riscv/cpu.h | 10 +++++++---
target/riscv/cpu_helper.c | 35 ++++++++++++++++++++++++++---------
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de275782e6..de4705bb57 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -82,9 +82,13 @@ enum {
#define VEXT_VERSION_0_07_1 0x00000701
-#define TRANSLATE_PMP_FAIL 2
-#define TRANSLATE_FAIL 1
-#define TRANSLATE_SUCCESS 0
+enum {
+ TRANSLATE_SUCCESS,
+ TRANSLATE_FAIL,
+ TRANSLATE_PMP_FAIL,
+ TRANSLATE_G_STAGE_FAIL
+};
+
#define MMU_USER_IDX 3
#define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 904899054d..ae66722d32 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -316,6 +316,8 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
* @physical: This will be set to the calculated physical address
* @prot: The returned protection attributes
* @addr: The virtual address to be translated
+ * @fault_pte_addr: If not NULL, this will be set to fault pte address
+ * when a error occurs on pte address translation.
* @access_type: The type of MMU access
* @mmu_idx: Indicates current privilege level
* @first_stage: Are we in first stage translation?
@@ -324,6 +326,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
*/
static int get_physical_address(CPURISCVState *env, hwaddr *physical,
int *prot, target_ulong addr,
+ target_ulong *fault_pte_addr,
int access_type, int mmu_idx,
bool first_stage, bool two_stage)
{
@@ -447,11 +450,14 @@ restart:
/* Do the second stage translation on the base PTE address. */
int vbase_ret = get_physical_address(env, &vbase, &vbase_prot,
- base, MMU_DATA_LOAD,
+ base, NULL, MMU_DATA_LOAD,
mmu_idx, false, true);
if (vbase_ret != TRANSLATE_SUCCESS) {
- return vbase_ret;
+ if (fault_pte_addr) {
+ *fault_pte_addr = (base + idx * ptesize) >> 2;
+ }
+ return TRANSLATE_G_STAGE_FAIL;
}
pte_addr = vbase + idx * ptesize;
@@ -632,13 +638,13 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
int prot;
int mmu_idx = cpu_mmu_index(&cpu->env, false);
- if (get_physical_address(env, &phys_addr, &prot, addr, 0, mmu_idx,
+ if (get_physical_address(env, &phys_addr, &prot, addr, NULL, 0, mmu_idx,
true, riscv_cpu_virt_enabled(env))) {
return -1;
}
if (riscv_cpu_virt_enabled(env)) {
- if (get_physical_address(env, &phys_addr, &prot, phys_addr,
+ if (get_physical_address(env, &phys_addr, &prot, phys_addr, NULL,
0, mmu_idx, false, true)) {
return -1;
}
@@ -727,19 +733,30 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
if (riscv_cpu_virt_enabled(env) ||
(riscv_cpu_two_stage_lookup(env) && access_type != MMU_INST_FETCH)) {
/* Two stage lookup */
- ret = get_physical_address(env, &pa, &prot, address, access_type,
+ ret = get_physical_address(env, &pa, &prot, address,
+ &env->guest_phys_fault_addr, access_type,
mmu_idx, true, true);
+ /*
+ * A G-stage exception may be triggered during two state lookup.
+ * And the env->guest_phys_fault_addr has already been set in
+ * get_physical_address().
+ */
+ if (ret == TRANSLATE_G_STAGE_FAIL) {
+ first_stage_error = false;
+ access_type = MMU_DATA_LOAD;
+ }
+
qemu_log_mask(CPU_LOG_MMU,
"%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
- if (ret != TRANSLATE_FAIL) {
+ if (ret == TRANSLATE_SUCCESS) {
/* Second stage lookup */
im_address = pa;
- ret = get_physical_address(env, &pa, &prot2, im_address,
+ ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
access_type, mmu_idx, false, true);
qemu_log_mask(CPU_LOG_MMU,
@@ -768,8 +785,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
}
} else {
/* Single stage lookup */
- ret = get_physical_address(env, &pa, &prot, address, access_type,
- mmu_idx, true, false);
+ ret = get_physical_address(env, &pa, &prot, address, NULL,
+ access_type, mmu_idx, true, false);
qemu_log_mask(CPU_LOG_MMU,
"%s address=%" VADDR_PRIx " ret %d physical "
--
2.19.1
On Wed, Oct 14, 2020 at 3:18 AM Yifei Jiang <jiangyifei@huawei.com> wrote: > > VS-stage translation at get_physical_address needs to translate pte > address by G-stage translation. But the G-stage translation error > can not be distinguished from VS-stage translation error in > riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte, > and this G-stage translation error must be handled by HS-mode. So > introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could > distinguish and raise it to HS-mode. > > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com> > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.h | 10 +++++++--- > target/riscv/cpu_helper.c | 35 ++++++++++++++++++++++++++--------- > 2 files changed, 33 insertions(+), 12 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index de275782e6..de4705bb57 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -82,9 +82,13 @@ enum { > > #define VEXT_VERSION_0_07_1 0x00000701 > > -#define TRANSLATE_PMP_FAIL 2 > -#define TRANSLATE_FAIL 1 > -#define TRANSLATE_SUCCESS 0 > +enum { > + TRANSLATE_SUCCESS, > + TRANSLATE_FAIL, > + TRANSLATE_PMP_FAIL, > + TRANSLATE_G_STAGE_FAIL > +}; > + > #define MMU_USER_IDX 3 > > #define MAX_RISCV_PMPS (16) > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 904899054d..ae66722d32 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -316,6 +316,8 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) > * @physical: This will be set to the calculated physical address > * @prot: The returned protection attributes > * @addr: The virtual address to be translated > + * @fault_pte_addr: If not NULL, this will be set to fault pte address > + * when a error occurs on pte address translation. > * @access_type: The type of MMU access > * @mmu_idx: Indicates current privilege level > * @first_stage: Are we in first stage translation? > @@ -324,6 +326,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) > */ > static int get_physical_address(CPURISCVState *env, hwaddr *physical, > int *prot, target_ulong addr, > + target_ulong *fault_pte_addr, > int access_type, int mmu_idx, > bool first_stage, bool two_stage) > { > @@ -447,11 +450,14 @@ restart: > > /* Do the second stage translation on the base PTE address. */ > int vbase_ret = get_physical_address(env, &vbase, &vbase_prot, > - base, MMU_DATA_LOAD, > + base, NULL, MMU_DATA_LOAD, > mmu_idx, false, true); > > if (vbase_ret != TRANSLATE_SUCCESS) { > - return vbase_ret; > + if (fault_pte_addr) { > + *fault_pte_addr = (base + idx * ptesize) >> 2; > + } > + return TRANSLATE_G_STAGE_FAIL; > } > > pte_addr = vbase + idx * ptesize; > @@ -632,13 +638,13 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) > int prot; > int mmu_idx = cpu_mmu_index(&cpu->env, false); > > - if (get_physical_address(env, &phys_addr, &prot, addr, 0, mmu_idx, > + if (get_physical_address(env, &phys_addr, &prot, addr, NULL, 0, mmu_idx, > true, riscv_cpu_virt_enabled(env))) { > return -1; > } > > if (riscv_cpu_virt_enabled(env)) { > - if (get_physical_address(env, &phys_addr, &prot, phys_addr, > + if (get_physical_address(env, &phys_addr, &prot, phys_addr, NULL, > 0, mmu_idx, false, true)) { > return -1; > } > @@ -727,19 +733,30 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > if (riscv_cpu_virt_enabled(env) || > (riscv_cpu_two_stage_lookup(env) && access_type != MMU_INST_FETCH)) { > /* Two stage lookup */ > - ret = get_physical_address(env, &pa, &prot, address, access_type, > + ret = get_physical_address(env, &pa, &prot, address, > + &env->guest_phys_fault_addr, access_type, > mmu_idx, true, true); > > + /* > + * A G-stage exception may be triggered during two state lookup. > + * And the env->guest_phys_fault_addr has already been set in > + * get_physical_address(). > + */ > + if (ret == TRANSLATE_G_STAGE_FAIL) { > + first_stage_error = false; > + access_type = MMU_DATA_LOAD; > + } > + > qemu_log_mask(CPU_LOG_MMU, > "%s 1st-stage address=%" VADDR_PRIx " ret %d physical " > TARGET_FMT_plx " prot %d\n", > __func__, address, ret, pa, prot); > > - if (ret != TRANSLATE_FAIL) { > + if (ret == TRANSLATE_SUCCESS) { > /* Second stage lookup */ > im_address = pa; > > - ret = get_physical_address(env, &pa, &prot2, im_address, > + ret = get_physical_address(env, &pa, &prot2, im_address, NULL, > access_type, mmu_idx, false, true); > > qemu_log_mask(CPU_LOG_MMU, > @@ -768,8 +785,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > } > } else { > /* Single stage lookup */ > - ret = get_physical_address(env, &pa, &prot, address, access_type, > - mmu_idx, true, false); > + ret = get_physical_address(env, &pa, &prot, address, NULL, > + access_type, mmu_idx, true, false); > > qemu_log_mask(CPU_LOG_MMU, > "%s address=%" VADDR_PRIx " ret %d physical " > -- > 2.19.1 > >
On 10/14/20 3:17 AM, Yifei Jiang wrote: > + if (fault_pte_addr) { > + *fault_pte_addr = (base + idx * ptesize) >> 2; The shift is wrong. It should be exactly like... > + } > + return TRANSLATE_G_STAGE_FAIL; > } > > pte_addr = vbase + idx * ptesize; ... this. r~
> -----Original Message----- > From: Richard Henderson [mailto:richard.henderson@linaro.org] > Sent: Thursday, October 15, 2020 4:22 AM > To: Jiangyifei <jiangyifei@huawei.com>; qemu-devel@nongnu.org; > qemu-riscv@nongnu.org > Cc: palmer@dabbelt.com; Alistair.Francis@wdc.com; > sagark@eecs.berkeley.edu; kbastian@mail.uni-paderborn.de; Zhangxiaofeng > (F) <victor.zhangxiaofeng@huawei.com>; Wubin (H) <wu.wubin@huawei.com>; > Zhanghailiang <zhang.zhanghailiang@huawei.com>; dengkai (A) > <dengkai1@huawei.com>; yinyipeng <yinyipeng1@huawei.com> > Subject: Re: [PATCH V3] target/riscv: raise exception to HS-mode at > get_physical_address > > On 10/14/20 3:17 AM, Yifei Jiang wrote: > > + if (fault_pte_addr) { > > + *fault_pte_addr = (base + idx * ptesize) >> 2; > > The shift is wrong. It should be exactly like... > We have tested in the VM migration. fault_pte_addr will eventually be assigned to htval register. Description of htval register according to the specification: When a guest-page-fault trap is taken into HS-mode, htval is written with either zero or the guest physical address that faulted, shifted right by 2 bits. In addition, fault_pte_addr is named after env->guest_phys_fault_addr, which makes sense in a sense. Yifei > > + } > > + return TRANSLATE_G_STAGE_FAIL; > > } > > > > pte_addr = vbase + idx * ptesize; > > ... this. > > > r~
On Wed, Oct 14, 2020 at 6:59 PM Jiangyifei <jiangyifei@huawei.com> wrote: > > > > -----Original Message----- > > From: Richard Henderson [mailto:richard.henderson@linaro.org] > > Sent: Thursday, October 15, 2020 4:22 AM > > To: Jiangyifei <jiangyifei@huawei.com>; qemu-devel@nongnu.org; > > qemu-riscv@nongnu.org > > Cc: palmer@dabbelt.com; Alistair.Francis@wdc.com; > > sagark@eecs.berkeley.edu; kbastian@mail.uni-paderborn.de; Zhangxiaofeng > > (F) <victor.zhangxiaofeng@huawei.com>; Wubin (H) <wu.wubin@huawei.com>; > > Zhanghailiang <zhang.zhanghailiang@huawei.com>; dengkai (A) > > <dengkai1@huawei.com>; yinyipeng <yinyipeng1@huawei.com> > > Subject: Re: [PATCH V3] target/riscv: raise exception to HS-mode at > > get_physical_address > > > > On 10/14/20 3:17 AM, Yifei Jiang wrote: > > > + if (fault_pte_addr) { > > > + *fault_pte_addr = (base + idx * ptesize) >> 2; > > > > The shift is wrong. It should be exactly like... > > > > We have tested in the VM migration. > > fault_pte_addr will eventually be assigned to htval register. > > Description of htval register according to the specification: > When a guest-page-fault trap is taken into HS-mode, htval is written with either zero > or the guest physical address that faulted, shifted right by 2 bits. Yep, I agree that the shift is correct, it's what we do when we set guest_phys_fault_addr in other places. It is a little confusing that we shift it in get_physical_address(), instead of when guest_phys_fault_addr is set. In this case you are setting guest_phys_fault_addr directly when calling get_physical_address(... &env->guest_phys_fault_addr ...). I have added this comment to make sure it's clear and applied it, I hope that's ok. diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 4ea9510c07..4652082df1 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -318,6 +318,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) * @addr: The virtual address to be translated * @fault_pte_addr: If not NULL, this will be set to fault pte address * when a error occurs on pte address translation. + * This will already be shifted to match htval. * @access_type: The type of MMU access * @mmu_idx: Indicates current privilege level * @first_stage: Are we in first stage translation? Alistair > > In addition, fault_pte_addr is named after env->guest_phys_fault_addr, which makes > sense in a sense. > > Yifei > > > > + } > > > + return TRANSLATE_G_STAGE_FAIL; > > > } > > > > > > pte_addr = vbase + idx * ptesize; > > > > ... this. > > > > > > r~
© 2016 - 2024 Red Hat, Inc.