Re: [edk2-devel] [PATCH v2 02/31] OvmfPkg: Create platform XenOvmf

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

On 04/09/19 13:08, Anthony PERARD wrote:
> This is a copy of OvmfX64, removing VirtIO and some SMM.
> 
> This new platform will be changed to make it works on two types of Xen
> guest: HVM and PVH.
> 
> Compare to OvmfX64, this patch:
> 
> - changed: PLATFORM_GUID, OUTPUT_DIRECTORY, FLASH_DEFINITION
> - removed: VirtioLib class resolution
> - removed: all UEFI_DRIVER modules for virtio devices
> - removed: DXE_SMM_DRIVER and SMM_CORE lib class resolutions
> - removed: DXE_SMM_DRIVER and SMM_CORE FDF rules
> - removed: Everything related to SMM_REQUIRE==true
> - removed: Everything related to SECURE_BOOT_ENABLE==true
> - removed: Everything related to TPM2_ENABLE==true
> - changed: PcdPciDisableBusEnumeration dynamic default flipped to TRUE
> - changed: default FD_SIZE_IN_KB to 2M.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/{OvmfPkgX64.dsc => XenOvmf.dsc}     | 202 +-------------------
>  OvmfPkg/{OvmfPkgIa32X64.fdf => XenOvmf.fdf} |  72 +------
>  2 files changed, 12 insertions(+), 262 deletions(-)

Nicely done (and nicely formatted for review too), thanks.

I have some suggestions:


(1) in the commit message, it might be worth mentioning (additionally)
that we undo commit d272449d9e1e ("OvmfPkg: raise DXEFV size to 11 MB",
2018-05-29).


(2) Please don't forget to rename & replace XenOvmf -> OvmfXen (in file
names, the commit message, and the patch body).

> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/XenOvmf.dsc
> similarity index 79%
> copy from OvmfPkg/OvmfPkgX64.dsc
> copy to OvmfPkg/XenOvmf.dsc
> index 2943e9e8af..bfe9190735 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/XenOvmf.dsc
> @@ -3,6 +3,7 @@
>  #
>  #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#  Copyright (c) 2019, Citrix Systems, Inc.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
> @@ -21,26 +22,22 @@
>  ################################################################################
>  [Defines]
>    PLATFORM_NAME                  = Ovmf
> -  PLATFORM_GUID                  = 5a9e7754-d81b-49ea-85ad-69eaa7b1539b
> +  PLATFORM_GUID                  = e3aa4fbe-9459-482d-bd40-d3f3b5f89d6e
>    PLATFORM_VERSION               = 0.1
>    DSC_SPECIFICATION              = 0x00010005
> -  OUTPUT_DIRECTORY               = Build/OvmfX64
> +  OUTPUT_DIRECTORY               = Build/XenOvmf
>    SUPPORTED_ARCHITECTURES        = X64
>    BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
>    SKUID_IDENTIFIER               = DEFAULT
> -  FLASH_DEFINITION               = OvmfPkg/OvmfPkgX64.fdf
> +  FLASH_DEFINITION               = OvmfPkg/XenOvmf.fdf
>  
>    #
>    # Defines for default states.  These can be changed on the command line.
>    # -D FLAG=VALUE
>    #
> -  DEFINE SECURE_BOOT_ENABLE      = FALSE
>    DEFINE NETWORK_IP6_ENABLE      = FALSE
>    DEFINE HTTP_BOOT_ENABLE        = FALSE
> -  DEFINE SMM_REQUIRE             = FALSE
>    DEFINE TLS_ENABLE              = FALSE
> -  DEFINE TPM2_ENABLE             = FALSE
> -  DEFINE TPM2_CONFIG_ENABLE      = FALSE
>    DEFINE USE_LEGACY_ISA_STACK    = FALSE
>  
>    #
> @@ -57,7 +54,7 @@ [Defines]
>  !ifdef $(FD_SIZE_4MB)
>    DEFINE FD_SIZE_IN_KB           = 4096
>  !else
> -  DEFINE FD_SIZE_IN_KB           = 4096
> +  DEFINE FD_SIZE_IN_KB           = 2048
>  !endif
>  !endif
>  !endif
> @@ -157,12 +154,9 @@ [LibraryClasses]
>    UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>    SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> -  VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>    LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
>    MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> -!if $(SMM_REQUIRE) == FALSE
>    LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> -!endif
>    CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
>    FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>  
> @@ -185,14 +179,8 @@ [LibraryClasses]
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
>  
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> -  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> -  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> -!else
>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> -!endif
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>  
>    TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> @@ -211,13 +199,7 @@ [LibraryClasses]
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>    XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>  
> -!if $(TPM2_ENABLE) == TRUE
> -  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> -  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
> -  Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> -!else
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> -!endif
>  
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -283,11 +265,6 @@ [LibraryClasses.common.PEIM]
>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>  
> -!if $(TPM2_ENABLE) == TRUE
> -  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> -  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> -!endif
> -
>  [LibraryClasses.common.DXE_CORE]
>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>    DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> @@ -357,20 +334,13 @@ [LibraryClasses.common.DXE_DRIVER]
>    PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> -!if $(SMM_REQUIRE) == TRUE
> -  LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
> -!else
>    LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
> -!endif
>  !ifdef $(SOURCE_DEBUG_ENABLE)
>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> -!if $(TPM2_ENABLE) == TRUE
> -  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
> -!endif
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -385,43 +355,6 @@ [LibraryClasses.common.UEFI_APPLICATION]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>  
> -[LibraryClasses.common.DXE_SMM_DRIVER]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -  MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> -  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -  SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> -  MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
> -  SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
> -  CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
> -!ifdef $(SOURCE_DEBUG_ENABLE)
> -  DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf
> -!endif
> -  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> -  PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> -
> -[LibraryClasses.common.SMM_CORE]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -  SmmCorePlatformHookLib|MdeModulePkg/Library/SmmCorePlatformHookLibNull/SmmCorePlatformHookLibNull.inf
> -  MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllocationLib.inf
> -  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -  SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> -  SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib/PiSmmCoreSmmServicesTableLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
> -  PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> -
>  ################################################################################
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
> @@ -436,10 +369,6 @@ [PcdsFeatureFlag]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> -!if $(SMM_REQUIRE) == TRUE
> -  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> -!endif
>  
>  [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> @@ -516,10 +445,6 @@ [PcdsFixedAtBuild]
>  
>    gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>  
> -!if $(SMM_REQUIRE) == TRUE
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
> -!endif
> -
>    # IRQs 5, 9, 10, 11 are level-triggered
>    gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
>  
> @@ -533,14 +458,11 @@ [PcdsFixedAtBuild]
>  ################################################################################
>  
>  [PcdsDynamicDefault]
> -  # only set when
> -  #   ($(SMM_REQUIRE) == FALSE)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> -
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
> @@ -573,18 +495,8 @@ [PcdsDynamicDefault]
>    # Set memory encryption mask
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
>  
> -!if $(SMM_REQUIRE) == TRUE
> -  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
> -!endif
> -
>    gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
>  
> -!if $(TPM2_ENABLE) == TRUE
> -  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> -!endif
> -
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform.
> @@ -620,32 +532,9 @@ [Components]
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
>    OvmfPkg/PlatformPei/PlatformPei.inf
> -  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
> -    <LibraryClasses>
> -!if $(SMM_REQUIRE) == TRUE
> -      LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
> -!endif
> -  }
> -!if $(SMM_REQUIRE) == TRUE
> -  OvmfPkg/SmmAccess/SmmAccessPei.inf
> -!endif
> +  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
>    UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>  
> -!if $(TPM2_ENABLE) == TRUE
> -  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> -  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> -    <LibraryClasses>
> -      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> -  }
> -!if $(TPM2_CONFIG_ENABLE) == TRUE
> -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> -!endif
> -!endif
> -
>    #
>    # DXE Phase modules
>    #
> @@ -664,15 +553,7 @@ [Components]
>  
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>  
> -  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> -    <LibraryClasses>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> -!endif
> -!if $(TPM2_ENABLE) == TRUE
> -      NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> -!endif
> -  }
> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  
>    MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>    PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
> @@ -712,11 +593,6 @@ [Components]
>        NULL|IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUiLib.inf
>  !endif
>    }
> -  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
> -  OvmfPkg/Virtio10Dxe/Virtio10.inf
> -  OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> -  OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> -  OvmfPkg/VirtioRngDxe/VirtioRng.inf
>    OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
>    OvmfPkg/XenBusDxe/XenBusDxe.inf
>    OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> @@ -755,7 +631,6 @@ [Components]
>  
>    OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
>    OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> -  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  
>    #
>    # ISA Support
> @@ -824,7 +699,6 @@ [Components]
>        NULL|OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
>    }
>  !endif
> -  OvmfPkg/VirtioNetDxe/VirtioNet.inf
>  
>    #
>    # Usb Support
> @@ -874,56 +748,10 @@ [Components]
>        gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>    }
>  
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -!endif
> -
>    OvmfPkg/PlatformDxe/Platform.inf
>    OvmfPkg/AmdSevDxe/AmdSevDxe.inf
>    OvmfPkg/IoMmuDxe/IoMmuDxe.inf
>  
> -!if $(SMM_REQUIRE) == TRUE
> -  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
> -  OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
> -  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> -
> -  #
> -  # SMM Initial Program Load (a DXE_RUNTIME_DRIVER)
> -  #
> -  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
> -
> -  #
> -  # SMM_CORE
> -  #
> -  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
> -
> -  #
> -  # Privileged drivers (DXE_SMM_DRIVER modules)
> -  #
> -  UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
> -  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf {
> -    <LibraryClasses>
> -      LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> -  }
> -  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf {
> -    <LibraryClasses>
> -      SmmCpuPlatformHookLib|UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/SmmCpuPlatformHookLibNull.inf
> -      SmmCpuFeaturesLib|OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> -  }
> -
> -  #
> -  # Variable driver stack (SMM)
> -  #
> -  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
> -  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> -  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {
> -    <LibraryClasses>
> -      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> -  }
> -  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> -
> -!else
> -
>    #
>    # Variable driver stack (non-SMM)
>    #
> @@ -937,17 +765,3 @@ [Components]
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
>    }
> -!endif
> -
> -!if $(TPM2_ENABLE) == TRUE
> -  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> -    <LibraryClasses>
> -      Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
> -      NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> -      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> -      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> -  }
> -!endif
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/XenOvmf.fdf
> similarity index 85%
> copy from OvmfPkg/OvmfPkgIa32X64.fdf
> copy to OvmfPkg/XenOvmf.fdf
> index 6c40540202..612ffb2e01 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/XenOvmf.fdf
> @@ -3,6 +3,7 @@
>  #
>  #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#  Copyright (c) 2019, Citrix Systems, Inc.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
> @@ -68,10 +69,10 @@ [FD.OVMF_CODE]
>  
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size          = 0xC00000
> +Size          = 0xB00000
>  ErasePolarity = 1
>  BlockSize     = 0x10000
> -NumBlocks     = 0xC0
> +NumBlocks     = 0xB0
>  
>  0x000000|0x006000
>  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +90,7 @@ [FD.MEMFD]
>  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>  
> -0x100000|0xB00000
> +0x100000|0xA00000
>  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>  
> @@ -160,16 +161,8 @@ [FV.PEIFV]
>  INF  OvmfPkg/PlatformPei/PlatformPei.inf
>  INF  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> -!if $(SMM_REQUIRE) == TRUE
> -INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
> -!endif
>  INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>  
> -!if $(TPM2_ENABLE) == TRUE
> -INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> -INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> -!endif
> -
>  ################################################################################
>  
>  [FV.DXEFV]
> @@ -197,9 +190,6 @@ [FV.DXEFV]
>    INF  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>    INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>    INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> -!if $(SMM_REQUIRE) == FALSE
> -  INF  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> -!endif
>  }

