svn commit: r290130 - head/sys/dev/ntb/ntb_hw

[John Baldwin]

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 at 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