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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.05.2025 16:11, Roger Pau Monné wrote:
> 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?

Yes, that's what I think it boils down to.

Jan