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

[Jan Beulich <jbeulich@xxxxxxxx>]

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;

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.)

Jan