On Wed, 4 Mar 2020 at 12:47, Leif Lindholm <l...@nuviainc.com> wrote:
>
> On Wed, Mar 04, 2020 at 12:42:50 +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.biesheu...@linaro.org>
>
> Reviewed-by: Leif Lindholm <l...@nuviainc.com>
Thanks
One thing occurred to me though, please see below
> > ---
> > Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S | 40 +++++++++
> > Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S | 40 +++++++++
> > Platform/ARM/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf | 8 ++
> > Platform/ARM/Library/ArmShellCmdRunAxf/RunAxf.c | 91
> > +++++++++-----------
> > 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..dd7aa48f7a02
> > --- /dev/null
> > +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/AArch64/Pivot.S
> > @@ -0,0 +1,40 @@
> > +//
> > +// 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 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..edb3505e5cb9
> > --- /dev/null
> > +++ b/Platform/ARM/Library/ArmShellCmdRunAxf/Arm/Pivot.S
> > @@ -0,0 +1,40 @@
> > +//
> > +// 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 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..efad5c4525ac 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,11 @@ ShellDynCmdRunAxfHandler (
> > }
> > }
> >
> > + //
> > + // Clean the loaded file to the PoC
> > + //
> > + WriteBackDataCacheRange (FileData, FileSize);
> > +
Since we are no longer touching the I-cache now, I should do a
InvalidateInstructionCacheRange (FileData, FileSize);
here as well. Mind if I fold that in?
> > // Program load list created.
> > // Shutdown UEFI, copy and jump to code.
> > if (!IsListEmpty (&LoadList) && !EFI_ERROR (Status)) {
> > @@ -315,14 +307,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
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55402): https://edk2.groups.io/g/devel/message/55402
Mute This Topic: https://groups.io/mt/71724128/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
