Re: [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load

[Igor Druzhinin <igor.druzhinin@citrix.com>]

On 17/06/2020 17:59, Paolo Bonzini wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 17/06/20 17:46, Laszlo Ersek wrote:
>>> That said, Igor's patch seems correct to me.  In fact, I'd even move
>>> DisableInterrupts before gBS->RestoreTPL unless there's a good reason
>>> not to do so.
>> OK, thank you!
>>
>> Igor, please confirm if you'd like to submit v2 with the update
>> suggested by Paolo, or if you prefer the current version. We're at the
>> beginning of the current development cycle, so I guess we can apply the
>> patch and see how it works in practice. If it ends up wreaking havoc on
>> some platforms, we can always revert the patch in time for the next
>> stable tag (edk2-stable202008).
> 
> For what it's worth "correct" means that I don't see anything that could
> break and in fact I find it good policy hygienic not to allow recursive
> interrupts.
> 
> v1 is okay for me too, so:
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, unfortunately it's not possible to move DisableInterrupts inside
TPL block as RestoreTPL would immediately enable them according to current
logic.

IMO RaiseTPL could technically save interrupt state inside it (in that
case it was disabled) and then honor it in RestoreTPL but that might have
more surprise consequences than the proposed change I reckon.

I will create a BZ ticket as requested.

Igor