On Wed, Oct 14, 2020 at 6:59 PM Jiangyifei <jiangyi...@huawei.com> wrote: > > > > -----Original Message----- > > From: Richard Henderson [mailto:richard.hender...@linaro.org] > > Sent: Thursday, October 15, 2020 4:22 AM > > To: Jiangyifei <jiangyi...@huawei.com>; qemu-devel@nongnu.org; > > qemu-ri...@nongnu.org > > Cc: pal...@dabbelt.com; alistair.fran...@wdc.com; > > sag...@eecs.berkeley.edu; kbast...@mail.uni-paderborn.de; Zhangxiaofeng > > (F) <victor.zhangxiaof...@huawei.com>; Wubin (H) <wu.wu...@huawei.com>; > > Zhanghailiang <zhang.zhanghaili...@huawei.com>; dengkai (A) > > <dengk...@huawei.com>; yinyipeng <yinyipe...@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~