(3) Here you diverge from the general pattern of:
- SMM_REQUIRE==TRUE --> drop the full block
- SMM_REQUIRE==FALSE --> drop the condition, keep the block

For consistency with the pattern, you sould *in theory* only drop the
condition.

Let's see what this change effects.

You remove the driver from the APRIORI DXE file. That means the DXE Core
will be at liberty to launch the driver at any time, provided DEPEXes
are satisfied.

This wouldn't be correct for the original FDF file, because there we
need an arbitration between this driver and the emulation driver,
dependent on flash detection. Therefore the original platform first
launches the flash driver, and falls back to the emu driver if flash is
not present. Except in case of SMM_REQUIRE, in which case *both* of
those drivers are replaced by the SMM flash driver (and no arbitration
occurs, because flash is a hard requirement).

On Xen, SMM_REQUIRE is indeed assumed FALSE, but we also don't really
need the arbitration, because we know the flash detection will always
fail. Therefore, it is correct to remove the flash driver from the
APRIORI DXE file -- but then, shouldn't we remove the driver itself from
the DSC and FDF files too?

I mean, if it doesn't matter *when* it is launched, then you don't need
to launch it *at all*, I believe.

If you agree, then it's OK (but not required) to split this change to a
separate patch -- as I said it diverges from the general pattern and may
deserve a bit more explanation.


