[PATCH] OvmfPkg: reserve igd memory by E820

[PATCH] OvmfPkg: reserve igd memory by E820

[Corvin Köhne]

Hi all,

I'd like to discuss the following patch. At the moment, it's bhyve 
specific but I'd like to merge it into stock OVMF too. I'm working 
on GPU passthrough support for Intels integrated graphics devices 
(Intel calls it GVT-d). In order to get GVT-d working properly, 
the guest bios should allocate two memory areas (Graphics Stolen 
Memory and OpRegion). I'm trying to create a platform independent 
patch which allows to allocate those regions.

At the moment, I'm using an E820 table that bhyve provides to the 
guest. Another approach would be to use the QemuLoader which is 
used for loader Qemu's ACPI tables. While this has the advantage 
that the guest bios allocates the memory by it's own, the 
hypervisor has less control over the address. E.g. the ACRN 
hypervisor uses an indentical mapping for the Graphics Stolen 
Memory on Tiger Lake platforms and above (see 
https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/hw/pci/passthrough.c#L519-L523).

What do you think about this patch? Do you prefer using the E820 table 
or the QemuLoader? Do you have any other ideas?

Btw: I'm using the appended patch with an own bhyve implementation for 
several month now without any issues.

Here's the patch:

Detecting Graphics Stolen Memory and OpRegion address and length is
platform specific. OVMF shouldn't be platform specific. Move the
platform specific part to bhyve. Bhyve detects Graphics Stolen
Memory and OpRegion address and length and passes these
information as E820 table to the firmware. This will additionally
allow us to pass other platform specific memory ranges to the guest in
the future.

Signed-off-by: Corvin Köhne <CorvinK@beckhoff.com>
Reviewed-by: Patrick Bruenn <p.bruenn@beckhoff.com>
---
 OvmfPkg/Bhyve/BhyveBhfX64.dsc             |   1 +
 OvmfPkg/Bhyve/PlatformPei/MemDetect.c     |  78 ++++++++++++++++++
 OvmfPkg/Bhyve/PlatformPei/Platform.c      | 127 ++++++++++++++++++++++++++++++
 OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |   1 +
 4 files changed, 207 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyveBhfX64.dsc b/OvmfPkg/Bhyve/BhyveBhfX64.dsc
index 0a81b1de75..03d1fb50d9 100644
--- a/OvmfPkg/Bhyve/BhyveBhfX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveBhfX64.dsc
@@ -284,6 +284,7 @@
 !endif
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
 
   MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
 
diff --git a/OvmfPkg/Bhyve/PlatformPei/MemDetect.c b/OvmfPkg/Bhyve/PlatformPei/MemDetect.c
index 1949e586a0..0681586927 100644
--- a/OvmfPkg/Bhyve/PlatformPei/MemDetect.c
+++ b/OvmfPkg/Bhyve/PlatformPei/MemDetect.c
@@ -30,6 +30,7 @@ Module Name:
 #include <Library/PcdLib.h>
 #include <Library/PciLib.h>
 #include <Library/PeimEntryPoint.h>
+#include <Library/QemuFwCfgLib.h>
 #include <Library/ResourcePublicationLib.h>
 #include <Library/MtrrLib.h>
 
@@ -357,6 +358,9 @@ PublishPeiMemory (
   VOID
   )
 {
+  EFI_E820_ENTRY64      E820Entry;
+  FIRMWARE_CONFIG_ITEM  Item;
+  UINTN                 ItemSize;
   EFI_STATUS            Status;
   EFI_PHYSICAL_ADDRESS  MemoryBase;
   UINT64                MemorySize;
@@ -418,6 +422,80 @@ PublishPeiMemory (
   }
 
   //
+  // Bhyve uses an E820 table to reserve certain kinds of memory like Graphics
+  // Stolen Memory. These reserved memory regions could overlap with the PEI
+  // core memory which leads to undefined behaviour. Check the E820 table for
+  // a memory region starting at or below MemoryBase which is equal or larger
+  // than MemorySize.
+  //
+  if (!EFI_ERROR (QemuFwCfgFindFile ("etc/e820", &Item, &ItemSize))) {
+    //
+    // Set a new base address based on E820 table.
+    //
+
+    EFI_PHYSICAL_ADDRESS  MaxAddress;
+    UINT64                EntryEnd;
+    UINT64                EndAddr;
+
+    if (ItemSize % sizeof (E820Entry) != 0) {
+      DEBUG ((DEBUG_INFO, "%a: invalid E820 size\n", __FUNCTION__));
+      return EFI_PROTOCOL_ERROR;
+    }
+
+    QemuFwCfgSelectItem (Item);
+
+    MaxAddress = MemoryBase + MemorySize;
+    MemoryBase = 0;
+
+    for (UINTN i = 0; i < ItemSize; i += sizeof (E820Entry)) {
+      QemuFwCfgReadBytes (sizeof (E820Entry), &E820Entry);
+
+      if (E820Entry.BaseAddr > MaxAddress) {
+        //
+        // E820 is always sorted in ascending order. For that reason, BaseAddr
+        // will always be larger than MaxAddress for the following entries.
+        //
+        break;
+      } else if (E820Entry.Type != EfiAcpiAddressRangeMemory) {
+        continue;
+      }
+
+      //
+      // Since MemoryBase should be at top of the memory, calculate the end
+      // address of the entry. Additionally, MemoryBase and MemorySize are page
+      // aligned. For that reason, page align EntryEnd too.
+      //
+      EntryEnd = (E820Entry.BaseAddr + E820Entry.Length) & ~EFI_PAGE_MASK;
+      if (E820Entry.BaseAddr > EntryEnd) {
+        //
+        // Either BaseAddr + Length overflows or the entry is smaller than a
+        // page. In both cases, we can't use it.
+        //
+        continue;
+      }
+
+      EndAddr = MIN (EntryEnd, MaxAddress);
+      if (EndAddr - E820Entry.BaseAddr < MemorySize) {
+        //
+        // E820 entry is too small for the Pei core memory.
+        //
+        continue;
+      }
+
+      MemoryBase = EndAddr - MemorySize;
+    }
+
+    if (MemoryBase == 0) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: Unable to find suitable PeiCore address\n",
+        __FUNCTION__
+        ));
+      return EFI_OUT_OF_RESOURCES;
+    }
+  }
+
+  //
   // Publish this memory to the PEI Core
   //
   Status = PublishSystemMemory (MemoryBase, MemorySize);
diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c b/OvmfPkg/Bhyve/PlatformPei/Platform.c
index eba7c60fce..81062dbf44 100644
--- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
+++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
@@ -27,6 +27,7 @@
 #include <Library/PciLib.h>
 #include <Library/PeimEntryPoint.h>
 #include <Library/PeiServicesLib.h>
+#include <Library/QemuFwCfgLib.h>
 #include <Library/ResourcePublicationLib.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <Ppi/MasterBootMode.h>
@@ -491,6 +492,118 @@ DebugDumpCmos (
   }
 }
 
+STATIC
+EFI_STATUS
+E820ReserveMemory (
+  VOID
+  )
+{
+  EFI_E820_ENTRY64      E820Entry;
+  FIRMWARE_CONFIG_ITEM  Item;
+  UINTN                 Size;
+  EFI_STATUS            Status;
+
+  Status = QemuFwCfgFindFile ("etc/e820", &Item, &Size);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "%a: E820 not found: %r\n", __FUNCTION__, Status));
+    return Status;
+  }
+
+  if (Size % sizeof (E820Entry) != 0) {
+    DEBUG ((DEBUG_INFO, "%a: invalid E820 size\n", __FUNCTION__));
+    return EFI_PROTOCOL_ERROR;
+  }
+
+  QemuFwCfgSelectItem (Item);
+  for (UINTN i = 0; i < Size; i += sizeof (E820Entry)) {
+    UINT64  Base;
+    UINT64  End;
+
+    QemuFwCfgReadBytes (sizeof (E820Entry), &E820Entry);
+
+    DEBUG_CODE (
+      CHAR8 *E820TypeName;
+      switch (E820Entry.Type) {
+      case EfiAcpiAddressRangeMemory:
+        E820TypeName = "RAM     ";
+        break;
+      case EfiAcpiAddressRangeReserved:
+        E820TypeName = "Reserved";
+        break;
+      case EfiAcpiAddressRangeACPI:
+        E820TypeName = "ACPI    ";
+        break;
+      case EfiAcpiAddressRangeNVS:
+        E820TypeName = "NVS     ";
+        break;
+      default:
+        E820TypeName = "Unknown ";
+        break;
+    }
+
+      DEBUG ((
+        DEBUG_INFO,
+        "E820 entry [ %16lx, %16lx] (%a)\n",
+        E820Entry.BaseAddr,
+        E820Entry.BaseAddr + E820Entry.Length,
+        E820TypeName
+        ));
+      );
+
+    //
+    // Round down base and round up length to page boundary
+    //
+    Base = E820Entry.BaseAddr & ~(UINT64)EFI_PAGE_MASK;
+    End  = ALIGN_VALUE (
+             E820Entry.BaseAddr + E820Entry.Length,
+             (UINT64)EFI_PAGE_SIZE
+             );
+
+    switch (E820Entry.Type) {
+      case EfiAcpiAddressRangeReserved:
+        BuildMemoryAllocationHob (
+          Base,
+          End - Base,
+          EfiReservedMemoryType
+          );
+        break;
+
+      case EfiAcpiAddressRangeACPI:
+        BuildMemoryAllocationHob (
+          Base,
+          End - Base,
+          EfiACPIReclaimMemory
+          );
+        break;
+
+      case EfiAcpiAddressRangeNVS:
+        BuildMemoryAllocationHob (
+          Base,
+          End - Base,
+          EfiACPIMemoryNVS
+          );
+        break;
+
+      case EfiAcpiAddressRangeMemory:
+        //
+        // EfiAcpiAddressRangeMemory is usable. Do not build an Allocation HOB.
+        //
+        break;
+
+      default:
+        DEBUG ((
+          DEBUG_ERROR,
+          "%a: Unknown E820 type %d\n",
+          __FUNCTION__,
+          E820Entry.Type
+          ));
+        return EFI_PROTOCOL_ERROR;
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
 VOID
 S3Verification (
   VOID
@@ -579,6 +692,10 @@ InitializePlatform (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
+  EFI_STATUS            Status;
+  FIRMWARE_CONFIG_ITEM  Item;
+  UINTN                 Size;
+
   DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
 
   //
@@ -591,6 +708,16 @@ InitializePlatform (
 
   DebugDumpCmos ();
 
+  //
+  // If an E820 table exists, Memory should be reserved by E820.
+  //
+  if (!EFI_ERROR (QemuFwCfgFindFile ("etc/e820", &Item, &Size))) {
+    Status = E820ReserveMemory ();
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
   BootModeInitialization ();
   AddressWidthInitialization ();
   MaxCpuCountInitialization ();
diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
index 739d63098b..557a8f890e 100644
--- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
@@ -59,6 +59,7 @@
   PeiServicesLib
   PeiServicesTablePointerLib
   PcdLib
+  QemuFwCfgLib
   ResourcePublicationLib
 
 [Pcd]
-- 
2.11.0

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Gerd Hoffmann]

On Mon, Apr 04, 2022 at 08:34:48AM +0200, Corvin Köhne wrote:
> Hi all,
> 
> I'd like to discuss the following patch. At the moment, it's bhyve 
> specific but I'd like to merge it into stock OVMF too. I'm working 
> on GPU passthrough support for Intels integrated graphics devices 
> (Intel calls it GVT-d). In order to get GVT-d working properly, 
> the guest bios should allocate two memory areas (Graphics Stolen 
> Memory and OpRegion). I'm trying to create a platform independent 
> patch which allows to allocate those regions.

Oh no.  Not that discussion again.

First, there is no need to communicate memory regions from the
hypervisor to the guest.  The IGD hardware has registers pointing
to the opregion and to stolen memory, so the guest can simply
allocate and initialize memory, then program the registers
accordingly.  Same procedure you have when initializing IGD on
physical hardware.

Second, that doesn't belong into OVMF.  Drivers for virtual/emulated
hardware are fine, but we certainly don't want enter the business of
maintaining drivers for physical hardware in OVMF.

An EFI driver in the igd option rom should be able to handle all of
the above just fine, and for Intel it should also be easy to cover
differences in hardware generations in the driver.  Bonus points
for also setting up a GOP for the IGD ;)

Unfortunately Intel didn't manage it yet to provide an igd option
rom for virtual machines (or fix their regular driver to also work
in virtual machines) even though this discussion is ongoing on
and off for years meanwhile :(

BTW: Do you talk about GVT-d (== build virtual IGD devices with some
resources of the physical device, roughly comparable to SR-IOV but
with the igd kernel driver instead of the hardware handling this)?
Or do you want pci-assign the complete igd device?

> At the moment, I'm using an E820 table that bhyve provides to the 
> guest. Another approach would be to use the QemuLoader which is 
> used for loader Qemu's ACPI tables. While this has the advantage 
> that the guest bios allocates the memory by it's own, the 
> hypervisor has less control over the address. E.g. the ACRN 
> hypervisor uses an indentical mapping for the Graphics Stolen 
> Memory on Tiger Lake platforms and above (see 
> https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/hw/pci/passthrough.c#L519-L523).

Lovely.  Intel should fix their broken windows drivers ...

The etc/e820 fw_cfg file can have both 'ram' and 'reserved' entries.

And, yes, adding a 'reserved' entry there for the region which requires
an identity mapping (to workaround the driver bug) is fine and should
make sure the region is not used for something else.  Everything else
should be handled by the igd efi driver / optionrom.

Also:  As far I know the stolen memory is only needed at boot, for the
EFI GOP.  Once the guest OS loaded the native drivers for the IGD the
stolen memory is not needed / not used any more, and in theory the
drivers should cope with a stolen memory region not being present just
fine.

For the opregion:  qemu has, for historical reasons, an (optional and
disabled by default) etc/igd-opregion fw_cfg file.  It was a quick hack
which ended up staying.  When a option rom is needed anyway the content
of the opregion can simply be passed to the guest as part of the option
rom image.  But if you prefer to use fw_cfg instead you should at least
use the same hack instead of inventing a new one ...

See also:
  https://gitlab.com/qemu-project/qemu/-/blob/master/docs/igd-assign.txt

Seabios uses etc/igd-opregion, guest code is here:
https://gitlab.com/qemu-project/seabios/-/blob/master/src/fw/pciinit.c#L291
Ideally we'd move that to a proper vgabios too ...

take care,
  Gerd

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Corvin Köhne]

Hi Gerd,

thanks for your reply.

> First, there is no need to communicate memory regions from the
> hypervisor to the guest.  The IGD hardware has registers pointing
> to the opregion and to stolen memory, so the guest can simply
> allocate and initialize memory, then program the registers
> accordingly.  Same procedure you have when initializing IGD on
> physical hardware.

As far as I know, on physical hardware it's done by UEFI. Sadly, the
Intel GOP driver doesn't do it.

> Second, that doesn't belong into OVMF.  Drivers for virtual/emulated
> hardware are fine, but we certainly don't want enter the business of
> maintaining drivers for physical hardware in OVMF.

Understandable.

> An EFI driver in the igd option rom should be able to handle all of
> the above just fine, and for Intel it should also be easy to cover
> differences in hardware generations in the driver.  Bonus points
> for also setting up a GOP for the IGD ;)

I'm already passing the Intel GOP as option rom to my bhyve VM.

> Unfortunately Intel didn't manage it yet to provide an igd option
> rom for virtual machines (or fix their regular driver to also work
> in virtual machines) even though this discussion is ongoing on
> and off for years meanwhile :(

I saw some of this discussions.

> BTW: Do you talk about GVT-d (== build virtual IGD devices with some
> resources of the physical device, roughly comparable to SR-IOV but
> with the igd kernel driver instead of the hardware handling this)?
> Or do you want pci-assign the complete igd device?

Intel has different terms for their GPU passthrough, GVT-d, GVT-g and
GVT-s. I'd like to use GVT-d which means pci-assigning the
complete igd device.

> Lovely.  Intel should fix their broken windows drivers ...
>
> The etc/e820 fw_cfg file can have both 'ram' and 'reserved' entries.
>
> And, yes, adding a 'reserved' entry there for the region which requires
> an identity mapping (to workaround the driver bug) is fine and should
> make sure the region is not used for something else.  Everything else
> should be handled by the igd efi driver / optionrom.

At the moment, my bhyve implementation allocates GSM and OpRegion in
guest memory, copies OpRegion into guest memory, inserts the GSM
and OpRegion addresses into the PCI registers and creates an E820
table.

In order to get the Intel GOP driver working, an additional
PlatformGopPolicy driver is required. I know that the
PlatformGopPolicy driver would never be merged to OVMF because
it's a platform dependent driver for physical hardware. However,
EfiRom can be used to create a proper option rom which includes
the PlatformGopPolicy and the GOP driver.

> Also:  As far I know the stolen memory is only needed at boot, for the
> EFI GOP.  Once the guest OS loaded the native drivers for the IGD the
> stolen memory is not needed / not used any more, and in theory the
> drivers should cope with a stolen memory region not being present just
> fine.

I'd like to use the Intel GOP driver.

> For the opregion:  qemu has, for historical reasons, an (optional and
> disabled by default) etc/igd-opregion fw_cfg file.  It was a quick hack
> which ended up staying.  When a option rom is needed anyway the content
> of the opregion can simply be passed to the guest as part of the option
> rom image.  But if you prefer to use fw_cfg instead you should at least
> use the same hack instead of inventing a new one ...
>
> See also:
>   https://gitlab.com/qemu-project/qemu/-/blob/master/docs/igd-assign.txt
>
> Seabios uses etc/igd-opregion, guest code is here:
> https://gitlab.com/qemu-project/seabios/-/blob/master/src/fw/pciinit.c#L291
> Ideally we'd move that to a proper vgabios too ...

Moving all of this into a proper option rom might be a good idea.
It'll require some work to create some tools which build a proper
rom for each system. However, with this solution there aren't any
hacks neither in the hypervisor nor in OVMF.


Best regards
Corvin


Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Gerd Hoffmann]

  Hi,

> > First, there is no need to communicate memory regions from the
> > hypervisor to the guest.  The IGD hardware has registers pointing
> > to the opregion and to stolen memory, so the guest can simply
> > allocate and initialize memory, then program the registers
> > accordingly.  Same procedure you have when initializing IGD on
> > physical hardware.
> 
> As far as I know, on physical hardware it's done by UEFI. Sadly, the
> Intel GOP driver doesn't do it.

Where does the intel gop driver come from?  Extracted from host
firmware?

> > BTW: Do you talk about GVT-d (== build virtual IGD devices with some
> > resources of the physical device, roughly comparable to SR-IOV but
> > with the igd kernel driver instead of the hardware handling this)?
> > Or do you want pci-assign the complete igd device?
> 
> Intel has different terms for their GPU passthrough, GVT-d, GVT-g and
> GVT-s. I'd like to use GVT-d which means pci-assigning the
> complete igd device.

Ah, ok, didn't notice the subtile differences with the small letter at
the end.  GVT-d + GVT-g is clear then.  What is GVT-s ?

> > Lovely.  Intel should fix their broken windows drivers ...
> >
> > The etc/e820 fw_cfg file can have both 'ram' and 'reserved' entries.
> >
> > And, yes, adding a 'reserved' entry there for the region which requires
> > an identity mapping (to workaround the driver bug) is fine and should
> > make sure the region is not used for something else.  Everything else
> > should be handled by the igd efi driver / optionrom.
> 
> At the moment, my bhyve implementation allocates GSM and OpRegion in
> guest memory, copies OpRegion into guest memory, inserts the GSM
> and OpRegion addresses into the PCI registers and creates an E820
> table.

Ideally the guest would allocate and initialize this all itself.
That is hard for the GSM though when it requires an identity
mapping.

Having the guest check whenever the GSM register points to reserved
memory and if so use it instead of allocating memory should work I
think.

> > For the opregion:  qemu has, for historical reasons, an (optional and
> > disabled by default) etc/igd-opregion fw_cfg file.  It was a quick hack
> > which ended up staying.  When a option rom is needed anyway the content
> > of the opregion can simply be passed to the guest as part of the option
> > rom image.  But if you prefer to use fw_cfg instead you should at least
> > use the same hack instead of inventing a new one ...
> >
> > See also:
> >   https://gitlab.com/qemu-project/qemu/-/blob/master/docs/igd-assign.txt
> >
> > Seabios uses etc/igd-opregion, guest code is here:
> > https://gitlab.com/qemu-project/seabios/-/blob/master/src/fw/pciinit.c#L291
> > Ideally we'd move that to a proper vgabios too ...
> 
> Moving all of this into a proper option rom might be a good idea.
> It'll require some work to create some tools which build a proper
> rom for each system.

Once we have the code for vgabios and PlatformGopPolicy we can roll them
with the intelgop driver into a rom image with EfiRom.  Ideally also add
opregion content to the rom somehow.

> However, with this solution there aren't any
> hacks neither in the hypervisor nor in OVMF.

Yep, that's the main point of the approach.  Make it self-contained and
not depend on specific behavior/support in hypervisor and/or firmware.

take care,
  Gerd

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Corvin Köhne]

Hi,

> Where does the intel gop driver come from?  Extracted from host
> firmware?

https://projectacrn.github.io/latest/tutorials/gpu-passthru.html#enable-the-gvt-d-gop-driver

I don't know if it's possible to extract it from host firmware within
OS context easily. According to Intel you should ask your board
manufacturer. We're producing our own motherboards with an own
BIOS. So, I'm reusing the Intel GOP driver from our host BIOS.

> Ah, ok, didn't notice the subtile differences with the small letter at
> the end.  GVT-d + GVT-g is clear then.  What is GVT-s ?

https://01.org/sites/default/files/downloads/igvt-g/gvtflyer.pdf

I'm not familiar with GVT-s. I just know that it exists.

> Ideally the guest would allocate and initialize this all itself.
> That is hard for the GSM though when it requires an identity
> mapping.
>
> Having the guest check whenever the GSM register points to reserved
> memory and if so use it instead of allocating memory should work I
> think.

To allocate GSM, the guest has to detect the size of GSM. Detecting
the size is platform dependent. Writing a small EFI driver to
detect the GSM size and to allocate a proper memory region should
be easy.

https://github.com/torvalds/linux/blob/master/arch/x86/kernel/early-quirks.c#L353-L460

> Once we have the code for vgabios and PlatformGopPolicy we can roll them
> with the intelgop driver into a rom image with EfiRom.  Ideally also add
> opregion content to the rom somehow.

Sounds good. Does these EFI drivers have to be maintained outside of
EDKII?


Best regards
Corvin


Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Gerd Hoffmann]

On Tue, Apr 05, 2022 at 07:47:38AM +0000, Corvin Köhne wrote:
> Hi,
> 
> > Where does the intel gop driver come from?  Extracted from host
> > firmware?
> 
> https://projectacrn.github.io/latest/tutorials/gpu-passthru.html#enable-the-gvt-d-gop-driver
> 
> I don't know if it's possible to extract it from host firmware within
> OS context easily.

Extracting from firmware updates is probably easier.

> According to Intel you should ask your board manufacturer.

I'm wondering why if the opregion initialization (which actually is
board-specific) is not done by the gop driver.

> https://01.org/sites/default/files/downloads/igvt-g/gvtflyer.pdf
> 
> I'm not familiar with GVT-s. I just know that it exists.

Seems to be higher level, sending a directx/opengl command stream,
apparently no hardware access for the guest.

> > Once we have the code for vgabios and PlatformGopPolicy we can roll them
> > with the intelgop driver into a rom image with EfiRom.  Ideally also add
> > opregion content to the rom somehow.
> 
> Sounds good. Does these EFI drivers have to be maintained outside of
> EDKII?

Hosting this in https://github.com/tianocore/edk2-staging is probably
one option (not fully sure about the policy there).

Creating a separate project for that might also make sense given that
the roms will not be generic but host-specific, so end-users will have
to install the tools needed to collect host-specific bits and to build
the final rom image.

take care,
  Gerd

Re: [PATCH] OvmfPkg: reserve igd memory by E820

[Corvin Köhne]

Hi,

> > I don't know if it's possible to extract it from host firmware within
> > OS context easily.
>
> Extracting from firmware updates is probably easier.

Good idea but I'll focus on the rest first because I have access to
the GOP driver for my systems.

> > According to Intel you should ask your board manufacturer.
>
> I'm wondering why if the opregion initialization (which actually is
> board-specific) is not done by the gop driver.

>From my experience and as far as I know, Intels GOP driver seems to be
platform independent. I think only Intel can tell us why.

> > > Once we have the code for vgabios and PlatformGopPolicy we can roll them
> > > with the intelgop driver into a rom image with EfiRom.  Ideally also add
> > > opregion content to the rom somehow.
> >
> > Sounds good. Does these EFI drivers have to be maintained outside of
> > EDKII?
>
> Hosting this in https://github.com/tianocore/edk2-staging is probably
> one option (not fully sure about the policy there).
>
> Creating a separate project for that might also make sense given that
> the roms will not be generic but host-specific, so end-users will have
> to install the tools needed to collect host-specific bits and to build
> the final rom image.

I'm going to start working on that locally. Maybe while working on it,
I'll have an idea where's the best place to maintain it.


Best regards
Corvin

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075