[PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Wei Liu]

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

The default behavior is to allow full PCI enumeration. This is the same
behavior as before this change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
+    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
+  else
+    gFullEnumeration = FALSE;
 
   //
   // 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..626ae99 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.PcdPciAllowFullEnumeration
 
 # [Event]
 #   ##
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index b627eb1..274d2e5 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 allowed.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|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 RFC v2 2/7] OvmfPkg: introduce E820.h

[Wei Liu]

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

Signed-off-by: Wei Liu <wei.liu2@citrix.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 RFC v2 3/7] 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 those bits as well. Currently only E820
map is in use.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/Include/Guid/XenInfo.h |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
index d512b0b..eaeab1a 100644
--- a/OvmfPkg/Include/Guid/XenInfo.h
+++ b/OvmfPkg/Include/Guid/XenInfo.h
@@ -18,6 +18,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define EFI_XEN_INFO_GUID \
     { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
 
+#pragma pack(1)
+typedef struct {
+  CHAR8 Signature[11]; /* XenHVMOVMF\0 */
+  CHAR8 Padding[3];
+  UINT8 Length;     /* Length of this struct */
+  UINT8 Checksum;   /* Set such that the sum over bytes 0..length == 0 */
+  /*
+   * Physical address of an array of tables_nr elements.
+   *
+   * Each element is a 32 bit value contianing the physical address
+   * of a BIOS table.
+   */
+  UINT32 Tables;
+  UINT32 TablesNr;
+  /*
+   * Physical address of the e820 table, contains e820_nr entries.
+   */
+  UINT32 E820;
+  UINT32 E820Nr;
+} EFI_XEN_OVMF_INFO;
+#pragma pack()
+
 typedef struct {
   ///
   /// Beginning of the hypercall page.
@@ -35,6 +57,11 @@ typedef struct {
   /// Hypervisor minor version.
   ///
   UINT16 VersionMinor;
+  ///
+  /// E820 map
+  ///
+  VOID *E820;
+  UINT16 E820EntryCount;
 } EFI_XEN_INFO;
 
 extern EFI_GUID gEfiXenInfoGuid;
-- 
1.7.10.4

[PATCH RFC v2 4/7] OvmfPkg: extract OVMF info passed by Xen hvmloader

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/PlatformPei/Xen.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
index a720b91..fbb2619 100644
--- a/OvmfPkg/PlatformPei/Xen.c
+++ b/OvmfPkg/PlatformPei/Xen.c
@@ -26,12 +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"
 
 EFI_XEN_INFO mXenInfo;
 
+#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
 
 /**
   Connects to the Hypervisor.
@@ -50,6 +53,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 +78,31 @@ XenConnect (
   /* TBD: Locate hvm_info and reserve it away. */
   mXenInfo.HvmInfo = NULL;
 
+  if (!AsciiStrCmp ((CHAR8 *) Info->Signature, "XenHVMOVMF")) {
+    EFI_E820_ENTRY *E820Map;
+    UINTN Loop, EntryCount, Base;
+
+    /* E820 map */
+    EntryCount = Info->E820Nr;
+    Base = Info->E820;
+
+    E820Map = AllocateZeroPool (sizeof(EFI_E820_ENTRY) * EntryCount);
+
+    if (!E820Map) {
+      FreePages (mXenInfo.HyperPages, TransferPages);
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    for (Loop = 0; Loop < EntryCount; Loop++) {
+      EFI_E820_ENTRY *src = (EFI_E820_ENTRY *)Base + Loop;
+      EFI_E820_ENTRY *dst = (EFI_E820_ENTRY *)E820Map + Loop;
+      CopyMem (dst, src, sizeof(EFI_E820_ENTRY));
+    }
+
+    mXenInfo.E820 = E820Map;
+    mXenInfo.E820EntryCount = EntryCount;
+  }
+
   BuildGuidDataHob (
     &gEfiXenInfoGuid,
     &mXenInfo,
-- 
1.7.10.4

[PATCH RFC v2 5/7] OvmfPkg: detect Xen earlier

[Wei Liu]

This is useful for initializing memory map.

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 fbb2619..aaa8c17 100644
--- a/OvmfPkg/PlatformPei/Xen.c
+++ b/OvmfPkg/PlatformPei/Xen.c
@@ -150,19 +150,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 RFC v2 6/7] OvmfPkg: introduce PublishPeiMemory

[Wei Liu]

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

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

Also fix wrong comment while I was there.

--
I know there's code duplication, but are you happy with this approach?
Probabaly more refactoring to the existing code is needed? I would like
to know this.

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 RFC v2 7/7] OvmfPkg: introduce XenMemMapInitialization

[Wei Liu]

This function parses Xen OVMF info and arrange memory maps accordingly.
It also sets PcdPciAllowFullEnumeration to false to prevent OVMF from
playing with PCI devices.

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      |   81 ++++++++++++++++++++++++++++++++++-
 OvmfPkg/PlatformPei/PlatformPei.inf |    1 +
 5 files changed, 89 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 760bd41..4b465fe 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.PcdPciAllowFullEnumeration|TRUE
 
 
 ################################################################################
@@ -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..d26145d 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.PcdPciAllowFullEnumeration|TRUE
 
 
 ################################################################################
@@ -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..b2792aa 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.PcdPciAllowFullEnumeration|TRUE
 
 
 ################################################################################
@@ -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 9b7828f..f9ffc25 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->E820EntryCount > 0) {
+    EFI_E820_ENTRY64 *E820Map, *Entry;
+    UINT16 Loop;
+
+    E820Map = Info->E820;
+    for (Loop = 0; Loop < Info->E820EntryCount; 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 (
@@ -346,7 +416,11 @@ InitializePlatform (
 
   XenLeaf = XenDetect ();
 
-  TopOfMemory = MemDetect ();
+  if (XenLeaf != 0) {
+    PublishPeiMemory ();
+    PcdSetBool (PcdPciAllowFullEnumeration, FALSE);
+  } else
+    TopOfMemory = MemDetect ();
 
   if (XenLeaf != 0) {
     DEBUG ((EFI_D_INFO, "Xen was detected\n"));
@@ -357,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..221afb2 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -65,6 +65,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
 
 [Ppis]
-- 
1.7.10.4

Re: [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Konrad Rzeszutek Wilk]

On Tue, Nov 19, 2013 at 08:38:45PM +0000, Wei Liu 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 those bits as well. Currently only E820
> map is in use.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/Include/Guid/XenInfo.h |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
> index d512b0b..eaeab1a 100644
> --- a/OvmfPkg/Include/Guid/XenInfo.h
> +++ b/OvmfPkg/Include/Guid/XenInfo.h
> @@ -18,6 +18,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define EFI_XEN_INFO_GUID \
>      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
>  
> +#pragma pack(1)
> +typedef struct {
> +  CHAR8 Signature[11]; /* XenHVMOVMF\0 */
> +  CHAR8 Padding[3];

Why the padding?
> +  UINT8 Length;     /* Length of this struct */
> +  UINT8 Checksum;   /* Set such that the sum over bytes 0..length == 0 */
> +  /*
> +   * Physical address of an array of tables_nr elements.
> +   *
> +   * Each element is a 32 bit value contianing the physical address
                                       ^^^^^^^^^^

> +   * of a BIOS table.
> +   */
> +  UINT32 Tables;
> +  UINT32 TablesNr;
> +  /*
> +   * Physical address of the e820 table, contains e820_nr entries.
> +   */
> +  UINT32 E820;
> +  UINT32 E820Nr;
> +} EFI_XEN_OVMF_INFO;
> +#pragma pack()
> +
>  typedef struct {
>    ///
>    /// Beginning of the hypercall page.
> @@ -35,6 +57,11 @@ typedef struct {
>    /// Hypervisor minor version.
>    ///
>    UINT16 VersionMinor;
> +  ///
> +  /// E820 map
> +  ///
> +  VOID *E820;
> +  UINT16 E820EntryCount;
>  } EFI_XEN_INFO;
>  
>  extern EFI_GUID gEfiXenInfoGuid;
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [edk2] [PATCH RFC v2 0/7] Make OVMF fully working with Xen

[Jordan Justen]

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Hi all
>
> Manage to pull together another prototype without any hack in it.

>From a quick glance, it looks like the right direction.

I'll review and test further.

One note for v3 is you'll need to review *Pkg/Contributions.txt and
add Contributed-under. (But, I would suggest waiting for a code
review/testing before sending out v3.)

Thanks!

-Jordan

> The first patch adds PcdPciAllowFullEnumeration in MdeModulePkg, which
> short-cuts full enumeration if set to false. This one should be helpful to QEMU
> as well.
>
> This patch set should work with QEMU / KVM as well (read: doesn't break). The
> code path is the same as before if Xen is not detected.
>
> Comments are welcomed.
>
> The tree can be found at:
>    http://xenbits.xen.org/git-http/people/liuw/ovmf.git rfc-v2
> starting from 4ba5c67e.
>
> Wei.
>
> Wei Liu (7):
>   MdeModulePkg: introduce PcdPciAllowFullEnumeration
>   OvmfPkg: introduce E820.h
>   OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo
>   OvmfPkg: extract OVMF info passed by Xen hvmloader
>   OvmfPkg: detect Xen earlier
>   OvmfPkg: introduce PublishPeiMemory
>   OvmfPkg: introduce XenMemMapInitialization
>
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c      |    5 +-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |    1 +
>  MdeModulePkg/MdeModulePkg.dec                |    3 +
>  OvmfPkg/Include/Guid/XenInfo.h               |   27 ++++++++
>  OvmfPkg/Include/IndustryStandard/E820.h      |   46 +++++++++++++
>  OvmfPkg/OvmfPkgIa32.dsc                      |    5 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                   |    5 +-
>  OvmfPkg/OvmfPkgX64.dsc                       |    5 +-
>  OvmfPkg/PlatformPei/MemDetect.c              |   36 ++++++++++-
>  OvmfPkg/PlatformPei/Platform.c               |   89 +++++++++++++++++++++++++-
>  OvmfPkg/PlatformPei/Platform.h               |   10 +++
>  OvmfPkg/PlatformPei/PlatformPei.inf          |    1 +
>  OvmfPkg/PlatformPei/Xen.c                    |   43 +++++++++----
>  13 files changed, 254 insertions(+), 22 deletions(-)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/E820.h
>
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Jordan Justen]

Feng, what do you think of this change to MdeModulePkg?

Wei, How about PcdPciDisableBusEnumeration instead?

-Jordan

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Platforms such as Xen already enumerates PCI bridges and devices. Use
> this PCD to control EDK2 behavior.
>
> The default behavior is to allow full PCI enumeration. This is the same
> behavior as before this change.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
> +  else
> +    gFullEnumeration = FALSE;
>
>    //
>    // 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..626ae99 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.PcdPciAllowFullEnumeration
>
>  # [Event]
>  #   ##
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index b627eb1..274d2e5 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 allowed.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|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
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 2/7] OvmfPkg: introduce E820.h

[Jordan Justen]

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> E820 definitions copied from IntelFrameworkModulePkg/Csm/
> LegacyBiosDxe/LegacyBiosInterface.h.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Review OvmfPkg/Contributions.txt and add Contributed-under. Then,
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
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Tian, Feng]

It's in internal review process. Module Owner would have response on this.

-----Original Message-----
From: Jordan Justen [mailto:jljusten@gmail.com] 
Sent: Monday, November 25, 2013 06:05
To: edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu
Cc: xen-devel
Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

Feng, what do you think of this change to MdeModulePkg?

Wei, How about PcdPciDisableBusEnumeration instead?

-Jordan

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Platforms such as Xen already enumerates PCI bridges and devices. Use 
> this PCD to control EDK2 behavior.
>
> The default behavior is to allow full PCI enumeration. This is the 
> same behavior as before this change.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle 
> + (Controller) ? FALSE : TRUE));  else
> +    gFullEnumeration = FALSE;
>
>    //
>    // 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..626ae99 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.PcdPciAllowFullEnumeration
>
>  # [Event]
>  #   ##
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index b627eb1..274d2e5 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|0x1000004
> 6
>
> +  ## This PCD specifies whether full PCI enumeration is allowed.
> +  
> + gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|BOOLE
> + AN|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
>
>
> ----------------------------------------------------------------------
> -------- Shape the Mobile Experience: Free Subscription Software 
> experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and 
> game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.c
> lktrk _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Jordan Justen]

On Tue, Nov 19, 2013 at 12:38 PM, 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 those bits as well. Currently only E820
> map is in use.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/Include/Guid/XenInfo.h |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
> index d512b0b..eaeab1a 100644
> --- a/OvmfPkg/Include/Guid/XenInfo.h
> +++ b/OvmfPkg/Include/Guid/XenInfo.h
> @@ -18,6 +18,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define EFI_XEN_INFO_GUID \
>      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
>
> +#pragma pack(1)
> +typedef struct {
> +  CHAR8 Signature[11]; /* XenHVMOVMF\0 */
> +  CHAR8 Padding[3];

Does this follow a convention used by Xen? Normally EDK II would use
either a 4 or 8 byte integer signature with SIGNATURE_32/64.

This information does not seem all that OVMF specific, but I guess
from Xen's perspective it is?

> +  UINT8 Length;     /* Length of this struct */
> +  UINT8 Checksum;   /* Set such that the sum over bytes 0..length == 0 */
> +  /*
> +   * Physical address of an array of tables_nr elements.

tables_nr?

> +   *
> +   * Each element is a 32 bit value contianing the physical address
> +   * of a BIOS table.
> +   */
> +  UINT32 Tables;

64-bit EFI_PHYSICAL_ADDRESS type?

> +  UINT32 TablesNr;

Could this be TableCount?

Is "Tables" unused in this patch series? The purpose doesn't seem
clear. (Perhaps an updated comment or commit message could help?)

> +  /*
> +   * Physical address of the e820 table, contains e820_nr entries.

e820_nr?

> +   */
> +  UINT32 E820;

64-bit EFI_PHYSICAL_ADDRESS type?

> +  UINT32 E820Nr;

Maybe E820EntriesCount instead?

> +} EFI_XEN_OVMF_INFO;
> +#pragma pack()

Since this struct defines a Xen <=> OVMF data transfer, maybe we
should consider a new include file? Or, maybe just a good comment on
the structure?

> +
>  typedef struct {
>    ///
>    /// Beginning of the hypercall page.
> @@ -35,6 +57,11 @@ typedef struct {
>    /// Hypervisor minor version.
>    ///
>    UINT16 VersionMinor;
> +  ///
> +  /// E820 map
> +  ///
> +  VOID *E820;
> +  UINT16 E820EntryCount;

I think we (previously) made a mistake with this structure. It is a
HOB, and therefore produced by PEI. HOBs can be consumed by DXE code.
This means that the VOID* members will be different between PEI and
DXE when the Ia32X64 build is used. Right now, I think only PEI looks
at the HOB. This struct is internal to OVMF, so less of a concern than
the Xen <=> OVMF interface.

-Jordan

>  } EFI_XEN_INFO;
>
>  extern EFI_GUID gEfiXenInfoGuid;
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Kinney, Michael D]

Jordan,

There is an alternate module already available.

	DuetPkg/PciBusNoEnumeration

The concept was to have a different implementation that was smaller and just produced PCI I/O handles for PCI devices that are already enumerated.

Thanks,

Mike

-----Original Message-----
From: Jordan Justen [mailto:jljusten@gmail.com] 
Sent: Sunday, November 24, 2013 2:05 PM
To: edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu
Cc: xen-devel
Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

Feng, what do you think of this change to MdeModulePkg?

Wei, How about PcdPciDisableBusEnumeration instead?

-Jordan

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Platforms such as Xen already enumerates PCI bridges and devices. Use
> this PCD to control EDK2 behavior.
>
> The default behavior is to allow full PCI enumeration. This is the same
> behavior as before this change.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
> +  else
> +    gFullEnumeration = FALSE;
>
>    //
>    // 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..626ae99 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.PcdPciAllowFullEnumeration
>
>  # [Event]
>  #   ##
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index b627eb1..274d2e5 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 allowed.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|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
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 4/7] OvmfPkg: extract OVMF info passed by Xen hvmloader

[Jordan Justen]

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  OvmfPkg/PlatformPei/Xen.c |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
> index a720b91..fbb2619 100644
> --- a/OvmfPkg/PlatformPei/Xen.c
> +++ b/OvmfPkg/PlatformPei/Xen.c
> @@ -26,12 +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"
>
>  EFI_XEN_INFO mXenInfo;
>
> +#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
>
>  /**
>    Connects to the Hypervisor.
> @@ -50,6 +53,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 +78,31 @@ XenConnect (
>    /* TBD: Locate hvm_info and reserve it away. */
>    mXenInfo.HvmInfo = NULL;
>
> +  if (!AsciiStrCmp ((CHAR8 *) Info->Signature, "XenHVMOVMF")) {

Would AsciiStrnCmp be a good idea here? Like I mentioned in the other
patch, we normally use an integer based signature.

> +    EFI_E820_ENTRY *E820Map;
> +    UINTN Loop, EntryCount, Base;
> +
> +    /* E820 map */
> +    EntryCount = Info->E820Nr;
> +    Base = Info->E820;
> +
> +    E820Map = AllocateZeroPool (sizeof(EFI_E820_ENTRY) * EntryCount);
> +
> +    if (!E820Map) {
> +      FreePages (mXenInfo.HyperPages, TransferPages);
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    for (Loop = 0; Loop < EntryCount; Loop++) {
> +      EFI_E820_ENTRY *src = (EFI_E820_ENTRY *)Base + Loop;
> +      EFI_E820_ENTRY *dst = (EFI_E820_ENTRY *)E820Map + Loop;
> +      CopyMem (dst, src, sizeof(EFI_E820_ENTRY));
> +    }

How about AllocateCopyPool and just copy the entire array in one shot?

-Jordan

> +
> +    mXenInfo.E820 = E820Map;
> +    mXenInfo.E820EntryCount = EntryCount;
> +  }
> +
>    BuildGuidDataHob (
>      &gEfiXenInfoGuid,
>      &mXenInfo,
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Jordan Justen]

On Sun, Nov 24, 2013 at 5:47 PM, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Jordan,
>
> There is an alternate module already available.
>
>         DuetPkg/PciBusNoEnumeration
>
> The concept was to have a different implementation that was smaller and just produced PCI I/O handles for PCI devices that are already enumerated.

This was already discussed (and tried). (See Xen emails from Wei.)

I'd really prefer to continue to use the MdeModulePkg version for
OVMF, and avoid extra Xen specific build flags which we haven't needed
previously.

The PCD seems very unobtrusive for the driver. (That surprised me.) It
should not even cause different code to be generated when a fixed-PCD
is used.

Would a setting of gFullEnumeration = FALSE be enough to allow DuetPkg
to use the MdeModulePkg module as well? Wei's work seems to give some
signs that the answer would be yes. Wouldn't we like to avoid code
duplication for such a complex module?

-Jordan

> -----Original Message-----
> From: Jordan Justen [mailto:jljusten@gmail.com]
> Sent: Sunday, November 24, 2013 2:05 PM
> To: edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu
> Cc: xen-devel
> Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration
>
> Feng, what do you think of this change to MdeModulePkg?
>
> Wei, How about PcdPciDisableBusEnumeration instead?
>
> -Jordan
>
> On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> Platforms such as Xen already enumerates PCI bridges and devices. Use
>> this PCD to control EDK2 behavior.
>>
>> The default behavior is to allow full PCI enumeration. This is the same
>> behavior as before this change.
>>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
>> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
>> +  else
>> +    gFullEnumeration = FALSE;
>>
>>    //
>>    // 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..626ae99 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.PcdPciAllowFullEnumeration
>>
>>  # [Event]
>>  #   ##
>> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
>> index b627eb1..274d2e5 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 allowed.
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|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
>>
>>
>> ------------------------------------------------------------------------------
>> Shape the Mobile Experience: Free Subscription
>> Software experts and developers: Be at the forefront of tech innovation.
>> Intel(R) Software Adrenaline delivers strategic insight and game-changing
>> conversations that shape the rapidly evolving mobile landscape. Sign up now.
>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Kinney, Michael D]

Jordan,

I agree that removing code duplication is a good idea.

I believe we can make the one in the MdeModulePkg functional everywhere.  Size will be the only remaining difference.  This can be addressed longer term.  If the proposed PCD is configured as FixedAtBuild in a DSC file, we should be able to get the optimized code generation for the PciBusDxe to remove all the code/data that is only used for PCI Enumeration.  Then the PciBusNoEnumeration could be retired.

I have no issues with adding a PCD to make the PciBusDxe in the MdeModulePkg skip enumeration all together.

Mike

-----Original Message-----
From: Jordan Justen [mailto:jljusten@gmail.com] 
Sent: Sunday, November 24, 2013 6:27 PM
To: edk2-devel@lists.sourceforge.net
Cc: Tian, Feng; Wei Liu; Kinney, Michael D; xen-devel
Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

On Sun, Nov 24, 2013 at 5:47 PM, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Jordan,
>
> There is an alternate module already available.
>
>         DuetPkg/PciBusNoEnumeration
>
> The concept was to have a different implementation that was smaller and just produced PCI I/O handles for PCI devices that are already enumerated.

This was already discussed (and tried). (See Xen emails from Wei.)

I'd really prefer to continue to use the MdeModulePkg version for
OVMF, and avoid extra Xen specific build flags which we haven't needed
previously.

The PCD seems very unobtrusive for the driver. (That surprised me.) It
should not even cause different code to be generated when a fixed-PCD
is used.

Would a setting of gFullEnumeration = FALSE be enough to allow DuetPkg
to use the MdeModulePkg module as well? Wei's work seems to give some
signs that the answer would be yes. Wouldn't we like to avoid code
duplication for such a complex module?

-Jordan

> -----Original Message-----
> From: Jordan Justen [mailto:jljusten@gmail.com]
> Sent: Sunday, November 24, 2013 2:05 PM
> To: edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu
> Cc: xen-devel
> Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration
>
> Feng, what do you think of this change to MdeModulePkg?
>
> Wei, How about PcdPciDisableBusEnumeration instead?
>
> -Jordan
>
> On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> Platforms such as Xen already enumerates PCI bridges and devices. Use
>> this PCD to control EDK2 behavior.
>>
>> The default behavior is to allow full PCI enumeration. This is the same
>> behavior as before this change.
>>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.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..49c204c 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 (PcdPciAllowFullEnumeration))
>> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? FALSE : TRUE));
>> +  else
>> +    gFullEnumeration = FALSE;
>>
>>    //
>>    // 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..626ae99 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.PcdPciAllowFullEnumeration
>>
>>  # [Event]
>>  #   ##
>> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
>> index b627eb1..274d2e5 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 allowed.
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|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
>>
>>
>> ------------------------------------------------------------------------------
>> Shape the Mobile Experience: Free Subscription
>> Software experts and developers: Be at the forefront of tech innovation.
>> Intel(R) Software Adrenaline delivers strategic insight and game-changing
>> conversations that shape the rapidly evolving mobile landscape. Sign up now.
>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Ian Campbell]

