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

[Alan Cox]

On 10/29/2015 12:02, John Baldwin wrote:
> 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.


I agree.  Commit it.


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