From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web10.8609.1576512304693557736 for ; Mon, 16 Dec 2019 08:05:05 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=YWNGNc1P; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: leif.lindholm@linaro.org) Received: by mail-wr1-f67.google.com with SMTP id q10so7909271wrm.11 for ; Mon, 16 Dec 2019 08:05:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=BPXt7S095SQpY0xvBbTUPFAuPRjiMxaEe2XLfshLqYY=; b=YWNGNc1Pl30m336danSft0y9+YHJYovcvdGvAg9baOTr6xmkwIA2l26E7/TodIhIgw du009gs8mJ+nvq5yB5ff8gcaEGsMNaWWM7rjaYsqbQ2qK5QT96oclbPqjyYoA2+T72o4 VjNPPfXLnU+4pUlVC8tWHlwEyody1QGsz3jgEk3t7imm3QCVZg98BhgHIIUCuTTX4pIA g7M1PAMp60+mjK/T85uKjZ/Cs1Zi8TZN6lycZvpgVVQOWUqUuPllrG7Z7f5YRHkODuAW H72ci01m39RWwsKqwji6I3XTaVZ6NVu56qesD1h+BJKjViuT+uSN7jB28o9cpVMUUTKr nOHw== 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=BPXt7S095SQpY0xvBbTUPFAuPRjiMxaEe2XLfshLqYY=; b=gnI97d6udItrcrl1G8ra1ERDLaux8wkb4qZQzOHaHgKukc7ZAslMNk1GFWWwfu1KOZ +hEmz3sKxB6+e8QgYRND+yOoG1yuXLSfKX/tjfUUzkeL662WB+kXFvH/luUxngoxQf58 WvJGDZp2BQ8XpOxk3R4XuNbt18hSLy9IV6+Rlw36/DDcSAdVzmxkMjucZzMBgme45szI y2T39+ZGr6EJB0cs0d0I4KFuYDoTzyahffAkymG7yAvxf3WveJR3qpkWV3Rvm+2l+yBd R8qaYycttxtwK2yqKwfxjxTkJycZFTrIMiriltWuDGSf1H6vTLU20k96yIpT8iP+cro4 DO3g== X-Gm-Message-State: APjAAAWzzHBmYCv7mHnyGoCkWRruL/Yq0uM1v+VoN04vfUkkQQR3XypL TNZMygeS++iS2TE9UG9EWsDSpQ== X-Google-Smtp-Source: APXvYqz+dbjvREclB6D/fhyF+JNROQWJ2oNSt9CfZq/8JfWR5bWZtC/PFlkquGs13cmTDoWhCWcEwQ== X-Received: by 2002:adf:dfc2:: with SMTP id q2mr30118188wrn.251.1576512302801; Mon, 16 Dec 2019 08:05:02 -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 n67sm22025580wmb.8.2019.12.16.08.05.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Dec 2019 08:05:01 -0800 (PST) Date: Mon, 16 Dec 2019 16:05:00 +0000 From: "Leif Lindholm" To: Tanmay Jagdale Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, Radoslaw Biernacki Subject: Re: [PATCH V2] SbsaQemu: EFI implementation for SbsaQemu platform Message-ID: <20191216160500.GK7359@bivouac.eciton.net> References: <20191215121625.11893-1-tanmay.jagdale@linaro.org> MIME-Version: 1.0 In-Reply-To: <20191215121625.11893-1-tanmay.jagdale@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Dec 15, 2019 at 17:46:25 +0530, Tanmay Jagdale wrote: > From: Radoslaw Biernacki > > Linaro enterprise group is coordinating work for adding SBSA compliant > virtual platform for QEMU. This patch adds initial support for platform > with nondiscoverable AHCI, VGA and single DRAM window over 32bit > address space. > > Using FDF to compose EFI flash images with ATF images. > Flash0 (secure) is used by BL1 and FIP (BL2 + BL31). > Flash1 contains EFI code and EFI variables. > > Signed-off-by: Radoslaw Biernacki > Signed-off-by: Tanmay Jagdale > --- > This patch adds initial support for Sbsa QEMU model. There will be > following patches which will add support for ACPI tables. > > Changes in V2: > - Fixed capitalization in the Readme.md file > > Maintainers.txt | 12 + > Platform/Qemu/SbsaQemu/Readme.md | 159 ++++ > Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 677 ++++++++++++++++++ > Platform/Qemu/SbsaQemu/SbsaQemu.fdf | 303 ++++++++ > Readme.md | 3 + > .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 55 ++ > .../SbsaQemuPlatformDxe.inf | 41 ++ > .../Qemu/Library/SbsaQemuLib/SbsaQemuHelper.S | 55 ++ > .../Qemu/Library/SbsaQemuLib/SbsaQemuLib.c | 139 ++++ > .../Qemu/Library/SbsaQemuLib/SbsaQemuLib.inf | 47 ++ > .../Qemu/Library/SbsaQemuLib/SbsaQemuMem.c | 80 +++ > .../SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c | 40 ++ > .../SbsaQemuNorFlashLib.inf | 29 + > .../SbsaQemuPciHostBridgeLib.c | 215 ++++++ > .../SbsaQemuPciHostBridgeLib.inf | 47 ++ > Silicon/Qemu/SbsaQemuPkg.dec | 39 + > 16 files changed, 1941 insertions(+) > create mode 100644 Platform/Qemu/SbsaQemu/Readme.md > create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemu.dsc > create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemu.fdf > create mode 100644 Silicon/Qemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c > create mode 100644 Silicon/Qemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf > create mode 100644 Silicon/Qemu/Library/SbsaQemuLib/SbsaQemuHelper.S > create mode 100644 Silicon/Qemu/Library/SbsaQemuLib/SbsaQemuLib.c > create mode 100644 Silicon/Qemu/Library/SbsaQemuLib/SbsaQemuLib.inf > create mode 100644 Silicon/Qemu/Library/SbsaQemuLib/SbsaQemuMem.c > create mode 100644 Silicon/Qemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c > create mode 100644 Silicon/Qemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf > create mode 100644 Silicon/Qemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c > create mode 100644 Silicon/Qemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.inf > create mode 100644 Silicon/Qemu/SbsaQemuPkg.dec > > diff --git a/Maintainers.txt b/Maintainers.txt > index cb7ae0a95d..320b38d049 100644 > --- a/Maintainers.txt > +++ b/Maintainers.txt > @@ -257,3 +257,15 @@ F: Platform/Socionext/ > F: Silicon/Socionext/ > M: Ard Biesheuvel > M: Leif Lindholm > + > +Silicon/Qemu/ > +M: Ard Biesheuvel > +M: Leif Lindholm > +R: Radoslaw Biernacki > +R: Tanmay Jagdale > + > +Platform/Qemu/SbsaQemu > +M: Ard Biesheuvel > +M: Leif Lindholm > +R: Radoslaw Biernacki > +R: Tanmay Jagdale > diff --git a/Platform/Qemu/SbsaQemu/Readme.md b/Platform/Qemu/SbsaQemu/Readme.md > new file mode 100644 > index 0000000000..fc2bd5a7ff > --- /dev/null > +++ b/Platform/Qemu/SbsaQemu/Readme.md > @@ -0,0 +1,159 @@ > +# Overview > + > +This directory holds UEFI implementation for Sbsa-ref machine which is faithful > +as possible to real hardware. In opposition to existing QEMU Virt machine which > +is suited for performing workloads, the purpose of this machine is development > +of firmware and OS for AARCH64 server alike platforms (like in situation where > +real HW is not available yet or the debugging is easier to control under > +emulation). The SBSAQemu name, was chosen because the modeled HW is aimed to > +follow the way that server-style armv8 machines are recommended to be set up. > +Implementation does not use fw_cfg nor DT provided by QEMU. (I will refer back here from a comment below.) > + > +# How to build (Linux Environment) > + > +## Prerequisites > + > +Build process for Sbsa-ref uses FDF file for flash image composition. This is > +different to what some might expect as you need to first build the ATF before > +building EDK2. > +Flash0 (secure) is used by BL1 and FIP (BL2 + BL31). > +Flash1 contains EFI code and EFI variables. > +For rest of the build process, typical prerequisites apply. > + > +## Obtaining source code > + > +1. Create a new directory on your local development machine for use as your > + workspace. This example uses `~/workspace`, modify as appropriate for your > + needs. In the next steps we will use `WORKSPACE` environment variable for > + reference to this directory. > + > + ``` > + cd ~/ > + mkdir workspace > + cd workspace > + export WORKSPACE=$PWD > + ``` We don't need this detailed instructions - top-level should tell us how to build EDK2 - just point out where the .dsc lives and that the build requires edk2-non-osi in PACKAGES_PATH for TF-A. > + > +1. Into that folder, clone: > + 1. [qemu](https://github.com/qemu/qemu.git) > + 2. [edk2](https://github.com/tianocore/edk2) > + 3. [edk2-platforms](https://github.com/tianocore/edk2-platforms) > + 4. [edk2-non-osi](https://github.com/tianocore/edk2-non-osi) Correct. > + 5. [atf](https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git) > + > + ``` > + cd $WORKSPACE > + git clone https://github.com/qemu/qemu.git > + git clone https://github.com/tianocore/edk2 > + git clone https://github.com/tianocore/edk2-platforms > + git clone https://git.linaro.org/people/tanmay.jagdale/edk2-non-osi.git Not correct. (This is another reason why it's often not really helpful to duplicate basic build instructions.) > + git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git > + ``` > + For Edk2 we need to initialize sub-modules (like OpenSSL) > + ``` > + cd edk2 > + git submodule init > + git submodule update > + ``` > + > +## Manual building > + > +1. Compile QEMU > + > + Sbsa-ref machine is fully SW emulated SBSA machine (ARM64). It aims to emulate > + the real HW as close as possible. It's purpose it to enable new feature > + development when HW for those features are not yet present on the market. > + It allows poking the HW to do all kinds of things (including errors) which are > + beyond control on real HW. It also allow easy simulations and debugging of > + FW-to-HW interactions. > + > + Keep in mind that all of the above is possible as this machine is fully > + emulated in SW. Sbsa-ref machine does not use any HW acceleration of your > + platform, even if you run it on ARM64 platform. The EL3 (and ARM TrustZone) > + is also emulated in SW. On reading this, I think it would make a lot more sense to move this introductory text to the start of the file. This whole block could replace the current introduction statement (which goes into a bit too much detail on the development history of the platform). > + > + Sbsa-ref machine was added into QEMU in v4.1.0 > + If your distribution package provides an earlier version then you need to > + compile QEMU from the source. Below is a short instruction how to compile > + without referring to QEMU docs. > + The `qemu-inst` is the install directory for QEMU in `workspace`. > + Modify those as appropriate for your needs. > + > + ``` > + cd $WORKSPACE I don't think we need to proscribe where people install things on their local machine. Suggest replacing this with something like INSTALL_PATH set to /usr/local, ~/local or whatever. > + mkdir qemu-inst > + cd qemu > + mkdir -p build-native > + cd build-native > + ../configure --target-list=aarch64-softmmu --prefix=$WORKSPACE/qemu-inst --prefix=$INSTALL_PATH > + make -j $NUM_CPUS install Where is NUM_CPUS set? I think the -j bit is redundant - we're showing people how to build *this*, not how to drive make. (Applies throughout.) > + ``` > + > + QEMU should be installed now in `$WORKSPACE/qemu-inst` $INSTALL_PATH > + > +1. Compile ATF I think we're supposed to call it TF-A these days. > + > + This step is only needed is users want to compile a custom ATF binary. Else, the edk2-non-osi directory contains prebuilt bl1.bin and fip.bin binaries which will be automatically used in the build process. > + As noted before, for Sbsa-ref machine we use FDF to compose two flash images. Those flash images need BL1, BL2 and BL31 from ATF in form of two files `bl1.bin` and `fip.bin`. Follow the instructions below to get those artifacts. > + > + ``` > + cd $WORKSPACE/atf > + export CROSS_COMPILE=aarch64-linux-gnu- Please drop above line. Cross compilation is adequately described elsewhere. > + make PLAT=sbsa all fip > + ``` > + Than copy `bl1.bin` and `fip.bin` to the top `workspace` directory: > + ``` > + cp build/sbsa/release/bl1.bin ../ > + cp build/sbsa/release/fip.bin ../ > + ``` The above is now actively incorrect - if someone rebuilds their own TF-A images, they need to drop them into the edk2-non-osi directory(*). > + > +1. edk2-platforms - SBSA QEMU UEFI implementation > + > + Compilation of BaseTools and preparation: > + > + ``` > + cd $WORKSPACE > + export PACKAGES_PATH=$WORKSPACE/edk2:$WORKSPACE/edk2-platforms:$WORKSPACE/edk2-non-osi > + make -C edk2/BaseTools > + export GCC5_AARCH64_PREFIX=aarch64-linux-gnu- See previous comment about cross compilation - drop above line. > + . edk2/edksetup.sh > + ``` > + > + Compilation UEFI for Sbsa QEMU: > + > + ``` > + cd $WORKSPACE > + build -n $NUM_CPUS -b RELEASE -a AARCH64 -t GCC5 -p edk2-platforms/Platform/Qemu/SbsaQemu/SbsaQemu.dsc You can drop the -n $NUM_CPUS here. (Not for the same reason as for make above, the edk2 build system may be new to people, but simply because it now automates this anyway.) > + ``` > + Copy SBSA_FLASH0.fd and SBSA_FLASH0.fd to top `workspace` directory. Than extend the file size to match the machine flash size. > + ``` > + cp Build/SbsaQemu/RELEASE_GCC5/FV/SBSA_FLASH[01].fd . > + truncate -s 256M SBSA_FLASH[01].fd > + ``` > + > +1. Testing Testing is what *we* do before we submit patches. For anyone else, this step is called running. > + > + The resulting SBSA_FLASH0.fd file will contain Secure flash0 image (ATF code). > + The SBSA_FLASH1.fd will contain Non-secure UEFI code and UEFI variables. > + > + You will boot to the UEFI console with following QEMU command line: > + ``` > + qemu-inst/bin/qemu-system-aarch64 -m 1024 -cpu cortex-a57 -M sbsa-ref -pflash SBSA_FLASH0.fd -pflash SBSA_FLASH1.fd -serial stdio Drop the qemu-inst/... bit. Why is there a -cpu flag in there? That should be completely redundant for sbsa-ref. > + ``` > + You can add XHCI controller with keyboard and mouse by: > + ``` > + -device qemu-xhci -device usb-mouse -device usb-kbd > + ``` > + You can add the hard drive to platform AHCI controller by `hda` parameter: > + ``` > + -hda disk1.img > + ``` > + For TEE and other secure development you might get use of secure serial which would require following commands. First create `secure_serial` fifo and read it from separate terminal (open new terminal emulator window for it): > + ``` > + mkfifo secure_serial > + tail -f secure_serial > + ``` > + Than on first console: > + ``` > + qemu-system-aarch64 -m 1024 -M sbsa-ref -pflash SBSA_FLASH0.fd -pflash SBSA_FLASH1.fd -serial stdio -hda disk1.img -serial file:secure_serial > + ``` > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > new file mode 100644 > index 0000000000..34efa4d83f > --- /dev/null > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > @@ -0,0 +1,303 @@ > +# > +# Copyright (c) 2019, Linaro Limited. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +################################################################################ > +# > +# FD Section for FLASH0 > +# > +# The [FD] Section is made up of the definition statements and a > +# description of what goes into the Flash Device Image. Each FD section > +# defines one flash "device" image. A flash device image may be one of > +# the following: Removable media bootable image (like a boot floppy > +# image,) an Option ROM image (that would be "flashed" into an add-in > +# card,) a System "Flash" image (that would be burned into a system's > +# flash) or an Update ("Capsule") image that will be used to update and > +# existing system flash. > +# > +################################################################################ > + > +[FD.SBSA_FLASH0] > +BaseAddress = 0x00000000 > +Size = 0x00200000 > +ErasePolarity = 1 > +BlockSize = 0x00001000 > +NumBlocks = 0x200 > + > +################################################################################ > +# > +# Following are lists of FD Region layout which correspond to the locations of different > +# images within the flash device. > +# > +# Regions must be defined in ascending order and may not overlap. > +# > +# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by > +# the pipe "|" character, followed by the size of the region, also in hex with the leading > +# "0x" characters. Like: > +# Offset|Size > +# PcdOffsetCName|PcdSizeCName > +# RegionType > +# > +################################################################################ > +## Place for Trusted Firmware > +# flash0 is secure so we put here the BL1 > +0x00000000|0x00008000 > +FILE = bl1.bin What? No! This breaks the functionality that was there in the 4 December submission and completely ignores edk2-non-osi. Please go through and diff the two contributions and ensure *only* what should be different is. Then resubmit that, with above documentation feedback addressed, as v3. And locally, ensure each patchset revision has its own working branch - any other workflow will guarantee this sort of thing happening. Regards, Leif