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
