diff mbox series

[v2,02/31] OvmfPkg: Create platform XenOvmf

Message ID 20190409110844.14746-3-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show
Series Specific platform to run OVMF in Xen PVH and HVM guests | expand

Commit Message

Anthony PERARD April 9, 2019, 11:08 a.m. UTC
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(-)

Comments

Laszlo Ersek April 10, 2019, 9:32 a.m. UTC | #1
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)
> -  }
>
Jordan Justen April 10, 2019, 9:48 a.m. UTC | #2
On 2019-04-09 04:08:15, 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 +-------------------

I guess we all want our name to be first :), but OvmfXen seems more
like the pattern that edk2 uses when making sub-platforms.

Also, in some cases we've used !ifdef type things to keep dsc and fdf
files common. Would that not be workable here? Maybe it would get too
ugly. :\

It could help to prevent having to sync dsc changes across, and
prevent changes from being omitted for Xen on accident.

-Jordan
Laszlo Ersek April 10, 2019, 2:27 p.m. UTC | #3
On 04/10/19 11:48, Jordan Justen wrote:
> On 2019-04-09 04:08:15, 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 +-------------------
> 
> I guess we all want our name to be first :), but OvmfXen seems more
> like the pattern that edk2 uses when making sub-platforms.
> 
> Also, in some cases we've used !ifdef type things to keep dsc and fdf
> files common. Would that not be workable here? Maybe it would get too
> ugly. :\

I've been happy with a similar Xen<->QEMU split under ArmVirtPkg.
Duplications in updates are usually not a huge burden, and keeping the
files separate has proved very helpful for determining
maintainer/reviewer/tester responsibility.

> It could help to prevent having to sync dsc changes across, and
> prevent changes from being omitted for Xen on accident.

True, but in my experience that's been the smaller problem. The larger
problem has been modifying Xen stuff in unintended ways (= regressing
Xen), because we can't test (or don't even notice) the Xen-side
implications of changes made to common source.

Personally I prefer another (DSC + FDF) pair under OvmfPkg (beyond the
three we already have) to another layer of (conditional?) includes. The
!if's that are being eliminated in this Xen-customized copy (i.e., in
this patch) are complex enough already :)

... It's not that I *generally* prefer duplication; as you say, we do
have a number of !includes already. I do prefer duplication specifically
for Xen vs. QEMU however.

Thanks
Laszlo
Ard Biesheuvel April 10, 2019, 4:27 p.m. UTC | #4
On Wed, 10 Apr 2019 at 07:27, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 04/10/19 11:48, Jordan Justen wrote:
> > On 2019-04-09 04:08:15, 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 +-------------------
> >
> > I guess we all want our name to be first :), but OvmfXen seems more
> > like the pattern that edk2 uses when making sub-platforms.
> >
> > Also, in some cases we've used !ifdef type things to keep dsc and fdf
> > files common. Would that not be workable here? Maybe it would get too
> > ugly. :\
>
> I've been happy with a similar Xen<->QEMU split under ArmVirtPkg.
> Duplications in updates are usually not a huge burden, and keeping the
> files separate has proved very helpful for determining
> maintainer/reviewer/tester responsibility.
>
> > It could help to prevent having to sync dsc changes across, and
> > prevent changes from being omitted for Xen on accident.
>
> True, but in my experience that's been the smaller problem. The larger
> problem has been modifying Xen stuff in unintended ways (= regressing
> Xen), because we can't test (or don't even notice) the Xen-side
> implications of changes made to common source.
>
> Personally I prefer another (DSC + FDF) pair under OvmfPkg (beyond the
> three we already have) to another layer of (conditional?) includes. The
> !if's that are being eliminated in this Xen-customized copy (i.e., in
> this patch) are complex enough already :)
>
> ... It's not that I *generally* prefer duplication; as you say, we do
> have a number of !includes already. I do prefer duplication specifically
> for Xen vs. QEMU however.
>

+1
Jordan Justen April 10, 2019, 6:37 p.m. UTC | #5
On 2019-04-10 07:27:15, Laszlo Ersek wrote:
> On 04/10/19 11:48, Jordan Justen wrote:
> > On 2019-04-09 04:08:15, 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 +-------------------
> > 
> > I guess we all want our name to be first :), but OvmfXen seems more
> > like the pattern that edk2 uses when making sub-platforms.

For example: ArmVirtPkg/ArmVirtXen.dsc

> > 
> > Also, in some cases we've used !ifdef type things to keep dsc and fdf
> > files common. Would that not be workable here? Maybe it would get too
> > ugly. :\
> 
> I've been happy with a similar Xen<->QEMU split under ArmVirtPkg.
> Duplications in updates are usually not a huge burden, and keeping the
> files separate has proved very helpful for determining
> maintainer/reviewer/tester responsibility.
> 
> > It could help to prevent having to sync dsc changes across, and
> > prevent changes from being omitted for Xen on accident.
> 
> True, but in my experience that's been the smaller problem. The larger
> problem has been modifying Xen stuff in unintended ways (= regressing
> Xen), because we can't test (or don't even notice) the Xen-side
> implications of changes made to common source.

