[PATCH v3 1/8] MdeModulePkg: introduce PcdPciDisableBusEnumeration

[PATCH v3 1/8] MdeModulePkg: introduce PcdPciDisableBusEnumeration

[Wei Liu]

Platforms such as Xen already enumerates PCI bridges and devices. Use
this PCD to control EDK2 behavior.

PcdPciDisableBusEnumeration is placed under [PcdFixedAtBuild,
PcdDynamic, PcdDynamicEx], so that it is possible to alter it during
runtime.

The default setting of this PCD is false (allow full PCI enumeration) to
preserve the same behavior before this change.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c      |    5 ++++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |    1 +
 MdeModulePkg/MdeModulePkg.dec                |    3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index 5afbb82..cc6be8b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -284,7 +284,10 @@ PciBusDriverBindingStart (
           );
   }  
 
-  gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
+  if (PcdGetBool (PcdPciDisableBusEnumeration))
+    gFullEnumeration = FALSE;
+  else
+    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
 
   //
   // Open Device Path Protocol for PCI root bridge
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 34eb672..917889c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -108,6 +108,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
 
 # [Event]
 #   ##
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index b627eb1..e86868b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -737,6 +737,9 @@
   ## This PCD specifies whether the Multi Root I/O virtualization support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport|FALSE|BOOLEAN|0x10000046
 
+  ## This PCD specifies whether full PCI enumeration is disabled.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE|BOOLEAN|0x10000048
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Single root I/O virtualization virtual function memory BAR alignment
   #  BITN set indicates 2 of n+12 power
-- 
1.7.10.4

[PATCH v3 2/8] OvmfPkg: introduce E820.h

[Wei Liu]

E820 definitions copied from IntelFrameworkModulePkg/Csm/
LegacyBiosDxe/LegacyBiosInterface.h.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h

diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
new file mode 100644
index 0000000..e7e0c25
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/E820.h
@@ -0,0 +1,46 @@
+/** @file
+
+Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013, Citrix Systems UK Ltd.
+
+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.
+
+**/
+
+
+#ifndef __E820_H__
+#define __E820_H__
+
+#pragma pack(1)
+
+typedef enum {
+  EfiAcpiAddressRangeMemory   = 1,
+  EfiAcpiAddressRangeReserved = 2,
+  EfiAcpiAddressRangeACPI     = 3,
+  EfiAcpiAddressRangeNVS      = 4
+} EFI_ACPI_MEMORY_TYPE;
+
+typedef struct {
+  UINT64                BaseAddr;
+  UINT64                Length;
+  EFI_ACPI_MEMORY_TYPE  Type;
+} EFI_E820_ENTRY64;
+
+typedef struct {
+  UINT32                BassAddrLow;
+  UINT32                BaseAddrHigh;
+  UINT32                LengthLow;
+  UINT32                LengthHigh;
+  EFI_ACPI_MEMORY_TYPE  Type;
+} EFI_E820_ENTRY;
+
+#pragma pack()
+
+#endif /* __E820_H__ */
-- 
1.7.10.4

[PATCH v3 3/8] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Wei Liu]

EFI_XEN_OVMF_INFO is defined to accept configurations from hvmloader. It
must match the definition on Xen side.

XenInfo is extended to include necessary bits as well. Currently only E820
map is in use.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/Include/Guid/XenInfo.h |    7 +++++++
 OvmfPkg/PlatformPei/Xen.h      |   44 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 OvmfPkg/PlatformPei/Xen.h

diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
index d512b0b..6146080 100644
--- a/OvmfPkg/Include/Guid/XenInfo.h
+++ b/OvmfPkg/Include/Guid/XenInfo.h
@@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #ifndef __XEN_INFO_H__
 #define __XEN_INFO_H__
 
