From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web12.15803.1583513660229670141 for ; Fri, 06 Mar 2020 08:54:20 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=Rp73uSt4; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id y17so3177353wrn.6 for ; Fri, 06 Mar 2020 08:54:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OgYdbmIZU7oo9WXzwfU1mEVEcrXP6/YQclRm/AAjoFc=; b=Rp73uSt45M9dHwXjcb7AFGKi0CdZg58RH6TZAQ9oJdlnuouy1Tb4hreKP9v5m1bbex mv6F9yBMwUEgABV12zDDkDrxgAyp2HXvuaiVJV2TJZSqqCo1krcD7fxE3hv9vKK8NB3L ZTV7twxmmg9Iv92ZGVtJzkPcxRMNE7fraFC/5lD1B/LYIa+e0ogDiNIfaIx74WbD6qAH d3VJ2I8FY1vk84O0N1n7u3G6TfWQsm7G8VAKxhh4PyYAbeAv4ZsCclL1iX1YBF95V+xT VJbBjRDgu78rZg1bswfZLhr/vUOdNGrJ+ppwZuPK9c/1vQ03sJbz1W0PADR6unnr2KNF feZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OgYdbmIZU7oo9WXzwfU1mEVEcrXP6/YQclRm/AAjoFc=; b=nFo751GN2Wl3fjNx8z88l1xjUncyWbxhA478LHh4M3laNEImAKwIcHV/6Vdrlyz4Qu unH3gfTpOc/z8MZmuirk4KDB7Kvs7xh5ha5oIs9InksT48fplhXrsTW68hhRIF+MRHwj /0Mu2dPQxVHjqJF7CDq4C+XqF602XSZV2CKZqpu/BV6AOCZjEQwJK0AO7P8IfT6uKV1I Dmd1iNtV6NExXkBH23hgdXi8VoW5M/Ksr5GM5todoMNwI7GIu0v8s4YWxcM3r1yzML38 ibqUCdHqNl3FlWOVTiue8IPvRzIYNbfGa6z8Y5IZlnJLYdUdYttsDkw99sJ2mz4qzthQ Y5Tg== X-Gm-Message-State: ANhLgQ2QOwE6LlRq5EWwePuDbh6dwN/StKQdJcZKCIde3JAbrS6pDezQ PJVm89/XhXkAobjunYdFVVv482TUOPjSBIkkXAKqVA== X-Google-Smtp-Source: ADFU+vtB4QwVNuayXbsZ4graTJhLWsOw+YW7oxUIevSz1qgepri3Hi8ZoPGwwJxyjQX0WuBWD9aaLgGQwd3JeHIYAmc= X-Received: by 2002:a5d:6051:: with SMTP id j17mr4882949wrt.151.1583513658593; Fri, 06 Mar 2020 08:54:18 -0800 (PST) MIME-Version: 1.0 References: <20200304175056.18107-1-ard.biesheuvel@linaro.org> <20200306105032.GE23627@bivouac.eciton.net> In-Reply-To: <20200306105032.GE23627@bivouac.eciton.net> From: "Ard Biesheuvel" Date: Fri, 6 Mar 2020 17:54:07 +0100 Message-ID: Subject: Re: [PATCH edk2-platforms v2 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance To: Leif Lindholm Cc: edk2-devel-groups-io Content-Type: text/plain; charset="UTF-8" On Fri, 6 Mar 2020 at 11:50, Leif Lindholm wrote: > > 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 > > --- > > 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 > > Thanks! Pushed as 828fcbf96a38..996047695a06 > > 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 > > + > > +// 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 > > + > > +// 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 > > > > +#include > > #include > > #include > > #include > > @@ -20,6 +21,8 @@ > > > > #include > > > > +#include > > + > > #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 > >