On Fri, 2026-05-22 at 14:25 -0300, 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: Alistair Francis <[email protected]> Alistair > --- > 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);
