From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web09.7284.1606984767451465825 for ; Thu, 03 Dec 2020 00:39:27 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cTKP7PjR; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1606984766; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ESX+OjxCB1GyabvZOHbep21BKBkqMFIZyaTXPAdLFNo=; b=cTKP7PjR2yHM2QBaxakKCUPQ9CAAg3b8FAtBkeTg/HrHWMT75CXZmuzDfZhyGdwWLzt6sq nqmxiW4dIxyoJu1pxHTTuZCJOsCM6EQfEpp9WNvgnz8BNMzNuDv7vUIk8C2lbbPDAiJWKq ELoFkrEsYqjHIGhfModiBQaq96SiA3g= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-182-I5Z0bgVEMMKuG9JPWPCVgQ-1; Thu, 03 Dec 2020 03:39:22 -0500 X-MC-Unique: I5Z0bgVEMMKuG9JPWPCVgQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C29C1185E489; Thu, 3 Dec 2020 08:39:20 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-182.ams2.redhat.com [10.36.113.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id 91D2519D9C; Thu, 3 Dec 2020 08:39:17 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 3/6] OvmfPkg/AmdSev: add Grub Firmware Volume Package To: devel@edk2.groups.io, jejb@linux.ibm.com Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" , Jordan Justen , Ard Biesheuvel References: <20201130202819.3910-1-jejb@linux.ibm.com> <20201130202819.3910-4-jejb@linux.ibm.com> From: "Laszlo Ersek" Message-ID: <3ba8cd32-c914-b5f0-7044-3ee91b3edd6a@redhat.com> Date: Thu, 3 Dec 2020 09:39:16 +0100 MIME-Version: 1.0 In-Reply-To: <20201130202819.3910-4-jejb@linux.ibm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/30/20 21:28, James Bottomley wrote: > This is used to package up the grub bootloader into a firmware volume > where it can be executed as a shell like the UEFI Shell. Grub itself > is built as a minimal entity into a Fv and then added as a boot > option. By default the UEFI shell isn't built but for debugging > purposes it can be enabled and will then be presented as a boot option > (This should never be allowed for secure boot in an external data > centre but may be useful for local debugging). Finally all other boot > options except grub and possibly the shell are stripped and the boot > timeout forced to 0 so the system will not enter a setup menu and will > only boot to grub. This is done by copying the > Library/PlatformBootManagerLib into Library/PlatformBootManagerLibGrub > and then customizing it. > > Boot failure is fatal to try to prevent secret theft. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3077 > Signed-off-by: James Bottomley > > --- > > v2: strip out s3 and qemu boot contacts make grub script robust and > don't build grub.efi each time > v3: add copyrights, untabify grub.sh, strip more from PlatformBootLibGrub. > --- > OvmfPkg/OvmfPkg.dec | 1 + > OvmfPkg/AmdSev/AmdSevX64.dsc | 21 +- > OvmfPkg/AmdSev/AmdSevX64.fdf | 7 +- > OvmfPkg/AmdSev/Grub/Grub.inf | 39 + > .../PlatformBootManagerLibGrub.inf | 71 + > .../PlatformBootManagerLibGrub/BdsPlatform.h | 175 ++ > .../PlatformBootManagerLibGrub/BdsPlatform.c | 1482 +++++++++++++++++ > .../PlatformBootManagerLibGrub/PlatformData.c | 214 +++ > OvmfPkg/AmdSev/Grub/.gitignore | 1 + > OvmfPkg/AmdSev/Grub/grub.cfg | 46 + > OvmfPkg/AmdSev/Grub/grub.sh | 93 ++ > 11 files changed, 2141 insertions(+), 9 deletions(-) > create mode 100644 OvmfPkg/AmdSev/Grub/Grub.inf > create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf > create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h > create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c > create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformData.c > create mode 100644 OvmfPkg/AmdSev/Grub/.gitignore > create mode 100644 OvmfPkg/AmdSev/Grub/grub.cfg > create mode 100644 OvmfPkg/AmdSev/Grub/grub.sh [...] > +VOID > +EFIAPI > +PlatformBootManagerBeforeConsole ( > + VOID > + ) > +{ > + EFI_HANDLE Handle; > + EFI_STATUS Status; > + UINT16 FrontPageTimeout = 0; (1) initialization of local variables is not permitted in the edk2 coding style; but we can fix this up at merge time. (also now I understand why you kept gEfiGlobalVariableGuid and UefiRuntimeServicesTableLib) > + > + DEBUG ((DEBUG_INFO, "PlatformBootManagerBeforeConsole\n")); > + InstallDevicePathCallback (); > + > + VisitAllInstancesOfProtocol (&gEfiPciRootBridgeIoProtocolGuid, > + ConnectRootBridge, NULL); > + > + // > + // Signal the ACPI platform driver that it can download QEMU ACPI tables. > + // > + EfiEventGroupSignal (&gRootBridgesConnectedEventGroupGuid); > + > + // > + // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers > + // the preparation of S3 system information. That logic has a hard dependency > + // on the presence of the FACS ACPI table. Since our ACPI tables are only > + // installed after PCI enumeration completes, we must not trigger the S3 save > + // earlier, hence we can't signal End-of-Dxe earlier. > + // > + EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid); > + > + // > + // Prevent further changes to LockBoxes or SMRAM. > + // > + Handle = NULL; > + Status = gBS->InstallProtocolInterface (&Handle, > + &gEfiDxeSmmReadyToLockProtocolGuid, EFI_NATIVE_INTERFACE, > + NULL); > + ASSERT_EFI_ERROR (Status); > + > + // > + // Dispatch deferred images after EndOfDxe event and ReadyToLock > + // installation. > + // > + EfiBootManagerDispatchDeferredImages (); > + > + PlatformInitializeConsole (gPlatformConsole); > + > + Status = gRT->SetVariable ( > + EFI_TIME_OUT_VARIABLE_NAME, > + &gEfiGlobalVariableGuid, > + (EFI_VARIABLE_NON_VOLATILE | > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS), > + sizeof FrontPageTimeout, > + &FrontPageTimeout > + ); > + // > + // Install both VIRTIO_DEVICE_PROTOCOL and (dependent) EFI_RNG_PROTOCOL > + // instances on Virtio PCI RNG devices. > + // > + VisitAllInstancesOfProtocol (&gEfiPciIoProtocolGuid, ConnectVirtioPciRng, > + NULL); > +} [...] with (1) fixed: Reviewed-by: Laszlo Ersek Thanks! Laszlo