[PATCH RFC 1/7] HACK: Use DeutPkg/PciRootBridgeNoEnumeration and PciBusNoEnumerationDxe

[PATCH RFC 1/7] HACK: Use DeutPkg/PciRootBridgeNoEnumeration and PciBusNoEnumerationDxe

[Wei Liu]

Xen hvmloader has already allocated resources for PCI devices. OVMF
should not need to fiddle with that.

Apparently this can be made configurable so that it benifits Xen /
Coreboot etc. Advise welcomed.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/OvmfPkgX64.dsc |    8 ++++++--
 OvmfPkg/OvmfPkgX64.fdf |    8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 53945d0..0f2b496 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -388,8 +388,12 @@
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
-  PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
-  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  # PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  # MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+
+  DuetPkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+  DuetPkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+
   PcAtChipsetPkg/KbcResetDxe/Reset.inf
   MdeModulePkg/Universal/Metronome/Metronome.inf {
     <LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 751b94b..67a5094 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -259,8 +259,12 @@ INF  PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
-INF  PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
-INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+# INF  PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+# INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+
+INF   DuetPkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+INF   DuetPkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+
 INF  PcAtChipsetPkg/KbcResetDxe/Reset.inf
 INF  MdeModulePkg/Universal/Metronome/Metronome.inf
 INF  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
-- 
1.7.10.4

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

[Wei Liu]

E820.h is extracted from LinuxBzimage.h. The definition is generally
useful in other cases as well so it deserves its own header.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/Include/IndustryStandard/E820.h         |   34 +++++++++++++++++++++++
 OvmfPkg/Include/IndustryStandard/LinuxBzimage.h |   14 ++--------
 2 files changed, 36 insertions(+), 12 deletions(-)
 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..c42a108
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/E820.h
@@ -0,0 +1,34 @@
+/** @file
+
+  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
+  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)
+
+#define E820_RAM		1
+#define E820_RESERVED		2
+#define E820_ACPI		3
+#define E820_NVS		4
+#define E820_UNUSABLE		5
+
+struct e820_entry {
+	UINT64 addr;		/* start of memory segment */
+	UINT64 size;		/* size of memory segment */
+	UINT32 type;		/* type of memory segment */
+};
+
+#pragma pack()
+
+#endif /* __E820_H__ */
diff --git a/OvmfPkg/Include/IndustryStandard/LinuxBzimage.h b/OvmfPkg/Include/IndustryStandard/LinuxBzimage.h
index b83b791..f0c82fe 100644
--- a/OvmfPkg/Include/IndustryStandard/LinuxBzimage.h
+++ b/OvmfPkg/Include/IndustryStandard/LinuxBzimage.h
@@ -14,15 +14,11 @@
 #ifndef __LINUX_BZIMAGE_H__
 #define __LINUX_BZIMAGE_H__
 
+#include <IndustryStandard/E820.h>
+
 #define BOOTSIG			0x1FE
 #define SETUP_HDR		0x53726448	/* 0x53726448 == "HdrS" */
 
