From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web09.10713.1583491836811343851 for ; Fri, 06 Mar 2020 02:50:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=CT5hZJgC; spf=pass (domain: nuviainc.com, ip: 209.85.128.67, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f67.google.com with SMTP id a141so1863730wme.2 for ; Fri, 06 Mar 2020 02:50:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=IzvsJH8wX6/IrbAw2zm4DwyZAxTWwKgwT0M0ViLT0Mg=; b=CT5hZJgCjcyDkBIu8A7p4R9qGAvn7YaJysRjygi/yoscYU5Bd5mdTyOkrK8HdLkvV1 6YFQwrOdOtZ/3nyRKHcQzX/CQOla+7DA0mQyy3uIkVxNaOgr8vGQwovJ4erc7WbSiU5M NbA/3z5YeToCl4VY2mbwA3KC5NB3luoTnzmzCEWTwKj+bvHfFO9F7/4kBbD+1Z0bstfd IF2RpZ4tglP1WDqwsx+zd0S8FPuM1DnVsy0TCB0j7aRWkgh5R4HzrEgnjaEko5aWKlt9 FLI6/cuSrDmJ99MVWHURHRQu3MOjqnYpk3jR/rUzpWgZ5/pZvfteNXdR5nLzN0XLrkyG gPgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=IzvsJH8wX6/IrbAw2zm4DwyZAxTWwKgwT0M0ViLT0Mg=; b=R7isLAAA06XTnynpDq+f7ClDMc5nJV7CM2Of8Ggss/CBIFDgtKjLQ1rD4SM4zfyMdv AWwgEpDKGxVqhmn0piGGJUplblb+e19KXrj4v5Hqg1qxRzqgut0N4Q8/fiMrH/xXpKoD ELQvm91VpPxsiu7EZZsewgw8YJiYe1YpFFAEAgIKvwaY1dfazI9h6iNTAQ191hbgxHVp pwIp0pzULahuTN6DqTCSmLUvG3RQMsuVxpEoyjNZP81zIjyrKRPOI+oNO4CVNX9BSyh6 IyytI5d5Q1qIl62/XZIuJnSgQ1OrKU0Dfv98g1/H47HFLgWTbXpSamSmpgMZKll33BbX z7SA== X-Gm-Message-State: ANhLgQ07/Em81ReeYjut80wlJVF+/CjDCGuShSf4aFyxqmykoItvREYg sFIhGP4T2YWzPs1hANSGdwKofg== X-Google-Smtp-Source: ADFU+vuYvGkeJ8Z6fkQzJgYOZtoVcTO9d+135EW1TH3k1aFtaE03WKy2RBLSYqadwtfMDHC8AxbMHA== X-Received: by 2002:a7b:cd11:: with SMTP id f17mr3374074wmj.161.1583491835186; Fri, 06 Mar 2020 02:50:35 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id q6sm4691474wmj.6.2020.03.06.02.50.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Mar 2020 02:50:34 -0800 (PST) Date: Fri, 6 Mar 2020 10:50:33 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: devel@edk2.groups.io Subject: Re: [PATCH edk2-platforms v2 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance Message-ID: <20200306105032.GE23627@bivouac.eciton.net> References: <20200304175056.18107-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200304175056.18107-1-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > 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 >