Re: [edk2-devel] [PATCH v3 5/5] MdeModulePkg: Extend NestedInterruptTplLib to support Arm CPUs

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 23 Jan 2024 at 16:31, Michael Brown <mcb30@ipxe.org> wrote:
>
> The only architecture-specific portion of NestedInterruptTplLib is in
> Iret.c, which must manipulate the interrupt stack frame such that the
> return-from-interrupt instruction will not re-enable interrupts.  The
> remaining logic in Tpl.c is architecture-agnostic.
>
> Add implementations of DisableInterruptsOnIret() for MDE_CPU_ARM and
> MDE_CPU_AARCH64.  In both cases, the saved IRQs-disabled and
> FIQs-disabled flags are set in the stored processor status register
> (matching the behaviour of DisableInterrupts(), which also sets both
> flags).
>
> Tested by patching ArmPkg's TimerDxe to use NestedInterruptTplLib and
> verifying that ArmVirtQemu passes the NestedInterruptTplLib self-tests
> for both ARM and AARCH64.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Michael Brown <mcb30@ipxe.org>

Thanks for this.

You can drop the FIQ bits, though: anything that can run Tianocore
will have the FIQs routed to the secure world, and all of the higher
level en/disable interrupt code only reasons about the IRQ line.

With that,

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  MdeModulePkg/MdeModulePkg.dsc                  |  2 +-
>  .../NestedInterruptTplLib.inf                  |  3 +++
>  .../Library/NestedInterruptTplLib/Iret.c       | 18 ++++++++++++++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 5b9ddfd26e75..4565b8e1b6e7 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -462,6 +462,7 @@ [Components.IA32, Components.X64, Components.AARCH64]
>
>  [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
>    MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
> +  MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
>    MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
>    MdeModulePkg/Core/Dxe/DxeMain.inf {
> @@ -526,7 +527,6 @@ [Components.IA32, Components.X64]
>    MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.inf
>    MdeModulePkg/Library/TraceHubDebugSysTLib/PeiTraceHubDebugSysTLib.inf
>    MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTLib.inf
> -  MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>
>  [Components.X64]
>    MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> diff --git a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> index e67d899b9446..1501f067d77f 100644
> --- a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> @@ -27,6 +27,9 @@ [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>
> +[Packages.ARM, Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> diff --git a/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
> index f6b2c51b6cc1..87cb74566730 100644
> --- a/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
> +++ b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
> @@ -9,6 +9,10 @@
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>
> +#if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_ARM)
> +#include <Library/ArmLib.h>
> +#endif
> +
>  #include "Iret.h"
>
>  /**
> @@ -54,6 +58,20 @@ DisableInterruptsOnIret (
>    Eflags.Bits.IF                          = 0;
>    SystemContext.SystemContextIa32->Eflags = Eflags.UintN;
>
> + #elif defined (MDE_CPU_AARCH64)
> +
> +  //
> +  // Set IRQ-disabled and FIQ-disabled flags.
> +  //
> +  SystemContext.SystemContextAArch64->SPSR |= (SPSR_I | SPSR_F);
> +
> + #elif defined (MDE_CPU_ARM)
> +
> +  //
> +  // Set IRQ-disabled and FIQ-disabled flags.
> +  //
> +  SystemContext.SystemContextArm->CPSR |= (CPSR_IRQ | CPSR_FIQ);
> +
>   #else
>
>    #error "Unsupported CPU"
> --
> 2.43.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114212): https://edk2.groups.io/g/devel/message/114212
Mute This Topic: https://groups.io/mt/103911611/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-