Il ven 1 mar 2024, 12:10 Michael Brown <mcb30@ipxe.org> ha scritto:

It feels as though this should be able to be cleanly modelled with a
single global state array

BOOLEAN mSavedInterruptState[TPL_HIGH_LEVEL]

Pretty much, yes. But you only have to write it when the interrupts are already disabled, so the bitmask works and makes it easier to clear "all values at NewTpl and above" with just an AND.

(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 think my patch works (modulo the 1 vs. 1U issue) for the second. Declaring the first to be invalid is a good idea IMO. Also it would only break in interrupt handlers and would revert to just causing a stack overflow in the interrupt storm scenario, so it wouldn't be too bad...

Paolo

I'll try to get a patch put together over the weekend.

Thanks,

Michael