-#define E820_RAM		1
-#define E820_RESERVED		2
-#define E820_ACPI		3
-#define E820_NVS		4
-#define E820_UNUSABLE		5
-
 #pragma pack(1)
 
 struct setup_header {
@@ -78,12 +74,6 @@ struct efi_info {
 	UINT32 efi_memmap_hi;
 };
 
-struct e820_entry {
-	UINT64 addr;		/* start of memory segment */
-	UINT64 size;		/* size of memory segment */
-	UINT32 type;		/* type of memory segment */
-};
-
 struct screen_info {
         UINT8  orig_x;           /* 0x00 */
         UINT8  orig_y;           /* 0x01 */
-- 
1.7.10.4

[PATCH RFC 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.

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

diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h
index d512b0b..c12960c 100644
--- a/OvmfPkg/Include/Guid/XenInfo.h
+++ b/OvmfPkg/Include/Guid/XenInfo.h
@@ -18,6 +18,19 @@ 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 */
+  UINT8 Length;     /* Length of this struct */
+  UINT8 Checksum;   /* Set such that the sum over bytes 0..length == 0 */
+  /*
+   * 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 +48,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 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..b4c1247 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")) {
+    struct e820_entry *E820Map;
+    UINTN Loop, EntryCount, Base;
+
+    /* E820 map */
+    EntryCount = Info->E820Nr;
+    Base = Info->E820;
+
+    E820Map = AllocateZeroPool (sizeof(struct e820_entry) * EntryCount);
+
+    if (!E820Map) {
+      FreePages (mXenInfo.HyperPages, TransferPages);
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    for (Loop = 0; Loop < EntryCount; Loop++) {
+      struct e820_entry *src = (struct e820_entry *)Base + Loop;
+      struct e820_entry *dst = (struct e820_entry *)E820Map + Loop;
+      CopyMem (dst, src, sizeof(struct e820_entry));
+    }
+
+    mXenInfo.E820 = E820Map;
+    mXenInfo.E820EntryCount = EntryCount;
+  }
+
   BuildGuidDataHob (
     &gEfiXenInfoGuid,
     &mXenInfo,
-- 
1.7.10.4

[PATCH RFC 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 b4c1247..3741eff 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 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 7/7] OvmfPkg: introduce XenMemMapInitialization

[Wei Liu]

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

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

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 9b7828f..ea5de66 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,75 @@ 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.
+  // IoResourceBase and IoResourceMax aren't actually necessary.
+  //
+  BuildResourceDescriptorHob (
+    EFI_RESOURCE_IO,
+    EFI_RESOURCE_ATTRIBUTE_PRESENT     |
+    EFI_RESOURCE_ATTRIBUTE_INITIALIZED,
+    0xC000,
+    0x4000
+    );
+
+  //
+  // Video memory + Legacy BIOS region
+  //
+  AddIoMemoryRangeHob (0x0A0000, BASE_1MB);
+
+  AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
+
+  //
+  // Parse RAM in E820 map
+  //
+  if (Info->E820EntryCount > 0) {
+    struct e820_entry *E820Map, *Entry;
+    UINT16 Loop;
+
+    E820Map = Info->E820;
+    for (Loop = 0; Loop < Info->E820EntryCount; Loop++) {
+      Entry = E820Map + Loop;
+
+      // only care about RAM
+      if (Entry->type != E820_RAM)
+        continue;
+
+      if (Entry->addr >= BASE_4GB)
+	AddUntestedMemoryBaseSizeHob (Entry->addr, Entry->size);
+      else
+	AddMemoryBaseSizeHob (Entry->addr, Entry->size);
+
+      MtrrSetMemoryAttribute (Entry->addr, Entry->size, CacheWriteBack);
+    }
+  }
+}
+
 
 VOID
 MemMapInitialization (
@@ -346,7 +419,10 @@ InitializePlatform (
 
   XenLeaf = XenDetect ();
 
-  TopOfMemory = MemDetect ();
+  if (XenLeaf == 0)
+    TopOfMemory = MemDetect ();
+  else
+    PublishPeiMemory ();
 
   if (XenLeaf != 0) {
     DEBUG ((EFI_D_INFO, "Xen was detected\n"));
@@ -357,7 +433,10 @@ InitializePlatform (
 
   PeiFvInitialization ();
 
-  MemMapInitialization (TopOfMemory);
+  if (XenLeaf != 0)
+    XenMemMapInitialization ();
+  else
+    MemMapInitialization (TopOfMemory);
 
   MiscInitialization ();
 
-- 
1.7.10.4

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Jordan Justen]

When I try to boot this on qemu or kvm, it asserts because the video
framebuffer address is NULL.

On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> This is a quick hack I pulled together last night to make OVMF work with Xen.
> I'm pretty sure there's many things that can be improved. ;-)
>
> Any advice is welcomed. I'm particular interested in comments such as how to
> organize code, what to refactor, how to achieve wider benifit for other use
> cases.
>
> Wei.
>
> Wei Liu (7):
>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
>     PciBusNoEnumerationDxe

These modules would need to move to PcAtChipsetPkg in the non-hack version.

Actually, it would be even better if the generic version of these
modules could be tweaked with a PCD to not do enumeration.

>   OvmfPkg: introduce E820.h

Can you copy the E820 structs from IntelFrameworkModulePkg/Csm/
LegacyBiosDxe/LegacyBiosInterface.h instead?

>   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

A quick glance makes me think the memmap stuff could tweaked, but I'll
wait until it seems like this actually could work before I look at
this too close. :)

-Jordan

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Wei Liu]

On Sat, Nov 16, 2013 at 3:31 AM, Jordan Justen <jljusten@gmail.com> wrote:
> When I try to boot this on qemu or kvm, it asserts because the video
> framebuffer address is NULL.
>

That's expected because of the hack. ;-)

> On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
>> This is a quick hack I pulled together last night to make OVMF work with Xen.
>> I'm pretty sure there's many things that can be improved. ;-)
>>
>> Any advice is welcomed. I'm particular interested in comments such as how to
>> organize code, what to refactor, how to achieve wider benifit for other use
>> cases.
>>
>> Wei.
>>
>> Wei Liu (7):
>>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
>>     PciBusNoEnumerationDxe
>
> These modules would need to move to PcAtChipsetPkg in the non-hack version.
>
> Actually, it would be even better if the generic version of these
> modules could be tweaked with a PCD to not do enumeration.
>

OK. I will look into it.

Just to make clear, you want to keep PcAtChipsetPkg (not moving
DuetPkg code in), and to make it able to skip enumeration with a PCD.

>>   OvmfPkg: introduce E820.h
>
> Can you copy the E820 structs from IntelFrameworkModulePkg/Csm/
> LegacyBiosDxe/LegacyBiosInterface.h instead?
>

OK, no problem.

Wei

>>   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
>
> A quick glance makes me think the memmap stuff could tweaked, but I'll
> wait until it seems like this actually could work before I look at
> this too close. :)
>
> -Jordan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Jordan Justen]

