[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [edk2-devel] [PATCH v2 04/31] OvmfPkg: Introduce XenPlatformPei



On 04/09/19 13:08, Anthony PERARD wrote:
> A copy of OvmfPkg/PlatformPei without some of QEMU specific
> initialization, Xen does not support QemuFwCfg.
> 
> This new module will be adjusted to accommodate Xen PVH.
> 
> fw_cfg dependents that have been removed, which are dynamically skipped
> when running PlatformPei on Xen:
> - GetFirstNonAddress(): controlling the 64-bit PCI MMIO aperture via the
> (experimental) "opt/ovmf/X-PciMmio64Mb" file
> - GetFirstNonAddress(): honoring the hotplug DIMM area
> ("etc/reserved-memory-end") in the placement of the 64-bit PCI MMIO
> aperture
> - NoexecDxeInitialization() is removed, so PcdPropertiesTableEnable and
> PcdSetNxForStack are left constant FALSE (not set dynamically from
> fw_cfg "opt/ovmf/PcdXxxx")
> - MaxCpuCountInitialization(), PublishPeiMemory(): the max CPU count is
> not taken from the QemuFwCfgItemSmpCpuCount fw_cfg key;
> PcdCpuMaxLogicalProcessorNumber is used intact and
> PcdCpuApInitTimeOutInMicroSeconds is never changed or used.
> - InitializeXenPlatform(), S3Verification(): S3 is assumed disabled (not
> consulting "etc/system-states" via QemuFwCfgS3Enabled()).
> - InstallFeatureControlCallback(): the feature control MSR is not set
> from "etc/msr_feature_control"
> (also removed FeatureControl.c as there is nothing been executed)
> 
> Also removed:
> - SMRAM/TSEG-related low mem size adjusting (PcdSmmSmramRequire is
> assumed FALSE) in PublishPeiMemory(),
> - QemuInitializeRam() entirely,
> 
> Xen related changes:
> - Have removed the module variable mXen, as it should be always true.
> - Have the platform PEI initialization fails if Xen has not been
>   detected.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  OvmfPkg/XenOvmf.dsc                                                        | 
>   2 +-
>  OvmfPkg/XenOvmf.fdf                                                        | 
>   2 +-
>  OvmfPkg/{PlatformPei/PlatformPei.inf => XenPlatformPei/XenPlatformPei.inf} | 
>  28 +-
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Cmos.h                             | 
>   2 +
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Platform.h                         | 
>  13 +-
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Xen.h                              | 
>   0
>  OvmfPkg/{PlatformPei => XenPlatformPei}/AmdSev.c                           | 
>  30 +-
>  OvmfPkg/{PlatformPei => XenPlatformPei}/ClearCache.c                       | 
>   1 +
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Cmos.c                             | 
>   2 +
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Fv.c                               | 
>  26 +-
>  OvmfPkg/XenPlatformPei/MemDetect.c                                         | 
> 427 ++++++++++++++++++++
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Platform.c                         | 
> 246 +----------
>  OvmfPkg/{PlatformPei => XenPlatformPei}/Xen.c                              | 
>   8 +-
>  13 files changed, 453 insertions(+), 334 deletions(-)

I reviewed this patch as follows:
- diffstat: only touchdes OvmfPkg/Xen*
- copyright notices in all new files: yes
- reviewed the output of:

  git show -b --color --find-copies-harder -C40 anthony_v2~$((31-4))

It looks good to me, with two suggestions:

(1) I think you could always assume

  mBootMode != BOOT_ON_S3_RESUME

I'm not saying you *should*, but you might prefer that, if it enabled
further simplifications. Up to you.


> 
> diff --git a/OvmfPkg/XenOvmf.dsc b/OvmfPkg/XenOvmf.dsc
> index 6161133fa8..7b8a1fdf6b 100644
> --- a/OvmfPkg/XenOvmf.dsc
> +++ b/OvmfPkg/XenOvmf.dsc
> @@ -531,7 +531,7 @@ [Components]
>    }
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
> -  OvmfPkg/PlatformPei/PlatformPei.inf
> +  OvmfPkg/XenPlatformPei/XenPlatformPei.inf
>    UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
>    UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>  
> diff --git a/OvmfPkg/XenOvmf.fdf b/OvmfPkg/XenOvmf.fdf
> index 292cf4b492..295e30ca3f 100644
> --- a/OvmfPkg/XenOvmf.fdf
> +++ b/OvmfPkg/XenOvmf.fdf
> @@ -158,7 +158,7 @@ [FV.PEIFV]
>  INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>  INF  
> MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
>  INF  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
> -INF  OvmfPkg/PlatformPei/PlatformPei.inf
> +INF  OvmfPkg/XenPlatformPei/XenPlatformPei.inf
>  INF  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
>  INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
> b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> similarity index 69%
> copy from OvmfPkg/PlatformPei/PlatformPei.inf
> copy to OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> index 5c8dd0fe6d..c8d25c115f 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> @@ -3,6 +3,7 @@
>  #
>  #  This module provides platform specific function to detect boot mode.
>  #  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<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
> @@ -16,11 +17,11 @@
>  
>  [Defines]
>    INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = PlatformPei
> -  FILE_GUID                      = 222c386d-5abc-4fb4-b124-fbb82488acf4
> +  BASE_NAME                      = XenPlatformPei
> +  FILE_GUID                      = f112a6ee-993a-4f0b-8295-e52029d9b4ba
>    MODULE_TYPE                    = PEIM
>    VERSION_STRING                 = 1.0
> -  ENTRY_POINT                    = InitializePlatform
> +  ENTRY_POINT                    = InitializeXenPlatform
>  
>  #
>  # The following information is for reference only and not required by the 
> build tools.
> @@ -33,7 +34,6 @@ [Sources]
>    ClearCache.c
>    Cmos.c
>    Cmos.h
> -  FeatureControl.c
>    Fv.c
>    MemDetect.c
>    Platform.c
> @@ -62,10 +62,7 @@ [LibraryClasses]
>    PciLib
>    ResourcePublicationLib
>    PeiServicesLib
> -  PeiServicesTablePointerLib
>    PeimEntryPoint
> -  QemuFwCfgLib
> -  QemuFwCfgS3Lib
>    MtrrLib
>    MemEncryptSevLib
>    PcdLib
> @@ -75,13 +72,8 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
> -  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
> -  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
> -  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
> -  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
> -  gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
> @@ -89,31 +81,19 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
> -  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
> -  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
>    gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
>  
>  [FixedPcd]
>    gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>  
> -[FeaturePcd]
> -  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> -
>  [Ppis]
>    gEfiPeiMasterBootModePpiGuid
>    gEfiPeiMpServicesPpiGuid
> diff --git a/OvmfPkg/PlatformPei/Cmos.h b/OvmfPkg/XenPlatformPei/Cmos.h
> similarity index 92%
> copy from OvmfPkg/PlatformPei/Cmos.h
> copy to OvmfPkg/XenPlatformPei/Cmos.h
> index 949f884c2c..defe677122 100644
> --- a/OvmfPkg/PlatformPei/Cmos.h
> +++ b/OvmfPkg/XenPlatformPei/Cmos.h
> @@ -2,6 +2,8 @@
>    PC/AT CMOS access routines
>  
>    Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<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
>    which accompanies this distribution.  The full text of the license may be 
> found at
> diff --git a/OvmfPkg/PlatformPei/Platform.h 
> b/OvmfPkg/XenPlatformPei/Platform.h
> similarity index 88%
> copy from OvmfPkg/PlatformPei/Platform.h
> copy to OvmfPkg/XenPlatformPei/Platform.h
> index b12a5c1f5f..6ccd5eb66c 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/XenPlatformPei/Platform.h
> @@ -2,6 +2,8 @@
>    Platform PEI module include file.
>  
>    Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<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
>    which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -78,11 +80,6 @@ PeiFvInitialization (
>    VOID
>    );
>  
> -VOID
> -InstallFeatureControlCallback (
> -  VOID
> -  );
> -
>  VOID
>  InstallClearCacheCallback (
>    VOID
> @@ -103,8 +100,6 @@ AmdSevInitialize (
>    VOID
>    );
>  
> -extern BOOLEAN mXen;
> -
>  VOID
>  XenPublishRamRegions (
>    VOID
> @@ -112,12 +107,8 @@ XenPublishRamRegions (
>  
>  extern EFI_BOOT_MODE mBootMode;
>  
> -extern BOOLEAN mS3Supported;
> -
>  extern UINT8 mPhysMemAddressWidth;
>  
> -extern UINT32 mMaxCpuCount;
> -
>  extern UINT16 mHostBridgeDevId;
>  
>  #endif // _PLATFORM_PEI_H_INCLUDED_
> diff --git a/OvmfPkg/PlatformPei/Xen.h b/OvmfPkg/XenPlatformPei/Xen.h
> similarity index 100%
> copy from OvmfPkg/PlatformPei/Xen.h
> copy to OvmfPkg/XenPlatformPei/Xen.h
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/XenPlatformPei/AmdSev.c
> similarity index 61%
> copy from OvmfPkg/PlatformPei/AmdSev.c
> copy to OvmfPkg/XenPlatformPei/AmdSev.c
> index 2e14eaf8c3..1a1f46291f 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/XenPlatformPei/AmdSev.c
> @@ -2,6 +2,7 @@
>    Initialize Secure Encrypted Virtualization (SEV) support
>  
>    Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<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
> @@ -16,7 +17,6 @@
>  // The package level header files this module uses
>  //
>  #include <Library/DebugLib.h>
> -#include <Library/HobLib.h>
>  #include <Library/MemEncryptSevLib.h>
>  #include <Library/PcdLib.h>
>  #include <PiPei.h>
> @@ -67,32 +67,4 @@ AmdSevInitialize (
>    //
>    PcdStatus = PcdSet32S (PcdOptionRomImageVerificationPolicy, 0x4);
>    ASSERT_RETURN_ERROR (PcdStatus);
> -
> -  //
> -  // When SMM is required, cover the pages containing the initial SMRAM Save
> -  // State Map with a memory allocation HOB:
> -  //
> -  // There's going to be a time interval between our decrypting those pages 
> for
> -  // SMBASE relocation and re-encrypting the same pages after SMBASE
> -  // relocation. We shall ensure that the DXE phase stay away from those 
> pages
> -  // until after re-encryption, in order to prevent an information leak to 
> the
> -  // hypervisor.
> -  //
> -  if (FeaturePcdGet (PcdSmmSmramRequire) && (mBootMode != 
> BOOT_ON_S3_RESUME)) {
> -    RETURN_STATUS LocateMapStatus;
> -    UINTN         MapPagesBase;
> -    UINTN         MapPagesCount;
> -
> -    LocateMapStatus = MemEncryptSevLocateInitialSmramSaveStateMapPages (
> -                        &MapPagesBase,
> -                        &MapPagesCount
> -                        );
> -    ASSERT_RETURN_ERROR (LocateMapStatus);
> -
> -    BuildMemoryAllocationHob (
> -      MapPagesBase,                      // BaseAddress
> -      EFI_PAGES_TO_SIZE (MapPagesCount), // Length
> -      EfiBootServicesData                // MemoryType
> -      );
> -  }
>  }
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c 
> b/OvmfPkg/XenPlatformPei/ClearCache.c
> similarity index 95%
> copy from OvmfPkg/PlatformPei/ClearCache.c
> copy to OvmfPkg/XenPlatformPei/ClearCache.c
> index 7d15fd925c..78e22124e3 100644
> --- a/OvmfPkg/PlatformPei/ClearCache.c
> +++ b/OvmfPkg/XenPlatformPei/ClearCache.c
> @@ -6,6 +6,7 @@
>    sake.
>  
>    Copyright (C) 2018, Red Hat, Inc.
> +  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 which accompanies this
> diff --git a/OvmfPkg/PlatformPei/Cmos.c b/OvmfPkg/XenPlatformPei/Cmos.c
> similarity index 92%
> copy from OvmfPkg/PlatformPei/Cmos.c
> copy to OvmfPkg/XenPlatformPei/Cmos.c
> index 48ed2cb8f4..174ec773ec 100644
> --- a/OvmfPkg/PlatformPei/Cmos.c
> +++ b/OvmfPkg/XenPlatformPei/Cmos.c
> @@ -2,6 +2,8 @@
>    PC/AT CMOS access routines
>  
>    Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<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
>    which accompanies this distribution.  The full text of the license may be 
> found at
> diff --git a/OvmfPkg/PlatformPei/Fv.c b/OvmfPkg/XenPlatformPei/Fv.c
> similarity index 72%
> copy from OvmfPkg/PlatformPei/Fv.c
> copy to OvmfPkg/XenPlatformPei/Fv.c
> index 248c585085..388e8768bf 100644
> --- a/OvmfPkg/PlatformPei/Fv.c
> +++ b/OvmfPkg/XenPlatformPei/Fv.c
> @@ -2,6 +2,8 @@
>    Build FV related hobs for platform.
>  
>    Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<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
>    which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -32,8 +34,6 @@ PeiFvInitialization (
>    VOID
>    )
>  {
> -  BOOLEAN SecureS3Needed;
> -
>    DEBUG ((EFI_D_INFO, "Platform PEI Firmware Volume Initialization\n"));
>  
>    //
> @@ -44,7 +44,7 @@ PeiFvInitialization (
>    BuildMemoryAllocationHob (
>      PcdGet32 (PcdOvmfPeiMemFvBase),
>      PcdGet32 (PcdOvmfPeiMemFvSize),
> -    mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
> +    EfiBootServicesData
>      );
>  
>    //
> @@ -52,8 +52,6 @@ PeiFvInitialization (
>    //
>    BuildFvHob (PcdGet32 (PcdOvmfDxeMemFvBase), PcdGet32 
> (PcdOvmfDxeMemFvSize));
>  
> -  SecureS3Needed = mS3Supported && FeaturePcdGet (PcdSmmSmramRequire);
> -
>    //
>    // Create a memory allocation HOB for the DXE FV.
>    //
> @@ -65,25 +63,9 @@ PeiFvInitialization (
>    BuildMemoryAllocationHob (
>      PcdGet32 (PcdOvmfDxeMemFvBase),
>      PcdGet32 (PcdOvmfDxeMemFvSize),
> -    SecureS3Needed ? EfiACPIMemoryNVS : EfiBootServicesData
> +    EfiBootServicesData
>      );
>  
> -  //
> -  // Additionally, said decompression will use temporary memory above the end
> -  // of DXEFV, so let's keep away the OS from there too.
> -  //
> -  if (SecureS3Needed) {
> -    UINT32 DxeMemFvEnd;
> -
> -    DxeMemFvEnd = PcdGet32 (PcdOvmfDxeMemFvBase) +
> -                  PcdGet32 (PcdOvmfDxeMemFvSize);
> -    BuildMemoryAllocationHob (
> -      DxeMemFvEnd,
> -      PcdGet32 (PcdOvmfDecompressionScratchEnd) - DxeMemFvEnd,
> -      EfiACPIMemoryNVS
> -      );
> -  }
> -
>    //
>    // Let PEI know about the DXE FV so it can find the DXE Core
>    //
> diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c 
> b/OvmfPkg/XenPlatformPei/MemDetect.c
> new file mode 100644
> index 0000000000..db3d387d1c
> --- /dev/null
> +++ b/OvmfPkg/XenPlatformPei/MemDetect.c
> @@ -0,0 +1,427 @@
> +/**@file
> +  Memory Detection for Virtual Machines.
> +
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<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
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +Module Name:
> +
> +  MemDetect.c
> +
> +**/
> +
> +//
> +// The package level header files this module uses
> +//
> +#include <IndustryStandard/Q35MchIch9.h>
> +#include <PiPei.h>
> +
> +//
> +// The Library classes this module consumes
> +//
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PciLib.h>
> +#include <Library/PeimEntryPoint.h>
> +#include <Library/ResourcePublicationLib.h>
> +
> +#include "Platform.h"
> +#include "Cmos.h"
> +
> +UINT8 mPhysMemAddressWidth;
> +
> +STATIC UINT32 mS3AcpiReservedMemoryBase;
> +STATIC UINT32 mS3AcpiReservedMemorySize;
> +
> +STATIC UINT16 mQ35TsegMbytes;
> +
> +VOID
> +Q35TsegMbytesInitialization (
> +  VOID
> +  )
> +{
> +  UINT16        ExtendedTsegMbytes;
> +  RETURN_STATUS PcdStatus;
> +
> +  if (mHostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: no TSEG (SMRAM) on host bridge DID=0x%04x; "
> +      "only DID=0x%04x (Q35) is supported\n",
> +      __FUNCTION__,
> +      mHostBridgeDevId,
> +      INTEL_Q35_MCH_DEVICE_ID
> +      ));
> +    ASSERT (FALSE);
> +    CpuDeadLoop ();
> +  }
> +
> +  //
> +  // Check if QEMU offers an extended TSEG.
> +  //
> +  // This can be seen from writing MCH_EXT_TSEG_MB_QUERY to the 
> MCH_EXT_TSEG_MB
> +  // register, and reading back the register.
> +  //
> +  // On a QEMU machine type that does not offer an extended TSEG, the initial
> +  // write overwrites whatever value a malicious guest OS may have placed in
> +  // the (unimplemented) register, before entering S3 or rebooting.
> +  // Subsequently, the read returns MCH_EXT_TSEG_MB_QUERY unchanged.
> +  //
> +  // On a QEMU machine type that offers an extended TSEG, the initial write
> +  // triggers an update to the register. Subsequently, the value read back
> +  // (which is guaranteed to differ from MCH_EXT_TSEG_MB_QUERY) tells us the
> +  // number of megabytes.
> +  //
> +  PciWrite16 (DRAMC_REGISTER_Q35 (MCH_EXT_TSEG_MB), MCH_EXT_TSEG_MB_QUERY);
> +  ExtendedTsegMbytes = PciRead16 (DRAMC_REGISTER_Q35 (MCH_EXT_TSEG_MB));
> +  if (ExtendedTsegMbytes == MCH_EXT_TSEG_MB_QUERY) {
> +    mQ35TsegMbytes = PcdGet16 (PcdQ35TsegMbytes);
> +    return;
> +  }
> +
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "%a: QEMU offers an extended TSEG (%d MB)\n",
> +    __FUNCTION__,
> +    ExtendedTsegMbytes
> +    ));
> +  PcdStatus = PcdSet16S (PcdQ35TsegMbytes, ExtendedTsegMbytes);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +  mQ35TsegMbytes = ExtendedTsegMbytes;
> +}
> +
> +
> +UINT32
> +GetSystemMemorySizeBelow4gb (
> +  VOID
> +  )
> +{
> +  UINT8 Cmos0x34;
> +  UINT8 Cmos0x35;
> +
> +  //
> +  // CMOS 0x34/0x35 specifies the system memory above 16 MB.
> +  // * CMOS(0x35) is the high byte
> +  // * CMOS(0x34) is the low byte
> +  // * The size is specified in 64kb chunks
> +  // * Since this is memory above 16MB, the 16MB must be added
> +  //   into the calculation to get the total memory size.
> +  //
> +
> +  Cmos0x34 = (UINT8) CmosRead8 (0x34);
> +  Cmos0x35 = (UINT8) CmosRead8 (0x35);
> +
> +  return (UINT32) (((UINTN)((Cmos0x35 << 8) + Cmos0x34) << 16) + SIZE_16MB);
> +}
> +
> +
> +STATIC
> +UINT64
> +GetSystemMemorySizeAbove4gb (
> +  )
> +{
> +  UINT32 Size;
> +  UINTN  CmosIndex;
> +
> +  //
> +  // CMOS 0x5b-0x5d specifies the system memory above 4GB MB.
> +  // * CMOS(0x5d) is the most significant size byte
> +  // * CMOS(0x5c) is the middle size byte
> +  // * CMOS(0x5b) is the least significant size byte
> +  // * The size is specified in 64kb chunks
> +  //
> +
> +  Size = 0;
> +  for (CmosIndex = 0x5d; CmosIndex >= 0x5b; CmosIndex--) {
> +    Size = (UINT32) (Size << 8) + (UINT32) CmosRead8 (CmosIndex);
> +  }
> +
> +  return LShiftU64 (Size, 16);
> +}
> +
> +
> +/**
> +  Return the highest address that DXE could possibly use, plus one.
> +**/
> +STATIC
> +UINT64
> +GetFirstNonAddress (
> +  VOID
> +  )
> +{
> +  UINT64               FirstNonAddress;
> +  UINT64               Pci64Base, Pci64Size;
> +  RETURN_STATUS        PcdStatus;
> +
> +  FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
> +
> +  //
> +  // If DXE is 32-bit, then we're done; PciBusDxe will degrade 64-bit MMIO
> +  // resources to 32-bit anyway. See DegradeResource() in
> +  // "PciResourceSupport.c".
> +  //
> +#ifdef MDE_CPU_IA32
> +  if (!FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
> +    return FirstNonAddress;
> +  }
> +#endif
> +
> +  //
> +  // Otherwise, in order to calculate the highest address plus one, we must
> +  // consider the 64-bit PCI host aperture too. Fetch the default size.
> +  //
> +  Pci64Size = PcdGet64 (PcdPciMmio64Size);
> +
> +  if (Pci64Size == 0) {
> +    if (mBootMode != BOOT_ON_S3_RESUME) {
> +      DEBUG ((EFI_D_INFO, "%a: disabling 64-bit PCI host aperture\n",
> +        __FUNCTION__));
> +      PcdStatus = PcdSet64S (PcdPciMmio64Size, 0);
> +      ASSERT_RETURN_ERROR (PcdStatus);
> +    }
> +
> +    //
> +    // There's nothing more to do; the amount of memory above 4GB fully
> +    // determines the highest address plus one. The memory hotplug area (see
> +    // below) plays no role for the firmware in this case.
> +    //
> +    return FirstNonAddress;
> +  }
> +
> +  //
> +  // SeaBIOS aligns both boundaries of the 64-bit PCI host aperture to 1GB, 
> so
> +  // that the host can map it with 1GB hugepages. Follow suit.
> +  //
> +  Pci64Base = ALIGN_VALUE (FirstNonAddress, (UINT64)SIZE_1GB);
> +  Pci64Size = ALIGN_VALUE (Pci64Size, (UINT64)SIZE_1GB);
> +
> +  //
> +  // The 64-bit PCI host aperture should also be "naturally" aligned. The
> +  // alignment is determined by rounding the size of the aperture down to the
> +  // next smaller or equal power of two. That is, align the aperture by the
> +  // largest BAR size that can fit into it.
> +  //
> +  Pci64Base = ALIGN_VALUE (Pci64Base, GetPowerOfTwo64 (Pci64Size));
> +
> +  if (mBootMode != BOOT_ON_S3_RESUME) {
> +    //
> +    // The core PciHostBridgeDxe driver will automatically add this range to
> +    // the GCD memory space map through our PciHostBridgeLib instance; here 
> we
> +    // only need to set the PCDs.
> +    //
> +    PcdStatus = PcdSet64S (PcdPciMmio64Base, Pci64Base);
> +    ASSERT_RETURN_ERROR (PcdStatus);
> +    PcdStatus = PcdSet64S (PcdPciMmio64Size, Pci64Size);
> +    ASSERT_RETURN_ERROR (PcdStatus);
> +
> +    DEBUG ((EFI_D_INFO, "%a: Pci64Base=0x%Lx Pci64Size=0x%Lx\n",
> +      __FUNCTION__, Pci64Base, Pci64Size));
> +  }
> +
> +  //
> +  // The useful address space ends with the 64-bit PCI host aperture.
> +  //
> +  FirstNonAddress = Pci64Base + Pci64Size;
> +  return FirstNonAddress;
> +}
> +
> +
> +/**
> +  Initialize the mPhysMemAddressWidth variable, based on guest RAM size.
> +**/
> +VOID
> +AddressWidthInitialization (
> +  VOID
> +  )
> +{
> +  UINT64 FirstNonAddress;
> +
> +  //
> +  // As guest-physical memory size grows, the permanent PEI RAM requirements
> +  // are dominated by the identity-mapping page tables built by the DXE IPL.
> +  // The DXL IPL keys off of the physical address bits advertized in the CPU
> +  // HOB. To conserve memory, we calculate the minimum address width here.
> +  //
> +  FirstNonAddress      = GetFirstNonAddress ();
> +  mPhysMemAddressWidth = (UINT8)HighBitSet64 (FirstNonAddress);
> +
> +  //
> +  // If FirstNonAddress is not an integral power of two, then we need an
> +  // additional bit.
> +  //
> +  if ((FirstNonAddress & (FirstNonAddress - 1)) != 0) {
> +    ++mPhysMemAddressWidth;
> +  }
> +
> +  //
> +  // The minimum address width is 36 (covers up to and excluding 64 GB, which
> +  // is the maximum for Ia32 + PAE). The theoretical architecture maximum for
> +  // X64 long mode is 52 bits, but the DXE IPL clamps that down to 48 bits. 
> We
> +  // can simply assert that here, since 48 bits are good enough for 256 TB.
> +  //
> +  if (mPhysMemAddressWidth <= 36) {
> +    mPhysMemAddressWidth = 36;
> +  }
> +  ASSERT (mPhysMemAddressWidth <= 48);
> +}
> +
> +
> +/**
> +  Calculate the cap for the permanent PEI memory.
> +**/
> +STATIC
> +UINT32
> +GetPeiMemoryCap (
> +  VOID
> +  )
> +{
> +  BOOLEAN Page1GSupport;
> +  UINT32  RegEax;
> +  UINT32  RegEdx;
> +  UINT32  Pml4Entries;
> +  UINT32  PdpEntries;
> +  UINTN   TotalPages;
> +
> +  //
> +  // If DXE is 32-bit, then just return the traditional 64 MB cap.
> +  //
> +#ifdef MDE_CPU_IA32
> +  if (!FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
> +    return SIZE_64MB;
> +  }
> +#endif
> +
> +  //
> +  // Dependent on physical address width, PEI memory allocations can be
> +  // dominated by the page tables built for 64-bit DXE. So we key the cap off
> +  // of those. The code below is based on CreateIdentityMappingPageTables() 
> in
> +  // "MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c".
> +  //
> +  Page1GSupport = FALSE;
> +  if (PcdGetBool (PcdUse1GPageTable)) {
> +    AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> +    if (RegEax >= 0x80000001) {
> +      AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
> +      if ((RegEdx & BIT26) != 0) {
> +        Page1GSupport = TRUE;
> +      }
> +    }
> +  }
> +
> +  if (mPhysMemAddressWidth <= 39) {
> +    Pml4Entries = 1;
> +    PdpEntries = 1 << (mPhysMemAddressWidth - 30);
> +    ASSERT (PdpEntries <= 0x200);
> +  } else {
> +    Pml4Entries = 1 << (mPhysMemAddressWidth - 39);
> +    ASSERT (Pml4Entries <= 0x200);
> +    PdpEntries = 512;
> +  }
> +
> +  TotalPages = Page1GSupport ? Pml4Entries + 1 :
> +                               (PdpEntries + 1) * Pml4Entries + 1;
> +  ASSERT (TotalPages <= 0x40201);
> +
> +  //
> +  // Add 64 MB for miscellaneous allocations. Note that for
> +  // mPhysMemAddressWidth values close to 36, the cap will actually be
> +  // dominated by this increment.
> +  //
> +  return (UINT32)(EFI_PAGES_TO_SIZE (TotalPages) + SIZE_64MB);
> +}
> +
> +
> +/**
> +  Publish PEI core memory
> +
> +  @return EFI_SUCCESS     The PEIM initialized successfully.
> +
> +**/
> +EFI_STATUS
> +PublishPeiMemory (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_PHYSICAL_ADDRESS        MemoryBase;
> +  UINT64                      MemorySize;
> +  UINT32                      LowerMemorySize;
> +  UINT32                      PeiMemoryCap;
> +
> +  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> +
> +  if (mBootMode == BOOT_ON_S3_RESUME) {
> +    MemoryBase = mS3AcpiReservedMemoryBase;
> +    MemorySize = mS3AcpiReservedMemorySize;
> +  } else {
> +    PeiMemoryCap = GetPeiMemoryCap ();
> +    DEBUG ((EFI_D_INFO, "%a: mPhysMemAddressWidth=%d PeiMemoryCap=%u KB\n",
> +      __FUNCTION__, mPhysMemAddressWidth, PeiMemoryCap >> 10));
> +
> +    //
> +    // Determine the range of memory to use during PEI
> +    //
> +    MemoryBase =
> +      PcdGet32 (PcdOvmfDxeMemFvBase) + PcdGet32 (PcdOvmfDxeMemFvSize);
> +    MemorySize = LowerMemorySize - MemoryBase;
> +    if (MemorySize > PeiMemoryCap) {
> +      MemoryBase = LowerMemorySize - PeiMemoryCap;
> +      MemorySize = PeiMemoryCap;
> +    }
> +  }
> +
> +  //
> +  // Publish this memory to the PEI Core
> +  //
> +  Status = PublishSystemMemory(MemoryBase, MemorySize);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
> +
> +/**
> +  Publish system RAM and reserve memory regions
> +
> +**/
> +VOID
> +InitializeRamRegions (
> +  VOID
> +  )
> +{
> +  XenPublishRamRegions ();
> +
> +  if (mBootMode != BOOT_ON_S3_RESUME) {
> +    //
> +    // Reserve the lock box storage area
> +    //
> +    // Since this memory range will be used on S3 resume, it must be
> +    // reserved as ACPI NVS.
> +    //
> +    // If S3 is unsupported, then various drivers might still write to the
> +    // LockBox area. We ought to prevent DXE from serving allocation requests
> +    // such that they would overlap the LockBox storage.
> +    //
> +    ZeroMem (
> +      (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> +      (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
> +      );
> +    BuildMemoryAllocationHob (
> +      (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> +      (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
> +      EfiBootServicesData
> +      );
> +  }
> +}
> diff --git a/OvmfPkg/PlatformPei/Platform.c 
> b/OvmfPkg/XenPlatformPei/Platform.c
> similarity index 56%
> copy from OvmfPkg/PlatformPei/Platform.c
> copy to OvmfPkg/XenPlatformPei/Platform.c
> index 22139a64cb..2d567a4760 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/XenPlatformPei/Platform.c
> @@ -3,6 +3,7 @@
>  
>    Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2011, Andrei Warkentin <andreiw@xxxxxxxxxxxx>
> +  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
> @@ -31,8 +32,6 @@
>  #include <Library/PciLib.h>
>  #include <Library/PeimEntryPoint.h>
>  #include <Library/PeiServicesLib.h>
> -#include <Library/QemuFwCfgLib.h>
> -#include <Library/QemuFwCfgS3Lib.h>
>  #include <Library/ResourcePublicationLib.h>
>  #include <Guid/MemoryTypeInformation.h>
>  #include <Ppi/MasterBootMode.h>
> @@ -67,9 +66,6 @@ UINT16 mHostBridgeDevId;
>  
>  EFI_BOOT_MODE mBootMode = BOOT_WITH_FULL_CONFIGURATION;
>  
> -BOOLEAN mS3Supported = FALSE;
> -
> -UINT32 mMaxCpuCount;
>  
>  VOID
>  AddIoMemoryBaseSizeHob (
> @@ -179,89 +175,6 @@ MemMapInitialization (
>    //
>    AddIoMemoryRangeHob (0x0A0000, BASE_1MB);
>  
> -  if (!mXen) {
> -    UINT32  TopOfLowRam;
> -    UINT64  PciExBarBase;
> -    UINT32  PciBase;
> -    UINT32  PciSize;
> -
> -    TopOfLowRam = GetSystemMemorySizeBelow4gb ();
> -    PciExBarBase = 0;
> -    if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> -      //
> -      // The MMCONFIG area is expected to fall between the top of low RAM and
> -      // the base of the 32-bit PCI host aperture.
> -      //
> -      PciExBarBase = FixedPcdGet64 (PcdPciExpressBaseAddress);
> -      ASSERT (TopOfLowRam <= PciExBarBase);
> -      ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
> -      PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
> -    } else {
> -      PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;
> -    }
> -
> -    //
> -    // address       purpose   size
> -    // ------------  --------  -------------------------
> -    // max(top, 2g)  PCI MMIO  0xFC000000 - max(top, 2g)
> -    // 0xFC000000    gap                           44 MB
> -    // 0xFEC00000    IO-APIC                        4 KB
> -    // 0xFEC01000    gap                         1020 KB
> -    // 0xFED00000    HPET                           1 KB
> -    // 0xFED00400    gap                          111 KB
> -    // 0xFED1C000    gap (PIIX4) / RCRB (ICH9)     16 KB
> -    // 0xFED20000    gap                          896 KB
> -    // 0xFEE00000    LAPIC                          1 MB
> -    //
> -    PciSize = 0xFC000000 - PciBase;
> -    AddIoMemoryBaseSizeHob (PciBase, PciSize);
> -    PcdStatus = PcdSet64S (PcdPciMmio32Base, PciBase);
> -    ASSERT_RETURN_ERROR (PcdStatus);
> -    PcdStatus = PcdSet64S (PcdPciMmio32Size, PciSize);
> -    ASSERT_RETURN_ERROR (PcdStatus);
> -
> -    AddIoMemoryBaseSizeHob (0xFEC00000, SIZE_4KB);
> -    AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB);
> -    if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> -      AddIoMemoryBaseSizeHob (ICH9_ROOT_COMPLEX_BASE, SIZE_16KB);
> -      //
> -      // Note: there should be an
> -      //
> -      //   AddIoMemoryBaseSizeHob (PciExBarBase, SIZE_256MB);
> -      //
> -      // call below, just like the one above for RCBA. However, Linux insists
> -      // that the MMCONFIG area be marked in the E820 or UEFI memory map as
> -      // "reserved memory" -- Linux does not content itself with a simple gap
> -      // in the memory map wherever the MCFG ACPI table points to.
> -      //
> -      // This appears to be a safety measure. The PCI Firmware Specification
> -      // (rev 3.1) says in 4.1.2. "MCFG Table Description": "The resources 
> can
> -      // *optionally* be returned in [...] EFIGetMemoryMap as reserved memory
> -      // [...]". (Emphasis added here.)
> -      //
> -      // Normally we add memory resource descriptor HOBs in
> -      // QemuInitializeRam(), and pre-allocate from those with memory
> -      // allocation HOBs in InitializeRamRegions(). However, the MMCONFIG 
> area
> -      // is most definitely not RAM; so, as an exception, cover it with
> -      // uncacheable reserved memory right here.
> -      //
> -      AddReservedMemoryBaseSizeHob (PciExBarBase, SIZE_256MB, FALSE);
> -      BuildMemoryAllocationHob (PciExBarBase, SIZE_256MB,
> -        EfiReservedMemoryType);
> -    }
> -    AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
> -
> -    //
> -    // On Q35, the IO Port space is available for PCI resource allocations 
> from
> -    // 0x6000 up.
> -    //
> -    if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> -      PciIoBase = 0x6000;
> -      PciIoSize = 0xA000;
> -      ASSERT ((ICH9_PMBASE_VALUE & 0xF000) < PciIoBase);
> -    }
> -  }
> -
>    //
>    // Add PCI IO Port space available for PCI resource allocations.
>    //
> @@ -278,71 +191,6 @@ MemMapInitialization (
>    ASSERT_RETURN_ERROR (PcdStatus);
>  }
>  
> -EFI_STATUS
> -GetNamedFwCfgBoolean (
> -  IN  CHAR8   *FwCfgFileName,
> -  OUT BOOLEAN *Setting
> -  )
> -{
> -  EFI_STATUS           Status;
> -  FIRMWARE_CONFIG_ITEM FwCfgItem;
> -  UINTN                FwCfgSize;
> -  UINT8                Value[3];
> -
> -  Status = QemuFwCfgFindFile (FwCfgFileName, &FwCfgItem, &FwCfgSize);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -  if (FwCfgSize > sizeof Value) {
> -    return EFI_BAD_BUFFER_SIZE;
> -  }
> -  QemuFwCfgSelectItem (FwCfgItem);
> -  QemuFwCfgReadBytes (FwCfgSize, Value);
> -
> -  if ((FwCfgSize == 1) ||
> -      (FwCfgSize == 2 && Value[1] == '\n') ||
> -      (FwCfgSize == 3 && Value[1] == '\r' && Value[2] == '\n')) {
> -    switch (Value[0]) {
> -      case '0':
> -      case 'n':
> -      case 'N':
> -        *Setting = FALSE;
> -        return EFI_SUCCESS;
> -
> -      case '1':
> -      case 'y':
> -      case 'Y':
> -        *Setting = TRUE;
> -        return EFI_SUCCESS;
> -
> -      default:
> -        break;
> -    }
> -  }
> -  return EFI_PROTOCOL_ERROR;
> -}
> -
> -#define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName)                   \
> -          do {                                                      \
> -            BOOLEAN       Setting;                                  \
> -            RETURN_STATUS PcdStatus;                                \
> -                                                                    \
> -            if (!EFI_ERROR (GetNamedFwCfgBoolean (                  \
> -                              "opt/ovmf/" #TokenName, &Setting))) { \
> -              PcdStatus = PcdSetBoolS (TokenName, Setting);         \
> -              ASSERT_RETURN_ERROR (PcdStatus);                      \
> -            }                                                       \
> -          } while (0)
> -
> -VOID
> -NoexecDxeInitialization (
> -  VOID
> -  )
> -{
> -  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdPropertiesTableEnable);
> -  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
> -}
> -
>  VOID
>  PciExBarInitialization (
>    VOID
> @@ -547,67 +395,6 @@ DebugDumpCmos (
>  }
>  
>  
> -VOID
> -S3Verification (
> -  VOID
> -  )
> -{
> -#if defined (MDE_CPU_X64)
> -  if (FeaturePcdGet (PcdSmmSmramRequire) && mS3Supported) {
> -    DEBUG ((EFI_D_ERROR,
> -      "%a: S3Resume2Pei doesn't support X64 PEI + SMM yet.\n", 
> __FUNCTION__));
> -    DEBUG ((EFI_D_ERROR,
> -      "%a: Please disable S3 on the QEMU command line (see the README),\n",
> -      __FUNCTION__));
> -    DEBUG ((EFI_D_ERROR,
> -      "%a: or build OVMF with \"OvmfPkgIa32X64.dsc\".\n", __FUNCTION__));
> -    ASSERT (FALSE);
> -    CpuDeadLoop ();
> -  }
> -#endif
> -}
> -
> -
> -/**
> -  Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg 
> modules.
> -  Set the mMaxCpuCount variable.
> -**/
> -VOID
> -MaxCpuCountInitialization (
> -  VOID
> -  )
> -{
> -  UINT16        ProcessorCount;
> -  RETURN_STATUS PcdStatus;
> -
> -  QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount);
> -  ProcessorCount = QemuFwCfgRead16 ();
> -  //
> -  // If the fw_cfg key or fw_cfg entirely is unavailable, load mMaxCpuCount
> -  // from the PCD default. No change to PCDs.
> -  //
> -  if (ProcessorCount == 0) {
> -    mMaxCpuCount = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
> -    return;
> -  }
> -  //
> -  // Otherwise, set mMaxCpuCount to the value reported by QEMU.
> -  //
> -  mMaxCpuCount = ProcessorCount;
> -  //
> -  // Additionally, tell UefiCpuPkg modules (a) the exact number of VCPUs, (b)
> -  // to wait, in the initial AP bringup, exactly as long as it takes for all 
> of
> -  // the APs to report in. For this, we set the longest representable timeout
> -  // (approx. 71 minutes).
> -  //
> -  PcdStatus = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, ProcessorCount);
> -  ASSERT_RETURN_ERROR (PcdStatus);
> -  PcdStatus = PcdSet32S (PcdCpuApInitTimeOutInMicroSeconds, MAX_UINT32);
> -  ASSERT_RETURN_ERROR (PcdStatus);
> -  DEBUG ((DEBUG_INFO, "%a: QEMU reports %d processor(s)\n", __FUNCTION__,
> -    ProcessorCount));
> -}
> -
>  
>  /**
>    Perform Platform PEI initialization.
> @@ -620,62 +407,43 @@ MaxCpuCountInitialization (
>  **/
>  EFI_STATUS
>  EFIAPI
> -InitializePlatform (
> +InitializeXenPlatform (
>    IN       EFI_PEI_FILE_HANDLE  FileHandle,
>    IN CONST EFI_PEI_SERVICES     **PeiServices
>    )
>  {
> -  EFI_STATUS    Status;
> -
>    DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
>  
>    DebugDumpCmos ();
>  
> -  XenDetect ();
> -
> -  if (QemuFwCfgS3Enabled ()) {
> -    DEBUG ((EFI_D_INFO, "S3 support was detected on QEMU\n"));
> -    mS3Supported = TRUE;
> -    Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
> -    ASSERT_EFI_ERROR (Status);
> +  if (!XenDetect ()) {
> +    DEBUG ((EFI_D_ERROR, "ERROR: Xen isn't detected\n"));
> +    CpuDeadLoop ();
>    }

(2) This is brand new code.

- In new code, we should use DEBUG_xxx, not EFI_D_xxx.

- Please insert an "ASSERT (FALSE);" before CpuDeadLoop (). The current
  code is not wrong, but assertion failures can be configured to have a
  disposition different from an internal CpuDeadLoop(), and
  ASSERT(FALSE) + CpuDeadLoop() is both safe and preserves this
  configurability. It's just an edk2 pattern that was recommended to me
  earlier and I've followed it too since.

With (2) updated (and (1) is up to you):

Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx>

I'll try to continue reviewing the series tomorrow.

Thanks,
Laszlo

>  
> -  S3Verification ();
>    BootModeInitialization ();
>    AddressWidthInitialization ();
> -  MaxCpuCountInitialization ();
>  
>    //
>    // Query Host Bridge DID
>    //
>    mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
>  
> -  if (FeaturePcdGet (PcdSmmSmramRequire)) {
> -    Q35TsegMbytesInitialization ();
> -  }
> -
>    PublishPeiMemory ();
>  
>    InitializeRamRegions ();
>  
> -  if (mXen) {
> -    DEBUG ((EFI_D_INFO, "Xen was detected\n"));
> -    InitializeXen ();
> -  }
> +  InitializeXen ();
>  
>    if (mBootMode != BOOT_ON_S3_RESUME) {
> -    if (!FeaturePcdGet (PcdSmmSmramRequire)) {
> -      ReserveEmuVariableNvStore ();
> -    }
> +    ReserveEmuVariableNvStore ();
>      PeiFvInitialization ();
>      MemMapInitialization ();
> -    NoexecDxeInitialization ();
>    }
>  
>    InstallClearCacheCallback ();
>    AmdSevInitialize ();
>    MiscInitialization ();
> -  InstallFeatureControlCallback ();
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
> similarity index 94%
> copy from OvmfPkg/PlatformPei/Xen.c
> copy to OvmfPkg/XenPlatformPei/Xen.c
> index ab38f97a67..7c82e5b69b 100644
> --- a/OvmfPkg/PlatformPei/Xen.c
> +++ b/OvmfPkg/XenPlatformPei/Xen.c
> @@ -3,6 +3,7 @@
>  
>    Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2011, Andrei Warkentin <andreiw@xxxxxxxxxxxx>
> +  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
> @@ -34,8 +35,6 @@
>  #include "Platform.h"
>  #include "Xen.h"
>  
> -BOOLEAN mXen = FALSE;
> -
>  STATIC UINT32 mXenLeaf = 0;
>  
>  EFI_XEN_INFO mXenInfo;
> @@ -143,7 +142,6 @@ XenDetect (
>                (UINT32 *) &Signature[8]);
>  
>      if (!AsciiStrCmp ((CHAR8 *) Signature, "XenVMMXenVMM")) {
> -      mXen = TRUE;
>        return TRUE;
>      }
>    }
> @@ -162,10 +160,6 @@ XenPublishRamRegions (
>    UINT32            E820EntriesCount;
>    EFI_STATUS        Status;
>  
> -  if (!mXen) {
> -    return;
> -  }
> -
>    DEBUG ((EFI_D_INFO, "Using memory map provided by Xen\n"));
>  
>    //
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.