On 23/01/2024 16:32, Laszlo Ersek wrote:
On 1/23/24 16:31, 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.
Restoring TPL to TPL_HIGH_LEVEL is always a no-op. Return immediately
from NestedInterruptRestoreTPL(TPL_HIGH_LEVEL), so that we do not need
to consider the effect of this possible invariant violation on the
remainder of the logic.
Feels like the handling logic might as well be "panic" here (except edk2
does not have a central panic API that's suitable for all platforms). I
probably missed the previous discussion that led to this patch. Either
way, it seems reasonable.
Acked-by: Laszlo Ersek <[email protected]>
Thank you. We can't panic because there are some bootloaders (*cough*
Microsoft *cough*) that illegally call RaiseTPL(TPL_HIGH_LEVEL) and then
even more illegally enable interrupts via STI. Gerd tracked this down
before, which lead to commit
https://github.com/tianocore/edk2/commit/bee67e0c1
I found another way to trigger a RestoreTPL(TPL_HIGH_LEVEL) while I was
testing the self-tests by deliberately breaking
DisableInterruptsOnIret() to fail to disable interrupts. This also
induces a situation in which we end up at TPL_HIGH_LEVEL with interrupts
enabled.
This ended up triggering an assertion (due to the invariant violation)
in NestedInterruptRestoreTPL() before reaching the point in the
self-test that would have reported a more meaningful error message.
Adding the bypass is justifiable on its own merits (as per the reasoning
in the commit), and it avoids needing to add clutter to the complex
logic in NestedInterruptRestoreTPL() just to ensure that the self-test
fails on the desired ASSERT().
I decided against trying to explain all that in the commit message, but
I can have a go if you think it needs to be captured. :)
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114220): https://edk2.groups.io/g/devel/message/114220
Mute This Topic: https://groups.io/mt/103911606/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
