Re: port-xen/58561 (panic: kernel diagnostic assertion, "x86_read_psl() == 0" failed: file, "/home/netbsd/10/src/sys/arch/x86/x86/pmap.c", line 3581)

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
Cc: Konrad Schroder <perseant%hhhh.org@localhost>, gnats-bugs%NetBSD.org@localhost,
	port-xen-maintainer%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost,
	gnats-admin%NetBSD.org@localhost, cherry%NetBSD.org@localhost
Subject: Re: port-xen/58561 (panic: kernel diagnostic assertion,
	"x86_read_psl() == 0" failed: file,
	"/home/netbsd/10/src/sys/arch/x86/x86/pmap.c", line 3581)
Date: Fri, 16 Jan 2026 00:44:04 +0000

 > Date: Thu, 15 Jan 2026 11:25:33 +0100
 > From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
 >=20
 > On Wed, Jan 14, 2026 at 11:00:10PM +0000, Taylor R Campbell wrote:
 > > > Date: Tue, 13 Jan 2026 15:50:10 +0100
 > > > From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
 > > >=20
 > > >  ENTRY(setusergs)
 > > > -	CLI(ax)
 > > > +	CLI2(ax, rdi)
 > > >  	swapgs
 > > >  	movw	%di, %gs
 > > >  	swapgs
 > >=20
 > > I think this can't be right.  %rdi is obviously not a free scratch
 > > register: it's the first argument register.  But also, this code isn't
 > > even used in Xen builds, is it?  Xen has its own setusergs in
 > > xenfunc.c, doesn't it?
 >=20
 > Right, I changed it to use %rsi. Also as you noted this is not used for
 > Xen at this time (which is why I didn't notice I guess), but I think it's
 > better to be correct and use the appropriate macro here (even if it always
 > end up being a single cli instruction at this time)
 
 OK, fair enough.
 
 > > Not immediately clear these registers are free here, can we
 > > double-check and document in a comment exactly what registers are
 > > involved on entry and exit to doreti?
 >=20
 > I added "rsi, rdi and r14 - free for local use". rsi was already used her=
 e,=20
 > r14 is also used by CHECK_ASTPENDING()/CLEAR_ASTPENDING() and rdi for the
 > call to trap().
 
 OK, thanks!
 
 > > > +	cmpq $0, 0(%r ## temp_reg);				\
 > >=20
 > > I always get confused about C symbols in MOV assembly directives.
 > > Does temp_reg at this point hold a (possibly null) pointer to the
 > > first character of the panic string, or a (possibly null) pointer to
 > > the _pointer to_ first character of the panic string?
 >=20
 > temp_reg holds the address of the pointer (that is, &panicstr), and this
 > cannot be NULL. So the cmpq checks the pointer itself.
 > I know this code works because I went there multiple times while debuggin=
 g :)
 
 Great!  Thanks, yes, my LEA suggestion was nonsense and my MOV
 suggestion was useless.  I was confusing
 
 	movq	$foo,%reg
 
 with
 
 	movq	foo,%reg
 
 > > > +#if defined(DIAGNOSTIC) && defined(XENPV)
 > > > +const char *cli_panic =3D "cli with preemtion";
 > > > +const char *sti_panic =3D "sti with interrupts enabled";
 > > > +#endif
 > >=20
 > > const char cli_panic[] =3D "cli with preemtion";
 > > const char sti_panic[] =3D "sti with interrupts enabled";
 > >=20
 > > Don't need a mutable pointer to the immutable string in memory -- just
 > > a symbol for the string is enough.
 >=20
 > Yes, but how do we do this in C ?
 
 I'm saying right now you're using
 
 const char *sli_panic =3D ...
 
 but if you use
 
 const char sli_panic[] =3D ...
 
 it'll save some .data space.
 
 $ head morespace.c lessspace.c
 =3D=3D> morespace.c <=3D=3D
 const char *msg =3D "hello";
 
 =3D=3D> lessspace.c <=3D=3D
 const char msg[] =3D "hello";
 $ make morespace.o lessspace.o
 cc -O2   -c morespace.c
 cc -O2   -c lessspace.c
 $ size morespace.o lessspace.o
    text	   data	    bss	    dec	    hex	filename
       6	      8	      0	     14	      e	morespace.o
       6	      0	      0	      6	      6	lessspace.o