From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web11.24360.1678466757775285935 for ; Fri, 10 Mar 2023 08:45:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=hFwQRI8h; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A3C8FB82355 for ; Fri, 10 Mar 2023 16:45:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 531D1C4339E for ; Fri, 10 Mar 2023 16:45:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1678466754; bh=84MonEiT87IFkvNi5zooJtS8HZbzSKGUzgKV9szNfhY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=hFwQRI8hjUfCYxtOgnY+NiSlJ/4Z9JpIhpYWPVGuKAdZsoRCsTUjeNr/roHy5u48w 7ZPJgndPHHAz7eGZ3l9PdS5KnM1r2AW1eGF49Ow607sR+oYJMtJXFAdwX5i25tMHK1 kpUKnMMH9087apyZX3TxgzaxoVEA9MV3ELBURGZAQ5FhNZBL4k3QXdhZJJ7KitNBPn 40EK5VMhdztd6YTDPzBtCxhA84ym/OTPwrr+Gu2RVXGVWLWcCNkMiIZWOzj+r5h02G ALy44eEs5bW6CKmbv9tsG5WvmwNhcYthCYoxVhgCev2LfsCOZ9Dtlvmw8r0WzeRBhv E2sJx0T7a1X/w== Received: by mail-lf1-f50.google.com with SMTP id m6so7408218lfq.5 for ; Fri, 10 Mar 2023 08:45:54 -0800 (PST) X-Gm-Message-State: AO0yUKWmvu+c+IzDIHoZU+8jdBkCr8R12clhIleO0xLdxqtvYLXOxzZG PkfAKuuJjMbhak5YqNS41bJHfiRmMhRGIbvc1L4= X-Google-Smtp-Source: AK7set9yGDkPcge9QvW9g+gA/7VJDecM6HPx+MJNNIGV8TPCbOVuQejkGdAAU2nbPnL5v6h7Hd6ZnfV0OQ4mJ6eLvAw= X-Received: by 2002:ac2:54b9:0:b0:4d8:62e5:4f66 with SMTP id w25-20020ac254b9000000b004d862e54f66mr8195522lfk.7.1678466752260; Fri, 10 Mar 2023 08:45:52 -0800 (PST) MIME-Version: 1.0 References: <20230306002205.1640-1-nathaniel.l.desimone@intel.com> In-Reply-To: <20230306002205.1640-1-nathaniel.l.desimone@intel.com> From: "Ard Biesheuvel" Date: Fri, 10 Mar 2023 17:45:41 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v1 0/6] Add Raspberry Pi Support to EmulatorPkg To: devel@edk2.groups.io, nathaniel.l.desimone@intel.com Cc: Andrew Fish , Ray Ni , Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Michael D Kinney , Chasel Chiu Content-Type: text/plain; charset="UTF-8" Hello Nate, On Mon, 6 Mar 2023 at 01:22, Nate DeSimone wrote: > > This patch series is the result of a fun weekend project that I > did in my spare time. It implements the changes nessesary for > EmulatorPkg to run on the Raspberry Pi. It should also work on > other 32-bit ARM systems, but I only specifically tested on > Raspberry Pi OS (formerly Raspbian.) > It works fine on 64-bit ARM systems that support 32-bit user space as well. I managed to build and run this on my Qualcomm Snapdragon based Lenovo Yoga C630 laptop running Debian Linux. So Tested-by: Ard Biesheuvel Also, good job!! This is really nice. One nit: I had to add -mfpu=vfpv3 to the compiler command line in Host.inf to work around a build error. > I ran into several interesting issues during the development of > this patch series and it ended up being a good learning > experience for me. Some things of note are: > > 1 - Assembly Code > > There are several pieces of assembly code in EmulatorPkg that need > to be rewritten when porting to a new machine architecture. This > code fell into two categories, stack manipulation and "gaskets." > > 2 - ABI Differences > > The most significant amount of assembly code is the "gasket" > functions. The gasket functions are responsible for converting from > the UNIX ABI to the EFI ABI. They enable EmulatorPkg to > support executing arbitary UEFI binaries without needing > recompilation specifically for EmulatorPkg. X86 has many ABI > specifications that vary based on target OS and compiler toolchain. > There have been several attempts to unify things on the x86 side, > but they usually just result in the addition of yet another ABI. > Things are much more uniform on ARM due to wide acceptance of the > AAPCS. Due to this, the ABI differences between UNIX and EFI boil > down to passing of variadic arguments and enum values. Neither of > these cases apply to the function signatures for the gaskets. > Therefore, on ARM these gaskets can be simple wrapper functions > written in C since the UNIX and EFI ABIs are identical... at least > for the set of functions that need gaskets. > Glad to hear that. I assume this would apply to AArch64 as well. > 3 - Instruction Cache Maintenance > > Because EmulatorPkg and edk2 in general contains a PE/COFF loader, > it counts as "self-modifying code". The way self modifying code is > handled on ARM is considerably more complex than x86. While on x86 > there is no requirement for self-modifying code to flush the > instruction cache, on ARM failure to do so will result in incorrect > behavior. Additionally, flushing the cache is considerably more > difficult on ARM. On x86, one can simply add a CLFLUSH or WBINVD > instruction; they are perfectly valid to execute while in Ring 3. > On ARM, flushing the cache requires one to write to system control > registers, which can only be done in EL1 (kernel mode) or higher. > Therefore, while flushing the cache can be done on in the x86 > version of EmulatorPkg without any interaction with the OS, on ARM > we need to invoke OS syscalls. > > To accomodate this, I have added a new EMU_IO_THUNK_PROTOCOL that > implements the CacheMaintenanceLib LibraryClass. This new > implementation uses the GCC intrinsic function > __builtin_clear_cache(), which on x86 gets converted into a CLFLUSH > instruction by the compiler. On ARM, it gets converted into a call > to the __clear_cache() API in libgcc. The ARM implementation of > libgcc invokes the ARM_cacheflush syscall (0xF0002), which only > exists in kernels built for the ARM architecture. > > I have added wrapper implementations of the CacheMaintenanceLib > LibraryClass for PEI and DXE that use the EMU_THUNK infrastructure > to acquire the EMU_CACHE_THUNK_PROTOCOL and invoke the equivilent > functions, which in turn will use the GCC intrinsic mentioned > above. > > I have only enable this version of CacheMaintenanceLib on the ARM > architecture, because it will take several thousand CPU > instructions to execute, as opposed to about ten on the current x86 > implementation. > I think this is reasonable, but I am not a EmulatorPkg maintainer so I will defer to them to comment on this approach. > Testing Done: > > Boots to shell on Raspberry Pi OS "bullseye." > > Cc: Andrew Fish > Cc: Ray Ni > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Michael D Kinney > Cc: Chasel Chiu > Signed-off-by: Nate DeSimone > > Nate DeSimone (6): > ArmPkg: Add ArmMmuNullLib Given the recent focus on memory protections - could we turn this into a EmuPkg specific one that calls the OS's mprotect() under the hood? > EmulatorPkg: Add ARM Build Target > EmulatorPkg: Fix PosixFileSystem function misspellings > EmulatorPkg: Add ARM support to UNIX Host App > EmulatorPkg: Add ARM support to EmuSec > EmulatorPkg: Add EmuCacheMaintenanceLib > > ArmPkg/ArmPkg.dsc | 2 + > ArmPkg/Library/ArmMmuNullLib/ArmMmuNullLib.c | 84 ++ > .../Library/ArmMmuNullLib/ArmMmuNullLib.inf | 36 + > EmulatorPkg/EmulatorPkg.dec | 4 +- > EmulatorPkg/EmulatorPkg.dsc | 29 +- > EmulatorPkg/EmulatorPkg.fdf | 6 +- > EmulatorPkg/Include/Protocol/EmuCache.h | 217 ++++ > .../DxeEmuCacheMaintenanceLib.c | 337 +++++ > .../DxeEmuCacheMaintenanceLib.inf | 37 + > .../PeiEmuCacheMaintenanceLib.c | 344 ++++++ > .../PeiEmuCacheMaintenanceLib.inf | 39 + > EmulatorPkg/Sec/Arm/SwitchRam.S | 32 + > EmulatorPkg/Sec/Arm/TempRam.c | 58 + > EmulatorPkg/Sec/Sec.inf | 9 +- > EmulatorPkg/Unix/Host/Arm/Gasket.c | 895 ++++++++++++++ > .../Unix/Host/Arm/GasketFunctionDefinitions.h | 1092 +++++++++++++++++ > EmulatorPkg/Unix/Host/Arm/SwitchStack.S | 39 + > EmulatorPkg/Unix/Host/CacheMaintenance.c | 284 +++++ > EmulatorPkg/Unix/Host/Gasket.h | 12 +- > EmulatorPkg/Unix/Host/Host.c | 5 +- > EmulatorPkg/Unix/Host/Host.h | 2 + > EmulatorPkg/Unix/Host/Host.inf | 14 +- > EmulatorPkg/Unix/Host/Ia32/Gasket.S | 31 +- > EmulatorPkg/Unix/Host/PosixFileSystem.c | 22 +- > EmulatorPkg/Unix/Host/X64/Gasket.S | 31 +- > EmulatorPkg/build.sh | 13 +- > 26 files changed, 3617 insertions(+), 57 deletions(-) > create mode 100644 ArmPkg/Library/ArmMmuNullLib/ArmMmuNullLib.c > create mode 100644 ArmPkg/Library/ArmMmuNullLib/ArmMmuNullLib.inf > create mode 100644 EmulatorPkg/Include/Protocol/EmuCache.h > create mode 100644 EmulatorPkg/Library/DxeEmuCacheMaintenanceLib/DxeEmuCacheMaintenanceLib.c > create mode 100644 EmulatorPkg/Library/DxeEmuCacheMaintenanceLib/DxeEmuCacheMaintenanceLib.inf > create mode 100644 EmulatorPkg/Library/PeiEmuCacheMaintenanceLib/PeiEmuCacheMaintenanceLib.c > create mode 100644 EmulatorPkg/Library/PeiEmuCacheMaintenanceLib/PeiEmuCacheMaintenanceLib.inf > create mode 100644 EmulatorPkg/Sec/Arm/SwitchRam.S > create mode 100644 EmulatorPkg/Sec/Arm/TempRam.c > create mode 100644 EmulatorPkg/Unix/Host/Arm/Gasket.c > create mode 100644 EmulatorPkg/Unix/Host/Arm/GasketFunctionDefinitions.h > create mode 100644 EmulatorPkg/Unix/Host/Arm/SwitchStack.S > create mode 100644 EmulatorPkg/Unix/Host/CacheMaintenance.c > > -- > 2.30.2 > > > > > >