Date: Mon, 21 Nov 2016 09:49:35 -0800 From: John Baldwin <jhb@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r308821 - head/sys/i386/i386 Message-ID: <2451060.DFakaYVcal@ralph.baldwin.cx> In-Reply-To: <20161120083131.GP54029@kib.kiev.ua> References: <201611190136.uAJ1aiZb091275@repo.freebsd.org> <20161120083131.GP54029@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday, November 20, 2016 10:31:31 AM Konstantin Belousov wrote: > On Sat, Nov 19, 2016 at 01:36:44AM +0000, John Baldwin wrote: > > Author: jhb > > Date: Sat Nov 19 01:36:44 2016 > > New Revision: 308821 > > URL: https://svnweb.freebsd.org/changeset/base/308821 > > > > Log: > > MFamd64: Various fatal page fault fixes. > > > > - If a page fault is triggered due to reserved bits in a PTE, treat it > > as a fatal fault and panic. > > - If PG_NX is in use, report whether a fatal page fault is due to an > > instruction fetch or a data access. > > - If a fatal page fault is due to reserved bits in a PTE, report that as > > the page fault type rather than a protection violation. > > > > MFC after: 1 month > > > > Modified: > > head/sys/i386/i386/trap.c > > > > Modified: head/sys/i386/i386/trap.c > > ============================================================================== > > --- head/sys/i386/i386/trap.c Sat Nov 19 01:34:12 2016 (r308820) > > +++ head/sys/i386/i386/trap.c Sat Nov 19 01:36:44 2016 (r308821) > > @@ -857,6 +857,14 @@ trap_pfault(frame, usermode, eva) > > } > > > > /* > > + * If the trap was caused by errant bits in the PTE then panic. > > + */ > > + if (frame->tf_err & PGEX_RSV) { > > + trap_fatal(frame, eva); > > + return (-1); > > + } > > + > > + /* > > * PGEX_I is defined only if the execute disable bit capability is > > * supported and enabled. > > */ > > @@ -926,9 +934,15 @@ trap_fatal(frame, eva) > > #endif > > if (type == T_PAGEFLT) { > > printf("fault virtual address = 0x%x\n", eva); > > - printf("fault code = %s %s, %s\n", > > + printf("fault code = %s %s%s, %s\n", > > code & PGEX_U ? "user" : "supervisor", > > code & PGEX_W ? "write" : "read", > > +#if defined(PAE) || defined(PAE_TABLES) > > + pg_nx != 0 ? > > + (code & PGEX_I ? " instruction" : " data") : > > +#endif > I suggest to remove #ifdef guards, and the pg_nx check there, as > well. The page fault exception error code bits have constant meaning > regardless of the kernel and CPU configuration, so if we get the I bit > set in the error word, we know that it was due to executing on PAE table > with bit 63 (nx) set. This is true even if kernel was not configured to > create such tables. > > In other words, it would give more correct and useful information, which > make it easier to track page tables corruption. The SDM claims that this bit is only defined if pg_nx is enabled, so an instruction fault when EFER_NXE is not enabled would report this bit as clear resulting in the trap output saying "data" when it was really "instruction". That is, on i386 without pg_nx, the I/D being zero can mean either instruction or data, not just data. I felt always printing data in that case could be misleading (someone might get a fatal trap and be confused that it reports a data fault when it is the instruction fetch that failed). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2451060.DFakaYVcal>