[Qemu-devel] [PATCH 1/5] target-arm: convert check_ap to get_rw_prot

[Andrew Jones]

Instead of mixing access permission checking with access permissions
to page protection flags translation, just do the translation, and
leave it to the caller to check the protection flags against the access
type. As this function only considers READ/WRITE, not EXEC, then name
it accordingly.

Signed-off-by: Andrew Jones <address@hidden>
---
 target-arm/helper.c | 47 +++++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1a1a00577e780..610f305c4d661 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4692,34 +4692,23 @@ static inline bool regime_is_user(CPUARMState *env, 
ARMMMUIdx mmu_idx)
     }
 }
 
-/* Check section/page access permissions.
-   Returns the page protection flags, or zero if the access is not
-   permitted.  */
-static inline int check_ap(CPUARMState *env, ARMMMUIdx mmu_idx,
-                           int ap, int domain_prot,
-                           int access_type)
-{
-    int prot_ro;
+/* Translate section/page access permissions to page
+ * R/W protection flags
+ */
+static inline int get_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx,
+                              int ap, int domain_prot)
+{
     bool is_user = regime_is_user(env, mmu_idx);
 
     if (domain_prot == 3) {
         return PAGE_READ | PAGE_WRITE;
     }
 
-    if (access_type == 1) {
-        prot_ro = 0;
-    } else {
-        prot_ro = PAGE_READ;
-    }
-
     switch (ap) {
     case 0:
         if (arm_feature(env, ARM_FEATURE_V7)) {
             return 0;
         }
-        if (access_type == 1) {
-            return 0;
-        }
         switch (regime_sctlr(env, mmu_idx) & (SCTLR_S | SCTLR_R)) {
         case SCTLR_S:
             return is_user ? 0 : PAGE_READ;
@@ -4732,7 +4721,7 @@ static inline int check_ap(CPUARMState *env, ARMMMUIdx 
mmu_idx,
         return is_user ? 0 : PAGE_READ | PAGE_WRITE;
     case 2:
         if (is_user) {
-            return prot_ro;
+            return PAGE_READ;
         } else {
             return PAGE_READ | PAGE_WRITE;
         }
@@ -4741,16 +4730,16 @@ static inline int check_ap(CPUARMState *env, ARMMMUIdx 
mmu_idx,
     case 4: /* Reserved.  */
         return 0;
     case 5:
-        return is_user ? 0 : prot_ro;
+        return is_user ? 0 : PAGE_READ;
     case 6:
-        return prot_ro;
+        return PAGE_READ;
     case 7:
         if (!arm_feature(env, ARM_FEATURE_V6K)) {
             return 0;
         }
-        return prot_ro;
+        return PAGE_READ;
     default:
-        abort();
+        g_assert_not_reached();
     }
 }
 
@@ -4872,12 +4861,12 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t 
address, int access_type,
         }
         code = 15;
     }
-    *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type);
-    if (!*prot) {
+    *prot = get_rw_prot(env, mmu_idx, ap, domain_prot);
+    *prot |= *prot ? PAGE_EXEC : 0;
+    if (!(*prot & (1 << access_type))) {
         /* Access permission fault.  */
         goto do_fault;
     }
-    *prot |= PAGE_EXEC;
     *phys_ptr = phys_addr;
     return 0;
 do_fault:
@@ -4993,14 +4982,12 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t 
address, int access_type,
             code = (code == 15) ? 6 : 3;
             goto do_fault;
         }
-        *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type);
-        if (!*prot) {
+        *prot = get_rw_prot(env, mmu_idx, ap, domain_prot);
+        *prot |= *prot && !xn ? PAGE_EXEC : 0;
+        if (!(*prot & (1 << access_type))) {
             /* Access permission fault.  */
             goto do_fault;
         }
-        if (!xn) {
-            *prot |= PAGE_EXEC;
-        }
     }
     *phys_ptr = phys_addr;
     return 0;
-- 
1.9.3