Subject: Re: ARM1136 panic on arm32_sync_icache()
To: Matt Thomas <matt@3am-software.com>
From: Todd Poynor <tpoynor@danger.com>
List: port-arm
Date: 11/08/2007 17:22:23
This is a multi-part message in MIME format.

--------------000503000501030001080800
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Matt Thomas wrote:
> On Nov 7, 2007, at 4:58 PM, Todd Poynor wrote:
...
>> The ARMv6 Virtually Indexed Physically Tagged cache maintenance system 
>> control coprocessor instructions that invalidate lines based on 
>> Modified Virtual Addresses throw Data Abort exceptions for virtual 
>> addresses not mapped in the PTE. ...
> 
> I prefer the of onfault since this error shouldn't normally happen.

Here's a patch against -current (but only tested on -4.0rc3) that adds a 
"safe" wrapper around cpu_icache_sync_range via pcb_onfault and returns 
zero or EFAULT to the caller.  Some notes on questionable stuff I 
wrestled with follow.

The wrapper is added to arch/arm/arm32/locore.S as a handy place to put 
it within the arm32/ family.  This might not be the best place for a 
cpufunc thing, but it needs some assym.h symbols only generated for 
arm32.  The protototype is added to arch/arm/include/cpufunc.h, although 
it is arm32-only.  (It would be nice to have fault catchers in C and do 
this in arch/arm/arm32/sys_machdep.c with a setjmp-style call.)

The wrapper knows the called routine doesn't mess with sp or r4.


-- 
Todd


--------------000503000501030001080800
Content-Type: text/x-patch;
 name="arm1136_icache_sync_range-crash.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="arm1136_icache_sync_range-crash.patch"

Index: netbsd-current-11082007/src/sys/arch/arm/arm32/genassym.cf
===================================================================
--- netbsd-current-11082007.orig/src/sys/arch/arm/arm32/genassym.cf
+++ netbsd-current-11082007/src/sys/arch/arm/arm32/genassym.cf
@@ -130,6 +130,7 @@ define	TRAPFRAMESIZE		sizeof(struct trap
 
 define	CF_IDCACHE_WBINV_ALL	offsetof(struct cpu_functions, cf_idcache_wbinv_all)
 define	CF_DCACHE_WB_RANGE	offsetof(struct cpu_functions, cf_dcache_wb_range)
+define  CF_ICACHE_SYNC_RANGE    offsetof(struct cpu_functions, cf_icache_sync_range)
 define	CF_TLB_FLUSHID_SE	offsetof(struct cpu_functions, cf_tlb_flushID_SE)
 define	CF_CONTEXT_SWITCH	offsetof(struct cpu_functions, cf_context_switch)
 define	CF_SLEEP		offsetof(struct cpu_functions, cf_sleep)
Index: netbsd-current-11082007/src/sys/arch/arm/arm32/locore.S
===================================================================
--- netbsd-current-11082007.orig/src/sys/arch/arm/arm32/locore.S
+++ netbsd-current-11082007/src/sys/arch/arm/arm32/locore.S
@@ -212,5 +212,39 @@ _C_LABEL(esym):	.word	_C_LABEL(end)
 ENTRY_NP(abort)
 	b	_C_LABEL(abort)
 
+#ifdef MULTIPROCESSOR
+.Lcpu_info:
+        .word   _C_LABEL(cpu_info)
+#else
+.Lcurpcb:
+        .word   _C_LABEL(curpcb)
+#endif
+
+ENTRY(cpufunc_safe_icache_sync_range)
+	stmfd	sp!, {lr}
+#ifdef MULTIPROCESSOR
+	/* XXX Probably not appropriate for non-Hydra SMPs */
+	stmfd   sp!, {r0-r1}
+	bl      _C_LABEL(cpu_number)
+	ldr     r4, .Lcpu_info
+	ldr     r4, [r4, r0, lsl #2]
+	ldr     r4, [r4, #CI_CURPCB]
+	ldmfd   sp!, {r0-r1}
+#else
+	ldr     r4, .Lcurpcb
+	ldr     r4, [r4]
+#endif
+	adr     r3, .Licache_sync_fault
+	str     r3, [r4, #PCB_ONFAULT]
+	ldr	r2, .Lcpufuncs
+	mov	lr, pc
+	ldr	pc, [r2, #CF_ICACHE_SYNC_RANGE]
+	mov     r0, #0x00000000
+	ldmfd	sp!, {pc}
+
+.Licache_sync_fault:
+	mov     r1, #0x00000000
+	str     r1, [r4, #PCB_ONFAULT]
+	ldmfd	sp!, {pc}
 
 /* End of locore.S */
Index: netbsd-current-11082007/src/sys/arch/arm/arm32/sys_machdep.c
===================================================================
--- netbsd-current-11082007.orig/src/sys/arch/arm/arm32/sys_machdep.c
+++ netbsd-current-11082007/src/sys/arch/arm/arm32/sys_machdep.c
@@ -70,10 +70,8 @@ arm32_sync_icache(p, args, retval)
 	if ((error = copyin(args, &ua, sizeof(ua))) != 0)
 		return (error);
 
-	cpu_icache_sync_range(ua.addr, ua.len);
-
-	*retval = 0;
-	return(0);
+	*retval = cpufunc_safe_icache_sync_range(ua.addr, ua.len);
+	return(*retval);
 }
 
 static int
Index: netbsd-current-11082007/src/sys/arch/arm/include/cpufunc.h
===================================================================
--- netbsd-current-11082007.orig/src/sys/arch/arm/include/cpufunc.h
+++ netbsd-current-11082007/src/sys/arch/arm/include/cpufunc.h
@@ -211,6 +211,7 @@ u_int	cpufunc_control		(u_int, u_int);
 void	cpufunc_domains		(u_int);
 u_int	cpufunc_faultstatus	(void);
 u_int	cpufunc_faultaddress	(void);
+int	cpufunc_safe_icache_sync_range(vaddr_t, vsize_t);
 
 #ifdef CPU_ARM2
 u_int	arm2_id			(void);


--------------000503000501030001080800--