On Sun, 2013-11-24 at 17:46 -0800, Jordan Justen wrote:

> > +#pragma pack(1)
> > +typedef struct {
> > +  CHAR8 Signature[11]; /* XenHVMOVMF\0 */
> > +  CHAR8 Padding[3];
> 
> Does this follow a convention used by Xen? Normally EDK II would use
> either a 4 or 8 byte integer signature with SIGNATURE_32/64.
> 
> This information does not seem all that OVMF specific, but I guess
> from Xen's perspective it is?

I think it's actually pretty similar to what we currently pass to
SeaBIOS but I don't think we would want to force ourselves to co-evolve
all three of those together, we can still take advantage of some common
code on the Xen side without tying the actual interfaces together.

You can see the SeaBIOS side in Xen at
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/firmware/hvmloader/seabios.c;h=dd7dfbe0e8a571e2239a1753d38fa0d1b8f0b5c7;hb=HEAD#l37

> Since this struct defines a Xen <=> OVMF data transfer, maybe we
> should consider a new include file? Or, maybe just a good comment on
> the structure?

FWIW It Xen=>OVMF only, not bidirectional.

Ian,

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration ... [and one more message]

[Wei Liu]

On Mon, Nov 25, 2013 at 03:12:04AM +0000, Kinney, Michael D wrote:
> Jordan,
> 
> I agree that removing code duplication is a good idea.
> 
> I believe we can make the one in the MdeModulePkg functional
> everywhere.  Size will be the only remaining difference.  This can be
> addressed longer term.  If the proposed PCD is configured as
> FixedAtBuild in a DSC file, we should be able to get the optimized
> code generation for the PciBusDxe to remove all the code/data that is
> only used for PCI Enumeration.  Then the PciBusNoEnumeration could be
> retired.
> 

Forgive my ignorance -- is it possible to change FixedAtBuild PCD during
runtime? Jordan had the idea to disable / enable PCI enumeration during
runtime, so that we can have one single binary for all OVMF users.

> I have no issues with adding a PCD to make the PciBusDxe in the
> MdeModulePkg skip enumeration all together.
> 

Cool, thank you for confirming this.

Wei.

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

[Wei Liu]

On Sun, Nov 24, 2013 at 02:05:01PM -0800, Jordan Justen wrote:
> Feng, what do you think of this change to MdeModulePkg?
> 
> Wei, How about PcdPciDisableBusEnumeration instead?
> 

I have no objection to the new name.

Wei.

> -Jordan

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Wei Liu]