(4) You might want to remove/revert SEV-related modules as well (I'm
unsure if AMD SEV works on Xen -- if it does, then please ignore):

- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf

(refer to "git blame" to see what to revert it to)

-
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf

- NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf

- OvmfPkg/AmdSevDxe/AmdSevDxe.inf
- OvmfPkg/IoMmuDxe/IoMmuDxe.inf

This can be a separate patch again.


(5) I think you could change the QemuFwCfgS3Lib resolution to
"BaseQemuFwCfgS3LibNull.inf", and then remove all the S3 modules --
search OvmfPkg, and generally the edk2 tree, for " PcdAcpiS3Enable".

See also module names having "S3" in the name, in the FDF/DSC files.

This would probably be a separate patch.

Thanks,
Laszlo

>  
>  #
> @@ -226,19 +216,10 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Metronome/Metronome.inf
>  INF  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
>  
> -INF  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
> -INF  OvmfPkg/Virtio10Dxe/Virtio10.inf
> -INF  OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> -INF  OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> -INF  OvmfPkg/VirtioRngDxe/VirtioRng.inf
>  INF  OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
>  INF  OvmfPkg/XenBusDxe/XenBusDxe.inf
>  INF  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>  
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  INF  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -!endif
> -
>  INF  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>  INF  MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>  INF  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> @@ -337,7 +318,6 @@ [FV.DXEFV]
>    INF  NetworkPkg/TlsDxe/TlsDxe.inf
>    INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
>  !endif
> -  INF  OvmfPkg/VirtioNetDxe/VirtioNet.inf
>  
>  #
>  # Usb Support
> @@ -357,31 +337,10 @@ [FV.DXEFV]
>  
>  INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
>  INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> -INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
>  INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
>  INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
>  
> -!if $(SMM_REQUIRE) == TRUE
> -INF  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
> -INF  OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
> -INF  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> -INF  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
> -INF  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
> -INF  UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
> -INF  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
> -INF  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> -
> -#
> -# Variable driver stack (SMM)
> -#
> -INF  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
> -INF  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> -INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> -INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> -
> -!else
> -
>  #
>  # Variable driver stack (non-SMM)
>  #
> @@ -389,14 +348,6 @@ [FV.DXEFV]
>  INF  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
>  INF  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>  INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> -!endif
> -
> -!if $(TPM2_ENABLE) == TRUE
> -INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> -!if $(TPM2_CONFIG_ENABLE) == TRUE
> -INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> -!endif
> -!endif
>  
>  ################################################################################
>  
> @@ -527,18 +478,3 @@ [Rule.Common.SEC.RESET_VECTOR]
>    FILE RAW = $(NAMED_GUID) {
>      RAW BIN   Align = 16   |.bin
>    }
> -
> -[Rule.Common.SMM_CORE]
> -  FILE SMM_CORE = $(NAMED_GUID) {
> -    PE32     PE32           $(INF_OUTPUT)/$(MODULE_NAME).efi
> -    UI       STRING="$(MODULE_NAME)" Optional
> -    VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> -  }
> -
> -[Rule.Common.DXE_SMM_DRIVER]
> -  FILE SMM = $(NAMED_GUID) {
> -    SMM_DEPEX    SMM_DEPEX Optional      $(INF_OUTPUT)/$(MODULE_NAME).depex
> -    PE32     PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
> -    UI       STRING="$(MODULE_NAME)" Optional
> -    VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> -  }
>