Ping: [PATCH] x86: enable interrupts around dump_execstate()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.12.2021 14:33, Jan Beulich wrote:
> On 16.12.2021 12:54, Andrew Cooper wrote:
>> On 13/12/2021 15:12, Jan Beulich wrote:
>>> show_hvm_stack() requires interrupts to be enabled to avoids triggering
>>> the consistency check in check_lock() for the p2m lock. To do so in
>>> spurious_interrupt() requires adding reentrancy protection / handling
>>> there.
>>>
>>> Fixes: adb715db698b ("x86/HVM: also dump stacks from 
>>> show_execution_state()")
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

There's a bug here which we need to deal with one way or another.
May I please ask for a response to the issues pointed out with
what you said in your earlier reply?

Thanks, Jan

>>> ---
>>> The obvious (but imo undesirable) alternative is to suppress the call to
>>> show_hvm_stack() when interrupts are disabled.
>>
>> show_execution_state() need to work in any context including the #DF
>> handler,
> 
> Why? There's no show_execution_state() on that path.
> 
>> and
>>
>>     /*
>>      * Stop interleaving prevention: The necessary P2M lookups
>>      * involve locking, which has to occur with IRQs enabled.
>>      */
>>     console_unlock_recursive_irqrestore(flags);
>>     
>>     show_hvm_stack(curr, regs);
>>
>> is looking distinctly dodgy...
> 
> Well, yes, it does. If you have any better idea ...
> 
>> For these kinds of purposes, it ought to be entirely fine to do a
>> lockless pagewalk of the p2m, because we have to maintain atomicity of
>> updates vs the hardware pagewalk anyway.  We do not care about any side
>> effects if the target isn't a RAM page.
>>
>> That ought to remove any IRQ problems from the equation.
> 
> First - how do you suggest to signal to the page walk logic that there
> should be no lock acquired? And then I don't think there's a direct
> relationship here with what we need to guarantee correct hardware page
> walk behavior. Unless you mean to suggest that here we want to rely on
> either locking or interrupts being off (the latter guaranteeing that
> flush IPIs wouldn't complete while we're still doing software walking
> here).
> 
> Jan
> 
>