On Sat, Nov 16, 2013 at 5:31 PM, Wei Liu <liuw@liuw.name> wrote:
> On Sat, Nov 16, 2013 at 3:31 AM, Jordan Justen <jljusten@gmail.com> wrote:
>> When I try to boot this on qemu or kvm, it asserts because the video
>> framebuffer address is NULL.
>
> That's expected because of the hack. ;-)

I see. But, I don't want to spend to much time considering it if is
not going to work with QEMU & KVM. :)

I think a simpler approach would be to retain the enumeration and
figure out a memory range that works with Xen.

>> On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
>>> Wei Liu (7):
>>>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
>>>     PciBusNoEnumerationDxe
>>
>> These modules would need to move to PcAtChipsetPkg in the non-hack version.
>>
>> Actually, it would be even better if the generic version of these
>> modules could be tweaked with a PCD to not do enumeration.

My first choice would be to add a PCD to MdeModulePkg/MdeModulePkg.dec
that prevents enumeration in
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf, and continue to use this
main driver. But, I don't know how easy those changes would be to
make. It probably would also mean changing
PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf by looking at the
same PCD. If some OVMF environments require enumeration, and some
don't then this would allow us to choose what will happen at runtime.

The only other option would be to move those modules to PcAtChipset.
But, first we'd have to prove that QEMU & KVM are okay with using
those drivers.

-Jordan

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Wei Liu]

On Sun, Nov 17, 2013 at 2:11 AM, Jordan Justen <jljusten@gmail.com> wrote:
> On Sat, Nov 16, 2013 at 5:31 PM, Wei Liu <liuw@liuw.name> wrote:
>> On Sat, Nov 16, 2013 at 3:31 AM, Jordan Justen <jljusten@gmail.com> wrote:
>>> When I try to boot this on qemu or kvm, it asserts because the video
>>> framebuffer address is NULL.
>>
>> That's expected because of the hack. ;-)
>
> I see. But, I don't want to spend to much time considering it if is
> not going to work with QEMU & KVM. :)
>
> I think a simpler approach would be to retain the enumeration and
> figure out a memory range that works with Xen.
>
>>> On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
>>>> Wei Liu (7):
>>>>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
>>>>     PciBusNoEnumerationDxe
>>>
>>> These modules would need to move to PcAtChipsetPkg in the non-hack version.
>>>
>>> Actually, it would be even better if the generic version of these
>>> modules could be tweaked with a PCD to not do enumeration.
>
> My first choice would be to add a PCD to MdeModulePkg/MdeModulePkg.dec
> that prevents enumeration in
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf, and continue to use this
> main driver. But, I don't know how easy those changes would be to
> make. It probably would also mean changing
> PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf by looking at the
> same PCD. If some OVMF environments require enumeration, and some
> don't then this would allow us to choose what will happen at runtime.
>
> The only other option would be to move those modules to PcAtChipset.
> But, first we'd have to prove that QEMU & KVM are okay with using
> those drivers.
>

Would it be simpler if we have ifdef in .dsc file to detect predefine
macros and choose different modules during build time?

Wei.

> -Jordan

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Wei Liu]