On Sun, Nov 24, 2013 at 05:46:42PM -0800, Jordan Justen wrote:
> On Tue, Nov 19, 2013 at 12:38 PM, 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 those bits as well. Currently only E820
> > map is in use.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  OvmfPkg/Include/Guid/XenInfo.h |   27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
> > index d512b0b..eaeab1a 100644
> > --- a/OvmfPkg/Include/Guid/XenInfo.h
> > +++ b/OvmfPkg/Include/Guid/XenInfo.h
> > @@ -18,6 +18,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #define EFI_XEN_INFO_GUID \
> >      { 0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d } }
> >
> > +#pragma pack(1)
> > +typedef struct {
> > +  CHAR8 Signature[11]; /* XenHVMOVMF\0 */
> > +  CHAR8 Padding[3];
> 
> Does this follow a convention used by Xen? Normally EDK II would use
> either a 4 or 8 byte integer signature with SIGNATURE_32/64.
> 
> This information does not seem all that OVMF specific, but I guess
> from Xen's perspective it is?
> 
> > +  UINT8 Length;     /* Length of this struct */
> > +  UINT8 Checksum;   /* Set such that the sum over bytes 0..length == 0 */
> > +  /*
> > +   * Physical address of an array of tables_nr elements.
> 
> tables_nr?
> 

I modified the actual members of the structure but I forgot to modify
comments. Sorry about this. I will fix them.

> > +   *
> > +   * Each element is a 32 bit value contianing the physical address
> > +   * of a BIOS table.
> > +   */
> > +  UINT32 Tables;
> 
> 64-bit EFI_PHYSICAL_ADDRESS type?
> 

I don't think I can change this. As this structure needs to be
identical to the one on Xen side. Hvmloader runs in 32 bit mode so the
address allocated should be UINT32.

> > +  UINT32 TablesNr;
> 
> Could this be TableCount?
> 
> Is "Tables" unused in this patch series? The purpose doesn't seem
> clear. (Perhaps an updated comment or commit message could help?)
> 

No, not yet. We need to reserve places for future usage though.
Otherwise we need to break this interface when we find out we need to
pass more information.

> > +  /*
> > +   * Physical address of the e820 table, contains e820_nr entries.
> 
> e820_nr?
> 
> > +   */
> > +  UINT32 E820;
> 
> 64-bit EFI_PHYSICAL_ADDRESS type?
> 
> > +  UINT32 E820Nr;
> 
> Maybe E820EntriesCount instead?
> 
> > +} EFI_XEN_OVMF_INFO;
> > +#pragma pack()
> 
> Since this struct defines a Xen <=> OVMF data transfer, maybe we
> should consider a new include file? Or, maybe just a good comment on
> the structure?
> 

