Re: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address

[Alistair Francis]

On Mon, Aug 24, 2020 at 1:43 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>

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?

> +                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
>
>
>