Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/NestedInterruptTplLib: replace ASSERT() with a warning logged.

["Michael Brown" <mcb30@ipxe.org>]

On 03/05/2023 08:19, Gerd Hoffmann wrote:
> OVMF can't guarantee that the ASSERT() doesn't happen.  Misbehaving
> EFI applications can trigger this.  So log a warning instead and try
> to continue.
> 
> Reproducer: Fetch windows 11 22H2 iso image, boot it in qemu with OVMF.
> 
> Traced to BootServices->Stall() being called with IPL=TPL_HIGH_LEVEL
> and Interrupts /enabled/ while windows is booting.
> 
> Cc: Michael Brown <mcb30@ipxe.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
> index e19d98878eb7..fdd7d15c4ba8 100644
> --- a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
> +++ b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
> @@ -39,7 +39,9 @@ NestedInterruptRaiseTPL (
>     //
>     ASSERT (GetInterruptState () == FALSE);
>     InterruptedTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> -  ASSERT (InterruptedTPL < TPL_HIGH_LEVEL);
> +  if (InterruptedTPL >= TPL_HIGH_LEVEL) {
> +    DEBUG ((DEBUG_WARN, "%a: Called at IPL %d\n", __func__, InterruptedTPL));
> +  }
>   
>     return InterruptedTPL;
>   }

While https://bugzilla.redhat.com/show_bug.cgi?id=2189136 continues to 
track the underlying Windows bug that leads to this assertion being 
triggered: I suspect that this patch will allow people to boot these 
buggy versions of Windows in OVMF, and I don't think it will make things 
any worse.

I would probably suggest changing DEBUG_WARN to DEBUG_ERROR since this 
represents a serious invariant violation being detected.  With that change:

   Reviewed-by: Michael Brown <mcb30@ipxe.org>

Thanks,

Michael