Re: ARM1136 panic on arm32_sync_icache()
Matt Thomas <matt <at> 3am-software.com>
2007-11-09 01:53:37 GMT
On Nov 8, 2007, at 5:22 PM, Todd Poynor wrote:
> 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
>
> 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
> <at> <at> -130,6 +130,7 <at> <at> 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
> <at> <at> -212,5 +212,39 <at> <at> _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}
stmfd sp!, {r4, 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
For a call, the stack should be 8-byte aligned.
> + ldr pc, [r2, #CF_ICACHE_SYNC_RANGE]
> + mov r0, #0x00000000
str r0, [r4, #PCB_ONFAULT]
> + ldmfd sp!, {pc}
ldmfd sp!, {r4, pc}
> +
> +.Licache_sync_fault:
> + mov r1, #0x00000000
> + str r1, [r4, #PCB_ONFAULT]
> + ldmfd sp!, {pc}
ldmfd sp!, {r4, pc}
I think these r1's should be r0.
> /* 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
> <at> <at> -70,10 +70,8 <at> <at> 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);
retval should stay 0. 0 should be returned.
> }
>
> 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
> <at> <at> -211,6 +211,7 <at> <at> 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);
>
RSS Feed