[Qemu-devel] [PATCH] SH: Fix TLB/MMU detection of code accesses.

[Vladimir Prus]

Current SH4 TLB emulation does strange thing about code accesses. For
code accesses, tlb_fill will have 2 passed for is_write parameter.
In SH case, tlb_fill calls cpu_sh4_handle_mmu_fault, which treats
data read and code read identically -- that is, the same value is
passed for the 'rw' parameter for get_physical_address. The latter
function then calls get_mmu_address -- which tries to figure if we're
doing code address or not -- by comparing env->pc with the address
being accessed. The code comment say "Hack", and in fact this sometimes
gets wrong results, which causes random crashes in the simulated program.

This patch fixes this, by stopping cpu_sh4_handle_mmu_fault from
erasing the data read/code read distinction.

- Volodya

        * target-sh4/helper.c (get_mmu_address):  Don't use
        hacks to determine if access is to code.  Just check 'rw'.
        (get_physical_address): Adjust to the fact that 'rw' can have
        3 values.
        (cpu_sh4_handle_mmu_fault): Don't treat code and data reads the
        same.
---
 target-sh4/helper.c |   47 ++++++++++++++---------------------------------
 1 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/target-sh4/helper.c b/target-sh4/helper.c
index f523f81..b1b02dd 100644
--- a/target-sh4/helper.c
+++ b/target-sh4/helper.c
@@ -369,14 +369,12 @@ static int get_mmu_address(CPUState * env, target_ulong * 
physical,
                           int *prot, target_ulong address,
                           int rw, int access_type)
 {
-    int use_asid, is_code, n;
+    int use_asid, n;
     tlb_t *matching = NULL;
 
-    use_asid = (env->mmucr & MMUCR_SV) == 0 || (env->sr & SR_MD) == 0;
-    is_code = env->pc == address;      /* Hack */
+    use_asid = (env->mmucr & MMUCR_SV) == 0 || (env->sr & SR_MD) == 0;    
 
-    /* Use a hack to find if this is an instruction or data access */
-    if (env->pc == address && !(rw & PAGE_WRITE)) {
+    if (rw == 2) {
        n = find_itlb_entry(env, address, use_asid, 1);
        if (n >= 0) {
            matching = &env->itlb[n];
@@ -392,13 +390,13 @@ static int get_mmu_address(CPUState * env, target_ulong * 
physical,
            switch ((matching->pr << 1) | ((env->sr & SR_MD) ? 1 : 0)) {
            case 0:             /* 000 */
            case 2:             /* 010 */
-               n = (rw & PAGE_WRITE) ? MMU_DTLB_VIOLATION_WRITE :
+             n = (rw == 1) ? MMU_DTLB_VIOLATION_WRITE :
                    MMU_DTLB_VIOLATION_READ;
                break;
            case 1:             /* 001 */
            case 4:             /* 100 */
            case 5:             /* 101 */
-               if (rw & PAGE_WRITE)
+               if (rw == 1)
                    n = MMU_DTLB_VIOLATION_WRITE;
                else
                    *prot = PAGE_READ;
@@ -406,11 +404,11 @@ static int get_mmu_address(CPUState * env, target_ulong * 
physical,
            case 3:             /* 011 */
            case 6:             /* 110 */
            case 7:             /* 111 */
-               *prot = rw & (PAGE_READ | PAGE_WRITE);
+               *prot = (rw == 1)? PAGE_WRITE : PAGE_READ;
                break;
            }
        } else if (n == MMU_DTLB_MISS) {
-           n = (rw & PAGE_WRITE) ? MMU_DTLB_MISS_WRITE :
+           n = (rw == 1) ? MMU_DTLB_MISS_WRITE :
                MMU_DTLB_MISS_READ;
        }
     }
@@ -436,8 +434,12 @@ int get_physical_address(CPUState * env, target_ulong * 
physical,
            && (address < 0xe0000000 || address > 0xe4000000)) {
            /* Unauthorized access in user mode (only store queues are 
available) */
            fprintf(stderr, "Unauthorized access\n");
-           return (rw & PAGE_WRITE) ? MMU_DTLB_MISS_WRITE :
-               MMU_DTLB_MISS_READ;
+           if (rw == 0)
+               return MMU_DTLB_MISS_READ;
+           else if (rw == 1)
+               return MMU_DTLB_MISS_WRITE;
+           else
+               return MMU_ITLB_MISS;
        }
        if (address >= 0x80000000 && address < 0xc0000000) {
            /* Mask upper 3 bits for P1 and P2 areas */
@@ -475,27 +477,6 @@ int cpu_sh4_handle_mmu_fault(CPUState * env, target_ulong 
address, int rw,
     target_ulong physical, page_offset, page_size;
     int prot, ret, access_type;
 
-    switch (rw) {
-    case 0:
-        rw = PAGE_READ;
-        break;
-    case 1:
-        rw = PAGE_WRITE;
-        break;
-    case 2: /* READ_ACCESS_TYPE == 2 defined in softmmu_template.h */
-        rw = PAGE_READ;
-        break;
-    default:
-        /* fatal error */
-        assert(0);
-    }
-
-    /* XXXXX */
-#if 0
-    fprintf(stderr, "%s pc %08x ad %08x rw %d mmu_idx %d smmu %d\n",
-           __func__, env->pc, address, rw, mmu_idx, is_softmmu);
-#endif
-
     access_type = ACCESS_INT;
     ret =
        get_physical_address(env, &physical, &prot, address, rw,
@@ -547,7 +528,7 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState * env, 
target_ulong addr)
     target_ulong physical;
     int prot;
 
-    get_physical_address(env, &physical, &prot, addr, PAGE_READ, 0);
+    get_physical_address(env, &physical, &prot, addr, 0, 0);
     return physical;
 }
 
-- 
1.5.3.5