> -----Original Message-----
> From: Alistair Francis [mailto:alistai...@gmail.com]
> Sent: Saturday, September 26, 2020 6:24 AM
> To: Jiangyifei <jiangyi...@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-ri...@nongnu.org>; Zhanghailiang
> <zhang.zhanghaili...@huawei.com>; Sagar Karandikar
> <sag...@eecs.berkeley.edu>; Bastian Koppelmann
> <kbast...@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaof...@huawei.com>; Alistair Francis
> <alistair.fran...@wdc.com>; yinyipeng <yinyipe...@huawei.com>; Palmer
> Dabbelt <pal...@dabbelt.com>; Wubin (H) <wu.wu...@huawei.com>;
> dengkai (A) <dengk...@huawei.com>
> Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> get_physical_address
>
> On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyi...@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 <jiangyi...@huawei.com>
> > Signed-off-by: Yipeng Yin <yinyipe...@huawei.com>
>
> Thanks for the patch!
>
> Sorry for the delay here.
>
> > ---
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_helper.c | 12 ++++++++++--
> > 2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > a804a5d0ba..8b3b368d6a 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -85,6 +85,7 @@ enum {
> > #define TRANSLATE_FAIL 1
> > #define TRANSLATE_SUCCESS 0
> > #define MMU_USER_IDX 3
> > +#define TRANSLATE_G_STAGE_FAIL 4
> >
> > #define MAX_RISCV_PMPS (16)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index fd1d373b6f..1635b09c41 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -440,7 +440,10 @@ restart:
> > mmu_idx,
> false,
> > true);
> >
> > if (vbase_ret != TRANSLATE_SUCCESS) {
> > - return vbase_ret;
> > + env->guest_phys_fault_addr = (base |
> > + (addr &
> > +
> (TARGET_PAGE_SIZE -
> > + 1))) >> 2;
>
> Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
Hi Alistair,
It's not easy to do that. The key is that the wrong address(the `base`
variable) is not visible to riscv_cpu_tlb_fill().
Because the wrong address may be from any level of PTE which can't be easily
obtained by riscv_cpu_tlb_fill().
The alternative is to add an out parameter in get_physical_address(). But it is
not either elegant.
What is your advice?
Best Regards,
Yifei
>
> > + return TRANSLATE_G_STAGE_FAIL;
> > }
> >
> > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17 @@
> > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > ret = get_physical_address(env, &pa, &prot, address,
> access_type,
> > mmu_idx, true, true);
> >
> > + 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_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
>
> Otherwise this patch looks correct.
>
> Alistair
>
> > /* Second stage lookup */
> > im_address = pa;
> >
> > --
> > 2.19.1
> >
> >
> >
