On Fri, Mar 1, 2024 at 10:27 AM Michael Brown <mc...@ipxe.org> wrote: > > My version is attached, feel free to reuse it (either entirely or > > partially) for a hypothetical v2. Apologies to you and Mike K for the > > confusion! > > I much prefer this version of the patch, because the explanations are > easier to follow and to reason about.
Thanks! I find the new logic to be quite appealing... now that I understand it. Hopefully this provides a way to find the best of both worlds. > There is an implicit assumption that if interrupts are disabled when > RaiseTPL() is called then we must have been called from an interrupt > handler. How sure are we that this assumption is correct? > > 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. > Maybe this is what we want? If so, then we > should probably phrase the comments in these terms instead of in terms > of being called from an interrupt handler. I think phrasing the comments with reference to interrupt handlers is fine, but it may be worth adding a comment to either mInterruptedTplMask or CoreRestoreTpl(), like +/// NOTE: Strictly speaking, this applies to anything that +/// calls RaiseTPL() with interrupts disabled, not just +/// interrupt handlers. Interrupt handlers are just the case +/// that we care the most about, because of the potential +/// for stack overflow. Paolo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116228): https://edk2.groups.io/g/devel/message/116228 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] -=-=-=-=-=-=-=-=-=-=-=-