On Sun, Nov 17, 2013 at 02:24:35AM +0000, Wei Liu wrote:
> On Sun, Nov 17, 2013 at 2:11 AM, Jordan Justen <jljusten@gmail.com> wrote:
> > On Sat, Nov 16, 2013 at 5:31 PM, Wei Liu <liuw@liuw.name> wrote:
> >> On Sat, Nov 16, 2013 at 3:31 AM, Jordan Justen <jljusten@gmail.com> wrote:
> >>> When I try to boot this on qemu or kvm, it asserts because the video
> >>> framebuffer address is NULL.
> >>
> >> That's expected because of the hack. ;-)
> >
> > I see. But, I don't want to spend to much time considering it if is
> > not going to work with QEMU & KVM. :)
> >
> > I think a simpler approach would be to retain the enumeration and
> > figure out a memory range that works with Xen.
> >
> >>> On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> >>>> Wei Liu (7):
> >>>>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
> >>>>     PciBusNoEnumerationDxe
> >>>
> >>> These modules would need to move to PcAtChipsetPkg in the non-hack version.
> >>>
> >>> Actually, it would be even better if the generic version of these
> >>> modules could be tweaked with a PCD to not do enumeration.
> >
> > My first choice would be to add a PCD to MdeModulePkg/MdeModulePkg.dec
> > that prevents enumeration in
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf, and continue to use this
> > main driver. But, I don't know how easy those changes would be to
> > make. It probably would also mean changing
> > PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf by looking at the
> > same PCD. If some OVMF environments require enumeration, and some
> > don't then this would allow us to choose what will happen at runtime.
> >
> > The only other option would be to move those modules to PcAtChipset.
> > But, first we'd have to prove that QEMU & KVM are okay with using
> > those drivers.
> >
> 
> Would it be simpler if we have ifdef in .dsc file to detect predefine
> macros and choose different modules during build time?
> 

Hmm... Just realized that my other email wasn't subscribed to edk2-devel
so my mails didn't go through.

Here is my proposal, seems to be shorter and simpler than modifying
modules outside OVMF.

Wei.

---8<---
>From 807e24daa8f96597f656e5e26aed1107acf0ae5c Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Fri, 15 Nov 2013 15:30:12 +0000
Subject: [PATCH] OvmfPkg: disable PCI enumeration if Xen is enabled

PCI enumeration is done by Xen's hvmloader.

Luckily DuetPkg has code that skips enumeration, simply make use of it
saves us lots of coding.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 OvmfPkg/OvmfPkgX64.dsc |    6 ++++++
 OvmfPkg/OvmfPkgX64.fdf |    6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 53945d0..4d5b25f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -388,8 +388,14 @@
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
+!ifdef $(XEN_ENABLE)
+  DuetPkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+  DuetPkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+!else
   PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+!endif
+
   PcAtChipsetPkg/KbcResetDxe/Reset.inf
   MdeModulePkg/Universal/Metronome/Metronome.inf {
     <LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 751b94b..2c2f70a 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -259,8 +259,14 @@ INF  PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
+!ifdef $(XEN_ENABLE)
+INF   DuetPkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+INF   DuetPkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+!else
 INF  PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+!endif
+
 INF  PcAtChipsetPkg/KbcResetDxe/Reset.inf
 INF  MdeModulePkg/Universal/Metronome/Metronome.inf
 INF  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
-- 
1.7.10.4


> Wei.
> 
> > -Jordan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Wei Liu]

On Sat, Nov 16, 2013 at 06:11:20PM -0800, Jordan Justen wrote:
> On Sat, Nov 16, 2013 at 5:31 PM, Wei Liu <liuw@liuw.name> wrote:
> > On Sat, Nov 16, 2013 at 3:31 AM, Jordan Justen <jljusten@gmail.com> wrote:
> >> When I try to boot this on qemu or kvm, it asserts because the video
> >> framebuffer address is NULL.
> >
> > That's expected because of the hack. ;-)
> 
> I see. But, I don't want to spend to much time considering it if is
> not going to work with QEMU & KVM. :)
> 
> I think a simpler approach would be to retain the enumeration and
> figure out a memory range that works with Xen.
> 
> >> On Fri, Nov 15, 2013 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> >>> Wei Liu (7):
> >>>   HACK: Use DeutPkg/PciRootBridgeNoEnumeration and
> >>>     PciBusNoEnumerationDxe
> >>
> >> These modules would need to move to PcAtChipsetPkg in the non-hack version.
> >>
> >> Actually, it would be even better if the generic version of these
> >> modules could be tweaked with a PCD to not do enumeration.
> 
> My first choice would be to add a PCD to MdeModulePkg/MdeModulePkg.dec
> that prevents enumeration in
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf, and continue to use this
> main driver. But, I don't know how easy those changes would be to
> make. It probably would also mean changing
> PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf by looking at the
> same PCD. If some OVMF environments require enumeration, and some
> don't then this would allow us to choose what will happen at runtime.
> 

