Re: [edk2-devel] [PATCH v2 0/2] OvmfPkg: Relax assertion that interrupts do not occur at TPL_HIGH_LEVEL

["Ard Biesheuvel" <ardb@kernel.org>]

(cc Liming)

On Tue, 9 May 2023 at 16:23, Michael Brown <mcb30@ipxe.org> wrote:
>
> On 09/05/2023 14:31, Laszlo Ersek wrote:
> > On 5/9/23 14:09, Michael Brown wrote:
> >> At TPL_HIGH_LEVEL, CPU interrupts are disabled (as per the UEFI
> >> specification) and so we should never encounter a situation in which
> >> an interrupt occurs at TPL_HIGH_LEVEL. The specification also
> >> restricts usage of TPL_HIGH_LEVEL to the firmware itself.
> >>
> >> However, nothing actually prevents a UEFI application from calling
> >> gBS->RaiseTPL(TPL_HIGH_LEVEL) and then violating the invariant by
> >> enabling interrupts via the STI or equivalent instruction. Some
> >> versions of the Microsoft Windows bootloader are known to do this.
> >>
> >> NestedInterruptTplLib maintains the invariant that interrupts are
> >> disabled at TPL_HIGH_LEVEL (even when performing the dark art of
> >> deliberately manipulating the stack so that IRET will return with
> >> interrupts still disabled), but does not itself rely on external code
> >> maintaining this invariant.
> >>
> >> Relax the assertion that the interrupted TPL is below TPL_HIGH_LEVEL
> >> to an error message, to allow UEFI applications such as these versions
> >> of the Microsoft Windows bootloader to continue to function.
> >>
> >> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=2189136
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Oliver Steffen <osteffen@redhat.com>
> >> Cc: Pawel Polawski <ppolawsk@redhat.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >>
> >> Michael Brown (2):
> >>    OvmfPkg: Clarify invariants for NestedInterruptTplLib
> >>    OvmfPkg: Relax assertion that interrupts do not occur at
> >>      TPL_HIGH_LEVEL
> >>
> >>   OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 31 +++++++++++++++++----
> >>   1 file changed, 26 insertions(+), 5 deletions(-)
> >>
> >
> > series
> > Acked-by: Laszlo Ersek <lersek@redhat.com>
>
> Thank you!
>
> Gerd: are you happy for your Reviewed-by to stand, since the only
> changes since v1 were to comment wording?

I'll assume that it stands if that was the only difference.

I've queued this up now - thanks all for the hard work.

(Note to Liming - this is definitely a candidate for the stable tag,
see the bugzilla link for details)


>  (My apologies for forgetting
> to include a v2 description in the cover letter.)
>
> Thanks,
>
> Michael
>
>