arm: cpu_switch() has bug?

Hi,

I have doubt if cpu_switch() of arm has a bug.

In swtch.S:L.334, if newtd->td_pcb (this is in stack pointer for
kernel) has an address accessed first for the old(current) thread,
data_abort_fault may occur.

When data_abort_fault occurs, data_abort_handler() tries to solve this
address from kernel_map. In this time, curthread and curpcb are
already updated in swtch.S:L.223-231. As this result,
data_abort_handler() will occur data_abort_fault in trap.c:L.301, again.

When I check, in other CPUs, after updating the root pointer of MMU,
curthread and curpcb are updated.

Would you please check this?

Thanks,
 Kohji Okuno


<<arm/arm/swtch.S:>>

215	ENTRY(cpu_switch)
216	        stmfd   sp!, {r4-r7, lr}
217	        mov     r6, r2 /* Save the mutex */
218	
219	.Lswitch_resume:
220	        /* rem: r0 = old lwp */
221	        /* rem: interrupts are disabled */
222	
223	        /* Process is now on a processor. */
224	        /* We have a new curthread now so make a note it */
225	        GET_CURTHREAD_PTR(r7)
226	        str     r1, [r7]
227	
228	        /* Hook in a new pcb */
229	        GET_PCPU(r7)
230	        ldr     r2, [r1, #TD_PCB]
231	        str     r2, [r7, #PC_CURPCB]
232	
233	        /* rem: r1 = new process */
234	        /* rem: interrupts are enabled */

 ==== SNIP ====

298	        /* rem: r2 = old PCB */
299	        /* rem: r9 = new PCB */
300	        /* rem: interrupts are enabled */
301	
302	#ifdef ARM_VFP_SUPPORT
303	        /*
304	         * vfp_store will clear pcpu->pc_vfpcthread, save 
305	         * registers and state, and modify the control as needed.
306	         * a future exception will bounce the backup settings in the fp unit.
307	         * XXX vfp_store can't change r4
308	         */
309	        GET_PCPU(r7)
310	        ldr     r8, [r7, #(PC_VFPCTHREAD)]
311	        cmp     r4, r8                          /* old thread used vfp? */
312	        bne     1f                              /* no, don't save */
313	        cmp     r1, r4                          /* same thread ? */
314	        beq     1f                              /* yes, skip vfp store */
315	#ifdef SMP
316	        ldr     r8, [r7, #(PC_CPU)]             /* last used on this cpu? */
317	        ldr     r3, [r2, #(PCB_VFPCPU)]
318	        cmp     r8, r3          /* last cpu to use these registers? */
319	        bne     1f              /* no. these values are stale */
320	#endif
321	        add     r0, r2, #(PCB_VFPSTATE)
322	        bl      _C_LABEL(vfp_store)
323	1:
324	#endif          /* ARM_VFP_SUPPORT */
325	
326	        /* r1 now free! */
327	
328	        /* Third phase : restore saved context */
329	
330	        /* rem: r2 = old PCB */
331	        /* rem: r9 = new PCB */
332	        /* rem: interrupts are enabled */
333	
334	        ldr     r5, [r9, #(PCB_DACR)]           /* r5 = new DACR */
335	        mov     r2, #DOMAIN_CLIENT
336	        cmp     r5, r2, lsl #(PMAP_DOMAIN_KERNEL * 2) /* Sw to kernel thread? */
337	        beq     .Lcs_context_switched        /* Yup. Don't flush cache */
338	        mrc     p15, 0, r0, c3, c0, 0           /* r0 = old DACR */


<<arm/arm/trap.c>>

224	void
225	data_abort_handler(trapframe_t *tf)
226	{
227	        struct vm_map *map;
228	        struct pcb *pcb;
229	        struct thread *td;
230	        u_int user, far, fsr;
231	        vm_prot_t ftype;
232	        void *onfault;
233	        vm_offset_t va;
234	        int error = 0;
235	        struct ksig ksig;
236	        struct proc *p;
237	        
238	
239	        /* Grab FAR/FSR before enabling interrupts */
240	        far = cpu_faultaddress();
241	        fsr = cpu_faultstatus();
242	#if 0
243	        printf("data abort: %p (from %p %p)\n", (void*)far, (void*)tf->tf_pc,
244	            (void*)tf->tf_svc_lr);
245	#endif
246	
247	        /* Update vmmeter statistics */
248	#if 0
249	        vmexp.traps++;
250	#endif
251	
252	        td = curthread;
253	        p = td->td_proc;
254	
255	        PCPU_INC(cnt.v_trap);
256	        /* Data abort came from user mode? */
257	        user = TRAP_USERMODE(tf);
258	
259	        if (user) {
260	                td->td_pticks = 0;
261	                td->td_frame = tf;              
262	                if (td->td_ucred != td->td_proc->p_ucred)
263	                        cred_update_thread(td);
264	                
265	        }
266	        /* Grab the current pcb */
267	        pcb = td->td_pcb;

    ==== SNIP ====

299	
300	        /* fusubailout is used by [fs]uswintr to avoid page faulting */
301	        if (__predict_false(pcb->pcb_onfault == fusubailout)) {
302	                tf->tf_r0 = EFAULT;
303	                tf->tf_pc = (register_t)(intptr_t) pcb->pcb_onfault;
304	                return;
305	        }