* [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
@ 2017-01-16 12:22 Jiaxin Wu
2017-01-16 20:33 ` Laszlo Ersek
0 siblings, 1 reply; 9+ messages in thread
From: Jiaxin Wu @ 2017-01-16 12:22 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Justen Jordan L, Gary Lin, Long Qin, Wu Jiaxin
v2:
* Remove the flag for NetworkPkg/IScsiDxe
This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
the CryptoPkg librarie.
Not only the secure boot feature requires the CryptoPkg libraries
(e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
features. Those modules can be always included since no build performance
impacts if they are not consumed.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Gary Lin <glin@suse.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
3 files changed, 18 insertions(+), 33 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index e97f7f0..6e53d9f 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -1,9 +1,9 @@
## @file
# EFI/Framework Open Virtual Machine Firmware (OVMF) platform
#
-# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -139,14 +139,15 @@
ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
- PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+ PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
!endif
@@ -164,13 +165,11 @@
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
[LibraryClasses.common]
-!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-!endif
[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
!ifdef $(DEBUG_ON_SERIAL_PORT)
@@ -256,13 +255,13 @@
DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
!else
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
+
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
-!endif
+
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
[LibraryClasses.common.UEFI_DRIVER]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
@@ -698,16 +697,12 @@
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
NetworkPkg/IScsiDxe/IScsiDxe.inf
!else
- MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!else
MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
!endif
!if $(HTTP_BOOT_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 8e3e04c..15db2d5 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -1,9 +1,9 @@
## @file
# EFI/Framework Open Virtual Machine Firmware (OVMF) platform
#
-# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -144,14 +144,15 @@
ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
- PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+ PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
!endif
@@ -169,13 +170,11 @@
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
[LibraryClasses.common]
-!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-!endif
[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
!ifdef $(DEBUG_ON_SERIAL_PORT)
@@ -261,13 +260,13 @@
DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
!else
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
+
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
-!endif
+
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
[LibraryClasses.common.UEFI_DRIVER]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
@@ -707,16 +706,12 @@
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
NetworkPkg/IScsiDxe/IScsiDxe.inf
!else
- MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!else
MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
!endif
!if $(HTTP_BOOT_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 6ec3fe0..9c6bdc2 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1,9 +1,9 @@
## @file
# EFI/Framework Open Virtual Machine Firmware (OVMF) platform
#
-# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -144,14 +144,15 @@
ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
- PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+ PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
!endif
@@ -169,13 +170,11 @@
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
[LibraryClasses.common]
-!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-!endif
[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
!ifdef $(DEBUG_ON_SERIAL_PORT)
@@ -261,13 +260,13 @@
DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
!else
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
+
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
-!endif
+
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
[LibraryClasses.common.UEFI_DRIVER]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
@@ -705,16 +704,12 @@
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
NetworkPkg/IScsiDxe/IScsiDxe.inf
!else
- MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!else
MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
!endif
!if $(HTTP_BOOT_ENABLE) == TRUE
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-16 12:22 [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries Jiaxin Wu
@ 2017-01-16 20:33 ` Laszlo Ersek
2017-01-17 1:08 ` Wu, Jiaxin
2017-01-17 1:54 ` Jordan Justen
0 siblings, 2 replies; 9+ messages in thread
From: Laszlo Ersek @ 2017-01-16 20:33 UTC (permalink / raw)
To: Jiaxin Wu, edk2-devel; +Cc: Justen Jordan L, Gary Lin, Long Qin, Michael Kinney
On 01/16/17 13:22, Jiaxin Wu wrote:
> v2:
> * Remove the flag for NetworkPkg/IScsiDxe
>
> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> the CryptoPkg librarie.
>
> Not only the secure boot feature requires the CryptoPkg libraries
> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> features. Those modules can be always included since no build performance
> impacts if they are not consumed.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Justen Jordan L <jordan.l.justen@intel.com>
> Cc: Gary Lin <glin@suse.com>
> Cc: Long Qin <qin.long@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> 3 files changed, 18 insertions(+), 33 deletions(-)
I disagree with this patch (assuming at least that I understand it
correctly).
Namely,
- unconditionally resolving OpensslLib in the DSC files, and
- unconditionally consuming OpensslLib in modules that are
unconditionally included in the DSC files,
makes OpenSSL a hard requirement for building OVMF.
Given that OpenSSL is not distributed as part of the edk2 tree, and
given that it's not even pulled in through an unmodified git submodule,
this patch would prevent people, IIUC, from building OVMF without
jumping through the hoops described in
CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
That's a bad thing, forcing people to download and patch OpenSSL even if
they don't care about any of the dependent features. (It is perfectly
possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
and iSCSI, in a virtual machine.)
If OpenSSL were distributed as part of edk2, or if OpenSSL were
presented as a plain (unmodified) git submodule in edk2, then I might agree.
For now, perhaps we can introduce an OPENSSL_ENABLE build option.
- Features that require OpenSSL no matter what, such as
SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
(I don't remember if the [Defines] section of the DSC file can set
macros conditionally, dependent on other macros, but I hope so.)
- Features that can utilize (but don't require) OpenSSL, such as
NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide conditional
DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
- The libraries and drivers that provide the crypto stuff (directly on
top of OpenSSL) should depend on OPENSSL_ENABLE.
In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE should
not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
In summary:
- SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
- TLS_ENABLE should auto-select OPENSSL_ENABLE.
- NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
(for the ISCSI driver).
- HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
- OPENSSL_ENABLE should control the CryptoPkg modules that directly
wrap the OpenSSL functionality, for edk2.
As a result, the following build option combinations would be valid
(listing some examples):
* -D SECURE_BOOT_ENABLE
It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
fine, otherwise it would break, as it should.
* -D NETWORK_IP6_ENABLE
You get the IPv6 stack, but no secure ISCSI.
* -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
available, the build breaks, as it should.
* -D HTTP_BOOT_ENABLE
You get HTTP boot, but not HTTPS boot.
* -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is useless
Same, no change.
* -D TLS_ENABLE
Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
the build breaks. Otherwise, the TLS drivers are included in the fw
binary. They might not be used by any edk2 module, but some 3rd party
UEFI application (launched from the shell, eg.) could.
* -D HTTP_BOOT_ENABLE -D TLS_ENABLE
HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
tree, the build breaks.
* -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
boot.
* -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
-D NETWORK_IP6_ENABLE
You get everything.
My point is, if we touch these build flags, then we should go the whole
way, and express their inter-dependencies precisely.
Thanks!
Laszlo
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index e97f7f0..6e53d9f 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -1,9 +1,9 @@
> ## @file
> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> #
> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> @@ -139,14 +139,15 @@
>
> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> !if $(NETWORK_IP6_ENABLE) == TRUE
> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> !endif
> @@ -164,13 +165,11 @@
> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>
> [LibraryClasses.common]
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> -!endif
>
> [LibraryClasses.common.SEC]
> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> !ifdef $(DEBUG_ON_SERIAL_PORT)
> @@ -256,13 +255,13 @@
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> !else
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> !endif
> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> +
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> -!endif
> +
> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> @@ -698,16 +697,12 @@
> NetworkPkg/TcpDxe/TcpDxe.inf
> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> NetworkPkg/IScsiDxe/IScsiDxe.inf
> !else
> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!else
> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> !endif
> !if $(HTTP_BOOT_ENABLE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 8e3e04c..15db2d5 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -1,9 +1,9 @@
> ## @file
> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> #
> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> @@ -144,14 +144,15 @@
>
> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> !if $(NETWORK_IP6_ENABLE) == TRUE
> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> !endif
> @@ -169,13 +170,11 @@
> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>
> [LibraryClasses.common]
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> -!endif
>
> [LibraryClasses.common.SEC]
> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> !ifdef $(DEBUG_ON_SERIAL_PORT)
> @@ -261,13 +260,13 @@
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> !else
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> !endif
> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> +
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> -!endif
> +
> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> @@ -707,16 +706,12 @@
> NetworkPkg/TcpDxe/TcpDxe.inf
> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> NetworkPkg/IScsiDxe/IScsiDxe.inf
> !else
> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!else
> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> !endif
> !if $(HTTP_BOOT_ENABLE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 6ec3fe0..9c6bdc2 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -1,9 +1,9 @@
> ## @file
> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> #
> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> @@ -144,14 +144,15 @@
>
> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> !if $(NETWORK_IP6_ENABLE) == TRUE
> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> !endif
> @@ -169,13 +170,11 @@
> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>
> [LibraryClasses.common]
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> -!endif
>
> [LibraryClasses.common.SEC]
> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> !ifdef $(DEBUG_ON_SERIAL_PORT)
> @@ -261,13 +260,13 @@
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> !else
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> !endif
> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> +
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> -!endif
> +
> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> @@ -705,16 +704,12 @@
> NetworkPkg/TcpDxe/TcpDxe.inf
> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> NetworkPkg/IScsiDxe/IScsiDxe.inf
> !else
> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!else
> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> !endif
> !if $(HTTP_BOOT_ENABLE) == TRUE
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-16 20:33 ` Laszlo Ersek
@ 2017-01-17 1:08 ` Wu, Jiaxin
2017-01-17 1:47 ` Laszlo Ersek
2017-01-17 1:54 ` Jordan Justen
1 sibling, 1 reply; 9+ messages in thread
From: Wu, Jiaxin @ 2017-01-17 1:08 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org
Cc: Justen, Jordan L, Gary Lin, Long, Qin, Kinney, Michael D
Laszlo,
I don't think this patch makes OpenSSL must requirement for building OVMF by default.
As I note in the commit log that "no build performance impacts" if OpenSSL related library is not consumed by any other modules. That also means "Including OpenSSL libraries unconditionally won't break OVMF build by default since all dependent modules are controlled by the defined flag with the false value."
Secure Boot feature is controlled by:
* DEFINE SECURE_BOOT_ENABLE = FALSE
ISCSIv6 requires OpenSSL, which is controlled by:
* DEFINE NETWORK_IP6_ENABLE = FALSE
IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it should be controlled by:
* DEFINE NETWORK_IP6_ENABLE = FALSE
(For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled, we should fix it.)
HTTPS/TLS will also be controlled by:
* DEFINE TLS_ENABLE = FALSE
Namely:
OpenSSL is required to follow Patch-HOWTO *only when needed*.
Of course, as you propose, we can also add OPENSSL_ENABLE flag to control all the OpenSSL libraries. But as I mentioned above, do you think it's necessary? I don't have strong opinion for OPENSSL_ENABLE flag, but makes the logic more complexity as you list below.
Thanks,
Jiaxin
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, January 17, 2017 4:33 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>;
> Long, Qin <qin.long@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> libraries
>
> On 01/16/17 13:22, Jiaxin Wu wrote:
> > v2:
> > * Remove the flag for NetworkPkg/IScsiDxe
> >
> > This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> > the CryptoPkg librarie.
> >
> > Not only the secure boot feature requires the CryptoPkg libraries
> > (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> > features. Those modules can be always included since no build performance
> > impacts if they are not consumed.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Justen Jordan L <jordan.l.justen@intel.com>
> > Cc: Gary Lin <glin@suse.com>
> > Cc: Long Qin <qin.long@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> > OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> > OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> > OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> > 3 files changed, 18 insertions(+), 33 deletions(-)
>
> I disagree with this patch (assuming at least that I understand it
> correctly).
>
> Namely,
> - unconditionally resolving OpensslLib in the DSC files, and
> - unconditionally consuming OpensslLib in modules that are
> unconditionally included in the DSC files,
>
> makes OpenSSL a hard requirement for building OVMF.
>
> Given that OpenSSL is not distributed as part of the edk2 tree, and
> given that it's not even pulled in through an unmodified git submodule,
> this patch would prevent people, IIUC, from building OVMF without
> jumping through the hoops described in
>
> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
>
> That's a bad thing, forcing people to download and patch OpenSSL even if
> they don't care about any of the dependent features. (It is perfectly
> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
> and iSCSI, in a virtual machine.)
>
> If OpenSSL were distributed as part of edk2, or if OpenSSL were
> presented as a plain (unmodified) git submodule in edk2, then I might agree.
>
> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
>
> - Features that require OpenSSL no matter what, such as
> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
>
> (I don't remember if the [Defines] section of the DSC file can set
> macros conditionally, dependent on other macros, but I hope so.)
>
> - Features that can utilize (but don't require) OpenSSL, such as
> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide conditional
> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
>
> - The libraries and drivers that provide the crypto stuff (directly on
> top of OpenSSL) should depend on OPENSSL_ENABLE.
>
> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE should
> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
>
> In summary:
> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
> (for the ISCSI driver).
> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
> wrap the OpenSSL functionality, for edk2.
>
> As a result, the following build option combinations would be valid
> (listing some examples):
>
> * -D SECURE_BOOT_ENABLE
>
> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
> fine, otherwise it would break, as it should.
>
> * -D NETWORK_IP6_ENABLE
>
> You get the IPv6 stack, but no secure ISCSI.
>
> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
>
> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
> available, the build breaks, as it should.
>
> * -D HTTP_BOOT_ENABLE
>
> You get HTTP boot, but not HTTPS boot.
>
> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is useless
>
> Same, no change.
>
> * -D TLS_ENABLE
>
> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
> the build breaks. Otherwise, the TLS drivers are included in the fw
> binary. They might not be used by any edk2 module, but some 3rd party
> UEFI application (launched from the shell, eg.) could.
>
> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
>
> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
> tree, the build breaks.
>
> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
> NETWORK_IP6_ENABLE
>
> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
> boot.
>
> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
> -D NETWORK_IP6_ENABLE
>
> You get everything.
>
> My point is, if we touch these build flags, then we should go the whole
> way, and express their inter-dependencies precisely.
>
> Thanks!
> Laszlo
>
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index e97f7f0..6e53d9f 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the
> BSD License
> > # which accompanies this distribution. The full text of the license may be
> found at
> > @@ -139,14 +139,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> ebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > -
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> easurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -164,13 +165,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> /BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -256,13 +255,13 @@
> >
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> >
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> nf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -698,16 +697,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 8e3e04c..15db2d5 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the
> BSD License
> > # which accompanies this distribution. The full text of the license may be
> found at
> > @@ -144,14 +144,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> ebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > -
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> easurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -169,13 +170,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> /BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -261,13 +260,13 @@
> >
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> >
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> nf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -707,16 +706,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 6ec3fe0..9c6bdc2 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the
> BSD License
> > # which accompanies this distribution. The full text of the license may be
> found at
> > @@ -144,14 +144,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> ebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > -
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> easurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -169,13 +170,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> /BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -261,13 +260,13 @@
> >
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> >
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> nf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -705,16 +704,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-17 1:08 ` Wu, Jiaxin
@ 2017-01-17 1:47 ` Laszlo Ersek
2017-01-17 2:56 ` Wu, Jiaxin
0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2017-01-17 1:47 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@ml01.01.org
Cc: Justen, Jordan L, Gary Lin, Long, Qin, Kinney, Michael D
On 01/17/17 02:08, Wu, Jiaxin wrote:
> Laszlo,
>
> I don't think this patch makes OpenSSL must requirement for building
> OVMF by default.
>
> As I note in the commit log that "no build performance impacts" if
> OpenSSL related library is not consumed by any other modules.
I saw that comment, and I didn't understand it. What do you mean by
"performance impact"? How quickly the tree builds? Or how quickly the
resultant firmware boots? My concerns aren't related to performance, but
whether OVMF builds at all, or not.
> That
> also means "Including OpenSSL libraries unconditionally won't break
> OVMF build by default since all dependent modules are controlled by
> the defined flag with the false value."
So practically the suggestion is to provide unconditional library
resolutions for the OpenSslLib, IntrinsicLib and BaseCryptLib classes,
regardless of whether those classes are actually used by any module.
I see the point, but then the commit message should be improved. It
should also explain that unused lib class resolutions that refer to
nonexistent INF files (for example when OpenSSL is missing from the
tree) do not cause build failures, unless the lib class is actually used.
The commit message could be
OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
>
> Secure Boot feature is controlled by:
> * DEFINE SECURE_BOOT_ENABLE = FALSE
>
> ISCSIv6 requires OpenSSL, which is controlled by:
> * DEFINE NETWORK_IP6_ENABLE = FALSE
That's not entirely right; currently you can build with -D
NETWORK_IP6_ENABLE and without OpenSSL (i.e., without -D
SECURE_BOOT_ENABLE, at the moment). It will use IScsiDxe from
MdeModulePkg, rather than from NetworkPkg.
Is your argument that such an IPv6 stack (that is, with IScsiDxe comes
from MdeModulePkg) is incomplete in itself? In other words, that a
complete IPv6 stack requires IScsiDxe from NetworkPkg, hence OpenSSL too?
In that case, the relevant parts of the OVMF DSC / FDF files should be
fixed in a separate patch, with a separate justification. Something like:
OvmfPkg: correct the set of modules included for the IPv6 stack
>
> IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it should be controlled by:
> * DEFINE NETWORK_IP6_ENABLE = FALSE
> (For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled, we should fix it.)
Yes, it could be part of the above-suggested IPv6-oriented patch.
>
> HTTPS/TLS will also be controlled by:
> * DEFINE TLS_ENABLE = FALSE
Makes sense.
(And then HTTP_BOOT_ENABLE should pull in different modules dependent on
TLS_ENABLE.)
> Namely:
> OpenSSL is required to follow Patch-HOWTO *only when needed*.
>
> Of course, as you propose, we can also add OPENSSL_ENABLE flag to
> control all the OpenSSL libraries. But as I mentioned above, do you
> think it's necessary? I don't have strong opinion for OPENSSL_ENABLE
> flag, but makes the logic more complexity as you list below.
No, with your explanation, it seems fine. I think in total we'll need
four patches:
* OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
Does what it says; commit message suggestions above.
* OvmfPkg: correct the set of modules included for the IPv6 stack
Fixes up IScsiDxe and IPSec, makes OpenSSL a hard requirement for
IPv6. (And documents the fact in the commit message.)
* OvmfPkg: pull in TLS modules with -D TLS_ENABLE
Resolves the TLS-specific library classes, and pulls in TLS drivers
(that are independent of HTTPS).
* OvmfPkg: enable HTTPS boot under (HTTP_BOOT_ENABLE + TLS_ENABLE)
Adds any TLS-specific customizations to existent HTTP_BOOT_ENABLE
parts.
What do you guys think?
I believe it would be preferable if one of you (Gary?) could submit the
whole 4-part series, with the other one (Jiaxin?) helping out with the
review. Would that work for you both?
Thanks!
Laszlo
>
> Thanks,
> Jiaxin
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Tuesday, January 17, 2017 4:33 AM
>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>;
>> Long, Qin <qin.long@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
>> libraries
>>
>> On 01/16/17 13:22, Jiaxin Wu wrote:
>>> v2:
>>> * Remove the flag for NetworkPkg/IScsiDxe
>>>
>>> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
>>> the CryptoPkg librarie.
>>>
>>> Not only the secure boot feature requires the CryptoPkg libraries
>>> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
>>> features. Those modules can be always included since no build performance
>>> impacts if they are not consumed.
>>>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Justen Jordan L <jordan.l.justen@intel.com>
>>> Cc: Gary Lin <glin@suse.com>
>>> Cc: Long Qin <qin.long@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>> ---
>>> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
>>> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
>>> 3 files changed, 18 insertions(+), 33 deletions(-)
>>
>> I disagree with this patch (assuming at least that I understand it
>> correctly).
>>
>> Namely,
>> - unconditionally resolving OpensslLib in the DSC files, and
>> - unconditionally consuming OpensslLib in modules that are
>> unconditionally included in the DSC files,
>>
>> makes OpenSSL a hard requirement for building OVMF.
>>
>> Given that OpenSSL is not distributed as part of the edk2 tree, and
>> given that it's not even pulled in through an unmodified git submodule,
>> this patch would prevent people, IIUC, from building OVMF without
>> jumping through the hoops described in
>>
>> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
>>
>> That's a bad thing, forcing people to download and patch OpenSSL even if
>> they don't care about any of the dependent features. (It is perfectly
>> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
>> and iSCSI, in a virtual machine.)
>>
>> If OpenSSL were distributed as part of edk2, or if OpenSSL were
>> presented as a plain (unmodified) git submodule in edk2, then I might agree.
>>
>> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
>>
>> - Features that require OpenSSL no matter what, such as
>> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
>>
>> (I don't remember if the [Defines] section of the DSC file can set
>> macros conditionally, dependent on other macros, but I hope so.)
>>
>> - Features that can utilize (but don't require) OpenSSL, such as
>> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide conditional
>> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
>>
>> - The libraries and drivers that provide the crypto stuff (directly on
>> top of OpenSSL) should depend on OPENSSL_ENABLE.
>>
>> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
>> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE should
>> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
>>
>> In summary:
>> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
>> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
>> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
>> (for the ISCSI driver).
>> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
>> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
>> wrap the OpenSSL functionality, for edk2.
>>
>> As a result, the following build option combinations would be valid
>> (listing some examples):
>>
>> * -D SECURE_BOOT_ENABLE
>>
>> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
>> fine, otherwise it would break, as it should.
>>
>> * -D NETWORK_IP6_ENABLE
>>
>> You get the IPv6 stack, but no secure ISCSI.
>>
>> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
>>
>> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
>> available, the build breaks, as it should.
>>
>> * -D HTTP_BOOT_ENABLE
>>
>> You get HTTP boot, but not HTTPS boot.
>>
>> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is useless
>>
>> Same, no change.
>>
>> * -D TLS_ENABLE
>>
>> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
>> the build breaks. Otherwise, the TLS drivers are included in the fw
>> binary. They might not be used by any edk2 module, but some 3rd party
>> UEFI application (launched from the shell, eg.) could.
>>
>> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
>>
>> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
>> tree, the build breaks.
>>
>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
>> NETWORK_IP6_ENABLE
>>
>> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
>> boot.
>>
>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
>> -D NETWORK_IP6_ENABLE
>>
>> You get everything.
>>
>> My point is, if we touch these build flags, then we should go the whole
>> way, and express their inter-dependencies precisely.
>>
>> Thanks!
>> Laszlo
>>
>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>> index e97f7f0..6e53d9f 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>> @@ -1,9 +1,9 @@
>>> ## @file
>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>> #
>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>> #
>>> # This program and the accompanying materials
>>> # are licensed and made available under the terms and conditions of the
>> BSD License
>>> # which accompanies this distribution. The full text of the license may be
>> found at
>>> @@ -139,14 +139,15 @@
>>>
>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>> ebugPrintErrorLevelLib.inf
>>>
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> -
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> +
>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>> easurementLib.inf
>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>> !endif
>>> @@ -164,13 +165,11 @@
>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>
>>> [LibraryClasses.common]
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>> -!endif
>>>
>>> [LibraryClasses.common.SEC]
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>> @@ -256,13 +255,13 @@
>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>> !else
>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>> nf
>>> !endif
>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>> -!endif
>>> +
>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>
>>> [LibraryClasses.common.UEFI_DRIVER]
>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>> @@ -698,16 +697,12 @@
>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>> !else
>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> -!endif
>>> -!else
>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> !endif
>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> index 8e3e04c..15db2d5 100644
>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> @@ -1,9 +1,9 @@
>>> ## @file
>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>> #
>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>> #
>>> # This program and the accompanying materials
>>> # are licensed and made available under the terms and conditions of the
>> BSD License
>>> # which accompanies this distribution. The full text of the license may be
>> found at
>>> @@ -144,14 +144,15 @@
>>>
>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>> ebugPrintErrorLevelLib.inf
>>>
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> -
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> +
>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>> easurementLib.inf
>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>> !endif
>>> @@ -169,13 +170,11 @@
>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>
>>> [LibraryClasses.common]
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>> -!endif
>>>
>>> [LibraryClasses.common.SEC]
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>> @@ -261,13 +260,13 @@
>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>> !else
>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>> nf
>>> !endif
>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>> -!endif
>>> +
>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>
>>> [LibraryClasses.common.UEFI_DRIVER]
>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>> @@ -707,16 +706,12 @@
>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>> !else
>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> -!endif
>>> -!else
>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> !endif
>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>> index 6ec3fe0..9c6bdc2 100644
>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>> @@ -1,9 +1,9 @@
>>> ## @file
>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>> #
>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>> #
>>> # This program and the accompanying materials
>>> # are licensed and made available under the terms and conditions of the
>> BSD License
>>> # which accompanies this distribution. The full text of the license may be
>> found at
>>> @@ -144,14 +144,15 @@
>>>
>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>> ebugPrintErrorLevelLib.inf
>>>
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> -
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> +
>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>> easurementLib.inf
>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>> !endif
>>> @@ -169,13 +170,11 @@
>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>
>>> [LibraryClasses.common]
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>> -!endif
>>>
>>> [LibraryClasses.common.SEC]
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>> @@ -261,13 +260,13 @@
>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>> !else
>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>> nf
>>> !endif
>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> +
>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>> -!endif
>>> +
>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>
>>> [LibraryClasses.common.UEFI_DRIVER]
>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>> @@ -705,16 +704,12 @@
>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>> !else
>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> -!endif
>>> -!else
>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> !endif
>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-16 20:33 ` Laszlo Ersek
2017-01-17 1:08 ` Wu, Jiaxin
@ 2017-01-17 1:54 ` Jordan Justen
1 sibling, 0 replies; 9+ messages in thread
From: Jordan Justen @ 2017-01-17 1:54 UTC (permalink / raw)
To: Laszlo Ersek, Jiaxin Wu, edk2-devel; +Cc: Gary Lin, Long Qin, Michael Kinney
On 2017-01-16 12:33:20, Laszlo Ersek wrote:
> On 01/16/17 13:22, Jiaxin Wu wrote:
> > v2:
> > * Remove the flag for NetworkPkg/IScsiDxe
> >
> > This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> > the CryptoPkg librarie.
> >
> > Not only the secure boot feature requires the CryptoPkg libraries
> > (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> > features. Those modules can be always included since no build performance
> > impacts if they are not consumed.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Justen Jordan L <jordan.l.justen@intel.com>
> > Cc: Gary Lin <glin@suse.com>
> > Cc: Long Qin <qin.long@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> > OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> > OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> > OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> > 3 files changed, 18 insertions(+), 33 deletions(-)
>
> I disagree with this patch (assuming at least that I understand it
> correctly).
>
> Namely,
> - unconditionally resolving OpensslLib in the DSC files, and
> - unconditionally consuming OpensslLib in modules that are
> unconditionally included in the DSC files,
>
> makes OpenSSL a hard requirement for building OVMF.
>
> Given that OpenSSL is not distributed as part of the edk2 tree, and
> given that it's not even pulled in through an unmodified git submodule,
> this patch would prevent people, IIUC, from building OVMF without
> jumping through the hoops described in
>
> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
>
> That's a bad thing, forcing people to download and patch OpenSSL even if
> they don't care about any of the dependent features. (It is perfectly
> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
> and iSCSI, in a virtual machine.)
>
> If OpenSSL were distributed as part of edk2, or if OpenSSL were
> presented as a plain (unmodified) git submodule in edk2, then I might agree.
I agree.
I'm not sure what the half-hearted support for OpenSSL in the EDK II
tree is about. Perhaps it is the license? (Isn't is always that when
it comes to OpenSSL?) If so, I wonder if other free software
alternatives have been considered.
There is also the build time and firmware space overhead of supporting
this. Since it is not a UEFI requirement, as Laszlo mentions it is
entirely possible to not care that it is missing from OVMF.
-Jordan
>
> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
>
> - Features that require OpenSSL no matter what, such as
> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
>
> (I don't remember if the [Defines] section of the DSC file can set
> macros conditionally, dependent on other macros, but I hope so.)
>
> - Features that can utilize (but don't require) OpenSSL, such as
> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide conditional
> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
>
> - The libraries and drivers that provide the crypto stuff (directly on
> top of OpenSSL) should depend on OPENSSL_ENABLE.
>
> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE should
> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
>
> In summary:
> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
> (for the ISCSI driver).
> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
> wrap the OpenSSL functionality, for edk2.
>
> As a result, the following build option combinations would be valid
> (listing some examples):
>
> * -D SECURE_BOOT_ENABLE
>
> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
> fine, otherwise it would break, as it should.
>
> * -D NETWORK_IP6_ENABLE
>
> You get the IPv6 stack, but no secure ISCSI.
>
> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
>
> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
> available, the build breaks, as it should.
>
> * -D HTTP_BOOT_ENABLE
>
> You get HTTP boot, but not HTTPS boot.
>
> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is useless
>
> Same, no change.
>
> * -D TLS_ENABLE
>
> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
> the build breaks. Otherwise, the TLS drivers are included in the fw
> binary. They might not be used by any edk2 module, but some 3rd party
> UEFI application (launched from the shell, eg.) could.
>
> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
>
> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
> tree, the build breaks.
>
> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
>
> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
> boot.
>
> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
> -D NETWORK_IP6_ENABLE
>
> You get everything.
>
> My point is, if we touch these build flags, then we should go the whole
> way, and express their inter-dependencies precisely.
>
> Thanks!
> Laszlo
>
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index e97f7f0..6e53d9f 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the BSD License
> > # which accompanies this distribution. The full text of the license may be found at
> > @@ -139,14 +139,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -164,13 +165,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -256,13 +255,13 @@
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -698,16 +697,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 8e3e04c..15db2d5 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the BSD License
> > # which accompanies this distribution. The full text of the license may be found at
> > @@ -144,14 +144,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -169,13 +170,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -261,13 +260,13 @@
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -707,16 +706,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 6ec3fe0..9c6bdc2 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -1,9 +1,9 @@
> > ## @file
> > # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > #
> > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the BSD License
> > # which accompanies this distribution. The full text of the license may be found at
> > @@ -144,14 +144,15 @@
> >
> > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> >
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > - PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > + PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > !if $(NETWORK_IP6_ENABLE) == TRUE
> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > !endif
> > @@ -169,13 +170,11 @@
> > SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >
> > [LibraryClasses.common]
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > -!endif
> >
> > [LibraryClasses.common.SEC]
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > !ifdef $(DEBUG_ON_SERIAL_PORT)
> > @@ -261,13 +260,13 @@
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > !else
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> > !endif
> > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > +
> > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > -!endif
> > +
> > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > @@ -705,16 +704,12 @@
> > NetworkPkg/TcpDxe/TcpDxe.inf
> > NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > -!if $(SECURE_BOOT_ENABLE) == TRUE
> > NetworkPkg/IScsiDxe/IScsiDxe.inf
> > !else
> > - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -!endif
> > -!else
> > MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > !endif
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-17 1:47 ` Laszlo Ersek
@ 2017-01-17 2:56 ` Wu, Jiaxin
2017-01-17 3:15 ` Laszlo Ersek
2017-01-17 3:35 ` Gary Lin
0 siblings, 2 replies; 9+ messages in thread
From: Wu, Jiaxin @ 2017-01-17 2:56 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org
Cc: Justen, Jordan L, Gary Lin, Long, Qin, Kinney, Michael D
> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> libraries
>
> On 01/17/17 02:08, Wu, Jiaxin wrote:
> > Laszlo,
> >
> > I don't think this patch makes OpenSSL must requirement for building
> > OVMF by default.
> >
> > As I note in the commit log that "no build performance impacts" if
> > OpenSSL related library is not consumed by any other modules.
>
> I saw that comment, and I didn't understand it. What do you mean by
> "performance impact"? How quickly the tree builds? Or how quickly the
> resultant firmware boots? My concerns aren't related to performance, but
> whether OVMF builds at all, or not.
>
> > That
> > also means "Including OpenSSL libraries unconditionally won't break
> > OVMF build by default since all dependent modules are controlled by
> > the defined flag with the false value."
>
> So practically the suggestion is to provide unconditional library
> resolutions for the OpenSslLib, IntrinsicLib and BaseCryptLib classes,
> regardless of whether those classes are actually used by any module.
>
Yes.
I thought "build performance" should include the build result and time consumption during the OVMF build. Sorry for the misunderstanding due to the ambiguity of "build performance impacts", and I agree to refine the commit log.
> I see the point, but then the commit message should be improved. It
> should also explain that unused lib class resolutions that refer to
> nonexistent INF files (for example when OpenSSL is missing from the
> tree) do not cause build failures, unless the lib class is actually used.
>
> The commit message could be
>
> OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
>
I don't have the strong opinion for the commit message change. That's also fine to me since we can reach an agreement:).
> >
> > Secure Boot feature is controlled by:
> > * DEFINE SECURE_BOOT_ENABLE = FALSE
> >
> > ISCSIv6 requires OpenSSL, which is controlled by:
> > * DEFINE NETWORK_IP6_ENABLE = FALSE
>
> That's not entirely right; currently you can build with -D
> NETWORK_IP6_ENABLE and without OpenSSL (i.e., without -D
> SECURE_BOOT_ENABLE, at the moment). It will use IScsiDxe from
> MdeModulePkg, rather than from NetworkPkg.
>
> Is your argument that such an IPv6 stack (that is, with IScsiDxe comes
> from MdeModulePkg) is incomplete in itself? In other words, that a
> complete IPv6 stack requires IScsiDxe from NetworkPkg, hence OpenSSL too?
Yes, that's my point.
>
> In that case, the relevant parts of the OVMF DSC / FDF files should be
> fixed in a separate patch, with a separate justification. Something like:
>
> OvmfPkg: correct the set of modules included for the IPv6 stack
>
Ok, that's fine the separate patch.
> >
> > IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it
> should be controlled by:
> > * DEFINE NETWORK_IP6_ENABLE = FALSE
> > (For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled, we
> should fix it.)
>
> Yes, it could be part of the above-suggested IPv6-oriented patch.
>
> >
> > HTTPS/TLS will also be controlled by:
> > * DEFINE TLS_ENABLE = FALSE
>
> Makes sense.
>
> (And then HTTP_BOOT_ENABLE should pull in different modules dependent on
> TLS_ENABLE.)
No, we can keep the current modules included in HTTP_BOOT_ENABLE, and make the TLS_ENABLE independently since TLS feature should not be limit to HTTP(S) feature.
As I explained to Gary, TLS can be treated as independent module, which can be leveraged by third part drivers/apps (e.g. EAP-TLS). No TLS means no HTTPS.
>
> > Namely:
> > OpenSSL is required to follow Patch-HOWTO *only when needed*.
> >
> > Of course, as you propose, we can also add OPENSSL_ENABLE flag to
> > control all the OpenSSL libraries. But as I mentioned above, do you
> > think it's necessary? I don't have strong opinion for OPENSSL_ENABLE
> > flag, but makes the logic more complexity as you list below.
>
> No, with your explanation, it seems fine. I think in total we'll need
> four patches:
>
> * OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
>
> Does what it says; commit message suggestions above.
>
> * OvmfPkg: correct the set of modules included for the IPv6 stack
>
> Fixes up IScsiDxe and IPSec, makes OpenSSL a hard requirement for
> IPv6. (And documents the fact in the commit message.)
>
> * OvmfPkg: pull in TLS modules with -D TLS_ENABLE
>
> Resolves the TLS-specific library classes, and pulls in TLS drivers
> (that are independent of HTTPS).
>
> * OvmfPkg: enable HTTPS boot under (HTTP_BOOT_ENABLE + TLS_ENABLE)
>
> Adds any TLS-specific customizations to existent HTTP_BOOT_ENABLE
> parts.
>
> What do you guys think?
>
We can combine the last two patches instead:
* OvmfPkg: Enable HTTPS/TLS feature under (HTTP_BOOT_ENABLE + TLS_ENABLE)
> I believe it would be preferable if one of you (Gary?) could submit the
> whole 4-part series, with the other one (Jiaxin?) helping out with the
> review. Would that work for you both?
>
I'm fine with the propose:).
Thanks,
Jiaxin
> Thanks!
> Laszlo
>
> >
> > Thanks,
> > Jiaxin
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Tuesday, January 17, 2017 4:33 AM
> >> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
> >> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>;
> >> Long, Qin <qin.long@intel.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>
> >> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> >> libraries
> >>
> >> On 01/16/17 13:22, Jiaxin Wu wrote:
> >>> v2:
> >>> * Remove the flag for NetworkPkg/IScsiDxe
> >>>
> >>> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> >>> the CryptoPkg librarie.
> >>>
> >>> Not only the secure boot feature requires the CryptoPkg libraries
> >>> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> >>> features. Those modules can be always included since no build
> performance
> >>> impacts if they are not consumed.
> >>>
> >>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>> Cc: Justen Jordan L <jordan.l.justen@intel.com>
> >>> Cc: Gary Lin <glin@suse.com>
> >>> Cc: Long Qin <qin.long@intel.com>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >>> ---
> >>> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> >>> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> >>> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> >>> 3 files changed, 18 insertions(+), 33 deletions(-)
> >>
> >> I disagree with this patch (assuming at least that I understand it
> >> correctly).
> >>
> >> Namely,
> >> - unconditionally resolving OpensslLib in the DSC files, and
> >> - unconditionally consuming OpensslLib in modules that are
> >> unconditionally included in the DSC files,
> >>
> >> makes OpenSSL a hard requirement for building OVMF.
> >>
> >> Given that OpenSSL is not distributed as part of the edk2 tree, and
> >> given that it's not even pulled in through an unmodified git submodule,
> >> this patch would prevent people, IIUC, from building OVMF without
> >> jumping through the hoops described in
> >>
> >> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> >>
> >> That's a bad thing, forcing people to download and patch OpenSSL even if
> >> they don't care about any of the dependent features. (It is perfectly
> >> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
> >> and iSCSI, in a virtual machine.)
> >>
> >> If OpenSSL were distributed as part of edk2, or if OpenSSL were
> >> presented as a plain (unmodified) git submodule in edk2, then I might agree.
> >>
> >> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
> >>
> >> - Features that require OpenSSL no matter what, such as
> >> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
> >>
> >> (I don't remember if the [Defines] section of the DSC file can set
> >> macros conditionally, dependent on other macros, but I hope so.)
> >>
> >> - Features that can utilize (but don't require) OpenSSL, such as
> >> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide
> conditional
> >> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
> >>
> >> - The libraries and drivers that provide the crypto stuff (directly on
> >> top of OpenSSL) should depend on OPENSSL_ENABLE.
> >>
> >> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
> >> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE
> should
> >> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
> >>
> >> In summary:
> >> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
> >> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
> >> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
> >> (for the ISCSI driver).
> >> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
> >> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
> >> wrap the OpenSSL functionality, for edk2.
> >>
> >> As a result, the following build option combinations would be valid
> >> (listing some examples):
> >>
> >> * -D SECURE_BOOT_ENABLE
> >>
> >> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
> >> fine, otherwise it would break, as it should.
> >>
> >> * -D NETWORK_IP6_ENABLE
> >>
> >> You get the IPv6 stack, but no secure ISCSI.
> >>
> >> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
> >>
> >> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
> >> available, the build breaks, as it should.
> >>
> >> * -D HTTP_BOOT_ENABLE
> >>
> >> You get HTTP boot, but not HTTPS boot.
> >>
> >> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is
> useless
> >>
> >> Same, no change.
> >>
> >> * -D TLS_ENABLE
> >>
> >> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
> >> the build breaks. Otherwise, the TLS drivers are included in the fw
> >> binary. They might not be used by any edk2 module, but some 3rd party
> >> UEFI application (launched from the shell, eg.) could.
> >>
> >> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
> >>
> >> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
> >> tree, the build breaks.
> >>
> >> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
> >> NETWORK_IP6_ENABLE
> >>
> >> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
> >> boot.
> >>
> >> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
> >> -D NETWORK_IP6_ENABLE
> >>
> >> You get everything.
> >>
> >> My point is, if we touch these build flags, then we should go the whole
> >> way, and express their inter-dependencies precisely.
> >>
> >> Thanks!
> >> Laszlo
> >>
> >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> >>> index e97f7f0..6e53d9f 100644
> >>> --- a/OvmfPkg/OvmfPkgIa32.dsc
> >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> >>> @@ -1,9 +1,9 @@
> >>> ## @file
> >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>> #
> >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>> #
> >>> # This program and the accompanying materials
> >>> # are licensed and made available under the terms and conditions of the
> >> BSD License
> >>> # which accompanies this distribution. The full text of the license may be
> >> found at
> >>> @@ -139,14 +139,15 @@
> >>>
> >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >> ebugPrintErrorLevelLib.inf
> >>>
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> -
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>> +
> >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >> easurementLib.inf
> >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>> !endif
> >>> @@ -164,13 +165,11 @@
> >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>
> >>> [LibraryClasses.common]
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>> -!endif
> >>>
> >>> [LibraryClasses.common.SEC]
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>> @@ -256,13 +255,13 @@
> >>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>> !else
> >>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >> nf
> >>> !endif
> >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>> -!endif
> >>> +
> >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>
> >>> [LibraryClasses.common.UEFI_DRIVER]
> >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>> @@ -698,16 +697,12 @@
> >>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>> !else
> >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> -!endif
> >>> -!else
> >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> !endif
> >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> >>> index 8e3e04c..15db2d5 100644
> >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> >>> @@ -1,9 +1,9 @@
> >>> ## @file
> >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>> #
> >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>> #
> >>> # This program and the accompanying materials
> >>> # are licensed and made available under the terms and conditions of the
> >> BSD License
> >>> # which accompanies this distribution. The full text of the license may be
> >> found at
> >>> @@ -144,14 +144,15 @@
> >>>
> >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >> ebugPrintErrorLevelLib.inf
> >>>
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> -
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>> +
> >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >> easurementLib.inf
> >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>> !endif
> >>> @@ -169,13 +170,11 @@
> >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>
> >>> [LibraryClasses.common]
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>> -!endif
> >>>
> >>> [LibraryClasses.common.SEC]
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>> @@ -261,13 +260,13 @@
> >>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>> !else
> >>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >> nf
> >>> !endif
> >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>> -!endif
> >>> +
> >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>
> >>> [LibraryClasses.common.UEFI_DRIVER]
> >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>> @@ -707,16 +706,12 @@
> >>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>> !else
> >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> -!endif
> >>> -!else
> >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> !endif
> >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> >>> index 6ec3fe0..9c6bdc2 100644
> >>> --- a/OvmfPkg/OvmfPkgX64.dsc
> >>> +++ b/OvmfPkg/OvmfPkgX64.dsc
> >>> @@ -1,9 +1,9 @@
> >>> ## @file
> >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>> #
> >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>> #
> >>> # This program and the accompanying materials
> >>> # are licensed and made available under the terms and conditions of the
> >> BSD License
> >>> # which accompanies this distribution. The full text of the license may be
> >> found at
> >>> @@ -144,14 +144,15 @@
> >>>
> >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >> ebugPrintErrorLevelLib.inf
> >>>
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> -
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>> +
> >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >> easurementLib.inf
> >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>> !endif
> >>> @@ -169,13 +170,11 @@
> >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>
> >>> [LibraryClasses.common]
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>> -!endif
> >>>
> >>> [LibraryClasses.common.SEC]
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>> @@ -261,13 +260,13 @@
> >>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>> !else
> >>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >> nf
> >>> !endif
> >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> +
> >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>> -!endif
> >>> +
> >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>
> >>> [LibraryClasses.common.UEFI_DRIVER]
> >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>> @@ -705,16 +704,12 @@
> >>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>> !else
> >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> -!endif
> >>> -!else
> >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>> !endif
> >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>>
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-17 2:56 ` Wu, Jiaxin
@ 2017-01-17 3:15 ` Laszlo Ersek
2017-01-17 3:20 ` Wu, Jiaxin
2017-01-17 3:35 ` Gary Lin
1 sibling, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2017-01-17 3:15 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@ml01.01.org
Cc: Justen, Jordan L, Gary Lin, Long, Qin, Kinney, Michael D
On 01/17/17 03:56, Wu, Jiaxin wrote:
>> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
>> libraries
>>
>> On 01/17/17 02:08, Wu, Jiaxin wrote:
>>> Laszlo,
>>>
>>> I don't think this patch makes OpenSSL must requirement for building
>>> OVMF by default.
>>>
>>> As I note in the commit log that "no build performance impacts" if
>>> OpenSSL related library is not consumed by any other modules.
>>
>> I saw that comment, and I didn't understand it. What do you mean by
>> "performance impact"? How quickly the tree builds? Or how quickly the
>> resultant firmware boots? My concerns aren't related to performance, but
>> whether OVMF builds at all, or not.
>>
>>> That
>>> also means "Including OpenSSL libraries unconditionally won't break
>>> OVMF build by default since all dependent modules are controlled by
>>> the defined flag with the false value."
>>
>> So practically the suggestion is to provide unconditional library
>> resolutions for the OpenSslLib, IntrinsicLib and BaseCryptLib classes,
>> regardless of whether those classes are actually used by any module.
>>
>
> Yes.
> I thought "build performance" should include the build result and time consumption during the OVMF build. Sorry for the misunderstanding due to the ambiguity of "build performance impacts", and I agree to refine the commit log.
>
>
>
>> I see the point, but then the commit message should be improved. It
>> should also explain that unused lib class resolutions that refer to
>> nonexistent INF files (for example when OpenSSL is missing from the
>> tree) do not cause build failures, unless the lib class is actually used.
>>
>> The commit message could be
>>
>> OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
>>
>
> I don't have the strong opinion for the commit message change. That's also fine to me since we can reach an agreement:).
>
>
>
>>>
>>> Secure Boot feature is controlled by:
>>> * DEFINE SECURE_BOOT_ENABLE = FALSE
>>>
>>> ISCSIv6 requires OpenSSL, which is controlled by:
>>> * DEFINE NETWORK_IP6_ENABLE = FALSE
>>
>> That's not entirely right; currently you can build with -D
>> NETWORK_IP6_ENABLE and without OpenSSL (i.e., without -D
>> SECURE_BOOT_ENABLE, at the moment). It will use IScsiDxe from
>> MdeModulePkg, rather than from NetworkPkg.
>>
>> Is your argument that such an IPv6 stack (that is, with IScsiDxe comes
>> from MdeModulePkg) is incomplete in itself? In other words, that a
>> complete IPv6 stack requires IScsiDxe from NetworkPkg, hence OpenSSL too?
>
> Yes, that's my point.
>
>
>
>>
>> In that case, the relevant parts of the OVMF DSC / FDF files should be
>> fixed in a separate patch, with a separate justification. Something like:
>>
>> OvmfPkg: correct the set of modules included for the IPv6 stack
>>
>
> Ok, that's fine the separate patch.
>
>
>
>>>
>>> IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it
>> should be controlled by:
>>> * DEFINE NETWORK_IP6_ENABLE = FALSE
>>> (For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled, we
>> should fix it.)
>>
>> Yes, it could be part of the above-suggested IPv6-oriented patch.
>>
>>>
>>> HTTPS/TLS will also be controlled by:
>>> * DEFINE TLS_ENABLE = FALSE
>>
>> Makes sense.
>>
>> (And then HTTP_BOOT_ENABLE should pull in different modules dependent on
>> TLS_ENABLE.)
>
> No, we can keep the current modules included in HTTP_BOOT_ENABLE, and make the TLS_ENABLE independently since TLS feature should not be limit to HTTP(S) feature.
>
> As I explained to Gary, TLS can be treated as independent module, which can be leveraged by third part drivers/apps (e.g. EAP-TLS). No TLS means no HTTPS.
>
>
>
>>
>>> Namely:
>>> OpenSSL is required to follow Patch-HOWTO *only when needed*.
>>>
>>> Of course, as you propose, we can also add OPENSSL_ENABLE flag to
>>> control all the OpenSSL libraries. But as I mentioned above, do you
>>> think it's necessary? I don't have strong opinion for OPENSSL_ENABLE
>>> flag, but makes the logic more complexity as you list below.
>>
>> No, with your explanation, it seems fine. I think in total we'll need
>> four patches:
>>
>> * OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
>>
>> Does what it says; commit message suggestions above.
>>
>> * OvmfPkg: correct the set of modules included for the IPv6 stack
>>
>> Fixes up IScsiDxe and IPSec, makes OpenSSL a hard requirement for
>> IPv6. (And documents the fact in the commit message.)
>>
>> * OvmfPkg: pull in TLS modules with -D TLS_ENABLE
>>
>> Resolves the TLS-specific library classes, and pulls in TLS drivers
>> (that are independent of HTTPS).
>>
>> * OvmfPkg: enable HTTPS boot under (HTTP_BOOT_ENABLE + TLS_ENABLE)
>>
>> Adds any TLS-specific customizations to existent HTTP_BOOT_ENABLE
>> parts.
>>
>> What do you guys think?
>>
>
> We can combine the last two patches instead:
>
> * OvmfPkg: Enable HTTPS/TLS feature under (HTTP_BOOT_ENABLE + TLS_ENABLE)
Hm, okay. So I guess the presence of TLS-related protocols (provided by
the drivers pulled in due to -D TLS_ENABLE) automatically enables HTTPS
when the firmware runs, in the drivers that are pulled in by -D
HTTP_BOOT_ENABLE?
In that case, I suggest the subject
OvmfPkg: pull in TLS modules with -D TLS_ENABLE (also enabling HTTPS)
and explain in the commit message that TLS_ENABLE and HTTP_BOOT_ENABLE
remain independent, but their intersection at build time produces HTTPS
capability dynamically, when the firmware runs. Is this correct?
Thanks!
Laszlo
>> I believe it would be preferable if one of you (Gary?) could submit the
>> whole 4-part series, with the other one (Jiaxin?) helping out with the
>> review. Would that work for you both?
>>
> I'm fine with the propose:).
>
> Thanks,
> Jiaxin
>
>
>
>
>> Thanks!
>> Laszlo
>>
>>>
>>> Thanks,
>>> Jiaxin
>>>
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>> Sent: Tuesday, January 17, 2017 4:33 AM
>>>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
>>>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>;
>>>> Long, Qin <qin.long@intel.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
>>>> libraries
>>>>
>>>> On 01/16/17 13:22, Jiaxin Wu wrote:
>>>>> v2:
>>>>> * Remove the flag for NetworkPkg/IScsiDxe
>>>>>
>>>>> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
>>>>> the CryptoPkg librarie.
>>>>>
>>>>> Not only the secure boot feature requires the CryptoPkg libraries
>>>>> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
>>>>> features. Those modules can be always included since no build
>> performance
>>>>> impacts if they are not consumed.
>>>>>
>>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>>> Cc: Justen Jordan L <jordan.l.justen@intel.com>
>>>>> Cc: Gary Lin <glin@suse.com>
>>>>> Cc: Long Qin <qin.long@intel.com>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>>>> ---
>>>>> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
>>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
>>>>> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
>>>>> 3 files changed, 18 insertions(+), 33 deletions(-)
>>>>
>>>> I disagree with this patch (assuming at least that I understand it
>>>> correctly).
>>>>
>>>> Namely,
>>>> - unconditionally resolving OpensslLib in the DSC files, and
>>>> - unconditionally consuming OpensslLib in modules that are
>>>> unconditionally included in the DSC files,
>>>>
>>>> makes OpenSSL a hard requirement for building OVMF.
>>>>
>>>> Given that OpenSSL is not distributed as part of the edk2 tree, and
>>>> given that it's not even pulled in through an unmodified git submodule,
>>>> this patch would prevent people, IIUC, from building OVMF without
>>>> jumping through the hoops described in
>>>>
>>>> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
>>>>
>>>> That's a bad thing, forcing people to download and patch OpenSSL even if
>>>> they don't care about any of the dependent features. (It is perfectly
>>>> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
>>>> and iSCSI, in a virtual machine.)
>>>>
>>>> If OpenSSL were distributed as part of edk2, or if OpenSSL were
>>>> presented as a plain (unmodified) git submodule in edk2, then I might agree.
>>>>
>>>> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
>>>>
>>>> - Features that require OpenSSL no matter what, such as
>>>> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
>>>>
>>>> (I don't remember if the [Defines] section of the DSC file can set
>>>> macros conditionally, dependent on other macros, but I hope so.)
>>>>
>>>> - Features that can utilize (but don't require) OpenSSL, such as
>>>> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide
>> conditional
>>>> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
>>>>
>>>> - The libraries and drivers that provide the crypto stuff (directly on
>>>> top of OpenSSL) should depend on OPENSSL_ENABLE.
>>>>
>>>> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
>>>> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE
>> should
>>>> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
>>>>
>>>> In summary:
>>>> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
>>>> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
>>>> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
>>>> (for the ISCSI driver).
>>>> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
>>>> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
>>>> wrap the OpenSSL functionality, for edk2.
>>>>
>>>> As a result, the following build option combinations would be valid
>>>> (listing some examples):
>>>>
>>>> * -D SECURE_BOOT_ENABLE
>>>>
>>>> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
>>>> fine, otherwise it would break, as it should.
>>>>
>>>> * -D NETWORK_IP6_ENABLE
>>>>
>>>> You get the IPv6 stack, but no secure ISCSI.
>>>>
>>>> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
>>>>
>>>> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
>>>> available, the build breaks, as it should.
>>>>
>>>> * -D HTTP_BOOT_ENABLE
>>>>
>>>> You get HTTP boot, but not HTTPS boot.
>>>>
>>>> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is
>> useless
>>>>
>>>> Same, no change.
>>>>
>>>> * -D TLS_ENABLE
>>>>
>>>> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
>>>> the build breaks. Otherwise, the TLS drivers are included in the fw
>>>> binary. They might not be used by any edk2 module, but some 3rd party
>>>> UEFI application (launched from the shell, eg.) could.
>>>>
>>>> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
>>>>
>>>> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
>>>> tree, the build breaks.
>>>>
>>>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
>>>> NETWORK_IP6_ENABLE
>>>>
>>>> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
>>>> boot.
>>>>
>>>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
>>>> -D NETWORK_IP6_ENABLE
>>>>
>>>> You get everything.
>>>>
>>>> My point is, if we touch these build flags, then we should go the whole
>>>> way, and express their inter-dependencies precisely.
>>>>
>>>> Thanks!
>>>> Laszlo
>>>>
>>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>>>> index e97f7f0..6e53d9f 100644
>>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>>>> @@ -1,9 +1,9 @@
>>>>> ## @file
>>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>>>> #
>>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>>>> #
>>>>> # This program and the accompanying materials
>>>>> # are licensed and made available under the terms and conditions of the
>>>> BSD License
>>>>> # which accompanies this distribution. The full text of the license may be
>>>> found at
>>>>> @@ -139,14 +139,15 @@
>>>>>
>>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>>>
>>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>>>
>>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>>>> ebugPrintErrorLevelLib.inf
>>>>>
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> -
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>>>> +
>>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>>
>>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>>>> easurementLib.inf
>>>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>>>> !endif
>>>>> @@ -164,13 +165,11 @@
>>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>>>
>>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>>>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>>>
>>>>> [LibraryClasses.common]
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>>>> -!endif
>>>>>
>>>>> [LibraryClasses.common.SEC]
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>>>> @@ -256,13 +255,13 @@
>>>>>
>>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>>>> !else
>>>>>
>>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>>>> nf
>>>>> !endif
>>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>>>> -!endif
>>>>> +
>>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>>>
>>>>> [LibraryClasses.common.UEFI_DRIVER]
>>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>>>> @@ -698,16 +697,12 @@
>>>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>>>> !else
>>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> -!endif
>>>>> -!else
>>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> !endif
>>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> index 8e3e04c..15db2d5 100644
>>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> @@ -1,9 +1,9 @@
>>>>> ## @file
>>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>>>> #
>>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>>>> #
>>>>> # This program and the accompanying materials
>>>>> # are licensed and made available under the terms and conditions of the
>>>> BSD License
>>>>> # which accompanies this distribution. The full text of the license may be
>>>> found at
>>>>> @@ -144,14 +144,15 @@
>>>>>
>>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>>>
>>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>>>
>>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>>>> ebugPrintErrorLevelLib.inf
>>>>>
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> -
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>>>> +
>>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>>
>>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>>>> easurementLib.inf
>>>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>>>> !endif
>>>>> @@ -169,13 +170,11 @@
>>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>>>
>>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>>>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>>>
>>>>> [LibraryClasses.common]
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>>>> -!endif
>>>>>
>>>>> [LibraryClasses.common.SEC]
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>>>> @@ -261,13 +260,13 @@
>>>>>
>>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>>>> !else
>>>>>
>>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>>>> nf
>>>>> !endif
>>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>>>> -!endif
>>>>> +
>>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>>>
>>>>> [LibraryClasses.common.UEFI_DRIVER]
>>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>>>> @@ -707,16 +706,12 @@
>>>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>>>> !else
>>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> -!endif
>>>>> -!else
>>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> !endif
>>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>>>> index 6ec3fe0..9c6bdc2 100644
>>>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>>>> @@ -1,9 +1,9 @@
>>>>> ## @file
>>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>>>>> #
>>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>>>> #
>>>>> # This program and the accompanying materials
>>>>> # are licensed and made available under the terms and conditions of the
>>>> BSD License
>>>>> # which accompanies this distribution. The full text of the license may be
>>>> found at
>>>>> @@ -144,14 +144,15 @@
>>>>>
>>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
>>>>>
>>>>
>> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
>>>>>
>>>>
>> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
>>>> ebugPrintErrorLevelLib.inf
>>>>>
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> -
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>>>> +
>>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>
>> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>>>>>
>>>>
>> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
>>>> easurementLib.inf
>>>>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>>>> !endif
>>>>> @@ -169,13 +170,11 @@
>>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>>>>>
>>>>
>> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
>>>> /BaseOrderedCollectionRedBlackTreeLib.inf
>>>>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>>>>>
>>>>> [LibraryClasses.common]
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>>>> -!endif
>>>>>
>>>>> [LibraryClasses.common.SEC]
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
>>>>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
>>>>> @@ -261,13 +260,13 @@
>>>>>
>>>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>>>> !else
>>>>>
>>>>
>> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
>>>> nf
>>>>> !endif
>>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> +
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>>>>> -!endif
>>>>> +
>>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>>>>
>>>>> [LibraryClasses.common.UEFI_DRIVER]
>>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>>>>> @@ -705,16 +704,12 @@
>>>>> NetworkPkg/TcpDxe/TcpDxe.inf
>>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
>>>>> !else
>>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> -!endif
>>>>> -!else
>>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>>>> !endif
>>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-17 3:15 ` Laszlo Ersek
@ 2017-01-17 3:20 ` Wu, Jiaxin
0 siblings, 0 replies; 9+ messages in thread
From: Wu, Jiaxin @ 2017-01-17 3:20 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org
Cc: Kinney, Michael D, Justen, Jordan L, Gary Lin, Long, Qin
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, January 17, 2017 11:15 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>; Long, Qin
> <qin.long@intel.com>
> Subject: Re: [edk2] [PATCH v2] OvmfPkg: Remove the flag control for the
> CryptoPkg libraries
>
> On 01/17/17 03:56, Wu, Jiaxin wrote:
> >> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> >> libraries
> >>
> >> On 01/17/17 02:08, Wu, Jiaxin wrote:
> >>> Laszlo,
> >>>
> >>> I don't think this patch makes OpenSSL must requirement for building
> >>> OVMF by default.
> >>>
> >>> As I note in the commit log that "no build performance impacts" if
> >>> OpenSSL related library is not consumed by any other modules.
> >>
> >> I saw that comment, and I didn't understand it. What do you mean by
> >> "performance impact"? How quickly the tree builds? Or how quickly the
> >> resultant firmware boots? My concerns aren't related to performance, but
> >> whether OVMF builds at all, or not.
> >>
> >>> That
> >>> also means "Including OpenSSL libraries unconditionally won't break
> >>> OVMF build by default since all dependent modules are controlled by
> >>> the defined flag with the false value."
> >>
> >> So practically the suggestion is to provide unconditional library
> >> resolutions for the OpenSslLib, IntrinsicLib and BaseCryptLib classes,
> >> regardless of whether those classes are actually used by any module.
> >>
> >
> > Yes.
> > I thought "build performance" should include the build result and time
> consumption during the OVMF build. Sorry for the misunderstanding due to the
> ambiguity of "build performance impacts", and I agree to refine the commit log.
> >
> >
> >
> >> I see the point, but then the commit message should be improved. It
> >> should also explain that unused lib class resolutions that refer to
> >> nonexistent INF files (for example when OpenSSL is missing from the
> >> tree) do not cause build failures, unless the lib class is actually used.
> >>
> >> The commit message could be
> >>
> >> OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
> >>
> >
> > I don't have the strong opinion for the commit message change. That's also
> fine to me since we can reach an agreement:).
> >
> >
> >
> >>>
> >>> Secure Boot feature is controlled by:
> >>> * DEFINE SECURE_BOOT_ENABLE = FALSE
> >>>
> >>> ISCSIv6 requires OpenSSL, which is controlled by:
> >>> * DEFINE NETWORK_IP6_ENABLE = FALSE
> >>
> >> That's not entirely right; currently you can build with -D
> >> NETWORK_IP6_ENABLE and without OpenSSL (i.e., without -D
> >> SECURE_BOOT_ENABLE, at the moment). It will use IScsiDxe from
> >> MdeModulePkg, rather than from NetworkPkg.
> >>
> >> Is your argument that such an IPv6 stack (that is, with IScsiDxe comes
> >> from MdeModulePkg) is incomplete in itself? In other words, that a
> >> complete IPv6 stack requires IScsiDxe from NetworkPkg, hence OpenSSL too?
> >
> > Yes, that's my point.
> >
> >
> >
> >>
> >> In that case, the relevant parts of the OVMF DSC / FDF files should be
> >> fixed in a separate patch, with a separate justification. Something like:
> >>
> >> OvmfPkg: correct the set of modules included for the IPv6 stack
> >>
> >
> > Ok, that's fine the separate patch.
> >
> >
> >
> >>>
> >>> IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it
> >> should be controlled by:
> >>> * DEFINE NETWORK_IP6_ENABLE = FALSE
> >>> (For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled,
> we
> >> should fix it.)
> >>
> >> Yes, it could be part of the above-suggested IPv6-oriented patch.
> >>
> >>>
> >>> HTTPS/TLS will also be controlled by:
> >>> * DEFINE TLS_ENABLE = FALSE
> >>
> >> Makes sense.
> >>
> >> (And then HTTP_BOOT_ENABLE should pull in different modules dependent
> on
> >> TLS_ENABLE.)
> >
> > No, we can keep the current modules included in HTTP_BOOT_ENABLE, and
> make the TLS_ENABLE independently since TLS feature should not be limit to
> HTTP(S) feature.
> >
> > As I explained to Gary, TLS can be treated as independent module, which can
> be leveraged by third part drivers/apps (e.g. EAP-TLS). No TLS means no HTTPS.
> >
> >
> >
> >>
> >>> Namely:
> >>> OpenSSL is required to follow Patch-HOWTO *only when needed*.
> >>>
> >>> Of course, as you propose, we can also add OPENSSL_ENABLE flag to
> >>> control all the OpenSSL libraries. But as I mentioned above, do you
> >>> think it's necessary? I don't have strong opinion for OPENSSL_ENABLE
> >>> flag, but makes the logic more complexity as you list below.
> >>
> >> No, with your explanation, it seems fine. I think in total we'll need
> >> four patches:
> >>
> >> * OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
> >>
> >> Does what it says; commit message suggestions above.
> >>
> >> * OvmfPkg: correct the set of modules included for the IPv6 stack
> >>
> >> Fixes up IScsiDxe and IPSec, makes OpenSSL a hard requirement for
> >> IPv6. (And documents the fact in the commit message.)
> >>
> >> * OvmfPkg: pull in TLS modules with -D TLS_ENABLE
> >>
> >> Resolves the TLS-specific library classes, and pulls in TLS drivers
> >> (that are independent of HTTPS).
> >>
> >> * OvmfPkg: enable HTTPS boot under (HTTP_BOOT_ENABLE + TLS_ENABLE)
> >>
> >> Adds any TLS-specific customizations to existent HTTP_BOOT_ENABLE
> >> parts.
> >>
> >> What do you guys think?
> >>
> >
> > We can combine the last two patches instead:
> >
> > * OvmfPkg: Enable HTTPS/TLS feature under (HTTP_BOOT_ENABLE +
> TLS_ENABLE)
>
> Hm, okay. So I guess the presence of TLS-related protocols (provided by
> the drivers pulled in due to -D TLS_ENABLE) automatically enables HTTPS
> when the firmware runs, in the drivers that are pulled in by -D
> HTTP_BOOT_ENABLE?
>
> In that case, I suggest the subject
>
> OvmfPkg: pull in TLS modules with -D TLS_ENABLE (also enabling HTTPS)
>
> and explain in the commit message that TLS_ENABLE and HTTP_BOOT_ENABLE
> remain independent, but their intersection at build time produces HTTPS
> capability dynamically, when the firmware runs. Is this correct?
Exactly.
Thanks,
Jiaxin
>
> Thanks!
> Laszlo
>
> >> I believe it would be preferable if one of you (Gary?) could submit the
> >> whole 4-part series, with the other one (Jiaxin?) helping out with the
> >> review. Would that work for you both?
> >>
> > I'm fine with the propose:).
> >
> > Thanks,
> > Jiaxin
> >
> >
> >
> >
> >> Thanks!
> >> Laszlo
> >>
> >>>
> >>> Thanks,
> >>> Jiaxin
> >>>
> >>>> -----Original Message-----
> >>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >>>> Sent: Tuesday, January 17, 2017 4:33 AM
> >>>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
> >>>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin
> <glin@suse.com>;
> >>>> Long, Qin <qin.long@intel.com>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>
> >>>> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the
> CryptoPkg
> >>>> libraries
> >>>>
> >>>> On 01/16/17 13:22, Jiaxin Wu wrote:
> >>>>> v2:
> >>>>> * Remove the flag for NetworkPkg/IScsiDxe
> >>>>>
> >>>>> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> >>>>> the CryptoPkg librarie.
> >>>>>
> >>>>> Not only the secure boot feature requires the CryptoPkg libraries
> >>>>> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> >>>>> features. Those modules can be always included since no build
> >> performance
> >>>>> impacts if they are not consumed.
> >>>>>
> >>>>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>>>> Cc: Justen Jordan L <jordan.l.justen@intel.com>
> >>>>> Cc: Gary Lin <glin@suse.com>
> >>>>> Cc: Long Qin <qin.long@intel.com>
> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >>>>> ---
> >>>>> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> >>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> >>>>> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> >>>>> 3 files changed, 18 insertions(+), 33 deletions(-)
> >>>>
> >>>> I disagree with this patch (assuming at least that I understand it
> >>>> correctly).
> >>>>
> >>>> Namely,
> >>>> - unconditionally resolving OpensslLib in the DSC files, and
> >>>> - unconditionally consuming OpensslLib in modules that are
> >>>> unconditionally included in the DSC files,
> >>>>
> >>>> makes OpenSSL a hard requirement for building OVMF.
> >>>>
> >>>> Given that OpenSSL is not distributed as part of the edk2 tree, and
> >>>> given that it's not even pulled in through an unmodified git submodule,
> >>>> this patch would prevent people, IIUC, from building OVMF without
> >>>> jumping through the hoops described in
> >>>>
> >>>> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> >>>>
> >>>> That's a bad thing, forcing people to download and patch OpenSSL even if
> >>>> they don't care about any of the dependent features. (It is perfectly
> >>>> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
> >>>> and iSCSI, in a virtual machine.)
> >>>>
> >>>> If OpenSSL were distributed as part of edk2, or if OpenSSL were
> >>>> presented as a plain (unmodified) git submodule in edk2, then I might
> agree.
> >>>>
> >>>> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
> >>>>
> >>>> - Features that require OpenSSL no matter what, such as
> >>>> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
> >>>>
> >>>> (I don't remember if the [Defines] section of the DSC file can set
> >>>> macros conditionally, dependent on other macros, but I hope so.)
> >>>>
> >>>> - Features that can utilize (but don't require) OpenSSL, such as
> >>>> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide
> >> conditional
> >>>> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
> >>>>
> >>>> - The libraries and drivers that provide the crypto stuff (directly on
> >>>> top of OpenSSL) should depend on OPENSSL_ENABLE.
> >>>>
> >>>> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
> >>>> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE
> >> should
> >>>> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
> >>>>
> >>>> In summary:
> >>>> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
> >>>> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
> >>>> - NETWORK_IP6_ENABLE should be customized based on
> OPENSSL_ENABLE
> >>>> (for the ISCSI driver).
> >>>> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
> >>>> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
> >>>> wrap the OpenSSL functionality, for edk2.
> >>>>
> >>>> As a result, the following build option combinations would be valid
> >>>> (listing some examples):
> >>>>
> >>>> * -D SECURE_BOOT_ENABLE
> >>>>
> >>>> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
> >>>> fine, otherwise it would break, as it should.
> >>>>
> >>>> * -D NETWORK_IP6_ENABLE
> >>>>
> >>>> You get the IPv6 stack, but no secure ISCSI.
> >>>>
> >>>> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
> >>>>
> >>>> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
> >>>> available, the build breaks, as it should.
> >>>>
> >>>> * -D HTTP_BOOT_ENABLE
> >>>>
> >>>> You get HTTP boot, but not HTTPS boot.
> >>>>
> >>>> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is
> >> useless
> >>>>
> >>>> Same, no change.
> >>>>
> >>>> * -D TLS_ENABLE
> >>>>
> >>>> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
> >>>> the build breaks. Otherwise, the TLS drivers are included in the fw
> >>>> binary. They might not be used by any edk2 module, but some 3rd party
> >>>> UEFI application (launched from the shell, eg.) could.
> >>>>
> >>>> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
> >>>>
> >>>> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
> >>>> tree, the build breaks.
> >>>>
> >>>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
> >>>> NETWORK_IP6_ENABLE
> >>>>
> >>>> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
> >>>> boot.
> >>>>
> >>>> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
> >>>> -D NETWORK_IP6_ENABLE
> >>>>
> >>>> You get everything.
> >>>>
> >>>> My point is, if we touch these build flags, then we should go the whole
> >>>> way, and express their inter-dependencies precisely.
> >>>>
> >>>> Thanks!
> >>>> Laszlo
> >>>>
> >>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> >>>>> index e97f7f0..6e53d9f 100644
> >>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
> >>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> >>>>> @@ -1,9 +1,9 @@
> >>>>> ## @file
> >>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>>>> #
> >>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>>>> #
> >>>>> # This program and the accompanying materials
> >>>>> # are licensed and made available under the terms and conditions of
> the
> >>>> BSD License
> >>>>> # which accompanies this distribution. The full text of the license may
> be
> >>>> found at
> >>>>> @@ -139,14 +139,15 @@
> >>>>>
> >>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>>>
> >>>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>>>
> >>>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >>>> ebugPrintErrorLevelLib.inf
> >>>>>
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> -
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>>>> +
> >>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>>
> >>>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >>>> easurementLib.inf
> >>>>>
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>>>> !endif
> >>>>> @@ -164,13 +165,11 @@
> >>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>>>
> >>>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >>>> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>>>>
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>>>
> >>>>> [LibraryClasses.common]
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>>>> -!endif
> >>>>>
> >>>>> [LibraryClasses.common.SEC]
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>>>>
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>>>> @@ -256,13 +255,13 @@
> >>>>>
> >>>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>>>> !else
> >>>>>
> >>>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >>>> nf
> >>>>> !endif
> >>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>>>> -!endif
> >>>>> +
> >>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>>>
> >>>>> [LibraryClasses.common.UEFI_DRIVER]
> >>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>>>> @@ -698,16 +697,12 @@
> >>>>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>>>> !else
> >>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> -!endif
> >>>>> -!else
> >>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> !endif
> >>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc
> b/OvmfPkg/OvmfPkgIa32X64.dsc
> >>>>> index 8e3e04c..15db2d5 100644
> >>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> >>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> >>>>> @@ -1,9 +1,9 @@
> >>>>> ## @file
> >>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>>>> #
> >>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>>>> #
> >>>>> # This program and the accompanying materials
> >>>>> # are licensed and made available under the terms and conditions of
> the
> >>>> BSD License
> >>>>> # which accompanies this distribution. The full text of the license may
> be
> >>>> found at
> >>>>> @@ -144,14 +144,15 @@
> >>>>>
> >>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>>>
> >>>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>>>
> >>>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >>>> ebugPrintErrorLevelLib.inf
> >>>>>
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> -
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>>>> +
> >>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>>
> >>>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >>>> easurementLib.inf
> >>>>>
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>>>> !endif
> >>>>> @@ -169,13 +170,11 @@
> >>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>>>
> >>>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >>>> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>>>>
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>>>
> >>>>> [LibraryClasses.common]
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>>>> -!endif
> >>>>>
> >>>>> [LibraryClasses.common.SEC]
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>>>>
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>>>> @@ -261,13 +260,13 @@
> >>>>>
> >>>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>>>> !else
> >>>>>
> >>>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >>>> nf
> >>>>> !endif
> >>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>>>> -!endif
> >>>>> +
> >>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>>>
> >>>>> [LibraryClasses.common.UEFI_DRIVER]
> >>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>>>> @@ -707,16 +706,12 @@
> >>>>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>>>> !else
> >>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> -!endif
> >>>>> -!else
> >>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> !endif
> >>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> >>>>> index 6ec3fe0..9c6bdc2 100644
> >>>>> --- a/OvmfPkg/OvmfPkgX64.dsc
> >>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
> >>>>> @@ -1,9 +1,9 @@
> >>>>> ## @file
> >>>>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> >>>>> #
> >>>>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >>>>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> >>>>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >>>>> #
> >>>>> # This program and the accompanying materials
> >>>>> # are licensed and made available under the terms and conditions of
> the
> >>>> BSD License
> >>>>> # which accompanies this distribution. The full text of the license may
> be
> >>>> found at
> >>>>> @@ -144,14 +144,15 @@
> >>>>>
> >>>>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >>>>>
> >>>>
> >>
> LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> >>>>>
> >>>>
> >>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> >>>> ebugPrintErrorLevelLib.inf
> >>>>>
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> -
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> >>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> >>>>> +
> >>>>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>
> >>
> PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >>>>>
> >>>>
> >>
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> >>>> easurementLib.inf
> >>>>>
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >>>>> !if $(NETWORK_IP6_ENABLE) == TRUE
> >>>>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> >>>>> !endif
> >>>>> @@ -169,13 +170,11 @@
> >>>>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> >>>>>
> >>>>
> >>
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> >>>> /BaseOrderedCollectionRedBlackTreeLib.inf
> >>>>>
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> >>>>>
> >>>>> [LibraryClasses.common]
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> >>>>> -!endif
> >>>>>
> >>>>> [LibraryClasses.common.SEC]
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> >>>>>
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> >>>>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> >>>>> @@ -261,13 +260,13 @@
> >>>>>
> >>>>
> >>
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> >>>>> !else
> >>>>>
> >>>>
> >>
> DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> >>>> nf
> >>>>> !endif
> >>>>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> +
> >>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >>>>> -!endif
> >>>>> +
> >>>>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> >>>>>
> >>>>> [LibraryClasses.common.UEFI_DRIVER]
> >>>>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> >>>>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> >>>>> @@ -705,16 +704,12 @@
> >>>>> NetworkPkg/TcpDxe/TcpDxe.inf
> >>>>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> >>>>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> >>>>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> >>>>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> >>>>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> >>>>> !else
> >>>>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> -!endif
> >>>>> -!else
> >>>>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >>>>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >>>>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> >>>>> !endif
> >>>>> !if $(HTTP_BOOT_ENABLE) == TRUE
> >>>>>
> >>>
> >
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries
2017-01-17 2:56 ` Wu, Jiaxin
2017-01-17 3:15 ` Laszlo Ersek
@ 2017-01-17 3:35 ` Gary Lin
1 sibling, 0 replies; 9+ messages in thread
From: Gary Lin @ 2017-01-17 3:35 UTC (permalink / raw)
To: Wu, Jiaxin, Laszlo Ersek
Cc: edk2-devel@ml01.01.org, Kinney, Michael D, Justen, Jordan L,
Long, Qin
On Tue, Jan 17, 2017 at 02:56:16AM +0000, Wu, Jiaxin wrote:
> > Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> > libraries
> >
> > On 01/17/17 02:08, Wu, Jiaxin wrote:
> > > Laszlo,
> > >
> > > I don't think this patch makes OpenSSL must requirement for building
> > > OVMF by default.
> > >
> > > As I note in the commit log that "no build performance impacts" if
> > > OpenSSL related library is not consumed by any other modules.
> >
> > I saw that comment, and I didn't understand it. What do you mean by
> > "performance impact"? How quickly the tree builds? Or how quickly the
> > resultant firmware boots? My concerns aren't related to performance, but
> > whether OVMF builds at all, or not.
> >
> > > That
> > > also means "Including OpenSSL libraries unconditionally won't break
> > > OVMF build by default since all dependent modules are controlled by
> > > the defined flag with the false value."
> >
> > So practically the suggestion is to provide unconditional library
> > resolutions for the OpenSslLib, IntrinsicLib and BaseCryptLib classes,
> > regardless of whether those classes are actually used by any module.
> >
>
> Yes.
> I thought "build performance" should include the build result and time consumption during the OVMF build. Sorry for the misunderstanding due to the ambiguity of "build performance impacts", and I agree to refine the commit log.
>
>
Just did a test on OvmfPkgX64.dsc and confirmed that providing those
library resolutions doesn't break the build without openssl.
>
> > I see the point, but then the commit message should be improved. It
> > should also explain that unused lib class resolutions that refer to
> > nonexistent INF files (for example when OpenSSL is missing from the
> > tree) do not cause build failures, unless the lib class is actually used.
> >
> > The commit message could be
> >
> > OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
> >
>
> I don't have the strong opinion for the commit message change. That's also fine to me since we can reach an agreement:).
>
>
>
> > >
> > > Secure Boot feature is controlled by:
> > > * DEFINE SECURE_BOOT_ENABLE = FALSE
> > >
> > > ISCSIv6 requires OpenSSL, which is controlled by:
> > > * DEFINE NETWORK_IP6_ENABLE = FALSE
> >
> > That's not entirely right; currently you can build with -D
> > NETWORK_IP6_ENABLE and without OpenSSL (i.e., without -D
> > SECURE_BOOT_ENABLE, at the moment). It will use IScsiDxe from
> > MdeModulePkg, rather than from NetworkPkg.
> >
> > Is your argument that such an IPv6 stack (that is, with IScsiDxe comes
> > from MdeModulePkg) is incomplete in itself? In other words, that a
> > complete IPv6 stack requires IScsiDxe from NetworkPkg, hence OpenSSL too?
>
> Yes, that's my point.
>
>
>
> >
> > In that case, the relevant parts of the OVMF DSC / FDF files should be
> > fixed in a separate patch, with a separate justification. Something like:
> >
> > OvmfPkg: correct the set of modules included for the IPv6 stack
> >
>
> Ok, that's fine the separate patch.
>
>
>
> > >
> > > IPsec is a mandatory part of IPv6, but is not an integral part of IPv4, then it
> > should be controlled by:
> > > * DEFINE NETWORK_IP6_ENABLE = FALSE
> > > (For IPsec, I just notice it's not included in OVMF platform if IPV6 enabled, we
> > should fix it.)
> >
> > Yes, it could be part of the above-suggested IPv6-oriented patch.
> >
> > >
> > > HTTPS/TLS will also be controlled by:
> > > * DEFINE TLS_ENABLE = FALSE
> >
> > Makes sense.
> >
> > (And then HTTP_BOOT_ENABLE should pull in different modules dependent on
> > TLS_ENABLE.)
>
> No, we can keep the current modules included in HTTP_BOOT_ENABLE, and make the TLS_ENABLE independently since TLS feature should not be limit to HTTP(S) feature.
>
> As I explained to Gary, TLS can be treated as independent module, which can be leveraged by third part drivers/apps (e.g. EAP-TLS). No TLS means no HTTPS.
>
>
>
> >
> > > Namely:
> > > OpenSSL is required to follow Patch-HOWTO *only when needed*.
> > >
> > > Of course, as you propose, we can also add OPENSSL_ENABLE flag to
> > > control all the OpenSSL libraries. But as I mentioned above, do you
> > > think it's necessary? I don't have strong opinion for OPENSSL_ENABLE
> > > flag, but makes the logic more complexity as you list below.
> >
> > No, with your explanation, it seems fine. I think in total we'll need
> > four patches:
> >
> > * OvmfPkg: always resolve OpenSslLib, IntrinsicLib and BaseCryptLib
> >
> > Does what it says; commit message suggestions above.
> >
> > * OvmfPkg: correct the set of modules included for the IPv6 stack
> >
> > Fixes up IScsiDxe and IPSec, makes OpenSSL a hard requirement for
> > IPv6. (And documents the fact in the commit message.)
> >
> > * OvmfPkg: pull in TLS modules with -D TLS_ENABLE
> >
> > Resolves the TLS-specific library classes, and pulls in TLS drivers
> > (that are independent of HTTPS).
> >
> > * OvmfPkg: enable HTTPS boot under (HTTP_BOOT_ENABLE + TLS_ENABLE)
> >
> > Adds any TLS-specific customizations to existent HTTP_BOOT_ENABLE
> > parts.
> >
> > What do you guys think?
> >
>
> We can combine the last two patches instead:
>
> * OvmfPkg: Enable HTTPS/TLS feature under (HTTP_BOOT_ENABLE + TLS_ENABLE)
>
Combining the last two patches makes sense to me since we don't really need
to change anything in HTTP_BOOT_ENABLE. Just add TlsLib, TlsDxe, and
TlsAuthConfigDxe.
>
>
> > I believe it would be preferable if one of you (Gary?) could submit the
> > whole 4-part series, with the other one (Jiaxin?) helping out with the
> > review. Would that work for you both?
> >
> I'm fine with the propose:).
>
I'm working on the patches now :)
Thanks,
Gary Lin
> Thanks,
> Jiaxin
>
>
>
>
> > Thanks!
> > Laszlo
> >
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > >> -----Original Message-----
> > >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> > >> Sent: Tuesday, January 17, 2017 4:33 AM
> > >> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@ml01.01.org
> > >> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gary Lin <glin@suse.com>;
> > >> Long, Qin <qin.long@intel.com>; Kinney, Michael D
> > >> <michael.d.kinney@intel.com>
> > >> Subject: Re: [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg
> > >> libraries
> > >>
> > >> On 01/16/17 13:22, Jiaxin Wu wrote:
> > >>> v2:
> > >>> * Remove the flag for NetworkPkg/IScsiDxe
> > >>>
> > >>> This patch is to remove the 'SECURE_BOOT_ENABLE' flag control for
> > >>> the CryptoPkg librarie.
> > >>>
> > >>> Not only the secure boot feature requires the CryptoPkg libraries
> > >>> (e.g, OpensslLib, BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS
> > >>> features. Those modules can be always included since no build
> > performance
> > >>> impacts if they are not consumed.
> > >>>
> > >>> Cc: Laszlo Ersek <lersek@redhat.com>
> > >>> Cc: Justen Jordan L <jordan.l.justen@intel.com>
> > >>> Cc: Gary Lin <glin@suse.com>
> > >>> Cc: Long Qin <qin.long@intel.com>
> > >>> Contributed-under: TianoCore Contribution Agreement 1.0
> > >>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > >>> ---
> > >>> OvmfPkg/OvmfPkgIa32.dsc | 17 ++++++-----------
> > >>> OvmfPkg/OvmfPkgIa32X64.dsc | 17 ++++++-----------
> > >>> OvmfPkg/OvmfPkgX64.dsc | 17 ++++++-----------
> > >>> 3 files changed, 18 insertions(+), 33 deletions(-)
> > >>
> > >> I disagree with this patch (assuming at least that I understand it
> > >> correctly).
> > >>
> > >> Namely,
> > >> - unconditionally resolving OpensslLib in the DSC files, and
> > >> - unconditionally consuming OpensslLib in modules that are
> > >> unconditionally included in the DSC files,
> > >>
> > >> makes OpenSSL a hard requirement for building OVMF.
> > >>
> > >> Given that OpenSSL is not distributed as part of the edk2 tree, and
> > >> given that it's not even pulled in through an unmodified git submodule,
> > >> this patch would prevent people, IIUC, from building OVMF without
> > >> jumping through the hoops described in
> > >>
> > >> CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> > >>
> > >> That's a bad thing, forcing people to download and patch OpenSSL even if
> > >> they don't care about any of the dependent features. (It is perfectly
> > >> possible to be uninterested in *all* of: Secure Boot, IpSec, HTTPS boot,
> > >> and iSCSI, in a virtual machine.)
> > >>
> > >> If OpenSSL were distributed as part of edk2, or if OpenSSL were
> > >> presented as a plain (unmodified) git submodule in edk2, then I might agree.
> > >>
> > >> For now, perhaps we can introduce an OPENSSL_ENABLE build option.
> > >>
> > >> - Features that require OpenSSL no matter what, such as
> > >> SECURE_BOOT_ENABLE, should auto-define OPENSSL_ENABLE.
> > >>
> > >> (I don't remember if the [Defines] section of the DSC file can set
> > >> macros conditionally, dependent on other macros, but I hope so.)
> > >>
> > >> - Features that can utilize (but don't require) OpenSSL, such as
> > >> NETWORK_IP6_ENABLE and HTTP_BOOT_ENABLE, should provide
> > conditional
> > >> DSC stanzas for both $(OPENSSL_ENABLE) == TRUE and == FALSE.
> > >>
> > >> - The libraries and drivers that provide the crypto stuff (directly on
> > >> top of OpenSSL) should depend on OPENSSL_ENABLE.
> > >>
> > >> In fact, looking at Gary's patch "OvmfPkg: Enable HTTPS for Ovmf" with
> > >> TLS_ENABLE, it seems like we need another layer. HTTP_BOOT_ENABLE
> > should
> > >> not be customized for OPENSSL_ENABLE, but for TLS_ENABLE.
> > >>
> > >> In summary:
> > >> - SECURE_BOOT_ENABLE should auto-select OPENSSL_ENABLE.
> > >> - TLS_ENABLE should auto-select OPENSSL_ENABLE.
> > >> - NETWORK_IP6_ENABLE should be customized based on OPENSSL_ENABLE
> > >> (for the ISCSI driver).
> > >> - HTTP_BOOT_ENABLE should be customized based on TLS_ENABLE.
> > >> - OPENSSL_ENABLE should control the CryptoPkg modules that directly
> > >> wrap the OpenSSL functionality, for edk2.
> > >>
> > >> As a result, the following build option combinations would be valid
> > >> (listing some examples):
> > >>
> > >> * -D SECURE_BOOT_ENABLE
> > >>
> > >> It would set OPENSSL_ENABLE. If OpenSSL is available, it would build
> > >> fine, otherwise it would break, as it should.
> > >>
> > >> * -D NETWORK_IP6_ENABLE
> > >>
> > >> You get the IPv6 stack, but no secure ISCSI.
> > >>
> > >> * -D NETWORK_IP6_ENABLE -D OPENSSL_ENABLE
> > >>
> > >> You get the IPv6 stack, with secure ISCSI. If OpenSSL is not
> > >> available, the build breaks, as it should.
> > >>
> > >> * -D HTTP_BOOT_ENABLE
> > >>
> > >> You get HTTP boot, but not HTTPS boot.
> > >>
> > >> * -D HTTP_BOOT_ENABLE -D OPENSSL_ENABLE <----- note that this is
> > useless
> > >>
> > >> Same, no change.
> > >>
> > >> * -D TLS_ENABLE
> > >>
> > >> Selects OPENSSL_ENABLE automatically. If OpenSSL is not available,
> > >> the build breaks. Otherwise, the TLS drivers are included in the fw
> > >> binary. They might not be used by any edk2 module, but some 3rd party
> > >> UEFI application (launched from the shell, eg.) could.
> > >>
> > >> * -D HTTP_BOOT_ENABLE -D TLS_ENABLE
> > >>
> > >> HTTP and HTTPS boot becomes available. If OpenSSL is absent from the
> > >> tree, the build breaks.
> > >>
> > >> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D
> > >> NETWORK_IP6_ENABLE
> > >>
> > >> You get Secure Boot, and secure ISCSI with IPv6, but not HTTPS
> > >> boot.
> > >>
> > >> * -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE -D TLS_ENABLE \
> > >> -D NETWORK_IP6_ENABLE
> > >>
> > >> You get everything.
> > >>
> > >> My point is, if we touch these build flags, then we should go the whole
> > >> way, and express their inter-dependencies precisely.
> > >>
> > >> Thanks!
> > >> Laszlo
> > >>
> > >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > >>> index e97f7f0..6e53d9f 100644
> > >>> --- a/OvmfPkg/OvmfPkgIa32.dsc
> > >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > >>> @@ -1,9 +1,9 @@
> > >>> ## @file
> > >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > >>> #
> > >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > >>> #
> > >>> # This program and the accompanying materials
> > >>> # are licensed and made available under the terms and conditions of the
> > >> BSD License
> > >>> # which accompanies this distribution. The full text of the license may be
> > >> found at
> > >>> @@ -139,14 +139,15 @@
> > >>>
> > >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > >>>
> > >>
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > >>>
> > >>
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> > >> ebugPrintErrorLevelLib.inf
> > >>>
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> -
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > >>> +
> > >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>>
> > >>
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> > >> easurementLib.inf
> > >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> > >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > >>> !endif
> > >>> @@ -164,13 +165,11 @@
> > >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > >>>
> > >>
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> > >> /BaseOrderedCollectionRedBlackTreeLib.inf
> > >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> > >>>
> > >>> [LibraryClasses.common]
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > >>> -!endif
> > >>>
> > >>> [LibraryClasses.common.SEC]
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> > >>> @@ -256,13 +255,13 @@
> > >>>
> > >>
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > >>> !else
> > >>>
> > >>
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> > >> nf
> > >>> !endif
> > >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > >>> -!endif
> > >>> +
> > >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> > >>>
> > >>> [LibraryClasses.common.UEFI_DRIVER]
> > >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >>> @@ -698,16 +697,12 @@
> > >>> NetworkPkg/TcpDxe/TcpDxe.inf
> > >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> > >>> !else
> > >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> -!endif
> > >>> -!else
> > >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> !endif
> > >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> > >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > >>> index 8e3e04c..15db2d5 100644
> > >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > >>> @@ -1,9 +1,9 @@
> > >>> ## @file
> > >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > >>> #
> > >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > >>> #
> > >>> # This program and the accompanying materials
> > >>> # are licensed and made available under the terms and conditions of the
> > >> BSD License
> > >>> # which accompanies this distribution. The full text of the license may be
> > >> found at
> > >>> @@ -144,14 +144,15 @@
> > >>>
> > >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > >>>
> > >>
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > >>>
> > >>
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> > >> ebugPrintErrorLevelLib.inf
> > >>>
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> -
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > >>> +
> > >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>>
> > >>
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> > >> easurementLib.inf
> > >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> > >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > >>> !endif
> > >>> @@ -169,13 +170,11 @@
> > >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > >>>
> > >>
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> > >> /BaseOrderedCollectionRedBlackTreeLib.inf
> > >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> > >>>
> > >>> [LibraryClasses.common]
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > >>> -!endif
> > >>>
> > >>> [LibraryClasses.common.SEC]
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> > >>> @@ -261,13 +260,13 @@
> > >>>
> > >>
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > >>> !else
> > >>>
> > >>
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> > >> nf
> > >>> !endif
> > >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > >>> -!endif
> > >>> +
> > >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> > >>>
> > >>> [LibraryClasses.common.UEFI_DRIVER]
> > >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >>> @@ -707,16 +706,12 @@
> > >>> NetworkPkg/TcpDxe/TcpDxe.inf
> > >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> > >>> !else
> > >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> -!endif
> > >>> -!else
> > >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> !endif
> > >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> > >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > >>> index 6ec3fe0..9c6bdc2 100644
> > >>> --- a/OvmfPkg/OvmfPkgX64.dsc
> > >>> +++ b/OvmfPkg/OvmfPkgX64.dsc
> > >>> @@ -1,9 +1,9 @@
> > >>> ## @file
> > >>> # EFI/Framework Open Virtual Machine Firmware (OVMF) platform
> > >>> #
> > >>> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> > >>> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > >>> #
> > >>> # This program and the accompanying materials
> > >>> # are licensed and made available under the terms and conditions of the
> > >> BSD License
> > >>> # which accompanies this distribution. The full text of the license may be
> > >> found at
> > >>> @@ -144,14 +144,15 @@
> > >>>
> > >>> ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > >>>
> > >>
> > LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> > >>>
> > >>
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseD
> > >> ebugPrintErrorLevelLib.inf
> > >>>
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> -
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > >>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > >>> +
> > >>> +!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>
> > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> > >>>
> > >>
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmM
> > >> easurementLib.inf
> > >>> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > >>> !if $(NETWORK_IP6_ENABLE) == TRUE
> > >>> TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > >>> !endif
> > >>> @@ -169,13 +170,11 @@
> > >>> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> > >>>
> > >>
> > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib
> > >> /BaseOrderedCollectionRedBlackTreeLib.inf
> > >>> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> > >>>
> > >>> [LibraryClasses.common]
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > >>> -!endif
> > >>>
> > >>> [LibraryClasses.common.SEC]
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> > >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > >>> !ifdef $(DEBUG_ON_SERIAL_PORT)
> > >>> @@ -261,13 +260,13 @@
> > >>>
> > >>
> > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > >>> !else
> > >>>
> > >>
> > DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.i
> > >> nf
> > >>> !endif
> > >>> UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> +
> > >>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > >>> -!endif
> > >>> +
> > >>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> > >>>
> > >>> [LibraryClasses.common.UEFI_DRIVER]
> > >>> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > >>> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >>> @@ -705,16 +704,12 @@
> > >>> NetworkPkg/TcpDxe/TcpDxe.inf
> > >>> NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > >>> NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > >>> NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > >>> NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> -!if $(SECURE_BOOT_ENABLE) == TRUE
> > >>> NetworkPkg/IScsiDxe/IScsiDxe.inf
> > >>> !else
> > >>> - MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> -!endif
> > >>> -!else
> > >>> MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > >>> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > >>> MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > >>> !endif
> > >>> !if $(HTTP_BOOT_ENABLE) == TRUE
> > >>>
> > >
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-17 3:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-16 12:22 [PATCH v2] OvmfPkg: Remove the flag control for the CryptoPkg libraries Jiaxin Wu
2017-01-16 20:33 ` Laszlo Ersek
2017-01-17 1:08 ` Wu, Jiaxin
2017-01-17 1:47 ` Laszlo Ersek
2017-01-17 2:56 ` Wu, Jiaxin
2017-01-17 3:15 ` Laszlo Ersek
2017-01-17 3:20 ` Wu, Jiaxin
2017-01-17 3:35 ` Gary Lin
2017-01-17 1:54 ` Jordan Justen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox