qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 22/28] target/riscv: Allow specifying MMU stage


From: Palmer Dabbelt
Subject: Re: [PATCH v1 22/28] target/riscv: Allow specifying MMU stage
Date: Wed, 16 Oct 2019 12:02:38 -0700 (PDT)

On Mon, 07 Oct 2019 11:05:33 PDT (-0700), address@hidden wrote:
On Thu, Oct 3, 2019 at 8:53 AM Palmer Dabbelt <address@hidden> wrote:

On Fri, 23 Aug 2019 16:38:47 PDT (-0700), Alistair Francis wrote:
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>  target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 098873c83e..9aa6906acd 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, 
target_ulong newpriv)
>   *
>   * Adapted from Spike's mmu_t::translate and mmu_t::walk
>   *
> + * @env: CPURISCVState
> + * @physical: This will be set to the calculated physical address
> + * @prot: The returned protection attributes
> + * @addr: The virtual address to be translated
> + * @access_type: The type of MMU access
> + * @mmu_idx: Indicates current privilege level
> + * @first_stage: Are we in first stage translation?
> + *               Second stage is used for hypervisor guest translation
>   */
>  static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>                                  int *prot, target_ulong addr,
> -                                int access_type, int mmu_idx)
> +                                int access_type, int mmu_idx,
> +                                bool first_stage)
>  {
>      /* NOTE: the env->pc value visible here will not be
>       * correct, but the value visible to the exception handler
> @@ -518,13 +527,23 @@ restart:
>  }
>
>  static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> -                                MMUAccessType access_type, bool 
pmp_violation)
> +                                MMUAccessType access_type, bool 
pmp_violation,
> +                                bool first_stage)
>  {
>      CPUState *cs = env_cpu(env);
> -    int page_fault_exceptions =
> -        (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> -        get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> -        !pmp_violation;
> +    int page_fault_exceptions;
> +    if (first_stage) {
> +        page_fault_exceptions =
> +            (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> +            get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> +            !pmp_violation;
> +            riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);

It might just be email, but the indentation looks wrong here.

Yep, fixed.


> +    } else {
> +        page_fault_exceptions =
> +            get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
> +            !pmp_violation;
> +            riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> +    }
>      switch (access_type) {
>      case MMU_INST_FETCH:
>          cs->exception_index = page_fault_exceptions ?
> @@ -551,7 +570,8 @@ 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(&cpu->env, &phys_addr, &prot, addr, 0, 
mmu_idx)) {
> +    if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
> +                             true)) {
>          return -1;
>      }
>      return phys_addr;
> @@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
>      qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
>                    __func__, address, access_type, mmu_idx);
>
> -    ret = get_physical_address(env, &pa, &prot, address, access_type, 
mmu_idx);
> +    ret = get_physical_address(env, &pa, &prot, address, access_type, 
mmu_idx,
> +                               true);
>
>      if (mode == PRV_M && access_type != MMU_INST_FETCH) {
>          if (get_field(*env->mstatus, MSTATUS_MPRV)) {
> @@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
>      } else if (probe) {
>          return false;
>      } else {
> -        raise_mmu_exception(env, address, access_type, pmp_violation);
> +        raise_mmu_exception(env, address, access_type, pmp_violation, true);
>          riscv_raise_exception(env, cs->exception_index, retaddr);
>      }
>  #else

I don't think it makes sense to split off these two (23 and 24, that add the
argument) out from the implementation.

The goal was just to make it easier to review. If you want them
combined I can easily combine them.

It's making it harder to read on my end :)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]