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

[Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp>]

Vladimir Prus wrote:
> 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.

I found that this patch still can be applied to the trunk HEAD, rev 5639,
and it really stabilizes SH-Linux system emulation : some segmentation
fault disappears.  Thanks.


> @@ -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;
>  	}
>      }

I think one more replace needed in get_mmu_address(), like following.
Isn't it?

     if (n >= 0) {
 	*physical = ((matching->ppn << 10) & ~(matching->size - 1)) |
 	    (address & (matching->size - 1));
-	if ((rw & PAGE_WRITE) & !matching->d)
+	if ((rw == 1) & !matching->d)
 	    n = MMU_DTLB_INITIAL_WRITE;
 	else
 	    n = MMU_OK;


> @@ -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;

To be more precise, these cases should not invoke TLB miss error exceptions
but address error exceptions, whose exception codes are 0x0e0, or 0x100, I guess.
It might be another small patch.


Regards,
Shin-ichiro KAWASAKI