Re: [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> Add a minimal, NULL class library called "PxeBcPcdProducerLib" for setting
> the "PcdIPv4PXESupport" and "PcdIPv6PXESupport" PCDs of NetworkPkg, from
> fw_cfg.
> 
> These PCDs control whether the UefiPxeBcDxe driver supports PXEv4 / PXEv6
> boot. If a PXE version is disabled, the corresponding LoadFile protocol
> instance is not produced by UefiPxeBcDxe, and so
> EfiBootManagerRefreshAllBootOption() in UefiBootManagerLib does not
> generate corresponding *new* boot options either. (Existent boot options
> are not deleted.)
> 
> Hook the library into the UefiPxeBcDxe driver. (The driver is already
> included from "NetworkComponents.dsc.inc", but we can list it again in the
> DSC file, for providing <LibraryClasses> overrides.)
> 
> In OVMF, the PCDs could be set in PlatformPei too, but ArmVirtQemu does
> not have fw_cfg access in the PEI phase. Hence a NULL class library that
> can be linked into UefiPxeBcDxe.
> 
> When listing the PCDs under [PcdsDynamicDefault], stick with the DEC
> default values.
> 
> QEMU switches:
> 
>    -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
> 
>    -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
> 
> The "opt/org.tianocore" prefix follows the "opt/RFQDN/" recommendation
> from QEMU's "docs/specs/fw_cfg.txt".
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   OvmfPkg/OvmfPkgIa32.dsc                                     |  9 +++++
>   OvmfPkg/OvmfPkgIa32X64.dsc                                  | 10 +++++
>   OvmfPkg/OvmfPkgX64.dsc                                      |  9 +++++
>   OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 +++++++++++++++++
>   OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c              | 39 ++++++++++++++++++++
>   5 files changed, 100 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 5e2972063110..fcd9779b5ba2 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -605,6 +605,10 @@ [PcdsDynamicDefault]
>     gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>   !endif
>   
> +  # IPv4 and IPv6 PXE Boot support.
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
>   [PcdsDynamicHii]
>   !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
>     gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -822,6 +826,11 @@ [Components]
>     #
>   !include NetworkPkg/NetworkComponents.dsc.inc
>   
> +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> +    <LibraryClasses>
> +      NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> +  }
> +
>   !if $(NETWORK_TLS_ENABLE) == TRUE
>     NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
>       <LibraryClasses>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 18e6909a33fa..1626d2415a2c 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -616,6 +616,11 @@ [PcdsDynamicDefault]
>     gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>   !endif
>   
> +[PcdsDynamicDefault.X64]
> +  # IPv4 and IPv6 PXE Boot support.
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
>   [PcdsDynamicHii]
>   !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
>     gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -834,6 +839,11 @@ [Components.X64]
>     #
>   !include NetworkPkg/NetworkComponents.dsc.inc
>   
> +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> +    <LibraryClasses>
> +      NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> +  }
> +
>   !if $(NETWORK_TLS_ENABLE) == TRUE
>     NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
>       <LibraryClasses>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 3d24cc4c1cfb..65cfe957761b 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -615,6 +615,10 @@ [PcdsDynamicDefault]
>     gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>   !endif
>   
> +  # IPv4 and IPv6 PXE Boot support.
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
>   [PcdsDynamicHii]
>   !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
>     gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -832,6 +836,11 @@ [Components]
>     #
>   !include NetworkPkg/NetworkComponents.dsc.inc
>   
> +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> +    <LibraryClasses>
> +      NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> +  }
> +
>   !if $(NETWORK_TLS_ENABLE) == TRUE
>     NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
>       <LibraryClasses>
> diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> new file mode 100644
> index 000000000000..948133228111
> --- /dev/null
> +++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +# Configure some PCDs dynamically for
> +# "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
> +#
> +# Copyright (C) 2020, Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PxeBcPcdProducerLib
> +  FILE_GUID                      = 1da2723f-52df-432a-8d03-6e8fa8acc107
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NULL
> +  CONSTRUCTOR                    = SetPxeBcPcds
> +
> +[Sources]
> +  PxeBcPcd.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  NetworkPkg/NetworkPkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  PcdLib
> +  QemuFwCfgSimpleParserLib
> +
> +[Pcd]
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport       ## SOMETIMES_PRODUCES
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport       ## SOMETIMES_PRODUCES
> diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> new file mode 100644
> index 000000000000..7ce236326fb4
> --- /dev/null
> +++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> @@ -0,0 +1,39 @@
> +/** @file
> +  Configure some PCDs dynamically for
> +  "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
> +
> +  Copyright (C) 2020, Red Hat, Inc.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/PcdLib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +SetPxeBcPcds (
> +  VOID
> +  )
> +{
> +  BOOLEAN       FwCfgBool;
> +  RETURN_STATUS PcdStatus;
> +
> +  if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv4PXESupport",
> +                       &FwCfgBool))) {
> +    PcdStatus = PcdSet8S (PcdIPv4PXESupport, FwCfgBool);

OK this is the 'current' interface, and NetworkPkg/UefiPxeBcDxe still 
uses the deprecated one 'PcdGet8()'.

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> +    if (RETURN_ERROR (PcdStatus)) {
> +      return PcdStatus;
> +    }
> +  }
> +
> +  if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv6PXESupport",
> +                       &FwCfgBool))) {
> +    PcdStatus = PcdSet8S (PcdIPv6PXESupport, FwCfgBool);
> +    if (RETURN_ERROR (PcdStatus)) {
> +      return PcdStatus;
> +    }
> +  }
> +
> +  return RETURN_SUCCESS;
> +}
>