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

[John Baldwin]

On Thursday, October 29, 2015 09:53:43 AM Konstantin Belousov wrote:
> On Thu, Oct 29, 2015 at 04:16:28AM +0000, Conrad E. Meyer wrote:
> > Author: cem
> > Date: Thu Oct 29 04:16:28 2015
> > New Revision: 290130
> > URL: https://svnweb.freebsd.org/changeset/base/290130
> > 
> > Log:
> >   ntb: Do not attempt to set write-combining on MWs
> >   
> >   AMD64 pmap assumes ranges will be in the DMAP, which isn't necessarily
> >   true for NTB memory windows (especially 64-bit BARs).
> I am not sure what do you mean.  pmap_change_attr() handles either DMAP
> or kernel mapped memory.

I think it assumes the DMAP is valid in the nested call here:

                        if (tmpva >= VM_MIN_KERNEL_ADDRESS) {
                                if (pa_start == pa_end) {
                                        /* Start physical address run. */
                                        pa_start = *pdpe & PG_PS_FRAME;
                                        pa_end = pa_start + NBPDP;
                                } else if (pa_end == (*pdpe & PG_PS_FRAME))
                                        pa_end += NBPDP;
                                else {
                                        /* Run ended, update direct map. */
                                        error = pmap_change_attr_locked(
                                            PHYS_TO_DMAP(pa_start),
                                            pa_end - pa_start, mode);
                                        if (error != 0)
                                                break;
                                        /* Start physical address run. */
                                        pa_start = *pdpe & PG_PS_FRAME;
                                        pa_end = pa_start + NBPDP;
                                }
                        }


That needs to do some sort of range checking on the (pa_start, pa_end)
range to only set values that are mapped in the DMAP.

In particular, that pmap_change_attr_locked() will fail here:

        /*
         * Pages that aren't mapped aren't supported.  Also break down 2MB pages
         * into 4KB pages if required.
         */
        for (tmpva = base; tmpva < base + size; ) {
                pdpe = pmap_pdpe(kernel_pmap, tmpva);
                if (*pdpe == 0)
                        return (EINVAL);

Since there won't be a valid pdpe, pde, or pte in the DMAP.

-- 
John Baldwin