Re: panic: mutex pmap not owned at ... efirt_machdep.c:255

On Tue, Aug 07, 2018 at 02:49:10PM -0700, Doug Ambrisko wrote:
> On Tue, Aug 07, 2018 at 08:29:49PM +0300, Konstantin Belousov wrote:
> | On Tue, Aug 07, 2018 at 11:50:44AM -0500, Kyle Evans wrote:
> | > On Tue, Aug 7, 2018 at 12:09 AM, Eitan Adler <eadler_at_freebsd.org> wrote:
> | > > On Mon, 6 Aug 2018 at 11:27, Kyle Evans <kevans_at_freebsd.org> wrote:
> | > >>
> | > >> On Sun, Aug 5, 2018 at 5:43 AM, Konstantin Belousov <kostikbel_at_gmail.com> wrote:
> | > >> > On Sat, Aug 04, 2018 at 09:46:39PM -0500, Kyle Evans wrote:
> | > >> >>
> | > >> >> He now gets a little further, but ends up with the same panic due to
> | > >> >> efirtc_probe trying to get time to verify the rtc's actually
> | > >> >> implemented. What kind of approach must we take to ensure curcpu is
> | > >> >> synced?
> | > >> >
> | > >> > It does not panic for me, when I load efirt.ko from the loader prompt.
> | > >> > Anyway, try this
> | > >>
> | > >> Right, I also don't get a panic on any of my machines from this.
> | > >> Hopefully he'll have a chance to try this soon.
> | > >
> | > > This change has no impact: it still panics in the same way as without the patch.
> | > >
> | > 
> | > That seems indicative of a bigger problem, since we use proc0
> | > throughout all these bits so we should still be dealing with the same
> | > pmap that got passed to pmap_pinit0 when we grab
> | > curthread->td_proc->p_vmspace->vm_pmap.
> | 
> | Can you confirm that you get the early efi_enter() call from rtc code,
> | when you preload the module or compile it into the kernel ?
> 
> When I ran into this, I did this change:
> 
> Index: dev/efidev/efirt.c
> ===================================================================
> --- dev/efidev/efirt.c	(revision 337264)
> +++ dev/efidev/efirt.c	(working copy)
> _at__at_ -257,7 +257,8 _at__at_
>  	if (efi_runtime == NULL)
>  		return (ENXIO);
>  	td = curthread;
> -	curpmap = &td->td_proc->p_vmspace->vm_pmap;
> +//	curpmap = &td->td_proc->p_vmspace->vm_pmap;
> +	curpmap = PCPU_GET(curpmap);
>  	PMAP_LOCK(curpmap);
>  	mtx_lock(&efi_lock);
>  	fpu_kern_enter(td, NULL, FPU_KERN_NOCTX);
> _at__at_ -272,7 +273,8 _at__at_
>  
>  	efi_arch_leave();
>  
> -	curpmap = &curproc->p_vmspace->vm_pmap;
> +//	curpmap = &curproc->p_vmspace->vm_pmap;
> +	curpmap = PCPU_GET(curpmap);
>  	td = curthread;
>  	fpu_kern_leave(td, NULL);
>  	mtx_unlock(&efi_lock);
> 
> Don't know if it is right.  Some previous code used both
> 	curpmap = PCPU_GET(curpmap);
> and
> 	curpmap = &td->td_proc->p_vmspace->vm_pmap;
> recently it was changes to only use
> 	curpmap = &td->td_proc->p_vmspace->vm_pmap;
> 
> Things seem to work after that.  I was able to repro. it with 
> qemu-system-x86_64 in UEFI mode.  I think it also failed in
> bhyve UEFI mode.

The pcpu curpmap and curproc vmspace pmap should be synced.  Esp. since
there is code relying on this early.  I do not want to paper it over.

In fact, try this please.  Ignore my previous change.

diff --git a/sys/amd64/amd64/pmap.c b/sys/amd64/amd64/pmap.c
index 572b2197453..4bce36cc0e5 100644
--- a/sys/amd64/amd64/pmap.c
+++ b/sys/amd64/amd64/pmap.c
_at__at_ -7536,7 +7536,8 _at__at_ pmap_activate_sw(struct thread *td)
 			PCPU_SET(kcr3, pmap->pm_cr3);
 			PCPU_SET(ucr3, pmap->pm_ucr3);
 		}
-	}
+	} else
+		PCPU_SET(curpmap, pmap);
 	if (pmap->pm_ucr3 != PMAP_NO_CR3) {
 		rsp0 = ((vm_offset_t)PCPU_PTR(pti_stack) +
 		    PC_PTI_STACK_SZ * sizeof(uint64_t)) & ~0xful;