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.web09.10010.1583322774421084160 for ; Wed, 04 Mar 2020 03:52:54 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=QlNRooRo; 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 j7so2005316wrp.13 for ; Wed, 04 Mar 2020 03:52:54 -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=MbGIhd29WtVxAtHRROv2h88r1XgZ5tdKaInNf81Gcos=; b=QlNRooRou7XD4zWl28l4e8A3C4aikh9f9N0xR9DU2aYfVFA1QsceyGRsHo/1tz5RyN fNtL1K+E0RJIda3D9sbtNIkAV1jVhx6LiO69Mx8/2/Vh0x6vtkw+2NDZZ+t9vXwzwV6n jqJJyCWlJTpvkaPgXekZVMhSxoaiVPEkfNrjlWgKMil9Rr+Wizq8zoF6fZP3MCrGn4zY IAiOegBkJGD7I5xrczRAMKNo0eF0cg9Q/LMMoCs7L/vGEto/lMUvv5Qvgb+NDogYeWPw tMbuwWsTx/Y0n72aauPY/X4BD2mlST/KgHIaeYn02VVe2c9X4DdR59KJ4rrViCqi42Dx 1kaQ== 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=MbGIhd29WtVxAtHRROv2h88r1XgZ5tdKaInNf81Gcos=; b=VPzGHWYvcChoHqDVmtdSVMfeJJfde33x8DP5cO6d79EDibNdpj5vdA17iizpUpgvpr G1FF5UUWiXsmPZJSn5eJWS46dqsU00c10E6Z3ZYq5izcPsAJmIrlUFnCdl6TXx7yG97f l6ucnmC6KvA1cD/w9FE7kfrjQjhSJvn8KcGG/H83a1QGFwklKSpOQKqN4Y+da+igTRQX 7BUOXEN9MNEMKIcAqVtYExVrwfoipIOzQEoCoqwfYzZeWtCYJni13A4pFXaH+J5/emNb 5OLlkQ2l51uzY5j+SMsB1G+T6DX+T1vemb7kyaxt0cxX5fhVFnTcnRohCDQkCgNGClHF 1u9w== X-Gm-Message-State: ANhLgQ03KeCwdKnauJZRhTd/t9gR8c1ZQdUI0wnZL3yCn1qKDyWWdFJh XVRa3Y4hNukXUwVOU91oS6ET/pIRLdCfixUHCKtUlw== X-Google-Smtp-Source: ADFU+vtuhSzIB4Xr5ZXrTEiMJUyPxMLUP7oBgyqK9MI2t1LnKO3H5XBVxd9P8wRJdPc/tQxq6k/CJqCAHKxz9jFNvds= X-Received: by 2002:a05:6000:110b:: with SMTP id z11mr3874150wrw.252.1583322772763; Wed, 04 Mar 2020 03:52:52 -0800 (PST) MIME-Version: 1.0 References: <20200304114250.6540-1-ard.biesheuvel@linaro.org> <20200304114735.GR23627@bivouac.eciton.net> In-Reply-To: <20200304114735.GR23627@bivouac.eciton.net> From: "Ard Biesheuvel" Date: Wed, 4 Mar 2020 12:52:41 +0100 Message-ID: Subject: Re: [PATCH edk2-platforms 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 Wed, 4 Mar 2020 at 12:47, Leif Lindholm 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 > > Reviewed-by: Leif Lindholm 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 > > + > > +// 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); > > + 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 > >