+/**
+ Timer Interrupt Handler.
+
+ @param InterruptType The type of interrupt that occured
+ @param SystemContext A pointer to the system context when the interrupt
occured
+**/
+VOID
+EFIAPI
+TimerInterruptHandler (
+ IN EFI_EXCEPTION_TYPE InterruptType,
+ IN EFI_SYSTEM_CONTEXT SystemContext
+ )
+{
+ EFI_TPL OriginalTPL;
+ UINT64 RiscvTimer;
+
+ OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+ if (mTimerNotifyFunction != NULL) {
+ mTimerNotifyFunction (mTimerPeriod);
+ }
+
+ RiscVDisableTimerInterrupt (); // Disable SMode timer int
+ RiscVClearPendingTimerInterrupt ();
+ if (mTimerPeriod == 0) {
+ gBS->RestoreTPL (OriginalTPL);
+ RiscVDisableTimerInterrupt (); // Disable SMode timer int
+ return;
+ }
+
+ RiscvTimer = RiscVReadTimer ();
+ SbiSetTimer (RiscvTimer += mTimerPeriod);
+ gBS->RestoreTPL (OriginalTPL);
+ RiscVEnableTimerInterrupt (); // enable SMode timer int
+}
This design looks as though it does not support nested timer interrupts.
The call to RestoreTPL() may invoke callbacks that may themselves
include delay loops that wait upon further timer interrupts. With the
above code, those timer interrupts will never arrive since the timer
interrupt is disabled at the point that you call RestoreTPL().
This will break device drivers such as those for USB network devices
that rely on nested timer interrupts.
The easiest fix is to use NestedTimerInterruptLib and change the code as
follows:
Add at the start of the function:
STATIC NESTED_INTERRUPT_STATE NestedInterruptState;
Change:
OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
to
OriginalTPL = NestedInterruptRaiseTPL ();
Change:
gBS->RestoreTPL (OriginalTPL);
RiscVEnableTimerInterrupt (); // enable SMode timer int
to
RiscVEnableTimerInterrupt (); // enable SMode timer int
NestedInterruptRestoreTPL (OriginalTpl, SystemContext,
&NestedInterruptState);
Note that the timer interrupt is then correctly re-enabled before the
call to NestedInterruptRestoreTPL(). NestedInterruptTplLib takes care
of the mess required behind the scenes to make this all provably safe.
You'll also need to extend Library/NestedInterruptTplLib/Iret.c to
support MDE_CPU_RISCV64: it should be fairly obvious what's needed there.
See commit https://github.com/tianocore/edk2/commit/a086f4a63b for the
equivalent fix being applied to OVMF.
You can find the background leading up to the creation of
NestedInterruptTplLib at:
https://bugzilla.tianocore.org/show_bug.cgi?id=4162
https://github.com/tianocore/edk2/commit/9bf473da4c
https://github.com/tianocore/edk2/commit/a24fbd6061
https://github.com/tianocore/edk2/commit/a086f4a63b
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99868): https://edk2.groups.io/g/devel/message/99868
Mute This Topic: https://groups.io/mt/96593498/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