+#include <Uefi/UefiBaseType.h>
+
 #define EFI_XEN_INFO_GUID \
     { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
 
@@ -35,6 +37,11 @@ typedef struct {
   /// Hypervisor minor version.
   ///
   UINT16 VersionMinor;
+  ///
+  /// E820 map
+  ///
+  EFI_PHYSICAL_ADDRESS E820;
+  UINT32 E820EntriesCount;
 } EFI_XEN_INFO;
 
 extern EFI_GUID gEfiXenInfoGuid;
diff --git a/OvmfPkg/PlatformPei/Xen.h b/OvmfPkg/PlatformPei/Xen.h
new file mode 100644
index 0000000..1885ede
--- /dev/null
+++ b/OvmfPkg/PlatformPei/Xen.h
@@ -0,0 +1,44 @@
+/** @file
+  Ovmf info structure passed by Xen
+
+Copyright (c) 2013, Wei Liu <wei.liu2@citrix.com>
+This program and the accompanying materials are licensed and made available under
+the terms and conditions of the BSD License that 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.
+
+**/
+
+#ifndef __XEN_H__
+#define __XEN_H__
+
+#include <Uefi/UefiBaseType.h>
+
+// Physical address of OVMF info
+#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
+
+// This structure must match the definition on Xen side
+#pragma pack(1)
+typedef struct {
+  CHAR8 Signature[14]; // XenHVMOVMF\0
+  UINT8 Length;        // Length of this structure
+  UINT8 Checksum;      // Set such that the sum over bytes 0..length == 0
+  //
+  // Physical address of an array of TablesCount elements.
+  //
+  // Each element contains the physical address of a BIOS table.
+  //
+  EFI_PHYSICAL_ADDRESS Tables;
+  UINT32 TablesCount;
+  //
+  // Physical address of the E820 table, contains E820EntriesCount entries.
+  //
+  EFI_PHYSICAL_ADDRESS E820;
+  UINT32 E820EntriesCount;
+} EFI_XEN_OVMF_INFO;
+#pragma pack()
+
+#endif /* __XEN_H__ */
-- 
1.7.10.4

[PATCH v3 4/8] OvmfPkg: extract OVMF info passed by Xen hvmloader

[Wei Liu]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/PlatformPei/Xen.c |   23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
index a720b91..737cb15 100644
--- a/OvmfPkg/PlatformPei/Xen.c
+++ b/OvmfPkg/PlatformPei/Xen.c
@@ -26,13 +26,15 @@
 #include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <IndustryStandard/E820.h>
 #include <Guid/XenInfo.h>
 
 #include "Platform.h"
+#include "Xen.h"
 
 EFI_XEN_INFO mXenInfo;
 
-
 /**
   Connects to the Hypervisor.
  
@@ -50,6 +52,9 @@ XenConnect (
   UINT32 TransferReg;
   UINT32 TransferPages;
   UINT32 XenVersion;
+  EFI_XEN_OVMF_INFO *Info = (EFI_XEN_OVMF_INFO *) OVMF_INFO_PHYSICAL_ADDRESS;
+
+  ZeroMem (&mXenInfo, sizeof(mXenInfo));
 
   AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL);
   mXenInfo.HyperPages = AllocatePages (TransferPages);
@@ -72,6 +77,22 @@ XenConnect (
   /* TBD: Locate hvm_info and reserve it away. */
   mXenInfo.HvmInfo = NULL;
 
+  if (!AsciiStrCmp ((CHAR8 *) Info->Signature, "XenHVMOVMF")) {
+    EFI_E820_ENTRY64 *E820Map;
+
+    E820Map = AllocateCopyPool (sizeof(EFI_E820_ENTRY64) * Info->E820EntriesCount,
+                                (VOID *)Info->E820);
+    ASSERT (E820Map != NULL);
+
+    if (!E820Map) {
+      FreePages (mXenInfo.HyperPages, TransferPages);
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    mXenInfo.E820 = (EFI_PHYSICAL_ADDRESS)E820Map;
+    mXenInfo.E820EntriesCount = Info->E820EntriesCount;
+  }
+
   BuildGuidDataHob (
     &gEfiXenInfoGuid,
     &mXenInfo,
-- 
1.7.10.4

[PATCH v3 5/8] OvmfPkg: detect Xen earlier

[Wei Liu]

This is useful for initializing memory map.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/PlatformPei/Platform.c |    8 +++++++-
 OvmfPkg/PlatformPei/Platform.h |    5 +++++
 OvmfPkg/PlatformPei/Xen.c      |   12 +-----------
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index fb56e99..9b7828f 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -338,14 +338,20 @@ InitializePlatform (
   )
 {
   EFI_PHYSICAL_ADDRESS  TopOfMemory;
+  UINT32 XenLeaf;
 
   DEBUG ((EFI_D_ERROR, "Platform PEIM Loaded\n"));
 
   DebugDumpCmos ();
 
+  XenLeaf = XenDetect ();
+
   TopOfMemory = MemDetect ();
 
-  InitializeXen ();
+  if (XenLeaf != 0) {
+    DEBUG ((EFI_D_INFO, "Xen was detected\n"));
+    InitializeXen (XenLeaf);
+  }
 
   ReserveEmuVariableNvStore ();
 
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 383e6a4..d63d124 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -69,6 +69,11 @@ PeiFvInitialization (
 
 EFI_STATUS
 InitializeXen (
+  UINT32 XenLeaf
+  );
+
+UINT32
+XenDetect (
   VOID
   );
 
diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
index 737cb15..7779a1c 100644
--- a/OvmfPkg/PlatformPei/Xen.c
+++ b/OvmfPkg/PlatformPei/Xen.c
@@ -140,19 +140,9 @@ XenDetect (
 **/
 EFI_STATUS
 InitializeXen (
-  VOID
+  UINT32 XenLeaf
   )
 {
-  UINT32 XenLeaf;
-
-  XenLeaf = XenDetect ();
-
-  if (XenLeaf == 0) {
-    return EFI_NOT_FOUND;
-  }
-
-  DEBUG ((EFI_D_INFO, "Xen was detected\n"));
-
   XenConnect (XenLeaf);
 
   //
-- 
1.7.10.4

[PATCH v3 6/8] OvmfPkg: introduce PublishPeiMemory

[Wei Liu]

MemDetect actully does too many things, the underlying platform might
want to have more control over memory layout.

Extract the functionality of publishing PEI memory to a dedicated
function.

Also fixed wrong comment while I was there.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/PlatformPei/MemDetect.c |   36 +++++++++++++++++++++++++++++++++++-
 OvmfPkg/PlatformPei/Platform.h  |    5 +++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 9f6ca19..dc44745 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -83,11 +83,45 @@ GetSystemMemorySizeAbove4gb (
   return LShiftU64 (Size, 16);
 }
 
+/**
+  Publish PEI core memory
+
+  @return EFI_SUCCESS     The PEIM initialized successfully.
+
+**/
+EFI_STATUS
+PublishPeiMemory(
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+  EFI_PHYSICAL_ADDRESS        MemoryBase;
+  UINT64                      MemorySize;
+  UINT64                      LowerMemorySize;
+
+  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
+
+  MemoryBase = PcdGet32 (PcdOvmfMemFvBase) + PcdGet32 (PcdOvmfMemFvSize);
+  MemorySize = LowerMemorySize - MemoryBase;
+  if (MemorySize > SIZE_64MB) {
+    MemoryBase = LowerMemorySize - SIZE_64MB;
+    MemorySize = SIZE_64MB;
+  }
+
+  //
+  // Publish this memory to the PEI Core
+  //
+  Status = PublishSystemMemory(MemoryBase, MemorySize);
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
 
 /**
   Peform Memory Detection
 
-  @return EFI_SUCCESS     The PEIM initialized successfully.
+  @return Top of memory
 
 **/
 EFI_PHYSICAL_ADDRESS
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index d63d124..01af2a9 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -57,6 +57,11 @@ AddUntestedMemoryRangeHob (
   EFI_PHYSICAL_ADDRESS        MemoryLimit
   );
 
+EFI_STATUS
+PublishPeiMemory(
+  VOID
+  );
+
 EFI_PHYSICAL_ADDRESS
 MemDetect (
   VOID
-- 
1.7.10.4

[PATCH v3 7/8] OvmfPkg: introduce XenMemMapInitialization

[Wei Liu]

This function parses Xen OVMF info and arrange memory maps accordingly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/PlatformPei/Platform.c |   70 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 9b7828f..c2e7ba9 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -34,6 +34,10 @@
 #include <Guid/MemoryTypeInformation.h>
 #include <Ppi/MasterBootMode.h>
 #include <IndustryStandard/Pci22.h>
+#include <Guid/XenInfo.h>
+#include <IndustryStandard/E820.h>
+#include <Library/ResourcePublicationLib.h>
+#include <Library/MtrrLib.h>
 
 #include "Platform.h"
 #include "Cmos.h"
@@ -163,6 +167,72 @@ AddUntestedMemoryRangeHob (
   AddUntestedMemoryBaseSizeHob (MemoryBase, (UINT64)(MemoryLimit - MemoryBase));
 }
 
+VOID
+XenMemMapInitialization (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE *GuidHob;
+  EFI_XEN_INFO *Info;
+
+  DEBUG ((EFI_D_ERROR, "Using memory map provided by Xen\n"));
+
+  GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
+
+  ASSERT (GuidHob != NULL);
+
+  Info = GET_GUID_HOB_DATA (GuidHob);
+
+  //
+  // Create Memory Type Information HOB
+  //
+  BuildGuidDataHob (
+    &gEfiMemoryTypeInformationGuid,
+    mDefaultMemoryTypeInformation,
+    sizeof(mDefaultMemoryTypeInformation)
+    );
+
+  //
+  // Add PCI IO Port space available for PCI resource allocations.
+  //
+  BuildResourceDescriptorHob (
+    EFI_RESOURCE_IO,
+    EFI_RESOURCE_ATTRIBUTE_PRESENT     |
+    EFI_RESOURCE_ATTRIBUTE_INITIALIZED,
+    0xC000,
+    0x4000
+    );
+
+  //
+  // Video memory + Legacy BIOS region
+  //
+  AddIoMemoryRangeHob (0x0A0000, BASE_1MB);
+
+  //
+  // Parse RAM in E820 map
+  //
+  if (Info->E820EntriesCount > 0) {
+    EFI_E820_ENTRY64 *E820Map, *Entry;
+    UINT16 Loop;
+
+    E820Map = (EFI_E820_ENTRY64 *) Info->E820;
+    for (Loop = 0; Loop < Info->E820EntriesCount; Loop++) {
+      Entry = E820Map + Loop;
+
+      // only care about RAM
+      if (Entry->Type != EfiAcpiAddressRangeMemory)
+        continue;
+
+      if (Entry->BaseAddr >= BASE_4GB)
+        AddUntestedMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
+      else
+        AddMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
+
+      MtrrSetMemoryAttribute (Entry->BaseAddr, Entry->Length, CacheWriteBack);
+    }
+  }
+}
+
 
 VOID
 MemMapInitialization (
-- 
1.7.10.4

[PATCH v3 8/8] OvmfPkg: enable Xen specific path

[Wei Liu]

This patch sets PcdPciDisableBusEnumeration to true then makes use of
PublishPeiMemory and XenMemMapInitialization to construct memory map for
Xen guest.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/OvmfPkgIa32.dsc             |    5 +++--
 OvmfPkg/OvmfPkgIa32X64.dsc          |    5 +++--
 OvmfPkg/OvmfPkgX64.dsc              |    5 +++--
 OvmfPkg/PlatformPei/Platform.c      |   11 +++++++++--
 OvmfPkg/PlatformPei/PlatformPei.inf |    1 +
 5 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 760bd41..d4851ed 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -222,7 +222,7 @@
 !else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 !endif
-  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
@@ -320,6 +320,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
 
 
 ################################################################################
@@ -342,7 +343,7 @@
   MdeModulePkg/Core/Pei/PeiMain.inf
   MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   }
   IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 268d722..d9072e7 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -227,7 +227,7 @@
 !else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 !endif
-  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
@@ -326,6 +326,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
 
 
 ################################################################################
@@ -348,7 +349,7 @@
   MdeModulePkg/Core/Pei/PeiMain.inf
   MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   }
   IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 53945d0..71b178e 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -227,7 +227,7 @@
 !else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 !endif
-  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
@@ -325,6 +325,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
 
 
 ################################################################################
@@ -347,7 +348,7 @@
   MdeModulePkg/Core/Pei/PeiMain.inf
   MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   }
   IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index c2e7ba9..8daf9f4 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -416,7 +416,11 @@ InitializePlatform (
 
   XenLeaf = XenDetect ();
 
-  TopOfMemory = MemDetect ();
+  if (XenLeaf != 0) {
+    PublishPeiMemory ();
+    PcdSetBool (PcdPciDisableBusEnumeration, TRUE);
+  } else
+    TopOfMemory = MemDetect ();
 
   if (XenLeaf != 0) {
     DEBUG ((EFI_D_INFO, "Xen was detected\n"));
@@ -427,7 +431,10 @@ InitializePlatform (
 
   PeiFvInitialization ();
 
-  MemMapInitialization (TopOfMemory);
+  if (XenLeaf != 0)
+    XenMemMapInitialization ();
+  else
+    MemMapInitialization (TopOfMemory);
 
   MiscInitialization ();
 
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 3d5cbbb..7fe9d47 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -65,6 +65,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
 
 [Ppis]
-- 
1.7.10.4

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Konrad Rzeszutek Wilk]

On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
> E820 definitions copied from IntelFrameworkModulePkg/Csm/
> LegacyBiosDxe/LegacyBiosInterface.h.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
> new file mode 100644
> index 0000000..e7e0c25
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/E820.h
> @@ -0,0 +1,46 @@
> +/** @file
> +
> +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>

<BR> ?

> +Copyright (c) 2013, Citrix Systems UK Ltd.
> +
> +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.
> +
> +**/
> +
> +
> +#ifndef __E820_H__
> +#define __E820_H__
> +
> +#pragma pack(1)
> +
> +typedef enum {
> +  EfiAcpiAddressRangeMemory   = 1,
> +  EfiAcpiAddressRangeReserved = 2,
> +  EfiAcpiAddressRangeACPI     = 3,
> +  EfiAcpiAddressRangeNVS      = 4

Odd, Linux also has E820_UNUSABLE

should that be in there as well?
> +} EFI_ACPI_MEMORY_TYPE;
> +
> +typedef struct {
> +  UINT64                BaseAddr;
> +  UINT64                Length;
> +  EFI_ACPI_MEMORY_TYPE  Type;
> +} EFI_E820_ENTRY64;
> +
> +typedef struct {
> +  UINT32                BassAddrLow;
> +  UINT32                BaseAddrHigh;
> +  UINT32                LengthLow;
> +  UINT32                LengthHigh;
> +  EFI_ACPI_MEMORY_TYPE  Type;
> +} EFI_E820_ENTRY;
> +
> +#pragma pack()
> +
> +#endif /* __E820_H__ */
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [edk2] [PATCH v3 1/8] MdeModulePkg: introduce PcdPciDisableBusEnumeration

[Kinney, Michael D]

Wei,

I recommend you put the declaration of the new PCD in the next section if MdeModulePkg.dec so it supports PatchableInModule as well.

Thanks,

Mike

-----Original Message-----
From: Wei Liu [mailto:wei.liu2@citrix.com] 
Sent: Tuesday, November 26, 2013 11:26 AM
To: edk2-devel@lists.sourceforge.net; xen-devel@lists.xen.org
Subject: [edk2] [PATCH v3 1/8] MdeModulePkg: introduce PcdPciDisableBusEnumeration

Platforms such as Xen already enumerates PCI bridges and devices. Use
this PCD to control EDK2 behavior.

PcdPciDisableBusEnumeration is placed under [PcdFixedAtBuild,
PcdDynamic, PcdDynamicEx], so that it is possible to alter it during
runtime.

The default setting of this PCD is false (allow full PCI enumeration) to
preserve the same behavior before this change.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c      |    5 ++++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |    1 +
 MdeModulePkg/MdeModulePkg.dec                |    3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index 5afbb82..cc6be8b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -284,7 +284,10 @@ PciBusDriverBindingStart (
           );
   }  
 
-  gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
+  if (PcdGetBool (PcdPciDisableBusEnumeration))
+    gFullEnumeration = FALSE;
+  else
+    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
 
   //
   // Open Device Path Protocol for PCI root bridge
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 34eb672..917889c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -108,6 +108,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
 
 # [Event]
 #   ##
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index b627eb1..e86868b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -737,6 +737,9 @@
   ## This PCD specifies whether the Multi Root I/O virtualization support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport|FALSE|BOOLEAN|0x10000046
 
+  ## This PCD specifies whether full PCI enumeration is disabled.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE|BOOLEAN|0x10000048
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Single root I/O virtualization virtual function memory BAR alignment
   #  BITN set indicates 2 of n+12 power
-- 
1.7.10.4


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Ian Campbell]

On Tue, 2013-11-26 at 14:47 -0500, Konrad Rzeszutek Wilk wrote:

> > +typedef enum {
> > +  EfiAcpiAddressRangeMemory   = 1,
> > +  EfiAcpiAddressRangeReserved = 2,
> > +  EfiAcpiAddressRangeACPI     = 3,
> > +  EfiAcpiAddressRangeNVS      = 4
> 
> Odd, Linux also has E820_UNUSABLE

== AddressRangeUnusuable = 5 in the ACPI 5.0 spec, which also defines
AddressRangeDisabled = 6.

> should that be in there as well?

I suppose it only really matters if something is using it...

Re: [edk2] [PATCH v3 1/8] MdeModulePkg: introduce PcdPciDisableBusEnumeration

[Wei Liu]

On Tue, Nov 26, 2013 at 10:26:12PM +0000, Kinney, Michael D wrote:
> Wei,
> 
> I recommend you put the declaration of the new PCD in the next section if MdeModulePkg.dec so it supports PatchableInModule as well.
> 

Thanks Mike. I will do this.

Wei.

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Wei Liu]

On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
> > LegacyBiosDxe/LegacyBiosInterface.h.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> > ---
> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
> > 
> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
> > new file mode 100644
> > index 0000000..e7e0c25
> > --- /dev/null
> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
> > @@ -0,0 +1,46 @@
> > +/** @file
> > +
> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> 
> <BR> ?
> 

Copy-n-paste error, phew.

<BR> stands for new line in HTML. I have no idea how it came up in the
original header.

> > +Copyright (c) 2013, Citrix Systems UK Ltd.
> > +
> > +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.
> > +
> > +**/
> > +
> > +
> > +#ifndef __E820_H__
> > +#define __E820_H__
> > +
> > +#pragma pack(1)
> > +
> > +typedef enum {
> > +  EfiAcpiAddressRangeMemory   = 1,
> > +  EfiAcpiAddressRangeReserved = 2,
> > +  EfiAcpiAddressRangeACPI     = 3,
> > +  EfiAcpiAddressRangeNVS      = 4
> 
> Odd, Linux also has E820_UNUSABLE
> 
> should that be in there as well?

The original header from where I copied these definitions doens't have
that and nobody is using that at the moment. I think we can add that in
later.

Wei.

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Jordan Justen]

On Wed, Nov 27, 2013 at 4:22 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
>> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
>> > LegacyBiosDxe/LegacyBiosInterface.h.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>> > ---
>> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
>> >  1 file changed, 46 insertions(+)
>> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
>> >
>> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
>> > new file mode 100644
>> > index 0000000..e7e0c25
>> > --- /dev/null
>> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
>> > @@ -0,0 +1,46 @@
>> > +/** @file
>> > +
>> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
>>
>> <BR> ?
>>
>
> Copy-n-paste error, phew.
>
> <BR> stands for new line in HTML. I have no idea how it came up in the
> original header.

Actually Intel wants that appended to all Intel copyright lines in EDK II files.

I'm not sure why, but I suspected something related to forcing a
newline in doxygen.

Hot seems to keep an eye on it, so maybe he knows.

-Jordan

>> > +Copyright (c) 2013, Citrix Systems UK Ltd.
>> > +
>> > +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.
>> > +
>> > +**/
>> > +
>> > +
>> > +#ifndef __E820_H__
>> > +#define __E820_H__
>> > +
>> > +#pragma pack(1)
>> > +
>> > +typedef enum {
>> > +  EfiAcpiAddressRangeMemory   = 1,
>> > +  EfiAcpiAddressRangeReserved = 2,
>> > +  EfiAcpiAddressRangeACPI     = 3,
>> > +  EfiAcpiAddressRangeNVS      = 4
>>
>> Odd, Linux also has E820_UNUSABLE
>>
>> should that be in there as well?
>
> The original header from where I copied these definitions doens't have
> that and nobody is using that at the moment. I think we can add that in
> later.
>
> Wei.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Wei Liu]

On Wed, Nov 27, 2013 at 04:18:38PM -0800, Jordan Justen wrote:
> On Wed, Nov 27, 2013 at 4:22 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
> >> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
> >> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
> >> > LegacyBiosDxe/LegacyBiosInterface.h.
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> >> > ---
> >> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
> >> >  1 file changed, 46 insertions(+)
> >> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
> >> >
> >> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > new file mode 100644
> >> > index 0000000..e7e0c25
> >> > --- /dev/null
> >> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > @@ -0,0 +1,46 @@
> >> > +/** @file
> >> > +
> >> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> >>
> >> <BR> ?
> >>
> >
> > Copy-n-paste error, phew.
> >
> > <BR> stands for new line in HTML. I have no idea how it came up in the
> > original header.
> 
> Actually Intel wants that appended to all Intel copyright lines in EDK II files.
> 
> I'm not sure why, but I suspected something related to forcing a
> newline in doxygen.
> 
> Hot seems to keep an eye on it, so maybe he knows.
> 

Oh that's really unusal requirement. ;-)

> -Jordan
> 
> >> > +Copyright (c) 2013, Citrix Systems UK Ltd.
> >> > +

In that case should I append <BR> to the above line?

Hot, can you clarify this to a humble non-Intel developer? :-P

Thanks
Wei.

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Tian, Hot]

Below is quoted from the EDKII coding standard spec:

7.3.2 Copyright Notice
The first line of the last paragraph of the file description is made up of the copyright
notice. The copyright notice must consist of the following text with the FIRST and
LAST symbols replaced with the year the file was created and the year the file was last
edited, respectively.
Copyright (C) FIRST - LAST, Intel Corporation. All rights reserved.<BR>
A file that has been created but not edited in subsequent years would have a copyright
notice with a single date, such as:
Copyright (C) 2007, Intel Corporation. All rights reserved.<BR>
If this file is subsequently edited, the copyright notice would be updated as follows.
Copyright (C) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
The FIRST - LAST format for the copyright date, as described above, is the only
format allowed. Do not use a comma separated list or keep updating a single date.
The <BR> at the end of the line is required. Since Doxygen uses XML for its internal
format, spaces and new lines are treated as a single space. The <BR> will force
Doxygen to start the following text, the license notice, on a new line.

Thanks,
Hot

-----Original Message-----
From: Jordan Justen [mailto:jljusten@gmail.com] 
Sent: Thursday, November 28, 2013 8:19 AM
To: Wei Liu
Cc: Konrad Rzeszutek Wilk; edk2-devel@lists.sourceforge.net; xen-devel; Tian, Hot; Justen, Jordan L
Subject: Re: [Xen-devel] [PATCH v3 2/8] OvmfPkg: introduce E820.h

On Wed, Nov 27, 2013 at 4:22 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
>> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
>> > LegacyBiosDxe/LegacyBiosInterface.h.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>> > ---
>> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
>> >  1 file changed, 46 insertions(+)
>> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
>> >
>> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
>> > new file mode 100644
>> > index 0000000..e7e0c25
>> > --- /dev/null
>> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
>> > @@ -0,0 +1,46 @@
>> > +/** @file
>> > +
>> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
>>
>> <BR> ?
>>
>
> Copy-n-paste error, phew.
>
> <BR> stands for new line in HTML. I have no idea how it came up in the
> original header.

Actually Intel wants that appended to all Intel copyright lines in EDK II files.

I'm not sure why, but I suspected something related to forcing a
newline in doxygen.

Hot seems to keep an eye on it, so maybe he knows.

-Jordan

>> > +Copyright (c) 2013, Citrix Systems UK Ltd.
>> > +
>> > +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.
>> > +
>> > +**/
>> > +
>> > +
>> > +#ifndef __E820_H__
>> > +#define __E820_H__
>> > +
>> > +#pragma pack(1)
>> > +
>> > +typedef enum {
>> > +  EfiAcpiAddressRangeMemory   = 1,
>> > +  EfiAcpiAddressRangeReserved = 2,
>> > +  EfiAcpiAddressRangeACPI     = 3,
>> > +  EfiAcpiAddressRangeNVS      = 4
>>
>> Odd, Linux also has E820_UNUSABLE
>>
>> should that be in there as well?
>
> The original header from where I copied these definitions doens't have
> that and nobody is using that at the moment. I think we can add that in
> later.
>
> Wei.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 3/8] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Jordan Justen]

On Tue, Nov 26, 2013 at 11:25 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> EFI_XEN_OVMF_INFO is defined to accept configurations from hvmloader. It
> must match the definition on Xen side.
>
> XenInfo is extended to include necessary bits as well. Currently only E820
> map is in use.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/Include/Guid/XenInfo.h |    7 +++++++
>  OvmfPkg/PlatformPei/Xen.h      |   44 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>  create mode 100644 OvmfPkg/PlatformPei/Xen.h
>
> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
> index d512b0b..6146080 100644
> --- a/OvmfPkg/Include/Guid/XenInfo.h
> +++ b/OvmfPkg/Include/Guid/XenInfo.h
> @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #ifndef __XEN_INFO_H__
>  #define __XEN_INFO_H__
>
> +#include <Uefi/UefiBaseType.h>
> +

I don't think we want this #include in Guid/XenInfo.h.

Can you make sure that the locations that #include Guid/XenInfo.h
include PiPei.h, PiDxe.h, or Uefi.h before Guid/XenInfo.h?

>  #define EFI_XEN_INFO_GUID \
>      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
>
> @@ -35,6 +37,11 @@ typedef struct {
>    /// Hypervisor minor version.
>    ///
>    UINT16 VersionMinor;
> +  ///
> +  /// E820 map
> +  ///
> +  EFI_PHYSICAL_ADDRESS E820;
> +  UINT32 E820EntriesCount;

Can you add this chunk into the next patch which initializes
E820/E820EntriesCount?

With those 2 changes, patches 3 & 4 are
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

>  } EFI_XEN_INFO;
>
>  extern EFI_GUID gEfiXenInfoGuid;
> diff --git a/OvmfPkg/PlatformPei/Xen.h b/OvmfPkg/PlatformPei/Xen.h
> new file mode 100644
> index 0000000..1885ede
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/Xen.h
> @@ -0,0 +1,44 @@
> +/** @file
> +  Ovmf info structure passed by Xen
> +
> +Copyright (c) 2013, Wei Liu <wei.liu2@citrix.com>
> +This program and the accompanying materials are licensed and made available under
> +the terms and conditions of the BSD License that 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.
> +
> +**/
> +
> +#ifndef __XEN_H__
> +#define __XEN_H__
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +// Physical address of OVMF info
> +#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
> +
> +// This structure must match the definition on Xen side
> +#pragma pack(1)
> +typedef struct {
> +  CHAR8 Signature[14]; // XenHVMOVMF\0
> +  UINT8 Length;        // Length of this structure
> +  UINT8 Checksum;      // Set such that the sum over bytes 0..length == 0
> +  //
> +  // Physical address of an array of TablesCount elements.
> +  //
> +  // Each element contains the physical address of a BIOS table.
> +  //
> +  EFI_PHYSICAL_ADDRESS Tables;
> +  UINT32 TablesCount;
> +  //
> +  // Physical address of the E820 table, contains E820EntriesCount entries.
> +  //
> +  EFI_PHYSICAL_ADDRESS E820;
> +  UINT32 E820EntriesCount;
> +} EFI_XEN_OVMF_INFO;
> +#pragma pack()
> +
> +#endif /* __XEN_H__ */
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 3/8] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Jordan Justen]

On Wed, Nov 27, 2013 at 5:09 PM, Jordan Justen <jljusten@gmail.com> wrote:
> On Tue, Nov 26, 2013 at 11:25 AM, Wei Liu <wei.liu2@citrix.com> wrote:
>> EFI_XEN_OVMF_INFO is defined to accept configurations from hvmloader. It
>> must match the definition on Xen side.
>>
>> XenInfo is extended to include necessary bits as well. Currently only E820
>> map is in use.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  OvmfPkg/Include/Guid/XenInfo.h |    7 +++++++
>>  OvmfPkg/PlatformPei/Xen.h      |   44 ++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 51 insertions(+)
>>  create mode 100644 OvmfPkg/PlatformPei/Xen.h
>>
>> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
>> index d512b0b..6146080 100644
>> --- a/OvmfPkg/Include/Guid/XenInfo.h
>> +++ b/OvmfPkg/Include/Guid/XenInfo.h
>> @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>>  #ifndef __XEN_INFO_H__
>>  #define __XEN_INFO_H__
>>
>> +#include <Uefi/UefiBaseType.h>
>> +
>
> I don't think we want this #include in Guid/XenInfo.h.
>
> Can you make sure that the locations that #include Guid/XenInfo.h
> include PiPei.h, PiDxe.h, or Uefi.h before Guid/XenInfo.h?
>
>>  #define EFI_XEN_INFO_GUID \
>>      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
>>
>> @@ -35,6 +37,11 @@ typedef struct {
>>    /// Hypervisor minor version.
>>    ///
>>    UINT16 VersionMinor;
>> +  ///
>> +  /// E820 map
>> +  ///
>> +  EFI_PHYSICAL_ADDRESS E820;
>> +  UINT32 E820EntriesCount;
>
> Can you add this chunk into the next patch which initializes
> E820/E820EntriesCount?

Actually ... this doesn't need to be added into EFI_XEN_INFO, since
PlatformPei is the only thing looking at the tables, right?

So, maybe you just need a new function:
EFI_STATUS
XenGetE820Entries (
  EFI_E820_ENTRY64 **Entries,
  UINTN *Count
  );

-Jordan

> With those 2 changes, patches 3 & 4 are
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>
>>  } EFI_XEN_INFO;
>>
>>  extern EFI_GUID gEfiXenInfoGuid;
>> diff --git a/OvmfPkg/PlatformPei/Xen.h b/OvmfPkg/PlatformPei/Xen.h
>> new file mode 100644
>> index 0000000..1885ede
>> --- /dev/null
>> +++ b/OvmfPkg/PlatformPei/Xen.h
>> @@ -0,0 +1,44 @@
>> +/** @file
>> +  Ovmf info structure passed by Xen
>> +
>> +Copyright (c) 2013, Wei Liu <wei.liu2@citrix.com>
>> +This program and the accompanying materials are licensed and made available under
>> +the terms and conditions of the BSD License that 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.
>> +
>> +**/
>> +
>> +#ifndef __XEN_H__
>> +#define __XEN_H__
>> +
>> +#include <Uefi/UefiBaseType.h>
>> +
>> +// Physical address of OVMF info
>> +#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
>> +
>> +// This structure must match the definition on Xen side
>> +#pragma pack(1)
>> +typedef struct {
>> +  CHAR8 Signature[14]; // XenHVMOVMF\0
>> +  UINT8 Length;        // Length of this structure
>> +  UINT8 Checksum;      // Set such that the sum over bytes 0..length == 0
>> +  //
>> +  // Physical address of an array of TablesCount elements.
>> +  //
>> +  // Each element contains the physical address of a BIOS table.
>> +  //
>> +  EFI_PHYSICAL_ADDRESS Tables;
>> +  UINT32 TablesCount;
>> +  //
>> +  // Physical address of the E820 table, contains E820EntriesCount entries.
>> +  //
>> +  EFI_PHYSICAL_ADDRESS E820;
>> +  UINT32 E820EntriesCount;
>> +} EFI_XEN_OVMF_INFO;
>> +#pragma pack()
>> +
>> +#endif /* __XEN_H__ */
>> --
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

Re: [PATCH v3 3/8] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Wei Liu]

On Wed, Nov 27, 2013 at 05:36:01PM -0800, Jordan Justen wrote:
> On Wed, Nov 27, 2013 at 5:09 PM, Jordan Justen <jljusten@gmail.com> wrote:
> > On Tue, Nov 26, 2013 at 11:25 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> >> EFI_XEN_OVMF_INFO is defined to accept configurations from hvmloader. It
> >> must match the definition on Xen side.
> >>
> >> XenInfo is extended to include necessary bits as well. Currently only E820
> >> map is in use.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> ---
> >>  OvmfPkg/Include/Guid/XenInfo.h |    7 +++++++
> >>  OvmfPkg/PlatformPei/Xen.h      |   44 ++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 51 insertions(+)
> >>  create mode 100644 OvmfPkg/PlatformPei/Xen.h
> >>
> >> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
> >> index d512b0b..6146080 100644
> >> --- a/OvmfPkg/Include/Guid/XenInfo.h
> >> +++ b/OvmfPkg/Include/Guid/XenInfo.h
> >> @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >>  #ifndef __XEN_INFO_H__
> >>  #define __XEN_INFO_H__
> >>
> >> +#include <Uefi/UefiBaseType.h>
> >> +
> >
> > I don't think we want this #include in Guid/XenInfo.h.
> >
> > Can you make sure that the locations that #include Guid/XenInfo.h
> > include PiPei.h, PiDxe.h, or Uefi.h before Guid/XenInfo.h?
> >
> >>  #define EFI_XEN_INFO_GUID \
> >>      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
> >>
> >> @@ -35,6 +37,11 @@ typedef struct {
> >>    /// Hypervisor minor version.
> >>    ///
> >>    UINT16 VersionMinor;
> >> +  ///
> >> +  /// E820 map
> >> +  ///
> >> +  EFI_PHYSICAL_ADDRESS E820;
> >> +  UINT32 E820EntriesCount;
> >
> > Can you add this chunk into the next patch which initializes
> > E820/E820EntriesCount?
> 
> Actually ... this doesn't need to be added into EFI_XEN_INFO, since
> PlatformPei is the only thing looking at the tables, right?
> 

At this stage, yes, we don't need HOB for this.

> So, maybe you just need a new function:
> EFI_STATUS
> XenGetE820Entries (
>   EFI_E820_ENTRY64 **Entries,
>   UINTN *Count
>   );
> 

Looks like a good approach.

Wei.

> -Jordan
> 

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Wei Liu]

On Thu, Nov 28, 2013 at 12:42:27AM +0000, Tian, Hot wrote:
> Below is quoted from the EDKII coding standard spec:
> 
Sorry I can understand the above paragraph, but I still cannot get the
expection for non-Intel copyright line.

> 7.3.2 Copyright Notice
> The first line of the last paragraph of the file description is made up of the copyright

You seem to be expecting only one line of copyright notice?

> notice. The copyright notice must consist of the following text with the FIRST and
> LAST symbols replaced with the year the file was created and the year the file was last
> edited, respectively.
> Copyright (C) FIRST - LAST, Intel Corporation. All rights reserved.<BR>
> A file that has been created but not edited in subsequent years would have a copyright
> notice with a single date, such as:
> Copyright (C) 2007, Intel Corporation. All rights reserved.<BR>
> If this file is subsequently edited, the copyright notice would be updated as follows.
> Copyright (C) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
> The FIRST - LAST format for the copyright date, as described above, is the only
> format allowed. Do not use a comma separated list or keep updating a single date.
> The <BR> at the end of the line is required. Since Doxygen uses XML for its internal
> format, spaces and new lines are treated as a single space. The <BR> will force
> Doxygen to start the following text, the license notice, on a new line.
> 

I mimic the pattern in OvmfPkg/VirtioBlkDxe/VirtioBlk.c and get.

/** @file

Copyright (c) 2013, Citrix Systems UK Ltd.
Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>

This program and the accompanying materials
are licensed and made available under the terms and conditions
of the BSD License which accompanies this distribution.  The
full text of the license may be found at
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.

**/

Is the above one OK?

Wei.

> Thanks,
> Hot
> 
> -----Original Message-----
> From: Jordan Justen [mailto:jljusten@gmail.com] 
> Sent: Thursday, November 28, 2013 8:19 AM
> To: Wei Liu
> Cc: Konrad Rzeszutek Wilk; edk2-devel@lists.sourceforge.net; xen-devel; Tian, Hot; Justen, Jordan L
> Subject: Re: [Xen-devel] [PATCH v3 2/8] OvmfPkg: introduce E820.h
> 
> On Wed, Nov 27, 2013 at 4:22 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
> >> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
> >> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
> >> > LegacyBiosDxe/LegacyBiosInterface.h.
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> >> > ---
> >> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
> >> >  1 file changed, 46 insertions(+)
> >> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
> >> >
> >> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > new file mode 100644
> >> > index 0000000..e7e0c25
> >> > --- /dev/null
> >> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > @@ -0,0 +1,46 @@
> >> > +/** @file
> >> > +
> >> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> >>
> >> <BR> ?
> >>
> >
> > Copy-n-paste error, phew.
> >
> > <BR> stands for new line in HTML. I have no idea how it came up in the
> > original header.
> 
> Actually Intel wants that appended to all Intel copyright lines in EDK II files.
> 
> I'm not sure why, but I suspected something related to forcing a
> newline in doxygen.
> 
> Hot seems to keep an eye on it, so maybe he knows.
> 
> -Jordan
> 
> >> > +Copyright (c) 2013, Citrix Systems UK Ltd.
> >> > +
> >> > +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.
> >> > +
> >> > +**/
> >> > +
> >> > +
> >> > +#ifndef __E820_H__
> >> > +#define __E820_H__
> >> > +
> >> > +#pragma pack(1)
> >> > +
> >> > +typedef enum {
> >> > +  EfiAcpiAddressRangeMemory   = 1,
> >> > +  EfiAcpiAddressRangeReserved = 2,
> >> > +  EfiAcpiAddressRangeACPI     = 3,
> >> > +  EfiAcpiAddressRangeNVS      = 4
> >>
> >> Odd, Linux also has E820_UNUSABLE
> >>
> >> should that be in there as well?
> >
> > The original header from where I copied these definitions doens't have
> > that and nobody is using that at the moment. I think we can add that in
> > later.
> >
> > Wei.
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [PATCH v3 2/8] OvmfPkg: introduce E820.h

[Tian, Hot]

You may follow your company's legal guidance on copyright format.

-----Original Message-----
From: Wei Liu [mailto:wei.liu2@citrix.com] 
Sent: Thursday, November 28, 2013 10:35 AM
To: Tian, Hot
Cc: Jordan Justen; Wei Liu; Konrad Rzeszutek Wilk; edk2-devel@lists.sourceforge.net; xen-devel; Justen, Jordan L
Subject: Re: [Xen-devel] [PATCH v3 2/8] OvmfPkg: introduce E820.h

On Thu, Nov 28, 2013 at 12:42:27AM +0000, Tian, Hot wrote:
> Below is quoted from the EDKII coding standard spec:
> 
Sorry I can understand the above paragraph, but I still cannot get the
expection for non-Intel copyright line.

> 7.3.2 Copyright Notice
> The first line of the last paragraph of the file description is made up of the copyright

You seem to be expecting only one line of copyright notice?

> notice. The copyright notice must consist of the following text with the FIRST and
> LAST symbols replaced with the year the file was created and the year the file was last
> edited, respectively.
> Copyright (C) FIRST - LAST, Intel Corporation. All rights reserved.<BR>
> A file that has been created but not edited in subsequent years would have a copyright
> notice with a single date, such as:
> Copyright (C) 2007, Intel Corporation. All rights reserved.<BR>
> If this file is subsequently edited, the copyright notice would be updated as follows.
> Copyright (C) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
> The FIRST - LAST format for the copyright date, as described above, is the only
> format allowed. Do not use a comma separated list or keep updating a single date.
> The <BR> at the end of the line is required. Since Doxygen uses XML for its internal
> format, spaces and new lines are treated as a single space. The <BR> will force
> Doxygen to start the following text, the license notice, on a new line.
> 

I mimic the pattern in OvmfPkg/VirtioBlkDxe/VirtioBlk.c and get.

/** @file

Copyright (c) 2013, Citrix Systems UK Ltd.
Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>

This program and the accompanying materials
are licensed and made available under the terms and conditions
of the BSD License which accompanies this distribution.  The
full text of the license may be found at
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.

**/

Is the above one OK?

Wei.

> Thanks,
> Hot
> 
> -----Original Message-----
> From: Jordan Justen [mailto:jljusten@gmail.com] 
> Sent: Thursday, November 28, 2013 8:19 AM
> To: Wei Liu
> Cc: Konrad Rzeszutek Wilk; edk2-devel@lists.sourceforge.net; xen-devel; Tian, Hot; Justen, Jordan L
> Subject: Re: [Xen-devel] [PATCH v3 2/8] OvmfPkg: introduce E820.h
> 
> On Wed, Nov 27, 2013 at 4:22 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Nov 26, 2013 at 02:47:13PM -0500, Konrad Rzeszutek Wilk wrote:
> >> On Tue, Nov 26, 2013 at 07:25:58PM +0000, Wei Liu wrote:
> >> > E820 definitions copied from IntelFrameworkModulePkg/Csm/
> >> > LegacyBiosDxe/LegacyBiosInterface.h.
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> >> > ---
> >> >  OvmfPkg/Include/IndustryStandard/E820.h |   46 +++++++++++++++++++++++++++++++
> >> >  1 file changed, 46 insertions(+)
> >> >  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
> >> >
> >> > diff --git a/OvmfPkg/Include/IndustryStandard/E820.h b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > new file mode 100644
> >> > index 0000000..e7e0c25
> >> > --- /dev/null
> >> > +++ b/OvmfPkg/Include/IndustryStandard/E820.h
> >> > @@ -0,0 +1,46 @@
> >> > +/** @file
> >> > +
> >> > +Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> >>
> >> <BR> ?
> >>
> >
> > Copy-n-paste error, phew.
> >
> > <BR> stands for new line in HTML. I have no idea how it came up in the
> > original header.
> 
> Actually Intel wants that appended to all Intel copyright lines in EDK II files.
> 
> I'm not sure why, but I suspected something related to forcing a
> newline in doxygen.
> 
> Hot seems to keep an eye on it, so maybe he knows.
> 
> -Jordan
> 
> >> > +Copyright (c) 2013, Citrix Systems UK Ltd.
> >> > +
> >> > +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.
> >> > +
> >> > +**/
> >> > +
> >> > +
> >> > +#ifndef __E820_H__
> >> > +#define __E820_H__
> >> > +
> >> > +#pragma pack(1)
> >> > +
> >> > +typedef enum {
> >> > +  EfiAcpiAddressRangeMemory   = 1,
> >> > +  EfiAcpiAddressRangeReserved = 2,
> >> > +  EfiAcpiAddressRangeACPI     = 3,
> >> > +  EfiAcpiAddressRangeNVS      = 4
> >>
> >> Odd, Linux also has E820_UNUSABLE
> >>
> >> should that be in there as well?
> >
> > The original header from where I copied these definitions doens't have
> > that and nobody is using that at the moment. I think we can add that in
> > later.
> >
> > Wei.
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [edk2] [PATCH v3 7/8] OvmfPkg: introduce XenMemMapInitialization

[Jordan Justen]

On Tue, Nov 26, 2013 at 11:26 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> This function parses Xen OVMF info and arrange memory maps accordingly.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/PlatformPei/Platform.c |   70 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 9b7828f..c2e7ba9 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -34,6 +34,10 @@
>  #include <Guid/MemoryTypeInformation.h>
>  #include <Ppi/MasterBootMode.h>
>  #include <IndustryStandard/Pci22.h>
> +#include <Guid/XenInfo.h>
> +#include <IndustryStandard/E820.h>
> +#include <Library/ResourcePublicationLib.h>
> +#include <Library/MtrrLib.h>
>
>  #include "Platform.h"
>  #include "Cmos.h"
> @@ -163,6 +167,72 @@ AddUntestedMemoryRangeHob (
>    AddUntestedMemoryBaseSizeHob (MemoryBase, (UINT64)(MemoryLimit - MemoryBase));
>  }
>
> +VOID
> +XenMemMapInitialization (
> +  VOID
> +  )
> +{
> +  EFI_HOB_GUID_TYPE *GuidHob;
> +  EFI_XEN_INFO *Info;
> +
> +  DEBUG ((EFI_D_ERROR, "Using memory map provided by Xen\n"));
> +
> +  GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
> +
> +  ASSERT (GuidHob != NULL);
> +
> +  Info = GET_GUID_HOB_DATA (GuidHob);
> +
> +  //
> +  // Create Memory Type Information HOB
> +  //
> +  BuildGuidDataHob (
> +    &gEfiMemoryTypeInformationGuid,
> +    mDefaultMemoryTypeInformation,
> +    sizeof(mDefaultMemoryTypeInformation)
> +    );
> +
> +  //
> +  // Add PCI IO Port space available for PCI resource allocations.
> +  //
> +  BuildResourceDescriptorHob (
> +    EFI_RESOURCE_IO,
> +    EFI_RESOURCE_ATTRIBUTE_PRESENT     |
> +    EFI_RESOURCE_ATTRIBUTE_INITIALIZED,
> +    0xC000,
> +    0x4000
> +    );
> +
> +  //
> +  // Video memory + Legacy BIOS region
> +  //
> +  AddIoMemoryRangeHob (0x0A0000, BASE_1MB);
> +
> +  //
> +  // Parse RAM in E820 map
> +  //
> +  if (Info->E820EntriesCount > 0) {
> +    EFI_E820_ENTRY64 *E820Map, *Entry;
> +    UINT16 Loop;
> +
> +    E820Map = (EFI_E820_ENTRY64 *) Info->E820;
> +    for (Loop = 0; Loop < Info->E820EntriesCount; Loop++) {
> +      Entry = E820Map + Loop;
> +
> +      // only care about RAM
> +      if (Entry->Type != EfiAcpiAddressRangeMemory)
> +        continue;

I think you are going to change this code base on the
XenGetE820Entries discussion in the patch 3 thread.

I just wanted to add that we always use curly-brackets:
  if () {
  }
even if the body is just a single line.

-Jordan

> +
> +      if (Entry->BaseAddr >= BASE_4GB)
> +        AddUntestedMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
> +      else
> +        AddMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
> +
> +      MtrrSetMemoryAttribute (Entry->BaseAddr, Entry->Length, CacheWriteBack);
> +    }
> +  }
> +}
> +
>
>  VOID
>  MemMapInitialization (
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH v3 6/8] OvmfPkg: introduce PublishPeiMemory

[Jordan Justen]

On Tue, Nov 26, 2013 at 11:26 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> MemDetect actully does too many things, the underlying platform might
> want to have more control over memory layout.
>
> Extract the functionality of publishing PEI memory to a dedicated
> function.
>
> Also fixed wrong comment while I was there.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/PlatformPei/MemDetect.c |   36 +++++++++++++++++++++++++++++++++++-
>  OvmfPkg/PlatformPei/Platform.h  |    5 +++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 9f6ca19..dc44745 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -83,11 +83,45 @@ GetSystemMemorySizeAbove4gb (
>    return LShiftU64 (Size, 16);
>  }
>
> +/**
> +  Publish PEI core memory
> +
> +  @return EFI_SUCCESS     The PEIM initialized successfully.
> +
> +**/
> +EFI_STATUS
> +PublishPeiMemory(

Space before open-paren.

> +  VOID
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_PHYSICAL_ADDRESS        MemoryBase;
> +  UINT64                      MemorySize;
> +  UINT64                      LowerMemorySize;
> +
> +  LowerMemorySize = GetSystemMemorySizeBelow4gb ();

I'm going to say
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
for patches 5 & 6, but I am not too happy with them.

For 5, I think maybe it would be nice to not require the details of
'XenLeaf' to leak out of Xen.c. I think XenDetect should return
BOOLEAN, and store XenLeaf in a static global in Xen.c.

For 6, I think it is inconsistent that Xen continues to use CMOS here,
but moves to the E820 tables otherwise. (GetSystemMemorySizeBelow4gb
could have a different path for Xen, or maybe PublishPeiMemory could
have an input parameter of MemoryLimit32Bit and get called by
MemDetect too.)

But, these don't seem critical, so to reduce thrash for you I'll let
you move forward with these patches as-is.

> +  MemoryBase = PcdGet32 (PcdOvmfMemFvBase) + PcdGet32 (PcdOvmfMemFvSize);
> +  MemorySize = LowerMemorySize - MemoryBase;
> +  if (MemorySize > SIZE_64MB) {
> +    MemoryBase = LowerMemorySize - SIZE_64MB;
> +    MemorySize = SIZE_64MB;
> +  }
> +
> +  //
> +  // Publish this memory to the PEI Core
> +  //
> +  Status = PublishSystemMemory(MemoryBase, MemorySize);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
>
>  /**
>    Peform Memory Detection
>
> -  @return EFI_SUCCESS     The PEIM initialized successfully.
> +  @return Top of memory
>
>  **/
>  EFI_PHYSICAL_ADDRESS
> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index d63d124..01af2a9 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -57,6 +57,11 @@ AddUntestedMemoryRangeHob (
>    EFI_PHYSICAL_ADDRESS        MemoryLimit
>    );
>
> +EFI_STATUS
> +PublishPeiMemory(

Space before open-paren.

-Jordan

> +  VOID
> +  );
> +
>  EFI_PHYSICAL_ADDRESS
>  MemDetect (
>    VOID
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [PATCH v3 8/8] OvmfPkg: enable Xen specific path

[Jordan Justen]

On Tue, Nov 26, 2013 at 11:26 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> This patch sets PcdPciDisableBusEnumeration to true then makes use of
> PublishPeiMemory and XenMemMapInitialization to construct memory map for
> Xen guest.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc             |    5 +++--
>  OvmfPkg/OvmfPkgIa32X64.dsc          |    5 +++--
>  OvmfPkg/OvmfPkgX64.dsc              |    5 +++--
>  OvmfPkg/PlatformPei/Platform.c      |   11 +++++++++--
>  OvmfPkg/PlatformPei/PlatformPei.inf |    1 +
>  5 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 760bd41..d4851ed 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -222,7 +222,7 @@
>  !else
>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>  !endif
> -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

What impact does this have on size for DEBUG and RELEASE builds. We
could just override it for the PciBus driver.

>    UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>
>  [LibraryClasses.common.DXE_DRIVER]
> @@ -320,6 +320,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
>
>
>  ################################################################################
> @@ -342,7 +343,7 @@
>    MdeModulePkg/Core/Pei/PeiMain.inf
>    MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
>      <LibraryClasses>
> -      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf

I don't think we need to change this.

-Jordan

>    }
>    IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 268d722..d9072e7 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -227,7 +227,7 @@
>  !else
>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>  !endif
> -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>    UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>
>  [LibraryClasses.common.DXE_DRIVER]
> @@ -326,6 +326,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
>
>
>  ################################################################################
> @@ -348,7 +349,7 @@
>    MdeModulePkg/Core/Pei/PeiMain.inf
>    MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
>      <LibraryClasses>
> -      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>    }
>    IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 53945d0..71b178e 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -227,7 +227,7 @@
>  !else
>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>  !endif
> -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>    UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>
>  [LibraryClasses.common.DXE_DRIVER]
> @@ -325,6 +325,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
>
>
>  ################################################################################
> @@ -347,7 +348,7 @@
>    MdeModulePkg/Core/Pei/PeiMain.inf
>    MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
>      <LibraryClasses>
> -      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>    }
>    IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index c2e7ba9..8daf9f4 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -416,7 +416,11 @@ InitializePlatform (
>
>    XenLeaf = XenDetect ();
>
> -  TopOfMemory = MemDetect ();
> +  if (XenLeaf != 0) {
> +    PublishPeiMemory ();
> +    PcdSetBool (PcdPciDisableBusEnumeration, TRUE);
> +  } else
> +    TopOfMemory = MemDetect ();
>
>    if (XenLeaf != 0) {
>      DEBUG ((EFI_D_INFO, "Xen was detected\n"));
> @@ -427,7 +431,10 @@ InitializePlatform (
>
>    PeiFvInitialization ();
>
> -  MemMapInitialization (TopOfMemory);
> +  if (XenLeaf != 0)
> +    XenMemMapInitialization ();
> +  else
> +    MemMapInitialization (TopOfMemory);
>
>    MiscInitialization ();
>
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 3d5cbbb..7fe9d47 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -65,6 +65,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
>
>  [Ppis]
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 8/8] OvmfPkg: enable Xen specific path

[Wei Liu]

On Thu, Nov 28, 2013 at 08:39:10PM -0800, Jordan Justen wrote:
> On Tue, Nov 26, 2013 at 11:26 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> > This patch sets PcdPciDisableBusEnumeration to true then makes use of
> > PublishPeiMemory and XenMemMapInitialization to construct memory map for
> > Xen guest.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc             |    5 +++--
> >  OvmfPkg/OvmfPkgIa32X64.dsc          |    5 +++--
> >  OvmfPkg/OvmfPkgX64.dsc              |    5 +++--
> >  OvmfPkg/PlatformPei/Platform.c      |   11 +++++++++--
> >  OvmfPkg/PlatformPei/PlatformPei.inf |    1 +
> >  5 files changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 760bd41..d4851ed 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -222,7 +222,7 @@
> >  !else
> >    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> >  !endif
> > -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> 
> What impact does this have on size for DEBUG and RELEASE builds. We
> could just override it for the PciBus driver.
> 

After reverting the above change and only override PciBusDxe, the size
of debug build is still the same. I think there's very high chance that
sizes for release build are the same as well. But I think it is
generally a good idea to only override it for the PciBus driver.

Debug build with this patch:
  MAINFV [69%Full] 8388608 total, 5803152 used, 2585456 free
  SECFV [19%Full] 212992 total, 42584 used, 170408 free
  FVMAIN_COMPACT [53%Full] 1753088 total, 929904 used, 823184 free
  DXEFV [99%Full] 5570560 total, 5560600 used, 9960 free

Override for PciBusDxe, debug build:
  MAINFV [69%Full] 8388608 total, 5803152 used, 2585456 free
  SECFV [19%Full] 212992 total, 42584 used, 170408 free
  FVMAIN_COMPACT [53%Full] 1753088 total, 929904 used, 823184 free
  DXEFV [99%Full] 5570560 total, 5560600 used, 9960 free


> >    UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
> >
> >  [LibraryClasses.common.DXE_DRIVER]
> > @@ -320,6 +320,7 @@
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
> >
> >
> >  ################################################################################
> > @@ -342,7 +343,7 @@
> >    MdeModulePkg/Core/Pei/PeiMain.inf
> >    MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
> >      <LibraryClasses>
> > -      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > +      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> 
> I don't think we need to change this.
> 

Yes, you're right. I will fix this.

Wei.

Re: [edk2] [PATCH v3 6/8] OvmfPkg: introduce PublishPeiMemory

[Wei Liu]

On Thu, Nov 28, 2013 at 08:31:11PM -0800, Jordan Justen wrote:
[...]
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_PHYSICAL_ADDRESS        MemoryBase;
> > +  UINT64                      MemorySize;
> > +  UINT64                      LowerMemorySize;
> > +
> > +  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> 
> I'm going to say
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> for patches 5 & 6, but I am not too happy with them.
> 
> For 5, I think maybe it would be nice to not require the details of
> 'XenLeaf' to leak out of Xen.c. I think XenDetect should return
> BOOLEAN, and store XenLeaf in a static global in Xen.c.
> 
> For 6, I think it is inconsistent that Xen continues to use CMOS here,
> but moves to the E820 tables otherwise. (GetSystemMemorySizeBelow4gb
> could have a different path for Xen, or maybe PublishPeiMemory could
> have an input parameter of MemoryLimit32Bit and get called by
> MemDetect too.)
> 
> But, these don't seem critical, so to reduce thrash for you I'll let
> you move forward with these patches as-is.
> 

Thanks for reviewing.

Wei.