Re: [edk2-devel] [PATCH 1/4] OvmfPkg/Amdsev: Base commit to build encrypted boot specific OVMF

["Laszlo Ersek" <lersek@redhat.com>]

On 11/12/20 01:13, James Bottomley wrote:
> This commit represents the file copied from OvmfPkgX64 with minor
> changes to change the build name.
> 
> This package will form the basis for adding Sev specific features.
> Since everything must go into a single rom file for attestation, the
> separated build of code and variables is eliminated.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc | 1024 ++++++++++++++++++++++++++++++++++
>  OvmfPkg/AmdSev/AmdSevX64.fdf |  506 +++++++++++++++++
>  2 files changed, 1530 insertions(+)
>  create mode 100644 OvmfPkg/AmdSev/AmdSevX64.dsc
>  create mode 100644 OvmfPkg/AmdSev/AmdSevX64.fdf
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> new file mode 100644
> index 0000000000..d1dfb8742f
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -0,0 +1,1024 @@
> +## @file
> +#  EFI/Framework Open Virtual Machine Firmware (OVMF) platform for SEV

(1) I suggest / request that we put "remote attestation" somewhere in
the above file-top comment.

> +#
> +#  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##

(2) In every new file created in this series, please prepend an IBM
Copyright Notice, to the original (C) notices (if any).

> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = Ovmf
> +  PLATFORM_GUID                  = 5a9e7754-d81b-49ea-85ad-69eaa7b1539b

(3) Please generate a new PLATFORM_GUID for this new platform with
"uuidgen".

> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x00010005
> +  OUTPUT_DIRECTORY               = Build/AmdSev
> +  SUPPORTED_ARCHITECTURES        = X64
> +  BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = OvmfPkg/AmdSev/AmdSevX64.fdf
> +
> +  #
> +  # Defines for default states.  These can be changed on the command line.
> +  # -D FLAG=VALUE
> +  #
> +  DEFINE SECURE_BOOT_ENABLE      = FALSE
> +  DEFINE SMM_REQUIRE             = FALSE

(4) SEV-ES doesn't support (to my knowledge) SMM, so we should strip
everything dependent on SMM_REQUIRE being TRUE (DSC and FDF files both).

(5) Given that SMM cannot protect Secure Boot, SECURE_BOOT_ENABLE too
should be assumed FALSE, and stuff dependent on SECURE_BOOT_ENABLE being
TRUE should be stripped.

> +  DEFINE SOURCE_DEBUG_ENABLE     = FALSE
> +  DEFINE TPM_ENABLE              = FALSE
> +  DEFINE TPM_CONFIG_ENABLE       = FALSE
> +
> +  #
> +  # Network definition
> +  #
> +  DEFINE NETWORK_TLS_ENABLE             = FALSE
> +  DEFINE NETWORK_IP6_ENABLE             = FALSE
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = TRUE

(6) My understanding is that netboot with this platform is never
desired. If that's the case, then please remove:
- all the NETWORK_* flags,
- the dependent DSC/FDF snippets,
- the gEfiNetworkPkgTokenSpaceGuid.* PCD defaults,
- and (in particular) all !include directives that refer to NetworkPkg/*

My goal with the above trimming is two-fold:

- avoid an implication for platform builders that they can meaningfully
tweak the -D flags for this platform,

- cut down on the size of the new DSC/FDF files (given that the above
fruits seem to hang low).

Thanks
Laszlo