On Sun, Mar 26, 2023 at 2:03 AM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Move the code that never loops outside of the loop.
> Unchain the if-return-else statements.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/cpu_helper.c | 234 +++++++++++++++++++++-----------------
> 1 file changed, 127 insertions(+), 107 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 00f70a3dd5..ce12dcec1d 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -879,6 +879,8 @@ static int get_physical_address(CPURISCVState *env,
> hwaddr *physical,
> }
>
> int ptshift = (levels - 1) * ptidxbits;
> + target_ulong pte;
> + hwaddr pte_addr;
> int i;
>
> #if !TCG_OVERSIZED_GUEST
> @@ -895,7 +897,6 @@ restart:
> }
>
> /* check that physical address of PTE is legal */
> - hwaddr pte_addr;
>
> if (two_stage && first_stage) {
> int vbase_prot;
> @@ -927,7 +928,6 @@ restart:
> return TRANSLATE_PMP_FAIL;
> }
>
> - target_ulong pte;
> if (riscv_cpu_mxl(env) == MXL_RV32) {
> pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> } else {
> @@ -952,120 +952,140 @@ restart:
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> return TRANSLATE_FAIL;
> - } else if (!pbmte && (pte & PTE_PBMT)) {
> + }
> + if (pte & (PTE_R | PTE_W | PTE_X)) {
> + goto leaf;
> + }
> +
> + /* Inner PTE, continue walking */
> + if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> return TRANSLATE_FAIL;
> - } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> - /* Inner PTE, continue walking */
> - if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> - return TRANSLATE_FAIL;
> - }
> - base = ppn << PGSHIFT;
> - } else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
> - /* Reserved leaf PTE flags: PTE_W */
> - return TRANSLATE_FAIL;
> - } else if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
> - /* Reserved leaf PTE flags: PTE_W + PTE_X */
> - return TRANSLATE_FAIL;
> - } else if ((pte & PTE_U) && ((mode != PRV_U) &&
> - (!sum || access_type == MMU_INST_FETCH))) {
> - /* User PTE flags when not U mode and mstatus.SUM is not set,
> - or the access type is an instruction fetch */
> - return TRANSLATE_FAIL;
> - } else if (!(pte & PTE_U) && (mode != PRV_S)) {
> - /* Supervisor PTE flags when not S mode */
> - return TRANSLATE_FAIL;
> - } else if (ppn & ((1ULL << ptshift) - 1)) {
> - /* Misaligned PPN */
> - return TRANSLATE_FAIL;
> - } else if (access_type == MMU_DATA_LOAD && !((pte & PTE_R) ||
> - ((pte & PTE_X) && mxr))) {
> - /* Read access check failed */
> - return TRANSLATE_FAIL;
> - } else if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
> - /* Write access check failed */
> - return TRANSLATE_FAIL;
> - } else if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
> - /* Fetch access check failed */
> - return TRANSLATE_FAIL;
> - } else {
> - /* if necessary, set accessed and dirty bits. */
> - target_ulong updated_pte = pte | PTE_A |
> + }
> + base = ppn << PGSHIFT;
> + }
> +
> + /* No leaf pte at any translation level. */
> + return TRANSLATE_FAIL;
> +
> + leaf:
> + if (ppn & ((1ULL << ptshift) - 1)) {
> + /* Misaligned PPN */
> + return TRANSLATE_FAIL;
> + }
> + if (!pbmte && (pte & PTE_PBMT)) {
> + /* Reserved without Svpbmt. */
> + return TRANSLATE_FAIL;
> + }
> + if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
> + /* Reserved leaf PTE flags: PTE_W */
> + return TRANSLATE_FAIL;
> + }
> + if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
> + /* Reserved leaf PTE flags: PTE_W + PTE_X */
> + return TRANSLATE_FAIL;
> + }
> + if ((pte & PTE_U) &&
> + ((mode != PRV_U) && (!sum || access_type == MMU_INST_FETCH))) {
> + /*
> + * User PTE flags when not U mode and mstatus.SUM is not set,
> + * or the access type is an instruction fetch.
> + */
> + return TRANSLATE_FAIL;
> + }
> + if (!(pte & PTE_U) && (mode != PRV_S)) {
> + /* Supervisor PTE flags when not S mode */
> + return TRANSLATE_FAIL;
> + }
> + if (access_type == MMU_DATA_LOAD &&
> + !((pte & PTE_R) || ((pte & PTE_X) && mxr))) {
> + /* Read access check failed */
> + return TRANSLATE_FAIL;
> + }
> + if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
> + /* Write access check failed */
> + return TRANSLATE_FAIL;
> + }
> + if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
> + /* Fetch access check failed */
> + return TRANSLATE_FAIL;
> + }
> +
> + /* If necessary, set accessed and dirty bits. */
> + target_ulong updated_pte = pte | PTE_A |
> (access_type == MMU_DATA_STORE ? PTE_D : 0);
>
> - /* Page table updates need to be atomic with MTTCG enabled */
> - if (updated_pte != pte) {
> - if (!hade) {
> - return TRANSLATE_FAIL;
> - }
> + /* Page table updates need to be atomic with MTTCG enabled */
> + if (updated_pte != pte) {
> + if (!hade) {
> + return TRANSLATE_FAIL;
> + }
>
> - /*
> - * - if accessed or dirty bits need updating, and the PTE is
> - * in RAM, then we do so atomically with a compare and
> swap.
> - * - if the PTE is in IO space or ROM, then it can't be
> updated
> - * and we return TRANSLATE_FAIL.
> - * - if the PTE changed by the time we went to update it,
> then
> - * it is no longer valid and we must re-walk the page
> table.
> - */
> - MemoryRegion *mr;
> - hwaddr l = sizeof(target_ulong), addr1;
> - mr = address_space_translate(cs->as, pte_addr,
> - &addr1, &l, false, MEMTXATTRS_UNSPECIFIED);
> - if (memory_region_is_ram(mr)) {
> - target_ulong *pte_pa =
> - qemu_map_ram_ptr(mr->ram_block, addr1);
> + /*
> + * - if accessed or dirty bits need updating, and the PTE is
> + * in RAM, then we do so atomically with a compare and swap.
> + * - if the PTE is in IO space or ROM, then it can't be updated
> + * and we return TRANSLATE_FAIL.
> + * - if the PTE changed by the time we went to update it, then
> + * it is no longer valid and we must re-walk the page table.
> + */
> + MemoryRegion *mr;
> + hwaddr l = sizeof(target_ulong), addr1;
> + mr = address_space_translate(cs->as, pte_addr, &addr1, &l, false,
> + MEMTXATTRS_UNSPECIFIED);
> + if (memory_region_is_ram(mr)) {
> + target_ulong *pte_pa = qemu_map_ram_ptr(mr->ram_block, addr1);
> #if TCG_OVERSIZED_GUEST
> - /* MTTCG is not enabled on oversized TCG guests so
> - * page table updates do not need to be atomic */
> - *pte_pa = pte = updated_pte;
> + /*
> + * MTTCG is not enabled on oversized TCG guests so
> + * page table updates do not need to be atomic.
> + */
> + *pte_pa = pte = updated_pte;
> #else
> - target_ulong old_pte =
> - qatomic_cmpxchg(pte_pa, pte, updated_pte);
> - if (old_pte != pte) {
> - goto restart;
> - } else {
> - pte = updated_pte;
> - }
> + target_ulong old_pte = qatomic_cmpxchg(pte_pa, pte, updated_pte);
> + if (old_pte != pte) {
> + goto restart;
> + }
> + pte = updated_pte;
> #endif
> - } else {
> - /* misconfigured PTE in ROM (AD bits are not preset) or
> - * PTE is in IO space and can't be updated atomically */
> - return TRANSLATE_FAIL;
> - }
> - }
> -
> - /* for superpage mappings, make a fake leaf PTE for the TLB's
> - benefit. */
> - target_ulong vpn = addr >> PGSHIFT;
> -
> - if (cpu->cfg.ext_svnapot && (pte & PTE_N)) {
> - napot_bits = ctzl(ppn) + 1;
> - if ((i != (levels - 1)) || (napot_bits != 4)) {
> - return TRANSLATE_FAIL;
> - }
> - }
> -
> - napot_mask = (1 << napot_bits) - 1;
> - *physical = (((ppn & ~napot_mask) | (vpn & napot_mask) |
> - (vpn & (((target_ulong)1 << ptshift) - 1))
> - ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
> -
> - /* set permissions on the TLB entry */
> - if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> - *prot |= PAGE_READ;
> - }
> - if ((pte & PTE_X)) {
> - *prot |= PAGE_EXEC;
> - }
> - /* add write permission on stores or if the page is already
> dirty,
> - so that we TLB miss on later writes to update the dirty bit */
> - if ((pte & PTE_W) &&
> - (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
> - *prot |= PAGE_WRITE;
> - }
> - return TRANSLATE_SUCCESS;
> + } else {
> + /*
> + * Misconfigured PTE in ROM (AD bits are not preset) or
> + * PTE is in IO space and can't be updated atomically.
> + */
> + return TRANSLATE_FAIL;
> }
> }
> - return TRANSLATE_FAIL;
> +
> + /* For superpage mappings, make a fake leaf PTE for the TLB's benefit. */
> + target_ulong vpn = addr >> PGSHIFT;
> +
> + if (cpu->cfg.ext_svnapot && (pte & PTE_N)) {
> + napot_bits = ctzl(ppn) + 1;
> + if ((i != (levels - 1)) || (napot_bits != 4)) {
> + return TRANSLATE_FAIL;
> + }
> + }
> +
> + napot_mask = (1 << napot_bits) - 1;
> + *physical = (((ppn & ~napot_mask) | (vpn & napot_mask) |
> + (vpn & (((target_ulong)1 << ptshift) - 1))
> + ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
> +
> + /* set permissions on the TLB entry */
> + if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> + *prot |= PAGE_READ;
> + }
> + if (pte & PTE_X) {
> + *prot |= PAGE_EXEC;
> + }
> + /*
> + * Add write permission on stores or if the page is already dirty,
> + * so that we TLB miss on later writes to update the dirty bit.
> + */
> + if ((pte & PTE_W) && (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
> + *prot |= PAGE_WRITE;
> + }
> + return TRANSLATE_SUCCESS;
> }
>
> static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> --
> 2.34.1
>
>
