* [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance
@ 2020-03-04 11:42 Ard Biesheuvel
2020-03-04 11:47 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 11:42 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
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>
---
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);
+
// 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance
2020-03-04 11:42 [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance Ard Biesheuvel
@ 2020-03-04 11:47 ` Leif Lindholm
2020-03-04 11:52 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2020-03-04 11:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel
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.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Thanks!
> ---
> 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);
> +
> // 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance
2020-03-04 11:47 ` Leif Lindholm
@ 2020-03-04 11:52 ` Ard Biesheuvel
2020-03-04 11:56 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 11:52 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io
On Wed, 4 Mar 2020 at 12:47, Leif Lindholm <leif@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.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif@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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance
2020-03-04 11:52 ` Ard Biesheuvel
@ 2020-03-04 11:56 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 11:56 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io
On Wed, 4 Mar 2020 at 12:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 4 Mar 2020 at 12:47, Leif Lindholm <leif@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.biesheuvel@linaro.org>
> >
> > Reviewed-by: Leif Lindholm <leif@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
> > > +
... and add a
bl ArmDisableDataCache
here
(in both instances)
> > > + 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
> > >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-04 11:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-04 11:42 [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance Ard Biesheuvel
2020-03-04 11:47 ` Leif Lindholm
2020-03-04 11:52 ` Ard Biesheuvel
2020-03-04 11:56 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox