Date: Thu, 08 May 2003 12:53:09 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: Peter Wemm <peter@wemm.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 30777 for review Message-ID: <XFMail.20030508125309.jhb@FreeBSD.org> In-Reply-To: <20030508151522.36B8D2A8AB@canning.wemm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 08-May-2003 Peter Wemm wrote: > John Baldwin wrote: >> >> On 08-May-2003 Peter Wemm wrote: >> > http://perforce.freebsd.org/chv.cgi?CH=30777 >> > >> > Change 30777 by peter@peter_overcee on 2003/05/08 01:21:28 >> > >> > Change the page IDTVEC back to an interrupt gate instead of a trap >> > gate. Otherwise we could preempt and %cr2 could be reused on another >> > process when it faults. >> > >> > Affected files ... >> > >> > .. //depot/projects/hammer/sys/amd64/amd64/machdep.c#12 edit >> > .. //depot/projects/hammer/sys/amd64/amd64/trap.c#6 edit >> > >> > Differences ... >> > >> > ==== //depot/projects/hammer/sys/amd64/amd64/machdep.c#12 (text+ko) ==== >> > >> > @@ -1219,7 +1219,7 @@ >> > setidt(11, &IDTVEC(missing), SDT_SYSTGT, SEL_KPL, 0); >> > setidt(12, &IDTVEC(stk), SDT_SYSTGT, SEL_KPL, 0); >> > setidt(13, &IDTVEC(prot), SDT_SYSTGT, SEL_KPL, 0); >> > - setidt(14, &IDTVEC(page), SDT_SYSTGT, SEL_KPL, 0); >> > + setidt(14, &IDTVEC(page), SDT_SYSIGT, SEL_KPL, 0); >> > setidt(15, &IDTVEC(rsvd), SDT_SYSTGT, SEL_KPL, 0); >> > setidt(16, &IDTVEC(fpu), SDT_SYSTGT, SEL_KPL, 0); >> > setidt(17, &IDTVEC(align), SDT_SYSTGT, SEL_KPL, 0); >> > >> > ==== //depot/projects/hammer/sys/amd64/amd64/trap.c#6 (text+ko) ==== >> > >> > @@ -213,9 +213,17 @@ >> > * do the VM lookup, so just consider it a fatal trap so the >> > * kernel can print out a useful trap message and even get >> > * to the debugger. >> > + * >> > + * Note that T_PAGEFLT is registered as an interrupt gate. T > his >> > + * is just like a trap gate, except interrupts are disabled. > This >> > + * happens to be critically important, because we could other > wise >> > + * preempt and run another process that may cause %cr2 to be >> > + * clobbered for something else. >> > */ >> > eva = rcr2(); >> > - if (PCPU_GET(spinlocks) != NULL) >> > + if (PCPU_GET(spinlocks) == NULL) >> > + enable_intr(); >> > + else >> > trap_fatal(&frame, eva); >> > } >> >> The spinlocks check only works if witness is on. What you want to >> do is check td_critnest > 0 instead. > > Hmm. I was just going by the i386 code. Is that wrong too, or is this > because I'm using the cheat implementation of the nonlazy critical masking? The i386 code is wrong, I fixed it this morning in the smpng branch. :) I need to make sure I didn't screw it up (make sure it boots ok and stuff) and then get it into HEAD. Then you can put the final version into amd64. It is actually less fatal to be wrong here on i386 because it has the lazy interrupt masking evil. -- John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20030508125309.jhb>