| Message ID | 20230325105429.1142530-21-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/riscv: MSTATUS_SUM + cleanups | expand |
On Sun, Mar 26, 2023 at 2:03 AM Richard Henderson <richard.henderson@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.henderson@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@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 > >
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,
Move the code that never loops outside of the loop. Unchain the if-return-else statements. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/riscv/cpu_helper.c | 234 +++++++++++++++++++++----------------- 1 file changed, 127 insertions(+), 107 deletions(-)