Re: port-macppc/58283: NetBSD/macppc 10.0 reproducible kernel PGM trappanic

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

The following reply was made to PR port-macppc/58283; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: mrg%eterna23.net@localhost, tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: port-macppc/58283: NetBSD/macppc 10.0 reproducible kernel PGM trappanic
Date: Tue, 11 Jun 2024 23:15:39 +0900

 > Hmm, indeed the panic no longer happens with the following patch.
 > (I'm not sure why netbsd-9 worked though)
 > 
 > ---
 > Index: altivec.c
 > ===================================================================
 > RCS file: /cvsroot/src/sys/arch/powerpc/oea/altivec.c,v
 > retrieving revision 1.34
 > diff -u -p -d -r1.34 altivec.c
 > --- altivec.c	30 Oct 2021 19:44:56 -0000	1.34
 > +++ altivec.c	10 Jun 2024 18:01:52 -0000
 > @@ -163,6 +163,10 @@ vec_restore_from_mcontext(struct lwp *l,
 >  
 >  	KASSERT(l == curlwp);
 >  
 > +	/* Nothing to do if no AltiVec context. */
 > +	if (!vec_used_p(l))
 > +		return;
 > +
 >  	/* we don't need to save the state, just drop it */
 >  	pcu_discard(&vec_ops, l, true);
 
 This is triggered by the following commit for PR/56471:
  https://github.com/NetBSD/src/commit/abf148505b7dc713c84cfb0a31b3849c863f4500
 
 >> - In vec_restore_from_mcontext() and vec_save_to_mcontext(), allows the
 >>   mcontext argument to be NULL.
 >> - In sendsig_sigcontext(), don't set PSL_VEC in the saved MSR; we can't
 >>   actually round-trip the AltiVec registers.  At least get them saved
 >>   into the PCB by calling vec_save_to_mcontext() (with a NULL mcontext
 >>   argument).
 >> - In compat_16_sys___sigreturn14(), call vec_restore_from_mcontext()
 >>   with a NULL mcontext argument, which will force any subsequent use
 >>   of AltiVec to re-load the AltiVec registers from the PCB.
 
 After this change vec_restore_from_mcontext() is unconditionally called
 from sendsig_sigcontext() in sys/arch/powerpc/powerpc/compat_16_machdep.c
 and it doesn't have the vec_used_p() check.
 
 I'm not sure if the caller should check it or not, but
 it's more consistent and less scary to have a vec_used_p() check
 in vec_restore_from_mcontext() as vec_save_to_mcontext() does,
 even current all fpu_restore_from_mcontext() calls in
 sys/arch/powerpc/powerpc/sig_machdep.c have checks
 against _UC_POWERPC_VEC or _UC_POWERPC_SPE.
 
 Thanks,
 ---
 Izumi Tsutsui