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

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

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

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

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

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

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

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