Re: [PATCH 2/2] x86/boot: attempt to print trace and panic on AP bring up stall

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, May 22, 2025 at 09:18:57AM +0200, Jan Beulich wrote:
> On 21.05.2025 18:55, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/traps.c
> > +++ b/xen/arch/x86/traps.c
> > @@ -714,13 +714,15 @@ static cpumask_t show_state_mask;
> >  static bool opt_show_all;
> >  boolean_param("async-show-all", opt_show_all);
> >  
> > +static bool force_show_all;
> > +
> >  static int cf_check nmi_show_execution_state(
> >      const struct cpu_user_regs *regs, int cpu)
> >  {
> >      if ( !cpumask_test_cpu(cpu, &show_state_mask) )
> >          return 0;
> >  
> > -    if ( opt_show_all )
> > +    if ( opt_show_all || force_show_all )
> >          show_execution_state(regs);
> >      else if ( guest_mode(regs) )
> >          printk(XENLOG_ERR "CPU%d\t%pv\t%04x:%p in guest\n",
> > @@ -734,6 +736,40 @@ static int cf_check nmi_show_execution_state(
> >      return 1;
> >  }
> >  
> > +void show_execution_state_nmi(const cpumask_t *mask, bool show_all)
> > +{
> > +    unsigned int msecs, pending;
> > +
> > +    force_show_all = show_all;

Sorry, I did send v2 before seeing your comments.

> Both forms of the call can, aiui, in principle race with one another.
> I think you want to avoid setting the static to false once it was set
> to true.
> 
> Furthermore, as long as all calls here with the 2nd argument being
> true are followed by panic() or alike, I see no reason why you couldn't
> simply re-use opt_show_all, setting that one to true. (Or else there
> would then also be some resetting of the new static.)

So basically do something like:

if ( show_all )
    opt_show_all = true;

And only overwrite opt_show_all when the caller requests full traces?

Thanks, Roger.