Does that mean you avoid changing ArmVirtXen.dsc entirely, or you try
to update it when it seems likely to not cause trouble? I could see
unintended breakage either way.

Anyway, it sounds like it's generally working out okay with
ArmVirtXen, so it seems okay to follow that under OvmfPkg.

-Jordan
Laszlo Ersek April 11, 2019, 7:34 a.m. UTC | #6
On 04/10/19 20:37, Jordan Justen wrote:
> On 2019-04-10 07:27:15, Laszlo Ersek wrote:
>> On 04/10/19 11:48, Jordan Justen wrote:
>>> On 2019-04-09 04:08:15, 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 +-------------------
>>>
>>> I guess we all want our name to be first :), but OvmfXen seems more
>>> like the pattern that edk2 uses when making sub-platforms.
> 
> For example: ArmVirtPkg/ArmVirtXen.dsc

Right, we all agree on this.

> 
>>>
>>> Also, in some cases we've used !ifdef type things to keep dsc and fdf
>>> files common. Would that not be workable here? Maybe it would get too
>>> ugly. :\
>>
>> I've been happy with a similar Xen<->QEMU split under ArmVirtPkg.
>> Duplications in updates are usually not a huge burden, and keeping the
>> files separate has proved very helpful for determining
>> maintainer/reviewer/tester responsibility.
>>
>>> It could help to prevent having to sync dsc changes across, and
>>> prevent changes from being omitted for Xen on accident.
>>
>> True, but in my experience that's been the smaller problem. The larger
>> problem has been modifying Xen stuff in unintended ways (= regressing
>> Xen), because we can't test (or don't even notice) the Xen-side
>> implications of changes made to common source.
> 
> Does that mean you avoid changing ArmVirtXen.dsc entirely, or you try
> to update it when it seems likely to not cause trouble? I could see
> unintended breakage either way.

Under ArmVirtPkg, we have three platforms: ArmVirtQemu,
ArmVirtQemuKernel, and ArmVirtXen. All of these have separate DSC files.
All include ArmVirt.dsc.inc.

In addition, all three have FDF files, but ArmVirtQemu and
ArmVirtQemuKernel share (include) ArmVirtQemuFvMain.fdf.inc.

ArmVirtQemu and ArmVirtQemuKernel differ in that the latter is supposed
to be launched with "-kernel" on QEMU, not via pflash.

I primarily care about ArmVirtQemu. I always start modifications there.
Then I evaluate whether the change makes sense for ArmVirtQemuKernel,
and rely on Ard to confirm that. ArmVirtXen is the last thought for me,
and I try to make a "best effort" to figure out the impact.

When I'm stongly in doubt re: Xen, I avoid modifying ArmVirtXen. When I
sort of suspect that modifying Xen is valid/necessary, then I (a) either
modify ArmVirtXen explicitly, or (b) modify ArmVirt.dsc.inc *AND* talk
about Xen explicitly in the commit message, and/or the cover letter.

Case (b) is quite rare. When it happens, it usually follows immediately
(a) -- I might change ArmVirtXen, and then realize something is now
shared between Xen and QEMU. It's then a separate decision whether I
actually want to merge that thing into ArmVirt.dsc.inc -- sometimes
that's not the best decision (exactly because it increases shared review
responsibility for the future).

In both case (a) and (b), I CC the Xen reviewers. The point is that
"Xen" is mentioned somewhere explicitly (diffstat or commit message).
This lets us decide more clearly whether Xen reviewers should
participate -- when they don't have to, that saves us all time, and so
when they *do* need to, their responsibility is both big and clear.

If, for QEMU feature development or bugfixing, we kept modifying files
that were by default processed in Xen builds too, then Xen people would
be bombarded with review requests they'd rather ignore. Their attention
would wane and they'd miss (or greatly delay) the reviews when they
should indeed follow up (and hopefully quickly).

It's not a 100% "scientific" workflow, and you are right that both
approaches can cause issues by omission. In my experience the split
tends to be safer (and easier on human resources), in practice.

> Anyway, it sounds like it's generally working out okay with
> ArmVirtXen, so it seems okay to follow that under OvmfPkg.

Thanks!
Laszlo

> 
> -Jordan
>
Anthony PERARD April 15, 2019, 10:53 a.m. UTC | #7
On Wed, Apr 10, 2019 at 11:32:10AM +0200, Laszlo Ersek wrote:
> 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.
> > 
>
> (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).

Will do.

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

Will do.

> > 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
> > @@ -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.

I think that was removed by mistake, otherwise I think I would have
removed QemuFlashFvbServicesRuntimeDxe completely. I do some tests, and
if I do need to remove it from the APRIORI list, I do that in a
separated patch.

> (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.

Thanks for the suggestion. It probably doesn't hurt to keep the
SEV-related modules, so I think I'll keep them for now.

> (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.

I'll have a look, thanks.
diff mbox series

Patch

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
 }
 
 #
@@ -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)
-  }