As this is one time and one way information transfer so that I don't
think we need dedicated header. I will add comments.

> > +
> >  typedef struct {
> >    ///
> >    /// Beginning of the hypercall page.
> > @@ -35,6 +57,11 @@ typedef struct {
> >    /// Hypervisor minor version.
> >    ///
> >    UINT16 VersionMinor;
> > +  ///
> > +  /// E820 map
> > +  ///
> > +  VOID *E820;
> > +  UINT16 E820EntryCount;
> 
> I think we (previously) made a mistake with this structure. It is a
> HOB, and therefore produced by PEI. HOBs can be consumed by DXE code.
> This means that the VOID* members will be different between PEI and
> DXE when the Ia32X64 build is used. Right now, I think only PEI looks
> at the HOB. This struct is internal to OVMF, so less of a concern than
> the Xen <=> OVMF interface.
> 

Oh, then this one should be EFI_PHYSICAL_ADDRESS. That should fix the
problem, right?

Wei.

Re: [edk2] [PATCH RFC v2 4/7] OvmfPkg: extract OVMF info passed by Xen hvmloader

[Wei Liu]

On Sun, Nov 24, 2013 at 06:01:03PM -0800, Jordan Justen wrote:
[...]
> >    mXenInfo.HyperPages = AllocatePages (TransferPages);
> > @@ -72,6 +78,31 @@ XenConnect (
> >    /* TBD: Locate hvm_info and reserve it away. */
> >    mXenInfo.HvmInfo = NULL;
> >
> > +  if (!AsciiStrCmp ((CHAR8 *) Info->Signature, "XenHVMOVMF")) {
> 
> Would AsciiStrnCmp be a good idea here? Like I mentioned in the other
> patch, we normally use an integer based signature.
> 

I think it is sufficient. We would like to keep the use that structure
to pass information. That structure is Xen => OVMF only  and consumed
internally by OVMF.

> > +    EFI_E820_ENTRY *E820Map;
> > +    UINTN Loop, EntryCount, Base;
> > +
> > +    /* E820 map */
> > +    EntryCount = Info->E820Nr;
> > +    Base = Info->E820;
> > +
> > +    E820Map = AllocateZeroPool (sizeof(EFI_E820_ENTRY) * EntryCount);
> > +
> > +    if (!E820Map) {
> > +      FreePages (mXenInfo.HyperPages, TransferPages);
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    for (Loop = 0; Loop < EntryCount; Loop++) {
> > +      EFI_E820_ENTRY *src = (EFI_E820_ENTRY *)Base + Loop;
> > +      EFI_E820_ENTRY *dst = (EFI_E820_ENTRY *)E820Map + Loop;
> > +      CopyMem (dst, src, sizeof(EFI_E820_ENTRY));
> > +    }
> 
> How about AllocateCopyPool and just copy the entire array in one shot?
> 

Sure, that seems much simpler. Thanks.

Wei.

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Jordan Justen]

On Mon, Nov 25, 2013 at 3:41 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Sun, Nov 24, 2013 at 05:46:42PM -0800, Jordan Justen wrote:
>> On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > +   *
>> > +   * Each element is a 32 bit value contianing the physical address
>> > +   * of a BIOS table.
>> > +   */
>> > +  UINT32 Tables;
>>
>> 64-bit EFI_PHYSICAL_ADDRESS type?
>>
>
> I don't think I can change this. As this structure needs to be
> identical to the one on Xen side. Hvmloader runs in 32 bit mode so the
> address allocated should be UINT32.

This sounds like an implementation detail on the hvmloader side. It
doesn't seem like a good idea to bake that into the data structure.

>> > +  UINT32 TablesNr;
>>
>> Could this be TableCount?
>>
>> Is "Tables" unused in this patch series? The purpose doesn't seem
>> clear. (Perhaps an updated comment or commit message could help?)
>>
>
> No, not yet. We need to reserve places for future usage though.
> Otherwise we need to break this interface when we find out we need to
> pass more information.

What about when the next blob type needs to be transferred? How about
something that E820 can fit into, and yet allow for more new blobs to
be passed?

How about something like:
#define XEN_HVM_INFO_SIGNATURE SIGNATURE_32 ('X', 'E', 'N', 'H')
#define XEN_HVM_INFO_ADDRESS BASE_4KB
#define XEN_HVM_INFO_VERSION 0

#define XEN_HVM_DATA_TYPE_E820 SIGNATURE_32 ('E', '8', '2', '0')

#pragma pack(1)
typedef struct {
  UINT32 Type;
  UINT32 Reserved;
  EFI_PHYSICAL_ADDRESS Address;
  UINT64 Length;
} XEN_HVM_INFO_ITEM;

typedef struct {
  UINT32 Signature;
  UINT8 Length;
  UINT8 Checksum;
  UINT8 Version;
  UINT8 Reserved;
  EFI_PHYSICAL_ADDRESS InfoItems;
  UINT32 InfoItemCount;
} XEN_HVM_INFO;
#pragma pack()

(The Reserved fields are for 64-bit alignment.)

You could also consider an E820 alternative:
#define XEN_HVM_DATA_TYPE_RAM_RANGE SIGNATURE_32 ('R', 'A', 'M', 'R')

>> > +} EFI_XEN_OVMF_INFO;
>> > +#pragma pack()
>>
>> Since this struct defines a Xen <=> OVMF data transfer, maybe we
>> should consider a new include file? Or, maybe just a good comment on
>> the structure?
>>
>
> As this is one time and one way information transfer so that I don't
> think we need dedicated header. I will add comments.

That seems a good reason to put it into a separate header. (Perhaps a
header only present within the one consuming driver.)

-Jordan

Re: [edk2] [PATCH RFC v2 7/7] OvmfPkg: introduce XenMemMapInitialization

[Jordan Justen]

Regarding patches 5-7, it seems like the mem-map code flow could be
more shared. It is a bit challenging to unravel things though.

I guess the only specific thing I can really point out is that
PcdPciAllowFullEnumeration should be initialized in a different patch,
and not within the mem-map init path.

-Jordan

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> This function parses Xen OVMF info and arrange memory maps accordingly.
> It also sets PcdPciAllowFullEnumeration to false to prevent OVMF from
> playing with PCI devices.
>
> 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      |   81 ++++++++++++++++++++++++++++++++++-
>  OvmfPkg/PlatformPei/PlatformPei.inf |    1 +
>  5 files changed, 89 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 760bd41..4b465fe 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.PcdPciAllowFullEnumeration|TRUE
>
>
>  ################################################################################
> @@ -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..d26145d 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.PcdPciAllowFullEnumeration|TRUE
>
>
>  ################################################################################
> @@ -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..b2792aa 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.PcdPciAllowFullEnumeration|TRUE
>
>
>  ################################################################################
> @@ -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 9b7828f..f9ffc25 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->E820EntryCount > 0) {
> +    EFI_E820_ENTRY64 *E820Map, *Entry;
> +    UINT16 Loop;
> +
> +    E820Map = Info->E820;
> +    for (Loop = 0; Loop < Info->E820EntryCount; 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 (
> @@ -346,7 +416,11 @@ InitializePlatform (
>
>    XenLeaf = XenDetect ();
>
> -  TopOfMemory = MemDetect ();
> +  if (XenLeaf != 0) {
> +    PublishPeiMemory ();
> +    PcdSetBool (PcdPciAllowFullEnumeration, FALSE);
> +  } else
> +    TopOfMemory = MemDetect ();
>
>    if (XenLeaf != 0) {
>      DEBUG ((EFI_D_INFO, "Xen was detected\n"));
> @@ -357,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..221afb2 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -65,6 +65,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
>
>  [Ppis]
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration ... [and one more message]

[Kinney, Michael D]

Wei,

The PCD declaration in the DEC file declares the PCD type the PCD is allowed to have.  For this specific PCD, you may want to allow it to be FixedAtBuild or PatahcbleInModule or Dynamic or DynamicEx.  This provides the maximum flexibility.

For future DUET use cases, the DSC file could set it to FixedAtBuild, so the full enumeration code could be optimized away.

For OVMF use cases where you want to be able to enable/disable enumeration, you can set it to DynamicEx in the DSC file.  PciBusDxe built with this config would include support for full and no enumeration.  If the DynamicEx subtype is set to HII, then a UEFI Variable setting can enable/disable enumeration and could be set in FD image externally.  Likewise, if the DynamicEx subtype is set to VPD (Vital Product Data), then the VPD area could be patched in the FD image externally.

Best regards,

Mike

-----Original Message-----
From: Wei Liu [mailto:wei.liu2@citrix.com] 
Sent: Monday, November 25, 2013 2:56 AM
To: Kinney, Michael D
Cc: Jordan Justen; edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu; xen-devel
Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration ... [and one more message]

On Mon, Nov 25, 2013 at 03:12:04AM +0000, Kinney, Michael D wrote:
> Jordan,
> 
> I agree that removing code duplication is a good idea.
> 
> I believe we can make the one in the MdeModulePkg functional
> everywhere.  Size will be the only remaining difference.  This can be
> addressed longer term.  If the proposed PCD is configured as
> FixedAtBuild in a DSC file, we should be able to get the optimized
> code generation for the PciBusDxe to remove all the code/data that is
> only used for PCI Enumeration.  Then the PciBusNoEnumeration could be
> retired.
> 

Forgive my ignorance -- is it possible to change FixedAtBuild PCD during
runtime? Jordan had the idea to disable / enable PCI enumeration during
runtime, so that we can have one single binary for all OVMF users.

> I have no issues with adding a PCD to make the PciBusDxe in the
> MdeModulePkg skip enumeration all together.
> 

Cool, thank you for confirming this.

Wei.

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Ian Campbell]

On Mon, 2013-11-25 at 12:00 -0800, Jordan Justen wrote:
> >> > +  UINT32 TablesNr;
> >>
> >> Could this be TableCount?
> >>
> >> Is "Tables" unused in this patch series? The purpose doesn't seem
> >> clear. (Perhaps an updated comment or commit message could help?)
> >>
> >
> > No, not yet. We need to reserve places for future usage though.
> > Otherwise we need to break this interface when we find out we need to
> > pass more information.

FWIW in the SeaBIOS world we pass a number of self describing BIOS
tables here. (specifically RSDP, MP table, PIR and the SMBIOS EP).

> What about when the next blob type needs to be transferred?

When I put the SeaBIOS version together my intention was that the length
field would serve as a proxy for the struct version should we ever need
to extend it.

>  How about
> something that E820 can fit into, and yet allow for more new blobs to
> be passed?
> 
> How about something like:
> #define XEN_HVM_INFO_SIGNATURE SIGNATURE_32 ('X', 'E', 'N', 'H')
> #define XEN_HVM_INFO_ADDRESS BASE_4KB
> #define XEN_HVM_INFO_VERSION 0
> 
> #define XEN_HVM_DATA_TYPE_E820 SIGNATURE_32 ('E', '8', '2', '0')
[...]

>From the Xen side unless there is some strong reason to deviate I would
prefer to stick with something which is similar to the existing
interface to other BIOSes, which is what Wei has proposed, rather than
reinvent it in a per-BIOS way. Ultimately this stuff won't be exposed to
UEFI outside of the Xen support code.

Ian.

Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration ... [and one more message]

[Wei Liu]

On Mon, Nov 25, 2013 at 11:27:14PM +0000, Kinney, Michael D wrote:
> Wei,
> 
> The PCD declaration in the DEC file declares the PCD type the PCD is
> allowed to have.  For this specific PCD, you may want to allow it to
> be FixedAtBuild or PatahcbleInModule or Dynamic or DynamicEx.  This
> provides the maximum flexibility.
> 
> For future DUET use cases, the DSC file could set it to FixedAtBuild,
> so the full enumeration code could be optimized away.
> 
> For OVMF use cases where you want to be able to enable/disable
> enumeration, you can set it to DynamicEx in the DSC file.  PciBusDxe
> built with this config would include support for full and no
> enumeration.  If the DynamicEx subtype is set to HII, then a UEFI
> Variable setting can enable/disable enumeration and could be set in FD
> image externally.  Likewise, if the DynamicEx subtype is set to VPD
> (Vital Product Data), then the VPD area could be patched in the FD
> image externally.
> 
> Best regards,
> 

Thanks for the explanation.

Currently in this patch this PCD in Mde is defined as [FixedAtBuild,
Dynamic, DynamicEx] so I suppose I am doing the right thing. Do you think
it is necessary to add in PatachbleInModule?

(note to self: state clearly the types of the PCD in the commit message
in next version)

Wei.

> Mike
> 
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com] 
> Sent: Monday, November 25, 2013 2:56 AM
> To: Kinney, Michael D
> Cc: Jordan Justen; edk2-devel@lists.sourceforge.net; Tian, Feng; Wei Liu; xen-devel
> Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration ... [and one more message]
> 
> On Mon, Nov 25, 2013 at 03:12:04AM +0000, Kinney, Michael D wrote:
> > Jordan,
> > 
> > I agree that removing code duplication is a good idea.
> > 
> > I believe we can make the one in the MdeModulePkg functional
> > everywhere.  Size will be the only remaining difference.  This can be
> > addressed longer term.  If the proposed PCD is configured as
> > FixedAtBuild in a DSC file, we should be able to get the optimized
> > code generation for the PciBusDxe to remove all the code/data that is
> > only used for PCI Enumeration.  Then the PciBusNoEnumeration could be
> > retired.
> > 
> 
> Forgive my ignorance -- is it possible to change FixedAtBuild PCD during
> runtime? Jordan had the idea to disable / enable PCI enumeration during
> runtime, so that we can have one single binary for all OVMF users.
> 
> > I have no issues with adding a PCD to make the PciBusDxe in the
> > MdeModulePkg skip enumeration all together.
> > 
> 
> Cool, thank you for confirming this.
> 
> Wei.

Re: [edk2] [PATCH RFC v2 7/7] OvmfPkg: introduce XenMemMapInitialization

[Wei Liu]

On Mon, Nov 25, 2013 at 12:38:04PM -0800, Jordan Justen wrote:
> Regarding patches 5-7, it seems like the mem-map code flow could be
> more shared. It is a bit challenging to unravel things though.
> 

Indeed. It's a bit unfortunate that this path is shared by all users of
OVMF. And from a previous email on edk2-devel I can see that QEMU / KVM
is also trying to deal with this now with different approach --
modifying MemMapInitialization -- that would not work with Xen. 

(What is interesting is that QEMU_PEI_INFO is very similar to what I
proposed for Xen in my RFC email. ;-) )

I cannot come up with a sensible solution for all platforms, so it makes
sense for me to keep and maintain a separate path, given that the path
is relatively short and straightforward.

> I guess the only specific thing I can really point out is that
> PcdPciAllowFullEnumeration should be initialized in a different patch,
> and not within the mem-map init path.
> 

Makes sense, I will fix this.

Wei.

Re: [edk2] [PATCH RFC v2 7/7] OvmfPkg: introduce XenMemMapInitialization

[Laszlo Ersek]

On 11/26/13 14:46, Wei Liu wrote:
> On Mon, Nov 25, 2013 at 12:38:04PM -0800, Jordan Justen wrote:
>> Regarding patches 5-7, it seems like the mem-map code flow could be
>> more shared. It is a bit challenging to unravel things though.
>>
> 
> Indeed. It's a bit unfortunate that this path is shared by all users of
> OVMF. And from a previous email on edk2-devel I can see that QEMU / KVM
> is also trying to deal with this now with different approach --
> modifying MemMapInitialization -- that would not work with Xen. 
> 
> (What is interesting is that QEMU_PEI_INFO is very similar to what I
> proposed for Xen in my RFC email. ;-) )
> 
> I cannot come up with a sensible solution for all platforms, so it makes
> sense for me to keep and maintain a separate path, given that the path
> is relatively short and straightforward.

I agree. Let's try to fix the memmap in PEI for Xen and Qemu
independently, and then we can extract the common bits. Hopefully.

(I'm putting the qemu part aside for now because we're obviously not
converging on how to fix that.)

Thanks
Laszlo

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Jordan Justen]

On Tue, Nov 26, 2013 at 2:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2013-11-25 at 12:00 -0800, Jordan Justen wrote:
>> What about when the next blob type needs to be transferred?
>
> When I put the SeaBIOS version together my intention was that the length
> field would serve as a proxy for the struct version should we ever need
> to extend it.

Okay. That should work.

> From the Xen side unless there is some strong reason to deviate I would
> prefer to stick with something which is similar to the existing
> interface to other BIOSes, which is what Wei has proposed, rather than
> reinvent it in a per-BIOS way. Ultimately this stuff won't be exposed to
> UEFI outside of the Xen support code.

Whoops. I asked if it followed a Xen convention, but then I missed
where you replied in the affirmative.

The biggest thing that bothers me is defining the addresses as 32-bit.
But, if this doesn't worry you guys, then I guess it is unlikely to
change soon.

-Jordan

Re: [edk2] [PATCH RFC v2 3/7] OvmfPkg: define EFI_XEN_OVMF_INFO and extend XenInfo

[Ian Campbell]

On Tue, 2013-11-26 at 08:46 -0800, Jordan Justen wrote:
> The biggest thing that bothers me is defining the addresses as 32-bit.
> But, if this doesn't worry you guys, then I guess it is unlikely to
> change soon.

I'd be happy with making them 64-bit.

Ian.