From owner-svn-src-head@freebsd.org Mon Nov 21 18:01:51 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DD6B1C4C5B7; Mon, 21 Nov 2016 18:01:51 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BD5BE1284; Mon, 21 Nov 2016 18:01:51 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 95DAF10A982; Mon, 21 Nov 2016 13:01:50 -0500 (EST) From: John Baldwin To: Konstantin Belousov Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r308821 - head/sys/i386/i386 Date: Mon, 21 Nov 2016 09:49:35 -0800 Message-ID: <2451060.DFakaYVcal@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-PRERELEASE; KDE/4.14.10; amd64; ; ) In-Reply-To: <20161120083131.GP54029@kib.kiev.ua> References: <201611190136.uAJ1aiZb091275@repo.freebsd.org> <20161120083131.GP54029@kib.kiev.ua> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Mon, 21 Nov 2016 13:01:50 -0500 (EST) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Nov 2016 18:01:52 -0000 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