Date
1 - 8 of 8
[edk2-staging/RiscV64QemuVirt PATCH V7 05/20] UefiCpuPkg: Add CpuTimerDxe module
|
Sunil V L
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
This DXE module initializes the timer interrupt handler and installs the Arch Timer protocol. Cc: Eric Dong <eric.dong@...> Cc: Ray Ni <ray.ni@...> Cc: Rahul Kumar <rahul1.kumar@...> Cc: Daniel Schaefer <git@...> Cc: Gerd Hoffmann <kraxel@...> Signed-off-by: Sunil V L <sunilvl@...> Acked-by: Abner Chang <abner.chang@...> --- UefiCpuPkg/UefiCpuPkg.dsc | 3 + UefiCpuPkg/CpuTimerDxe/CpuTimerDxe.inf | 51 ++++ UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.h | 177 ++++++++++++ UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.c | 294 ++++++++++++++++++++ UefiCpuPkg/CpuTimerDxe/CpuTimer.uni | 14 + UefiCpuPkg/CpuTimerDxe/CpuTimerExtra.uni | 12 + 6 files changed, 551 insertions(+) diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index f9a46089d2c7..96f6770281fe 100644 --- a/UefiCpuPkg/UefiCpuPkg.dsc +++ b/UefiCpuPkg/UefiCpuPkg.dsc @@ -195,5 +195,8 @@ [Components.IA32, Components.X64] [Components.X64] UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf +[Components.RISCV64] + UefiCpuPkg/CpuTimerDxe/CpuTimerDxe.inf + [BuildOptions] *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES diff --git a/UefiCpuPkg/CpuTimerDxe/CpuTimerDxe.inf b/UefiCpuPkg/CpuTimerDxe/CpuTimerDxe.inf new file mode 100644 index 000000000000..d7706328b591 --- /dev/null +++ b/UefiCpuPkg/CpuTimerDxe/CpuTimerDxe.inf @@ -0,0 +1,51 @@ +## @file +# Timer Arch protocol module +# +# Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR> +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x0001001b + BASE_NAME = CpuTimerDxe + MODULE_UNI_FILE = CpuTimer.uni + FILE_GUID = 055DDAC6-9142-4013-BF20-FC2E5BC325C9 + MODULE_TYPE = DXE_DRIVER + VERSION_STRING = 1.0 + ENTRY_POINT = TimerDriverInitialize +# +# The following information is for reference only and not required by the build +# tools. +# +# VALID_ARCHITECTURES = RISCV64 +# +[Packages] + MdePkg/MdePkg.dec + UefiCpuPkg/UefiCpuPkg.dec + +[LibraryClasses] + BaseLib + DebugLib + IoLib + CpuLib + UefiBootServicesTableLib + UefiDriverEntryPoint + +[LibraryClasses.RISCV64] + RiscVSbiLib + +[Sources.RISCV64] + RiscV64/Timer.h + RiscV64/Timer.c + +[Protocols] + gEfiCpuArchProtocolGuid ## CONSUMES + gEfiTimerArchProtocolGuid ## PRODUCES + +[Depex] + gEfiCpuArchProtocolGuid + +[UserExtensions.TianoCore."ExtraFiles"] + CpuTimerExtra.uni diff --git a/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.h b/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.h new file mode 100644 index 000000000000..586eb0cfadb4 --- /dev/null +++ b/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.h @@ -0,0 +1,177 @@ +/** @file + RISC-V Timer Architectural Protocol definitions + + Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR> + + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#ifndef TIMER_H_ +#define TIMER_H_ + +#include <PiDxe.h> + +#include <Protocol/Cpu.h> +#include <Protocol/Timer.h> + +#include <Library/UefiBootServicesTableLib.h> +#include <Library/BaseLib.h> +#include <Library/DebugLib.h> +#include <Library/IoLib.h> + +// +// RISC-V use 100us timer. +// The default timer tick duration is set to 10 ms = 10 * 1000 * 10 100 ns units +// +#define DEFAULT_TIMER_TICK_DURATION 100000 + +extern VOID +RiscvSetTimerPeriod ( + UINT32 TimerPeriod + ); + +// +// Function Prototypes +// + +/** + Initialize the Timer Architectural Protocol driver + + @param ImageHandle ImageHandle of the loaded driver + @param SystemTable Pointer to the System Table + + @retval EFI_SUCCESS Timer Architectural Protocol created + @retval EFI_OUT_OF_RESOURCES Not enough resources available to initialize driver. + @retval EFI_DEVICE_ERROR A device error occured attempting to initialize the driver. + +**/ +EFI_STATUS +EFIAPI +TimerDriverInitialize ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +; + +/** + + This function adjusts the period of timer interrupts to the value specified + by TimerPeriod. If the timer period is updated, then the selected timer + period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is returned. + If an error occurs while attempting to update the timer period, then the + timer hardware will be put back in its state prior to this call, and + EFI_DEVICE_ERROR is returned. If TimerPeriod is 0, then the timer interrupt + is disabled. This is not the same as disabling the CPU's interrupts. + Instead, it must either turn off the timer hardware, or it must adjust the + interrupt controller so that a CPU interrupt is not generated when the timer + interrupt fires. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param NotifyFunction The rate to program the timer interrupt in 100 nS units. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is + returned. If the timer is programmable, then the timer period + will be rounded up to the nearest timer period that is supported + by the timer hardware. If TimerPeriod is set to 0, then the + timer interrupts will be disabled. + + @retval EFI_SUCCESS The timer period was changed. + @retval EFI_UNSUPPORTED The platform cannot change the period of the timer interrupt. + @retval EFI_DEVICE_ERROR The timer period could not be changed due to a device error. + +**/ +EFI_STATUS +EFIAPI +TimerDriverRegisterHandler ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + IN EFI_TIMER_NOTIFY NotifyFunction + ) +; + +/** + + This function adjusts the period of timer interrupts to the value specified + by TimerPeriod. If the timer period is updated, then the selected timer + period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is returned. + If an error occurs while attempting to update the timer period, then the + timer hardware will be put back in its state prior to this call, and + EFI_DEVICE_ERROR is returned. If TimerPeriod is 0, then the timer interrupt + is disabled. This is not the same as disabling the CPU's interrupts. + Instead, it must either turn off the timer hardware, or it must adjust the + interrupt controller so that a CPU interrupt is not generated when the timer + interrupt fires. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param TimerPeriod The rate to program the timer interrupt in 100 nS units. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is + returned. If the timer is programmable, then the timer period + will be rounded up to the nearest timer period that is supported + by the timer hardware. If TimerPeriod is set to 0, then the + timer interrupts will be disabled. + + @retval EFI_SUCCESS The timer period was changed. + @retval EFI_UNSUPPORTED The platform cannot change the period of the timer interrupt. + @retval EFI_DEVICE_ERROR The timer period could not be changed due to a device error. + +**/ +EFI_STATUS +EFIAPI +TimerDriverSetTimerPeriod ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + IN UINT64 TimerPeriod + ) +; + +/** + + This function retrieves the period of timer interrupts in 100 ns units, + returns that value in TimerPeriod, and returns EFI_SUCCESS. If TimerPeriod + is NULL, then EFI_INVALID_PARAMETER is returned. If a TimerPeriod of 0 is + returned, then the timer is currently disabled. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param TimerPeriod A pointer to the timer period to retrieve in 100 ns units. If + 0 is returned, then the timer is currently disabled. + + @retval EFI_SUCCESS The timer period was returned in TimerPeriod. + @retval EFI_INVALID_PARAMETER TimerPeriod is NULL. + +**/ +EFI_STATUS +EFIAPI +TimerDriverGetTimerPeriod ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + OUT UINT64 *TimerPeriod + ) +; + +/** + + This function generates a soft timer interrupt. If the platform does not support soft + timer interrupts, then EFI_UNSUPPORTED is returned. Otherwise, EFI_SUCCESS is returned. + If a handler has been registered through the EFI_TIMER_ARCH_PROTOCOL.RegisterHandler() + service, then a soft timer interrupt will be generated. If the timer interrupt is + enabled when this service is called, then the registered handler will be invoked. The + registered handler should not be able to distinguish a hardware-generated timer + interrupt from a software-generated timer interrupt. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + + @retval EFI_SUCCESS The soft timer interrupt was generated. + @retval EFI_UNSUPPORTEDT The platform does not support the generation of soft timer interrupts. + +**/ +EFI_STATUS +EFIAPI +TimerDriverGenerateSoftInterrupt ( + IN EFI_TIMER_ARCH_PROTOCOL *This + ) +; + +#endif diff --git a/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.c b/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.c new file mode 100644 index 000000000000..db153f715e60 --- /dev/null +++ b/UefiCpuPkg/CpuTimerDxe/RiscV64/Timer.c @@ -0,0 +1,294 @@ +/** @file + RISC-V Timer Architectural Protocol + + Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR> + + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include <Library/BaseLib.h> +#include <Library/BaseRiscVSbiLib.h> +#include "Timer.h" + +// +// The handle onto which the Timer Architectural Protocol will be installed +// +STATIC EFI_HANDLE mTimerHandle = NULL; + +// +// The Timer Architectural Protocol that this driver produces +// +EFI_TIMER_ARCH_PROTOCOL mTimer = { + TimerDriverRegisterHandler, + TimerDriverSetTimerPeriod, + TimerDriverGetTimerPeriod, + TimerDriverGenerateSoftInterrupt +}; + +// +// Pointer to the CPU Architectural Protocol instance +// +EFI_CPU_ARCH_PROTOCOL *mCpu; + +// +// The notification function to call on every timer interrupt. +// A bug in the compiler prevents us from initializing this here. +// +STATIC EFI_TIMER_NOTIFY mTimerNotifyFunction; + +// +// The current period of the timer interrupt +// +STATIC UINT64 mTimerPeriod = 0; + +/** + 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 function registers the handler NotifyFunction so it is called every time + the timer interrupt fires. It also passes the amount of time since the last + handler call to the NotifyFunction. If NotifyFunction is NULL, then the + handler is unregistered. If the handler is registered, then EFI_SUCCESS is + returned. If the CPU does not support registering a timer interrupt handler, + then EFI_UNSUPPORTED is returned. If an attempt is made to register a handler + when a handler is already registered, then EFI_ALREADY_STARTED is returned. + If an attempt is made to unregister a handler when a handler is not registered, + then EFI_INVALID_PARAMETER is returned. If an error occurs attempting to + register the NotifyFunction with the timer interrupt, then EFI_DEVICE_ERROR + is returned. + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param NotifyFunction The function to call when a timer interrupt fires. This + function executes at TPL_HIGH_LEVEL. The DXE Core will + register a handler for the timer interrupt, so it can know + how much time has passed. This information is used to + signal timer based events. NULL will unregister the handler. + + @retval EFI_SUCCESS The timer handler was registered. + @retval EFI_UNSUPPORTED The platform does not support timer interrupts. + @retval EFI_ALREADY_STARTED NotifyFunction is not NULL, and a handler is already + registered. + @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not + previously registered. + @retval EFI_DEVICE_ERROR The timer handler could not be registered. + +**/ +EFI_STATUS +EFIAPI +TimerDriverRegisterHandler ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + IN EFI_TIMER_NOTIFY NotifyFunction + ) +{ + DEBUG ((DEBUG_INFO, "TimerDriverRegisterHandler(0x%lx) called\n", NotifyFunction)); + mTimerNotifyFunction = NotifyFunction; + return EFI_SUCCESS; +} + +/** + + This function adjusts the period of timer interrupts to the value specified + by TimerPeriod. If the timer period is updated, then the selected timer + period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is returned. + If an error occurs while attempting to update the timer period, then the + timer hardware will be put back in its state prior to this call, and + EFI_DEVICE_ERROR is returned. If TimerPeriod is 0, then the timer interrupt + is disabled. This is not the same as disabling the CPU's interrupts. + Instead, it must either turn off the timer hardware, or it must adjust the + interrupt controller so that a CPU interrupt is not generated when the timer + interrupt fires. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param TimerPeriod The rate to program the timer interrupt in 100 nS units. If + the timer hardware is not programmable, then EFI_UNSUPPORTED is + returned. If the timer is programmable, then the timer period + will be rounded up to the nearest timer period that is supported + by the timer hardware. If TimerPeriod is set to 0, then the + timer interrupts will be disabled. + + @retval EFI_SUCCESS The timer period was changed. + @retval EFI_UNSUPPORTED The platform cannot change the period of the timer interrupt. + @retval EFI_DEVICE_ERROR The timer period could not be changed due to a device error. + +**/ +EFI_STATUS +EFIAPI +TimerDriverSetTimerPeriod ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + IN UINT64 TimerPeriod + ) +{ + UINT64 RiscvTimer; + + DEBUG ((DEBUG_INFO, "TimerDriverSetTimerPeriod(0x%lx)\n", TimerPeriod)); + + if (TimerPeriod == 0) { + mTimerPeriod = 0; + RiscVDisableTimerInterrupt (); // Disable SMode timer int + return EFI_SUCCESS; + } + + mTimerPeriod = TimerPeriod / 10; // convert unit from 100ns to 1us + RiscvTimer = RiscVReadTimer (); + SbiSetTimer (RiscvTimer + mTimerPeriod); + + mCpu->EnableInterrupt (mCpu); + RiscVEnableTimerInterrupt (); // enable SMode timer int + return EFI_SUCCESS; +} + +/** + + This function retrieves the period of timer interrupts in 100 ns units, + returns that value in TimerPeriod, and returns EFI_SUCCESS. If TimerPeriod + is NULL, then EFI_INVALID_PARAMETER is returned. If a TimerPeriod of 0 is + returned, then the timer is currently disabled. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + @param TimerPeriod A pointer to the timer period to retrieve in 100 ns units. If + 0 is returned, then the timer is currently disabled. + + @retval EFI_SUCCESS The timer period was returned in TimerPeriod. + @retval EFI_INVALID_PARAMETER TimerPeriod is NULL. + +**/ +EFI_STATUS +EFIAPI +TimerDriverGetTimerPeriod ( + IN EFI_TIMER_ARCH_PROTOCOL *This, + OUT UINT64 *TimerPeriod + ) +{ + *TimerPeriod = mTimerPeriod; + return EFI_SUCCESS; +} + +/** + + This function generates a soft timer interrupt. If the platform does not support soft + timer interrupts, then EFI_UNSUPPORTED is returned. Otherwise, EFI_SUCCESS is returned. + If a handler has been registered through the EFI_TIMER_ARCH_PROTOCOL.RegisterHandler() + service, then a soft timer interrupt will be generated. If the timer interrupt is + enabled when this service is called, then the registered handler will be invoked. The + registered handler should not be able to distinguish a hardware-generated timer + interrupt from a software-generated timer interrupt. + + + @param This The EFI_TIMER_ARCH_PROTOCOL instance. + + @retval EFI_SUCCESS The soft timer interrupt was generated. + @retval EFI_UNSUPPORTEDT The platform does not support the generation of soft timer interrupts. + +**/ +EFI_STATUS +EFIAPI +TimerDriverGenerateSoftInterrupt ( + IN EFI_TIMER_ARCH_PROTOCOL *This + ) +{ + return EFI_SUCCESS; +} + +/** + Initialize the Timer Architectural Protocol driver + + @param ImageHandle ImageHandle of the loaded driver + @param SystemTable Pointer to the System Table + + @retval EFI_SUCCESS Timer Architectural Protocol created + @retval EFI_OUT_OF_RESOURCES Not enough resources available to initialize driver. + @retval EFI_DEVICE_ERROR A device error occured attempting to initialize the driver. + +**/ +EFI_STATUS +EFIAPI +TimerDriverInitialize ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + EFI_STATUS Status; + + // + // Initialize the pointer to our notify function. + // + mTimerNotifyFunction = NULL; + + // + // Make sure the Timer Architectural Protocol is not already installed in the system + // + ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiTimerArchProtocolGuid); + + // + // Find the CPU architectural protocol. + // + Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu); + ASSERT_EFI_ERROR (Status); + + // + // Force the timer to be disabled + // + Status = TimerDriverSetTimerPeriod (&mTimer, 0); + ASSERT_EFI_ERROR (Status); + + // + // Install interrupt handler for RISC-V Timer. + // + Status = mCpu->RegisterInterruptHandler (mCpu, EXCEPT_RISCV_TIMER_INT, TimerInterruptHandler); + ASSERT_EFI_ERROR (Status); + + // + // Force the timer to be enabled at its default period + // + Status = TimerDriverSetTimerPeriod (&mTimer, DEFAULT_TIMER_TICK_DURATION); + ASSERT_EFI_ERROR (Status); + + // + // Install the Timer Architectural Protocol onto a new handle + // + Status = gBS->InstallMultipleProtocolInterfaces ( + &mTimerHandle, + &gEfiTimerArchProtocolGuid, + &mTimer, + NULL + ); + ASSERT_EFI_ERROR (Status); + return Status; +} diff --git a/UefiCpuPkg/CpuTimerDxe/CpuTimer.uni b/UefiCpuPkg/CpuTimerDxe/CpuTimer.uni new file mode 100644 index 000000000000..76de1f3f352a --- /dev/null +++ b/UefiCpuPkg/CpuTimerDxe/CpuTimer.uni @@ -0,0 +1,14 @@ +// /** @file +// +// Timer Arch protocol strings. +// +// Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All rights reserved.<BR> +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// +// **/ + + +#string STR_MODULE_ABSTRACT #language en-US "Timer driver that provides Timer Arch protocol" + +#string STR_MODULE_DESCRIPTION #language en-US "Timer driver that provides Timer Arch protocol." diff --git a/UefiCpuPkg/CpuTimerDxe/CpuTimerExtra.uni b/UefiCpuPkg/CpuTimerDxe/CpuTimerExtra.uni new file mode 100644 index 000000000000..ceb93a7ce82f --- /dev/null +++ b/UefiCpuPkg/CpuTimerDxe/CpuTimerExtra.uni @@ -0,0 +1,12 @@ +// /** @file +// Timer Localized Strings and Content +// +// Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All rights reserved.<BR> +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// +// **/ + +#string STR_PROPERTIES_MODULE_NAME +#language en-US +"Timer DXE Driver" -- 2.38.0 |
|
|
|
Michael D Kinney
Hi Sunil,
toggle quoted message
Show quoted text
One comment on the name of this module. CpuTimerDxe is a very generic name, but the Implementation is for RISCV only and depends on SBL services. Correct? If I look at other TimerDxe modules, they have in their name some indication of the timer hardware/software dependencies in their name, * LocalApicTimerDxe.inf * HpetTimerDxe.inf How about RiscVTimerDxe or RiscVSbiTimerLib? Mike -----Original Message----- |
|
|
|
Sunil V L
On Wed, Feb 08, 2023 at 06:02:44PM +0000, Kinney, Michael D wrote:
Hi Sunil,Thanks!, Mike. Your suggestion makes sense. Since most of the drivers in UefiCpuPkg were named as Cpu*, I named it like this. Let me change it to RiscVTimerDxe. Thanks! Sunil |
|
|
|
Ni, Ray
I am curious if this module will support other arch as well? If it's RiscV specific, there is no need for creating the RiscV64 folder.
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Michael Brown
+/**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 |
|
|
|
Sunil V L
On Thu, Feb 09, 2023 at 10:17:56AM +0000, Michael Brown wrote:
Hi Michael!,+/**This design looks as though it does not support nested timer interrupts. Thanks a lot for this feedback and background. We are aware of few issues in this module. Currently, it is mostly porting what exists today in edk2-platforms repo. We want to add all these additional fixes after this basic thing is merged. That way we will have git history instead of combining all fixes single commit. Andrei has a patch ready and waiting for this to get merged. We can either combine this with his patch or create one more. Would that strategy be fine with you? Thanks! Sunil The easiest fix is to use NestedTimerInterruptLib and change the code as |
|
|
|
Michael Brown
On 09/02/2023 10:28, Sunil V L wrote:
Sure, as long as someone other than me is keeping track of the need to fix this bug. My work here is done. :)Thanks a lot for this feedback and background. We are aware of few issues+ gBS->RestoreTPL (OriginalTPL);This design looks as though it does not support nested timer interrupts. Thanks, Michael |
|
|
|
Sunil V L
On Thu, Feb 09, 2023 at 10:30:58AM +0000, Michael Brown wrote:
On 09/02/2023 10:28, Sunil V L wrote:Sure, I will keep track of this. We will need your help to review the changes.Sure, as long as someone other than me is keeping track of the need to fixThanks a lot for this feedback and background. We are aware of few issues+ gBS->RestoreTPL (OriginalTPL);This design looks as though it does not support nested timer interrupts. Hope that is fine. Thanks! Sunil |
|
|