Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Jan 2008 01:35:15 +0100
From:      Olivier Houchard <mlfbsd@ci0.org>
To:        Mark Tinguely <tinguely@casselton.net>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: ARM pmap cache flushed after PT modification.
Message-ID:  <20080128003515.GA32514@ci0.org>
In-Reply-To: <200801271606.m0RG6b1C036780@casselton.net>
References:  <20080127011417.GA19569@ci0.org> <200801271606.m0RG6b1C036780@casselton.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jan 27, 2008 at 10:06:37AM -0600, Mark Tinguely wrote:
> 
> >  I like your work, and am about to commit it, however, just a small point :
> >
> >  On Thu, Jan 17, 2008 at 09:48:54AM -0600, Mark Tinguely wrote:
> >  > In pmap_nuke_pv(), the vm_page_flag_clear(pg, PG_WRITEABLE) has been moved
> >  > to the pmap_fix_cache(). 
> >  > 
> >
> >  	if ((pve->pv_flags & PVF_NC) && ((pm == pmap_kernel()) ||
> >  	     (pve->pv_flags & PVF_WRITE) || !(pve->pv_flags & PVF_MWC)))
> >  		pmap_fix_cache(pg, pm, 0);
> >
> >  You only call pmap_fix_cache() if the PVF_NC bit is set, so 
> >  vm_page_flag_clear(pg, PG_WRITEABLE) won't be called, and PVF_MOD won't be
> >  removed, if we're removing the only writeable entry for a cacheable page, or
> >  did I miss something ?
> 
> You are correct, great catch.
> 
> Before, the cache fixing routine was called when a write mapping was removed.
> In addition, I really should call the cache fixing routine when caching
> was stopped and a kernel mapping was removed (this is a read kernel mapping
> but obviously there are user mapping(s) and now they may be able to
> re-enable caching) or a read mapping is being remove when there is at
> most one user write in this process space.
> 
> 	if ((pve->pv_flags & PVF_WRITE) || ((pve->pv_flags & PVF_NC) &&
> 	    ((pm == pmap_kernel()) || !(pve->pv_flags & PVF_MWC))))
> 		pmap_fix_cache(pg, pm, 0);
> 

I'm just nitpicking here, but maybe, if the page is cacheable, it would be
cheapier to just parse the mapping list once, instead of calling pmap_fix_cache
Something like this :
	if ((pve->pv_flags & PVF_NC) && ((pm == pmap_kernel()) ||
	     (pve->pv_flags & PVF_WRITE) || !(pve->pv_flags & PVF_MWC)))
		pmap_fix_cache(pg, pm, 0);
	else {
		TAILQ_FOREACH(pve, &pg->md.pv_list, pv_list)
		    if (pve->pv_flags & PVF_WRITE)
			    break;
		if (!pve) {
			pg->md.pvh_attrs &= ~PVF_MOD;
			vm_page_flag_clear(pg, PG_WRITEABLE);
		}
	}
> 			----
> I have been reading a lot in the ARM11 (aka ARMv6) technical reference
> manual. ARMv6 has significant cache changes to simplify page sharing
> and decrease cache flushing. The single core ARMv6 MMU could use the
> existing pmap code but is not as efficient as it could be. To maximize
> the single core MMU (and bypass all this pmap_fix_cache() and flushing
> the cache when switching processes), page coloring needs to be added
> (2 colors for 32KB caches and 4 colors for 64KB caches). The multicores
> does not need the page coloring, but changes to the pmap can help SMP
> support and efficiency issues.
> 

Yeah I'd love to work with armv6, but haven't been able to get my hand on
hardware yet.
I haven't read the doc yet, but I thought armv6 had a physically tagged cache.
So why would we need to flush it on context switch at all ?
I know NetBSD is starting to implement armv6 support (what they had before
for arm11 was just mimicking armv5). It would be interessant to watch their
approach.

Regards,

Olivier



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080128003515.GA32514>