> That would then be a bug in HpetTimer, which ought to be fixed. If > HpetTimer were to be used on a platform where the NotifyFunction > correctly assumes that it is called at TPL_HIGH_LEVEL and does something > that would break at a lower level, then this could lead to undefined > behaviour.
In theory, it could happen that the NotifyFunction may not raise TPL to HIGH. In real world, NotifyFunction (CoreTimerTick() in DxeCore) does raise TPL to HIGH. I agree binding the NotifyFunction to CoreTimerTick() is not the best. But it could help to reduce the complexity of the problem. > As a pure code change, I do agree that it solves the problem and it's a > much simpler approach. However, it is a breaking change to the > specification and I think it would need be handled as such. > > The minimal specification change I can think of that would make this > possible would be to relax the wording on NotifyFunction in the next > version of the PI specification to say that > > * the NotifyFunction can be called at any TPL level > > * the NotifyFunction will raise TPL to TPL_HIGH_LEVEL, restore TPL back > to the original TPL before returning > > * the NotifyFunction may re-enable interrupts during its execution, and > that the caller must be prepared to be re-entered before NotifyFunction > returns > > * the timer interrupt must have been rearmed before calling NotifyFunction > > * the NotifyFunction must guarantee that it never reaches a state in > which the TPL has been restored to the original level with CPU > interrupts enabled. I agree with you about the above PI spec clarifications. Would you like to write a PI spec ECR? But I do not think the PI spec version stored in the PI system table needs to reflect whether a DxeCore implementation follows the clarification. Since the DxeCore::CoreTimerTick() implementation raises TPL to HIGH in the very first version created in more than 10 years ago, I think it's safe for TimerInterruptHandler() assumes CoreTimerTick() will raise TPL to HIGH so that TimerInterruptHandler() does not need to raise TPL to HIGH. (I agree changing the spec version is the most correct way if we review the problem in a very theorical way.) I really want to keep the UEFI world simple with the bug fixed. (The cost is assumption on existing DxeCore::CoreTimerTick().) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114409): https://edk2.groups.io/g/devel/message/114409 Mute This Topic: https://groups.io/mt/103950154/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
