Subject: Re: SIR Reset with todays sources
To: Eduardo Horvath <eeh@NetBSD.org>
From: Chuck Silvers <chuq@chuq.com>
List: port-sparc64
Date: 03/27/2007 10:10:18
--kORqDWCi7qDJ0mEj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Mar 26, 2007 at 04:18:28PM +0000, Eduardo Horvath wrote:
> On Sun, 25 Mar 2007, Martin Husemann wrote:
> 
> > On Sun, Mar 25, 2007 at 08:59:31PM +0200, Martin Husemann wrote:
> > > But changing the one in pmap_remove_all to set the secondary context to
> > > -1 instead of 0 does the trick.
> > 
> > On second thought: removing that stxa(CTX_SECONDARY, ASI_DMMU, 0) completely
> > does the trick too.
> > 
> > I'm not sure there could be any data accesses happening between this and the
> > removal of the mapping in pmap_remove(), and if those would happen, if still
> > allowing them would be evil. I suppose we can ignore this.
> 
> What the heck are we trying to do here?  Setting the secondary context to 
> 0 merely makes user accesses (ASI_AIUS) use the kernel pmap, which means 
> you are allowing the kernel to trash itself, which is bogus.  Also, if 
> code tries to flush the secondary context out of the MMU it will flush out 
> he nucleus (kernel) context instead of whatever user context used to be 
> there.  What Chuck may have been trying to do there is flush the secondary 
> context, which is probably best done by calling sp_tlb_flush_ctx() in 
> locore.s, which, in theory, should DTRT.

the idea was that we shouldn't access the user address space anymore at
this point, since the purpose of pmap_remove_all() is to tell the pmap
that we're about to call pmap_remove() on every mapping in the pmap.
I don't recall why I thought that setting the secondary context to 0
was a good thing, I agree that it's bogus and should be removed.

the problem here seems to be that there's still one user window in the
register file when we're in pmap_remove_all(), which is flushed when
we call ctx_free().  however, since there's no mapping in context 0
at that address, we crash with a kernel fault.  adding just a single
save/restore pair before the stxa() makes the current problem go away,
but we need to use write_user_windows() to make sure that they're all gone
before we call ctx_free().

I did not intend to flush the TLB there, since the context will not be
reused until we cycle through all the context IDs, at which point
we invalidate all user TLB entries and start from ID 1 again.
so any stale TLB entries for this context that are left around
after pmap_remove_all() are harmless.

-Chuck

--kORqDWCi7qDJ0mEj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.sir"

Index: src/sys/arch/sparc64/sparc64/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/pmap.c,v
retrieving revision 1.187
diff -u -p -r1.187 pmap.c
--- src/sys/arch/sparc64/sparc64/pmap.c	12 Mar 2007 18:18:28 -0000	1.187
+++ src/sys/arch/sparc64/sparc64/pmap.c	27 Mar 2007 17:09:17 -0000
@@ -1846,7 +1846,7 @@ pmap_remove_all(pm)
 	if (pm == pmap_kernel()) {
 		return;
 	}
-	stxa(CTX_SECONDARY, ASI_DMMU, 0);
+	write_user_windows();
 	pm->pm_refs = 0;
 	ctx_free(pm);
 	blast_dcache();

--kORqDWCi7qDJ0mEj--