Second thought on this.

The main advantage to have a runtime Pcd over build time static
configuration is that we can use single binary for all OVMF use cases.
However currently Xen hvmloader statically embeds firmware blobs in
itself so it doesn't really matter if OVMF is configurable at runtime.
So this option will not benifit much unless we overhaul toolstack (libxl
and hvmloader) to support loading additional firmware blobs.

And per Andrew Fish: 

"The https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Bus/Pci/PciBusDxe/
driver assumes you are following the PI Spec model and a full PCI
enumeration is requires and a set of chipset/platform specific protocols
are provided to make the PCI enumerate code generic."

So I'm not quite sure whether to stop enumeration in MdeModulePkg is the
right choice.

> The only other option would be to move those modules to PcAtChipset.
> But, first we'd have to prove that QEMU & KVM are okay with using
> those drivers.
> 

I presume QEMU / KVM will still need enumeration, so they are not likely
to make use of those modules.

Corrections welcomed.

Wei.

> -Jordan
> 
> ------------------------------------------------------------------------------
> DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
> OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
> Free app hosting. Or install the open source package on any LAMP server.
> Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
> http://pubads.g.doubleclick.net/gampad/clk?id=63469471&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 0/7] OvmfPkg: make OVMF fully working with Xen

[Gerd Hoffmann]

  Hi,

> Second thought on this.
> 
> The main advantage to have a runtime Pcd over build time static
> configuration is that we can use single binary for all OVMF use cases.
> However currently Xen hvmloader statically embeds firmware blobs in
> itself so it doesn't really matter if OVMF is configurable at runtime.
> So this option will not benifit much unless we overhaul toolstack (libxl
> and hvmloader) to support loading additional firmware blobs.

[ same goes for coreboot btw, there isn't much benefit in
  switching at runtime ]

> > The only other option would be to move those modules to PcAtChipset.
> > But, first we'd have to prove that QEMU & KVM are okay with using
> > those drivers.
> > 
> 
> I presume QEMU / KVM will still need enumeration, so they are not likely
> to make use of those modules.

Correct.

cheers,
  Gerd

Re: [edk2] [PATCH RFC 0/7] OvmfPkg: make OVMF fully working with Xen

[Jordan Justen]

On Mon, Nov 18, 2013 at 2:26 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Sat, Nov 16, 2013 at 06:11:20PM -0800, Jordan Justen wrote:
>> My first choice would be to add a PCD to MdeModulePkg/MdeModulePkg.dec
>> that prevents enumeration in
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf, and continue to use this
>> main driver. But, I don't know how easy those changes would be to
>> make. It probably would also mean changing
>> PcAtChipsetPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf by looking at the
>> same PCD. If some OVMF environments require enumeration, and some
>> don't then this would allow us to choose what will happen at runtime.
>
> Second thought on this.
>
> The main advantage to have a runtime Pcd over build time static
> configuration is that we can use single binary for all OVMF use cases.
> However currently Xen hvmloader statically embeds firmware blobs in
> itself so it doesn't really matter if OVMF is configurable at runtime.
> So this option will not benifit much unless we overhaul toolstack (libxl
> and hvmloader) to support loading additional firmware blobs.

This is a good argument. I definitely wanted to keep a single OVMF
build working for Xen & QEMU if possible, but it seems like there is
little value if it is not easy to load a different version in Xen.

It seems that Xen supporting flash emulation will raise a few
uncomfortable questions. (Well, that is not exclusive to Xen. :)

> And per Andrew Fish:
>
> "The https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Bus/Pci/PciBusDxe/
> driver assumes you are following the PI Spec model and a full PCI
> enumeration is requires and a set of chipset/platform specific protocols
> are provided to make the PCI enumerate code generic."
>
> So I'm not quite sure whether to stop enumeration in MdeModulePkg is the
> right choice.

Obviously the PCD should default to enumeration, but I think there is
still value in not duplicating the module assuming there is a lot of
code overlap. It would be nice to continue to use the same module that
most platform use.

-Jordan