On 01/03/2024 09:33, Paolo Bonzini wrote:
On Fri, Mar 1, 2024 at 10:27 AM Michael Brown <mc...@ipxe.org> wrote:
It's possible that it doesn't matter. The new logic will effectively
mean that RestoreTPL() will restore not only the TPL but also the
interrupts-enabled state to whatever existed at the time of the
corresponding RaiseTPL().
Right: that's what my comment says
+ // However, when the handler calls RestoreTPL
+ // before returning, we want to keep interrupts disabled. This
+ // restores the exact state at the beginning of the handler,
+ // before the call to RaiseTPL(): low TPL and interrupts disabled.
but indeed it applies beyond interrupt handlers. It might even be a bugfix.
Right. I'm leaning towards treating this as a bugfix: essentially
tightening up the semantics of RestoreTPL() to mean:
- any callbacks in the range OldTpl < Tpl < gEfiCurrentTpl will be
dispatched with interrupts unconditionally enabled
- the TPL will be restored to OldTpl
- the interrupt state will be restored to the value it had when the TPL
was last raised from OldTpl
It feels as though this should be able to be cleanly modelled with a
single global state array
BOOLEAN mSavedInterruptState[TPL_HIGH_LEVEL]
(or possibly a bitmask, though using the array avoids having to disable
interrupts just to write a value).
I still need to think through the subtleties, to make sure it could cope
with pathological edge cases such as
OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
...
gBS->RestoreTPL (OldTpl);
gBS->RestoreTPL (OldTpl);
or
OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL - 1);
gBS->RaiseTPL (TPL_HIGH_LEVEL);
..
gBS->RestoreTPL (OldTpl);
I think that at least one of the above pathological usage patterns would
break the existing mInterruptedTplMask patches, since they currently
clear state in RestoreTPL() and so will not correctly handle a duplicate
call to RestoreTPL().
I'll try to get a patch put together over the weekend.
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116232): https://edk2.groups.io/g/devel/message/116232
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]
-=-=-=-=-=-=-=-=-=-=-=-
