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

[Alistair Francis <alistair23@gmail.com>]

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