On Fri, May 22, 2026 at 02:25:02PM +0800, Daniel Henrique Barboza wrote:
> We're not doing anything special w.r.t PMA (Physical Memory Access)
> related faults, handling them like regular faults that will eventually
> turn to be regular page faults.
> 
> Turns out we can't do that.  Priv spec section "Virtual Address
> Translation Process" mentions:
> 
> "If a store to the PTE at address a+va.vpn[i]×PTESIZE would violate a
> PMA or PMP check, raise an access-fault exception corresponding to the
> original access type."
> 
> This means that we should handle PMA violations with access faults, like
> we're already doing with PMP.  One clear code path where we should throw
> a PMA failure, exposed by [1], is the error return from
> address_space_ld* call.
> 
> There's a separated issue with the error code being returned by them (it
> always return DECODE_ERROR even with 'rejected' reads) that we're going
> to work around it by assuming that we did a good job with the PTE
> address sanitization beforehand, and interpret that the error here is
> related to PMA.  This is of course not ideal but fixing this QEMU API is
> out of scope for this work.
> 
> All this said, we'll set the new pmp_pma_violation flag when we have
> either a PMP or a PMA fault, and everything else shall fall into place.
> 
> [1] https://gitlab.com/qemu-project/qemu/-/work_items/3502
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3502
> Signed-off-by: Daniel Henrique Barboza <[email protected]>
Reviewed-by: Chao Liu <[email protected]>

> ---
>  target/riscv/cpu.h        |  3 ++-
>  target/riscv/cpu_helper.c | 34 +++++++++++++++++++++++++---------
>  2 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7d79c7a5a7..e5e1687177 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -131,7 +131,8 @@ enum {
>      TRANSLATE_SUCCESS,
>      TRANSLATE_FAIL,
>      TRANSLATE_PMP_FAIL,
> -    TRANSLATE_G_STAGE_FAIL
> +    TRANSLATE_G_STAGE_FAIL,
> +    TRANSLATE_PMA_FAIL,
>  };
>  
>  /* Extension context status */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 17305e1bb7..5303d9f7f4 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1424,7 +1424,22 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>          }
>  
>          if (res != MEMTX_OK) {
> -            return TRANSLATE_FAIL;
> +            /*
> +             * The result of address_space_* APIs above does not take into
> +             * consideration reject reads, putting all errors in the same
> +             * cathegory (DECODE_ERROR), although there's a clear
> +             * distinction between a rejected read versus other errors
> +             * (see memory_region_dispatch_read() ->
> +             * memory_region_access_valid()).  This is something that
> +             * we might have to deal with core QEMU logic some other
> +             * day.
> +             *
> +             * For this particular error path, given that we made checks
> +             * w.r.t legal PTE address before calling those APIs, we'll
> +             * assume that anything != MEMTX_OK means a rejected read,
> +             * i.e. a PMA error.
> +             */
> +            return TRANSLATE_PMA_FAIL;
>          }
>  
>          if (riscv_cpu_sxl(env) == MXL_RV32) {
> @@ -1668,7 +1683,8 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>  }
>  
>  static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> -                                MMUAccessType access_type, bool 
> pmp_violation,
> +                                MMUAccessType access_type,
> +                                bool pmp_pma_violation,
>                                  bool first_stage, bool two_stage,
>                                  bool two_stage_indirect)
>  {
> @@ -1676,7 +1692,7 @@ static void raise_mmu_exception(CPURISCVState *env, 
> target_ulong address,
>  
>      switch (access_type) {
>      case MMU_INST_FETCH:
> -        if (pmp_violation) {
> +        if (pmp_pma_violation) {
>              cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
>          } else if (env->virt_enabled && !first_stage) {
>              cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
> @@ -1685,7 +1701,7 @@ static void raise_mmu_exception(CPURISCVState *env, 
> target_ulong address,
>          }
>          break;
>      case MMU_DATA_LOAD:
> -        if (pmp_violation) {
> +        if (pmp_pma_violation) {
>              cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
>          } else if (two_stage && !first_stage) {
>              cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
> @@ -1694,7 +1710,7 @@ static void raise_mmu_exception(CPURISCVState *env, 
> target_ulong address,
>          }
>          break;
>      case MMU_DATA_STORE:
> -        if (pmp_violation) {
> +        if (pmp_pma_violation) {
>              cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
>          } else if (two_stage && !first_stage) {
>              cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
> @@ -1820,7 +1836,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, 
> int size,
>      vaddr im_address;
>      hwaddr pa = 0;
>      int prot, prot2, prot_pmp;
> -    bool pmp_violation = false;
> +    bool pmp_pma_violation = false;
>      bool first_stage_error = true;
>      bool two_stage_lookup = mmuidx_2stage(mmu_idx);
>      bool two_stage_indirect_error = false;
> @@ -1921,8 +1937,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, 
> int size,
>          }
>      }
>  
> -    if (ret == TRANSLATE_PMP_FAIL) {
> -        pmp_violation = true;
> +    if (ret == TRANSLATE_PMP_FAIL || ret == TRANSLATE_PMA_FAIL) {
> +        pmp_pma_violation = true;
>      }
>  
>      if (ret == TRANSLATE_SUCCESS) {
> @@ -1949,7 +1965,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, 
> int size,
>          cpu_check_watchpoint(cs, address, size, MEMTXATTRS_UNSPECIFIED,
>                               wp_access, retaddr);
>  
> -        raise_mmu_exception(env, address, access_type, pmp_violation,
> +        raise_mmu_exception(env, address, access_type, pmp_pma_violation,
>                              first_stage_error, two_stage_lookup,
>                              two_stage_indirect_error);
>          cpu_loop_exit_restore(cs, retaddr);
> -- 
> 2.43.0
> 

Reply via email to