Subject: Re: kernel: supervisor trap asynchronous system trap, code=0
To: YAMAMOTO Takashi <email@example.com>
From: Manuel Bouyer <firstname.lastname@example.org>
Date: 06/30/2007 14:33:27
On Sat, Jun 30, 2007 at 08:09:47PM +0900, YAMAMOTO Takashi wrote:
> > Index: i386/locore.S
> > ===================================================================
> > RCS file: /cvsroot/src/sys/arch/xen/i386/locore.S,v
> > retrieving revision 1.25
> > diff -u -r1.25 locore.S
> > --- i386/locore.S 17 May 2007 14:51:35 -0000 1.25
> > +++ i386/locore.S 28 Jun 2007 13:27:49 -0000
> > @@ -663,6 +663,7 @@
> > * Switch to newlwp's stack.
> > */
> > + CLI(%ebx)
> > movl L_ADDR(%edi),%ebx
> > movl PCB_EBP(%ebx),%ebp
> > movl PCB_ESP(%ebx),%esp
> can you explain why it's necessary?
I didn't find why an interrupt coming in there would cause problem,
but I can confirm that this prevent the KASSERT panic that was being reported.
Also, I notice that the code in netbsd-4 does the process stack swicthing
with interrupts disabled.
> anyway, please don't call ras_lookup with interrupt disabled.
OK. I just wanted to make sure the new context was fully installed before
re-taking interrupts. I didn't notice a C function was being called.
> > @@ -780,9 +789,29 @@
> > call _C_LABEL(trap)
> > addl $4,%esp
> > jmp .Lsyscall_checkast
> > -1: STI(%eax)
> > - CHECK_DEFERRED_SWITCH(%eax)
> > +1: CHECK_DEFERRED_SWITCH(%eax)
> > jnz 9f
> > + STIC(%eax)
> > + jz 14f
> > + call _C_LABEL(stipending)
> > + testl %eax,%eax
> > + jz 14f
> > + /* process pending interrupts */
> > + CLI(%eax)
> > + movl CPUVAR(ILEVEL), %ebx
> > + movl $.Lsyscall_resume, %esi # address to resume loop at
> > +.Lsyscall_resume:
> > + movl %ebx,%eax # get cpl
> > + movl CPUVAR(IUNMASK)(,%eax,4),%eax
> > + andl CPUVAR(IPENDING),%eax # any non-masked bits left?
> > + jz 17f
> > + bsrl %eax,%eax
> > + btrl %eax,CPUVAR(IPENDING)
> > + movl CPUVAR(ISOURCES)(,%eax,4),%eax
> > + jmp *IS_RESUME(%eax)
> > +17: movl %ebx, CPUVAR(ILEVEL) #restore cpl
> > + jmp .Lsyscall_checkast
> > +14:
> > #ifndef DIAGNOSTIC
> > INTRFASTEXIT
> > #else /* DIAGNOSTIC */
> can't these duplicated code be shared?
it could probably, but this would make some more call/return, or jump.
I'm not sure it's worth it.
> i'm not sure why stipending() was needed in the first place,
> in addition to __sti() and do_hypervisor_callback().
__sti() is intended for C code, but it takes an extrat trap and 4 context
switches to process pending interrupts. processing interrupts in the current
context it more efficient.
Manuel Bouyer <email@example.com>
NetBSD: 26 ans d'experience feront toujours la difference