On 29/02/2024 16:43, Kinney, Michael D wrote:
Hi Michael,
Can you provide a pointer to the UEFI Spec statement this breaks?
II-9.7.1.3 RestoreTPL():
"When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has
been installed, then the full version of the Boot Service RestoreTPL()
can be made available. When an attempt is made to restore the TPL level
to level below EFI_TPL_HIGH_LEVEL, then the DXE Foundation should use
the services of the EFI_CPU_ARCH_PROTOCOL to enable interrupts."
I suspect this is sufficient to veto the proposed design, though we
could argue that the loosely worded "should" is technically not "must".
If we still want to proceed with this design, then I have several other
questions:
- How does the proposed patch react to an interrupt occurring
(illegally) at TPL_HIGH_LEVEL (as happens with some versions of
Windows)? As far as I can tell, it will result in mInterruptedTplMask
having bit 31 set but never cleared. What impact will this have?
- How does the proposed patch react to potentially mismatched
RaisedTPL()/RestoreTPL() calls (e.g. oldTpl = RaiseTPL(TPL_CALLBACK)
followed by RaiseTPL(TPL_NOTIFY) followed by a single RestoreTPL(oldTpl))?
I believe the proposed patch is attempting to establish a new invariant
as follows:
Once an interrupt has occured at a given TPL, then that *TPL* is
conceptually considered to be in an "interrupted" state. The *only*
thing that can clear this "interrupted" state from the TPL is to return
from the interrupt handler.
Note that this conceptual definition does not perfectly align with the
bit flags in mInterruptedTplMask, since those bits will necessarily be
set only some time after the interrupt occurs, and will have to be
cleared before returning from the interrupt. However, it is the
conceptual definition that is relevant to the invariant.
The new invariant is that no code may execute at an "interrupted" TPL
with interrupts enabled. It is legitimate for code to raise to a higher
TPL and to enable interrupts while there, and it is legitimate for code
to execute in an "interrupted" TPL with interrupts disabled, but it is
not legitimate for any code to reenable interrupts while still at an
"interrupted" TPL.
It would be good to call out this invariant explicitly, so that authors
of interrupt handlers are aware of the restrictions. It would also
clarify some of the logic (e.g. it provides the reason why interrupts
must be disabled before lowering gEfiCurrentTpl in CoreRestoreTpl()).
It's also generally easier to reason about a stated invariant than to
extrapolate from a list of complicated examples.
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116178): https://edk2.groups.io/g/devel/message/116178
Mute This Topic: https://groups.io/mt/104642317/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
