From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.45766.1683194974018837736 for ; Thu, 04 May 2023 03:09:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=DBLGzXVK; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683194972; 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: in-reply-to:in-reply-to:references:references; bh=PDEXi+KC8QPDtm5vlk56ZLuo+fsjxhEu3d9iq2Tgxws=; b=DBLGzXVKrnozMpYLUCzisUkpIP3N6+xBxPtosR7HgrHNXbwarzRFlZxM4fJjhwrpaf2deo KMkyk1/7/vHvQ6vGUr658zRS6tAvapHrEVOwJY4V8wq3zni9SJe0DgnM/yu12XgcJ82U9S +8uVycUaZsxo4S7ST1TJWlx1XMUY1UE= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-674-piTribsDMqeAZrQPgMj9mQ-1; Thu, 04 May 2023 06:09:31 -0400 X-MC-Unique: piTribsDMqeAZrQPgMj9mQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 574BA1C07548; Thu, 4 May 2023 10:09:31 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.48]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DEE7A492C13; Thu, 4 May 2023 10:09:30 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 6775C1800922; Thu, 4 May 2023 12:09:29 +0200 (CEST) Date: Thu, 4 May 2023 12:09:29 +0200 From: "Gerd Hoffmann" To: devel@edk2.groups.io Cc: Sebastien Boeuf , Pawel Polawski , Ard Biesheuvel , Jiewen Yao , Jordan Justen , Oliver Steffen Subject: Re: [PATCH v2 1/1] OvmfPkg: replace SECURE_BOOT_FEATURE_ENABLED with PcdSecureBootSupported Message-ID: References: <20230421065544.53525-1-kraxel@redhat.com> MIME-Version: 1.0 In-Reply-To: <20230421065544.53525-1-kraxel@redhat.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Ping. On Fri, Apr 21, 2023 at 08:55:44AM +0200, Gerd Hoffmann wrote: > Drop the '-D SECURE_BOOT_FEATURE_ENABLED' compile time option, > use a new FeaturePcd instead. > > Signed-off-by: Gerd Hoffmann > --- > OvmfPkg/OvmfPkg.dec | 3 +++ > OvmfPkg/CloudHv/CloudHvX64.dsc | 10 +--------- > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 10 +--------- > OvmfPkg/Microvm/MicrovmX64.dsc | 10 +--------- > OvmfPkg/OvmfPkgIa32.dsc | 10 +--------- > OvmfPkg/OvmfPkgIa32X64.dsc | 10 +--------- > OvmfPkg/OvmfPkgX64.dsc | 10 +--------- > OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf | 2 ++ > OvmfPkg/PlatformPei/PlatformPei.inf | 1 + > OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c | 11 ++++------- > OvmfPkg/PlatformPei/Platform.c | 7 ++++--- > 11 files changed, 20 insertions(+), 64 deletions(-) > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index 749fbd3b6bf4..03ae29e7b034 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -488,6 +488,9 @@ [PcdsFeatureFlag] > # used by OVMF, the varstore pflash chip, LockBox etc). > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|FALSE|BOOLEAN|0x1e > > + ## This feature flag indicates the firmware build supports secure boot. > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|FALSE|BOOLEAN|0x6d > + > ## Informs modules (including pre-DXE-phase modules) whether the platform > # firmware contains a CSM (Compatibility Support Module). > # > diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc > index cc2dd925bc94..2a1139daaa19 100644 > --- a/OvmfPkg/CloudHv/CloudHvX64.dsc > +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc > @@ -93,15 +93,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > !include NetworkPkg/NetworkBuildOptions.dsc.inc > > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > @@ -477,6 +468,7 @@ [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc > index f73440905540..d4403f11a7c6 100644 > --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc > +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc > @@ -90,15 +90,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D TDX_PEI_LESS_BOOT > GCC:*_*_*_CC_FLAGS = -D TDX_PEI_LESS_BOOT > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 > XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000 > @@ -387,6 +378,7 @@ [PcdsFeatureFlag] > gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE > !endif > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc > index e9aab515592f..6fc11cc4d192 100644 > --- a/OvmfPkg/Microvm/MicrovmX64.dsc > +++ b/OvmfPkg/Microvm/MicrovmX64.dsc > @@ -91,15 +91,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > !include NetworkPkg/NetworkBuildOptions.dsc.inc > > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > @@ -473,6 +464,7 @@ [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 86177bb94899..16916ec58247 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -94,15 +94,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > !include NetworkPkg/NetworkBuildOptions.dsc.inc > > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > @@ -484,6 +475,7 @@ [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 065b54450647..936d763269c1 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -98,15 +98,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > !include NetworkPkg/NetworkBuildOptions.dsc.inc > > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > @@ -490,6 +481,7 @@ [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 3d405cd4ade0..1c763b27def1 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -112,15 +112,6 @@ [BuildOptions] > INTEL:*_*_*_CC_FLAGS = /D TDX_GUEST_SUPPORTED > GCC:*_*_*_CC_FLAGS = -D TDX_GUEST_SUPPORTED > > - # > - # SECURE_BOOT_FEATURE_ENABLED > - # > -!if $(SECURE_BOOT_ENABLE) == TRUE > - MSFT:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - INTEL:*_*_*_CC_FLAGS = /D SECURE_BOOT_FEATURE_ENABLED > - GCC:*_*_*_CC_FLAGS = -D SECURE_BOOT_FEATURE_ENABLED > -!endif > - > !include NetworkPkg/NetworkBuildOptions.dsc.inc > > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] > @@ -511,6 +502,7 @@ [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > !if $(SECURE_BOOT_ENABLE) == TRUE > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE > !endif > > diff --git a/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf b/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf > index 8cda78d0d0b4..f152c5504661 100644 > --- a/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf > +++ b/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf > @@ -47,6 +47,8 @@ [LibraryClasses] > [Protocols] > gEfiSimpleFileSystemProtocolGuid ## CONSUMES > > +[Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported > > [Guids] > gEfiFileInfoGuid > diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf > index 1fadadeb5565..3934aeed9514 100644 > --- a/OvmfPkg/PlatformPei/PlatformPei.inf > +++ b/OvmfPkg/PlatformPei/PlatformPei.inf > @@ -94,6 +94,7 @@ [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase > gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr > gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize > + gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootSupported > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > diff --git a/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c b/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c > index 72289da35819..d4139b911528 100644 > --- a/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c > +++ b/OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c > @@ -28,14 +28,12 @@ ConnectNvVarsToFileSystem ( > IN EFI_HANDLE FsHandle > ) > { > - #ifdef SECURE_BOOT_FEATURE_ENABLED > - > - return EFI_UNSUPPORTED; > - > - #else > - > EFI_STATUS Status; > > + if (FeaturePcdGet (PcdSecureBootSupported)) { > + return EFI_UNSUPPORTED; > + } > + > // > // We might fail to load the variable, since the file system initially > // will not have the NvVars file. > @@ -52,7 +50,6 @@ ConnectNvVarsToFileSystem ( > } > > return Status; > - #endif > } > > /** > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c > index cc9384ba5c4e..c56247e294f2 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -222,9 +222,10 @@ ReserveEmuVariableNvStore ( > VariableStore = (EFI_PHYSICAL_ADDRESS)(UINTN)PlatformReserveEmuVariableNvStore (); > PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); > > - #ifdef SECURE_BOOT_FEATURE_ENABLED > - PlatformInitEmuVariableNvStore ((VOID *)(UINTN)VariableStore); > - #endif > + if (FeaturePcdGet (PcdSecureBootSupported)) { > + // restore emulated VarStore from pristine ROM copy > + PlatformInitEmuVariableNvStore ((VOID *)(UINTN)VariableStore); > + } > > ASSERT_RETURN_ERROR (PcdStatus); > } > -- > 2.40.0 > --