Re: vcpu_show_execution_state() difference between Arm and x86

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.09.2021 13:48, Julien Grall wrote:
> On 02/09/2021 07:45, Jan Beulich wrote:
>> On 01.09.2021 20:11, Julien Grall wrote:
>>> I looked at the original commit to find out the reason to use the
>>> console lock. AFAICT, this was to allow console_force_unlock() to work
>>> properly. But it is not entirely clear why we couldn't get a new lock
>>> (with IRQ enabled) that could be forced unlocked in that function.
>>>
>>> Can either you or Andrew clarify it?
>>
>> AIUI any new lock would need to be IRQ-safe as well, as the lock
>> would be on paths taken to output stuff when the system crashed.
> 
> Hmmm... Just to confirm, what you are saying is some of the callers of 
> vcpu_show_execution_state() & co may do it with IRQ-disabled. Is that 
> correct?

No, that's not what I was saying. What I was saying is that crash-
safety requires an IRQ-safe lock, because the approach taken here
should imo match that in show_execution_state(). And _that_
function can be called with IRQs in either state.

> I have tried to play with it on Arm but then I realized that 
> check_lock() is not properly working on Arm because we don't call 
> spin_debug_enable() at boot. :/ So it would make sense why we never saw 
> any issue there...

Oops.

>> Hence it would be pointless to introduce yet another lock when the
>> one we have is already good enough. But I may be missing aspects,
>> in which case I'd have to defer to Andrew.
>>
>>> The other solution I can think off is buffering the output for
>>> show_registers and only print it once at the end. The downside is we may
>>> not get any output if there is an issue in the middle of the dump.
>>
>> Indeed; I'd prefer to avoid that, the more that it may be hard to
>> predict how much output there's going to be.
> 
> And it is not going to work as we couldn't grab the P2M lock with IRQ 
> disabled.
> 
> On Arm, the only problem is going to be the P2M lock for dump the guest 
> trace. A possible controversial approach for Arm is to just not dump the 
> guest stack or move it outside of the new lock and dump when IRQ is 
> enabled (I am not aware of any places where the guest stack will be 
> dumped and we have IRQ disabled).

Well, you could certainly omit the stack dump on Arm. I for one find
it quite useful every now and then. On x86, that is.

Jan