Date: Thu, 29 Oct 2015 10:02:09 -0700 From: John Baldwin <jhb@freebsd.org> To: Steven Wahl <Steve_Wahl@dell.com> Cc: "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "kib@freebsd.org" <kib@freebsd.org>, "cem@freebsd.org" <cem@freebsd.org>, Eric Van Gyzen <Eric.VanGyzen@compellent.com>, "?Alan L. Cox" <alc@freebsd.org> Subject: Re: svn commit: r290130 - head/sys/dev/ntb/ntb_hw Message-ID: <1968754.iCngWsIWpR@ralph.baldwin.cx> In-Reply-To: <bbf3208d2a4d40998c2154402208c016@mspexmb1.Beer.Town> References: <bbf3208d2a4d40998c2154402208c016@mspexmb1.Beer.Town>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <steve_wahl@dell.com>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1968754.iCngWsIWpR>