Re: [Qemu-devel] [edk2] [edk2 PATCH 0/1] OvmfPkg: grab ACPI tables from QEMU

[Laszlo Ersek <lersek@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

On 11/19/13 09:54, Gerd Hoffmann wrote:
>   Hi,
>
>>   ACPI PciWindow32: Base=0xA0000000 End=0xFEEFFFFF Length=0x5EF00000
>
>>   begin32=c0000000 end32=fec00000 begin64=0 end64=0
>
> qemu & seabios pick a 32bit window size which allows to cover it with
> a single mtrr entry.  Size must be a power of two for that to work.
>
>   [root@fedora ~]# cat /proc/mtrr
>   reg00: base=0x080000000 ( 2048MB), size= 2048MB, count=1: uncachable
>
> So we have three cases for piix (as you've figured in the source code
> already).  Start at 0x80000000 (2G window), 0xc0000000 (1G window) and
> 0xe0000000 (512M window).
>
> btw: q35 has a fixed 1G window, max low ram addr is 0xb000000, the
> remaining address space (0xb0000000 -> 0xc0000000) is used for
> mmconfig.
>
>> I guess the range starting at 0xc0000000 is somehow "incompatible"
>> with the EFI memory map. (I can't actually explain this idea because,
>> again, this second range is a proper subset of the former, and its
>> size is still 1004MB.)
>
> Probably efi places the gfx memory bar somewhere between 0xa0000000
> and 0xc0000000.  Sets up efifb accordingly.  Then the linux kernel
> loads and figures "Oh, that bar is outside the 0xc0000000+ window" and
> goes remap it -> efifb writes go into nowhere.

Thank you for the explanation.

How do I fix it though? :) I could change the MMIO HOBs in PEI
(OvmfPkg/PlatformPei/Platform.c):

  //
  // Video memory + Legacy BIOS region
  //
  AddIoMemoryRangeHob (0x0A0000, BASE_1MB);

  //
  // address       purpose   size
  // ------------  --------  -------------------------
  // max(top, 2g)  PCI MMIO  0xFC000000 - max(top, 2g)
  // 0xFC000000    gap                           44 MB
  // 0xFEC00000    IO-APIC                        4 KB
  // 0xFEC01000    gap                         1020 KB
  // 0xFED00000    HPET                           1 KB
  // 0xFED00400    gap                         1023 KB
  // 0xFEE00000    LAPIC                          1 MB
  //
  AddIoMemoryRangeHob (TopOfMemory < BASE_2GB ? BASE_2GB : TopOfMemory, 0xFC000000);
  AddIoMemoryBaseSizeHob (0xFEC00000, SIZE_4KB);
  AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB);
  AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);

to imitate the same three cases. The HOB with the lowest address
produced here would affect the BAR placement as well.

... Yes, I tested the attached patch, and it makes the display work in
both 2560MB guests.

However, I don't like the idea of hardwiring those boundaries here. (The
current values are also hardwired, but they are better: they are not the
consequence of "SeaBIOS has done it like this forever" -- inter-firmware
dependency, especially when they aren't each other's payloads, is bad
IMHO.) We'd need something dynamic here, like a memory map from qemu.

... Which puts us in the same boat with Wei :)

Thanks
Laszlo

[-- Attachment #2: 0001-OvmfPkg-PlatformPei-follow-SeaBIOS-tradition-with-32.patch --]
[-- Type: text/plain, Size: 1679 bytes --]

From 9cf2af82399d7d7a9717ff6ac17860b66c705a64 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Tue, 19 Nov 2013 13:07:41 +0100
Subject: [PATCH] OvmfPkg/PlatformPei: follow SeaBIOS tradition with 32-bit PCI
 hole placement

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/PlatformPei/Platform.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index fb56e99..5bc0d74 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -197,7 +197,7 @@ MemMapInitialization (
   //
   // address       purpose   size
   // ------------  --------  -------------------------
-  // max(top, 2g)  PCI MMIO  0xFC000000 - max(top, 2g)
+  // 2G/3G/3.5G    PCI MMIO    0xFC000000 - 2G/3G/3.5G
   // 0xFC000000    gap                           44 MB
   // 0xFEC00000    IO-APIC                        4 KB
   // 0xFEC01000    gap                         1020 KB
@@ -205,7 +205,9 @@ MemMapInitialization (
   // 0xFED00400    gap                         1023 KB
   // 0xFEE00000    LAPIC                          1 MB
   //
-  AddIoMemoryRangeHob (TopOfMemory < BASE_2GB ? BASE_2GB : TopOfMemory, 0xFC000000);
+  AddIoMemoryRangeHob (TopOfMemory <= 0x80000000 ? 0x80000000 :
+                       TopOfMemory <= 0xC0000000 ? 0xC0000000 :
+                       0xE0000000, 0xFC000000);
   AddIoMemoryBaseSizeHob (0xFEC00000, SIZE_4KB);
   AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB);
   AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
-- 
1.8.3.1