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.web11.9951.1583322459121726982 for ; Wed, 04 Mar 2020 03:47:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=MZ2cZJps; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id q8so2017941wrm.4 for ; Wed, 04 Mar 2020 03:47:38 -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=5al+9l5SFUmqgW8IhXJ+U2ERhFvdD7KOE5Kvz1GrDBA=; b=MZ2cZJps3+0O0QFrevK+ZZ28dmScTvSF/KwbDElzHQtyWPnYVrlf/HgGDH87QjS0ki EKb4yuvrchiI8jO5tM+gePGN3kgV9PUoCL8uBhxJZpBEym3cCkfBKtBiISkevSBhHNsz VTq0ZutldUEaQCJJm1WzZHKH3eSYSIzrB00jdMdJPSsaqRo6hIJycIO0Eg3cUvVh9Odv jtYUrc0OiSKX+GxcJIulzqAIG3srhFvBz3jmkvw4ahHYyR+8ZlZmD5f4Nt/V1FbgYZ53 q6X+dwwYbN0+c1p+wF2fb/OwUPyoABEqr7tLFH7QxgnGLyVxYlWWU0KLcUaK/nxzALai yjXg== 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=5al+9l5SFUmqgW8IhXJ+U2ERhFvdD7KOE5Kvz1GrDBA=; b=TCk5L1XgJnOTUjYxHd9W1e0FmkCBxqtr6WRJouey/kL6HYdRYqGi+6UVCfgerILEih e7oTz46r1x2nUHYEqiyyCANDEKpdwl/wSwFOwuNkXT7UsTvrR9oPbKw5ZQIM7Thle3zU NqJlyw4mBMuYfkbTDF+yBz5/KWSw5NLqjOb/J7LsqymRn4PuX9DJ1ihwpnw6pUdd4bQS MBGsxflm/+dJsnTdU8ixGoX5f+t6/KecUq24/3jhwFzuve+4zfzEUFUd5714G+Ymx9fw mGGlNIFYRSDaN9clDtdkYhAZmbduSW3K5iEdLHDPjDF18bvtOcILWNUc1EskXi7J2wIN u+zg== X-Gm-Message-State: ANhLgQ21a2LNerogoX10R2QUYKCha9NWjxRZe+xqvY2sgHWY/bSfb8rQ MHiUd+ogL9aJRl9TQzwAJh3gGw== X-Google-Smtp-Source: ADFU+vu8pl4cBgxSgZFHm57NG5yQhWBRxjnjiD0/gsjZOmn0Gi23uBNdJeLBH4vY28AAran6Zat+UQ== X-Received: by 2002:adf:df82:: with SMTP id z2mr3647260wrl.46.1583322457566; Wed, 04 Mar 2020 03:47:37 -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 m21sm3736995wmi.27.2020.03.04.03.47.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Mar 2020 03:47:37 -0800 (PST) Date: Wed, 4 Mar 2020 11:47:35 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: devel@edk2.groups.io Subject: Re: [PATCH edk2-platforms 1/1] Platform/ARM/ArmShellCmdRunAxf: switch to by-VA cache maintenance Message-ID: <20200304114735.GR23627@bivouac.eciton.net> References: <20200304114250.6540-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200304114250.6540-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 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 Reviewed-by: Leif Lindholm 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 > + > +// 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 > + > +// 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 > > +#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,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 >