From: "Leif Lindholm" <leif@nuviainc.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: devel@edk2.groups.io
Subject: Re: [PATCH edk2-platforms v2 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance
Date: Fri, 6 Mar 2020 10:50:33 +0000 [thread overview]
Message-ID: <20200306105032.GE23627@bivouac.eciton.net> (raw)
In-Reply-To: <20200304175056.18107-1-ard.biesheuvel@linaro.org>
On Wed, Mar 04, 2020 at 18:50:56 +0100, Ard Biesheuvel wrote:
> Currently, the 'runaxf' shell command that exists only on ARM's own
> development platforms deals with the caches in an unsafe manner, as
> it relies on set/way maintenance to ensure that the ELF image it has
> put into memory, as well as the running image itself is visible in
> memory after the MMU and caches are disabled.
>
> So let's switch to by-VA maintenance for the currently running image,
> as well as the ELF image, and use a helper in assembly to ensure that
> we are not relying on the stack between the disabling of the MMU and
> the invocation of the ELF image.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2:
> - call ArmDisableMmu() directly from the pivot code, instead of via an
> indirect branch
> - move ArmDisableDataCache() call into the pivot code as well
> - perform I-cache invalidation on the loaded ELF image, but leave the
> I-cache enabled
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S | 41 +++++++++
> Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S | 41 +++++++++
> Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf | 8 ++
> Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c | 89 +++++++++-----------
> 4 files changed, 131 insertions(+), 48 deletions(-)
>
> diff --git a/Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S b/Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S
> new file mode 100644
> index 000000000000..1ab4b15f127b
> --- /dev/null
> +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S
> @@ -0,0 +1,41 @@
> +//
> +// Copyright (c) 2020, ARM Limited. All rights reserved.
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +
> +#include <AsmMacroIoLibV8.h>
> +
> +// VOID
> +// RunAxfPivot (
> +// IN ELF_ENTRYPOINT ElfEntry
> +// IN UINTN Arg0,
> +// IN UINTN Arg1,
> +// IN UINTN Arg2,
> +// IN UINTN Arg3
> +// );
> +ASM_FUNC(RunAxfPivot)
> + // Preserve ElfEntry() and its arguments
> + // Since we will not be returning from this function, we can clobber
> + // callee preserved register instead.
> + mov x19, x0
> + mov x20, x1
> + mov x21, x2
> + mov x22, x3
> + mov x23, x4
> +
> + bl ArmDisableDataCache
> + bl ArmDisableMmu
> +
> + // Load ElfEntry()'s arguments into x0...x3
> + mov x0, x20
> + mov x1, x21
> + mov x2, x22
> + mov x3, x23
> +
> + // Call ElfEntry()
> + blr x19
> +
> +0:wfi
> + wfe
> + b 0b
> diff --git a/Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S b/Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S
> new file mode 100644
> index 000000000000..50efcfacdfe6
> --- /dev/null
> +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S
> @@ -0,0 +1,41 @@
> +//
> +// Copyright (c) 2020, ARM Limited. All rights reserved.
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +
> +#include <AsmMacroIoLibV8.h>
> +
> +// VOID
> +// RunAxfPivot (
> +// IN ELF_ENTRYPOINT ElfEntry
> +// IN UINTN Arg0,
> +// IN UINTN Arg1,
> +// IN UINTN Arg2,
> +// IN UINTN Arg3
> +// );
> +ASM_FUNC(RunAxfPivot)
> + // Preserve ElfEntry() and its arguments without using the stack.
> + // Since we will not be returning from this function, we can clobber
> + // callee preserved register instead.
> + mov r4, r0
> + mov r5, r1
> + mov r6, r2
> + mov r7, r3
> + pop {r8}
> +
> + bl ArmDisableDataCache
> + bl ArmDisableMmu
> +
> + // Load ElfEntry()'s arguments into x0...x3
> + mov r0, r5
> + mov r1, r6
> + mov r2, r7
> + mov r3, r8
> +
> + // Call ElfEntry()
> + blx r4
> +
> +0:wfi
> + wfe
> + b 0b
> diff --git a/Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf b/Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> index 74c6a0e84fdf..7c27a765bd5c 100644
> --- a/Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> @@ -27,6 +27,12 @@ [Sources.common]
> elf64.h
> elf_common.h
>
> +[Sources.AARCH64]
> + AArch64/Pivot.S
> +
> +[Sources.ARM]
> + Arm/Pivot.S
> +
> [Packages]
> ArmPkg/ArmPkg.dec
> MdeModulePkg/MdeModulePkg.dec
> @@ -37,11 +43,13 @@ [Packages]
> [LibraryClasses]
> ArmLib
> BaseLib
> + CacheMaintenanceLib
> DebugLib
> HiiLib
> ShellLib
>
> [Protocols]
> + gEfiLoadedImageProtocolGuid
> gEfiShellDynamicCommandProtocolGuid
>
> [Guids]
> diff --git a/Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c b/Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c
> index 153aed2ab6bd..dbad50ae866a 100644
> --- a/Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c
> +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c
> @@ -10,6 +10,7 @@
>
> #include <Guid/GlobalVariable.h>
>
> +#include <Library/CacheMaintenanceLib.h>
> #include <Library/PrintLib.h>
> #include <Library/HandleParsingLib.h>
> #include <Library/DevicePathLib.h>
> @@ -20,6 +21,8 @@
>
> #include <Library/ArmLib.h>
>
> +#include <Protocol/LoadedImage.h>
> +
> #include "ArmShellCmdRunAxf.h"
> #include "ElfLoader.h"
> #include "BootMonFsLoader.h"
> @@ -85,37 +88,21 @@ ShutdownUefiBootServices (
> return Status;
> }
>
> -
> -STATIC
> -EFI_STATUS
> -PreparePlatformHardware (
> - VOID
> - )
> -{
> - //Note: Interrupts will be disabled by the GIC driver when ExitBootServices() will be called.
> -
> - // Clean before Disable else the Stack gets corrupted with old data.
> - ArmCleanDataCache ();
> - ArmDisableDataCache ();
> - // Invalidate all the entries that might have snuck in.
> - ArmInvalidateDataCache ();
> -
> - // Disable and invalidate the instruction cache
> - ArmDisableInstructionCache ();
> - ArmInvalidateInstructionCache ();
> -
> - // Turn off MMU
> - ArmDisableMmu();
> -
> - return EFI_SUCCESS;
> -}
> -
> // Process arguments to pass to AXF?
> STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> {NULL, TypeMax}
> };
>
>
> +VOID
> +RunAxfPivot (
> + IN ELF_ENTRYPOINT ElfEntry,
> + IN UINTN Arg0,
> + IN UINTN Arg1,
> + IN UINTN Arg2,
> + IN UINTN Arg3
> + );
> +
> /**
> This is the shell command handler function pointer callback type. This
> function handles the command when it is invoked in the shell.
> @@ -139,23 +126,23 @@ ShellDynCmdRunAxfHandler (
> IN EFI_SHELL_PROTOCOL *Shell
> )
> {
> - LIST_ENTRY *ParamPackage;
> - EFI_STATUS Status;
> - SHELL_STATUS ShellStatus;
> - SHELL_FILE_HANDLE FileHandle;
> - ELF_ENTRYPOINT StartElf;
> - CONST CHAR16 *FileName;
> - EFI_FILE_INFO *Info;
> - UINTN FileSize;
> - VOID *FileData;
> - VOID *Entrypoint;
> - LIST_ENTRY LoadList;
> - LIST_ENTRY *Node;
> - LIST_ENTRY *NextNode;
> - RUNAXF_LOAD_LIST *LoadNode;
> - CHAR16 *TmpFileName;
> - CHAR16 *TmpChar16;
> -
> + LIST_ENTRY *ParamPackage;
> + EFI_STATUS Status;
> + SHELL_STATUS ShellStatus;
> + SHELL_FILE_HANDLE FileHandle;
> + ELF_ENTRYPOINT StartElf;
> + CONST CHAR16 *FileName;
> + EFI_FILE_INFO *Info;
> + UINTN FileSize;
> + VOID *FileData;
> + VOID *Entrypoint;
> + LIST_ENTRY LoadList;
> + LIST_ENTRY *Node;
> + LIST_ENTRY *NextNode;
> + RUNAXF_LOAD_LIST *LoadNode;
> + CHAR16 *TmpFileName;
> + CHAR16 *TmpChar16;
> + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
>
> ShellStatus = SHELL_SUCCESS;
> FileHandle = NULL;
> @@ -291,6 +278,9 @@ ShellDynCmdRunAxfHandler (
> }
> }
>
> + WriteBackDataCacheRange (FileData, FileSize);
> + InvalidateInstructionCacheRange (FileData, FileSize);
> +
> // Program load list created.
> // Shutdown UEFI, copy and jump to code.
> if (!IsListEmpty (&LoadList) && !EFI_ERROR (Status)) {
> @@ -315,14 +305,17 @@ ShellDynCmdRunAxfHandler (
> Node = GetNextNode (&LoadList, Node);
> }
>
> - //
> - // Switch off interrupts, caches, mmu, etc
> - //
> - Status = PreparePlatformHardware ();
> + Status = gBS->HandleProtocol (gImageHandle, &gEfiLoadedImageProtocolGuid,
> + (VOID **)&LoadedImage);
> ASSERT_EFI_ERROR (Status);
>
> - StartElf = (ELF_ENTRYPOINT)Entrypoint;
> - StartElf (0,0,0,0);
> + //
> + // Ensure that the currently running image is clean to the PoC so we can
> + // safely keep executing it with the MMU and caches off
> + //
> + WriteBackDataCacheRange (LoadedImage->ImageBase, LoadedImage->ImageSize);
> +
> + RunAxfPivot (StartElf, 0, 0, 0, 0);
>
> // We should never get here.. But if we do, spin..
> ASSERT (FALSE);
> --
> 2.17.1
>
next prev parent reply other threads:[~2020-03-06 10:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-04 17:50 [PATCH edk2-platforms v2 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance Ard Biesheuvel
2020-03-06 10:50 ` Leif Lindholm [this message]
2020-03-06 16:54 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200306105032.GE23627@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox