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 <riastradh%NetBSD.org@localhost>]

> Date: Thu, 15 Jan 2026 11:25:33 +0100
> From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
> 
> 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>
> > > 
> > >  ENTRY(setusergs)
> > > -	CLI(ax)
> > > +	CLI2(ax, rdi)
> > >  	swapgs
> > >  	movw	%di, %gs
> > >  	swapgs
> > 
> > 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?
> 
> 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?
> 
> I added "rsi, rdi and r14 - free for local use". rsi was already used here, 
> r14 is also used by CHECK_ASTPENDING()/CLEAR_ASTPENDING() and rdi for the
> call to trap().

OK, thanks!

> > > +	cmpq $0, 0(%r ## temp_reg);				\
> > 
> > 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?
> 
> 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 debugging :)

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 = "cli with preemtion";
> > > +const char *sti_panic = "sti with interrupts enabled";
> > > +#endif
> > 
> > const char cli_panic[] = "cli with preemtion";
> > const char sti_panic[] = "sti with interrupts enabled";
> > 
> > Don't need a mutable pointer to the immutable string in memory -- just
> > a symbol for the string is enough.
> 
> Yes, but how do we do this in C ?

I'm saying right now you're using

const char *sli_panic = ...

but if you use

const char sli_panic[] = ...

it'll save some .data space.

$ head morespace.c lessspace.c
==> morespace.c <==
const char *msg = "hello";

==> lessspace.c <==
const char msg[] = "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