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(-)
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.1On 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
>
>
© 2016 - 2026 Red Hat, Inc.