From owner-svn-src-all@freebsd.org Thu Oct 29 17:02:32 2015 Return-Path: Delivered-To: svn-src-all@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 A68F2A205FF for ; Thu, 29 Oct 2015 17:02:32 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.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 79DF412FF; Thu, 29 Oct 2015 17:02:32 +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 bigwig.baldwin.cx (Postfix) with ESMTPSA id 838DCB922; Thu, 29 Oct 2015 13:02:31 -0400 (EDT) From: John Baldwin To: Steven Wahl Cc: "svn-src-all@freebsd.org" , "kib@freebsd.org" , "cem@freebsd.org" , Eric Van Gyzen , "?Alan L. Cox" Subject: Re: svn commit: r290130 - head/sys/dev/ntb/ntb_hw Date: Thu, 29 Oct 2015 10:02:09 -0700 Message-ID: <1968754.iCngWsIWpR@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: References: 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.2.7 (bigwig.baldwin.cx); Thu, 29 Oct 2015 13:02:31 -0400 (EDT) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Oct 2015 17:02:32 -0000 On Thursday, October 29, 2015 04:42:20 PM Steven Wahl wrote: > We ran into this exact problem, pmap_change_attr not working right with large bars. I had been working up to seeing if this compiles on the current head, introducing myself to the community, seeing if this would be accepted. > > But looks like it's needed sooner, so in case it might save you some time, here's the patch we developed for this problem. > > --> Steve Wahl, Dell Compellent, Eden Prairie, MN Adding alc@, but on first blush this looks correct to me. > > commit 7d112aa8767390cb9dd020325a9f23aaac7fe5a0 > Author: swahl > Date: Thu Oct 1 14:36:48 2015 -0500 > > CQ00492954: FreeBSD traps on call to pmap_change_attr() with large PLX BAR > > If an address passed to pmap_change_attr() refers to a virtual address, > the function must also change the direct mapping of this same > region (if any) to match, or intel says the result is undefined. > > But the original code did not check if the virtual address actually fell > within the direct mapped region before attempting to make this change. > The attempt to look up the direct mapped page entries returned NULL, and > this was dereferenced causing a panic. > > This is fixed by checking whether the address is outside of the direct > mapped range before trying to change the direct mapped entries. > > diff --git a/src/sys/amd64/amd64/pmap.c b/src/sys/amd64/amd64/pmap.c > index fe09ace..dee22de 100644 > --- a/src/sys/amd64/amd64/pmap.c > +++ b/src/sys/amd64/amd64/pmap.c > @@ -6268,7 +6268,7 @@ pmap_change_attr_locked(vm_offset_t va, vm_size_t size, int mode) > */ > for (tmpva = base; tmpva < base + size; ) { > pdpe = pmap_pdpe(kernel_pmap, tmpva); > - if (*pdpe == 0) > + if (pdpe == NULL || *pdpe == 0) > return (EINVAL); > if (*pdpe & PG_PS) { > /* > @@ -6341,7 +6341,8 @@ pmap_change_attr_locked(vm_offset_t va, vm_size_t size, int mode) > X86_PG_PDE_CACHE); > changed = TRUE; > } > - if (tmpva >= VM_MIN_KERNEL_ADDRESS) { > + if (tmpva >= VM_MIN_KERNEL_ADDRESS && > + (*pdpe & PG_PS_FRAME) < dmaplimit) { > if (pa_start == pa_end) { > /* Start physical address run. */ > pa_start = *pdpe & PG_PS_FRAME; > @@ -6370,7 +6371,8 @@ pmap_change_attr_locked(vm_offset_t va, vm_size_t size, int mode) > X86_PG_PDE_CACHE); > changed = TRUE; > } > - if (tmpva >= VM_MIN_KERNEL_ADDRESS) { > + if (tmpva >= VM_MIN_KERNEL_ADDRESS && > + (*pde & PG_PS_FRAME) < dmaplimit) { > if (pa_start == pa_end) { > /* Start physical address run. */ > pa_start = *pde & PG_PS_FRAME; > @@ -6397,7 +6399,8 @@ pmap_change_attr_locked(vm_offset_t va, vm_size_t size, int mode) > X86_PG_PTE_CACHE); > changed = TRUE; > } > - if (tmpva >= VM_MIN_KERNEL_ADDRESS) { > + if (tmpva >= VM_MIN_KERNEL_ADDRESS && > + (*pte & PG_FRAME) < dmaplimit) { > if (pa_start == pa_end) { > /* Start physical address run. */ > pa_start = *pte & PG_FRAME; > -- John Baldwin