[edk2,edk2,0/1] OvmfPkg: grab ACPI tables from QEMU

On 11/12/13 16:11, Laszlo Ersek wrote:

> - Boot progress bar and console (efifb) continue to work (tested with
>   both 1GB and 5GB guest sizes).

Turns out one can't be diligent enough.

This patch causes (or exposes, dependent on your POV) breakage. It
breaks the cirrus video output for both RHEL-6 and Windows 2012 R2
guests, very early during boot, *if* the guest RAM size is 2560 MB.

(That amount is just an example, but a good example (= an esp. bad value).)

When the guest has this much RAM, then the built-in OVMF algorithm
advertises the following 32-bit PCI window (no 64-bit PCI window) in
\_SB.PCI0._CRS:

  ACPI PciWindow32: Base=0xA0000000 End=0xFEEFFFFF Length=0x5EF00000
  ACPI PciWindow64: Base=0x00000000 End=0x00000000 Length=0x00000000

(inclusive End). That is, the windows starts right above the
conventional memory; 0xA0000000 == 2560M.

However, the ACPI table exported by qemu says

  begin32=c0000000 end32=fec00000 begin64=0 end64=0

(exclusive end32).

Even though this latter range is a proper subset of the former, it
breaks the cirrus display in both said guests (didn't test more guests).

When I applied this PoC patch on qemu, the displays started to work:

f->pci_address_space,

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.)

The comment before the w32.begin assignments is interesting. It dates
back to qemu commit 3459a625 ("pci: store PCI hole ranges in guestinfo
structure"). It could be incompatible with the *current* OVMF memory
map, but of course the OVMF memory map could be wrong as well.

I've no idea how to fix this disagreement.

Thanks
Laszlo

On Tue, 19 Nov 2013 00:34:31 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> On 11/12/13 16:11, Laszlo Ersek wrote:
> 
> > - Boot progress bar and console (efifb) continue to work (tested with
> >   both 1GB and 5GB guest sizes).
> 
> Turns out one can't be diligent enough.
> 
> This patch causes (or exposes, dependent on your POV) breakage. It
> breaks the cirrus video output for both RHEL-6 and Windows 2012 R2
> guests, very early during boot, *if* the guest RAM size is 2560 MB.
> 
> (That amount is just an example, but a good example (= an esp. bad value).)
> 
> When the guest has this much RAM, then the built-in OVMF algorithm
> advertises the following 32-bit PCI window (no 64-bit PCI window) in
> \_SB.PCI0._CRS:
> 
>   ACPI PciWindow32: Base=0xA0000000 End=0xFEEFFFFF Length=0x5EF00000
looks wrong, it should honor window advertised by QEMU exported ACPI table.

>   ACPI PciWindow64: Base=0x00000000 End=0x00000000 Length=0x00000000
> 
> (inclusive End). That is, the windows starts right above the
> conventional memory; 0xA0000000 == 2560M.
> 
> However, the ACPI table exported by qemu says
> 
>   begin32=c0000000 end32=fec00000 begin64=0 end64=0
> 
> (exclusive end32).
> 
> Even though this latter range is a proper subset of the former, it
> breaks the cirrus display in both said guests (didn't test more guests).
> 
> When I applied this PoC patch on qemu, the displays started to work:
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index edc974e..305b786 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -348,11 +348,11 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>      /* Set PCI window size the way seabios has always done it. */
>      /* Power of 2 so bios can cover it with a single MTRR */
>      if (ram_size <= 0x80000000) {
>          i440fx->pci_info.w32.begin = 0x80000000;
>      } else if (ram_size <= 0xc0000000) {
> -        i440fx->pci_info.w32.begin = 0xc0000000;
> +        i440fx->pci_info.w32.begin = 0xa0000000;
You cant just move it for compatibility reasons,
since similar code is hardcoded in Seabios since forever.


>      } else {
>          i440fx->pci_info.w32.begin = 0xe0000000;
>      }
> 
>      memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole",
> f->pci_address_space,
> 
> 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.)
> 
> The comment before the w32.begin assignments is interesting. It dates
> back to qemu commit 3459a625 ("pci: store PCI hole ranges in guestinfo
> structure"). It could be incompatible with the *current* OVMF memory
> map, but of course the OVMF memory map could be wrong as well.
> 
> I've no idea how to fix this disagreement.
> 
> Thanks
> Laszlo
>

On 11/19/13 01:04, Igor Mammedov wrote:
> On Tue, 19 Nov 2013 00:34:31 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 11/12/13 16:11, Laszlo Ersek wrote:
>>
>>> - Boot progress bar and console (efifb) continue to work (tested with
>>>   both 1GB and 5GB guest sizes).
>>
>> Turns out one can't be diligent enough.
>>
>> This patch causes (or exposes, dependent on your POV) breakage. It
>> breaks the cirrus video output for both RHEL-6 and Windows 2012 R2
>> guests, very early during boot, *if* the guest RAM size is 2560 MB.
>>
>> (That amount is just an example, but a good example (= an esp. bad value).)
>>
>> When the guest has this much RAM, then the built-in OVMF algorithm
>> advertises the following 32-bit PCI window (no 64-bit PCI window) in
>> \_SB.PCI0._CRS:
>>
>>   ACPI PciWindow32: Base=0xA0000000 End=0xFEEFFFFF Length=0x5EF00000
> looks wrong, it should honor window advertised by QEMU exported ACPI table.

Sorry, I wasn't clear enough.

When you run OVMF without this patch on any qemu version, or when you
run OVMF with this patch on a qemu that doesn't export the
/etc/acpi/tables fw_cfg file, then OVMF relies on its own code to
determine the window boundaries, and installs DSDT+SSDT tables of its
own making. The values I quoted above originate from this OVMF
functionality.

>>   ACPI PciWindow64: Base=0x00000000 End=0x00000000 Length=0x00000000
>>
>> (inclusive End). That is, the windows starts right above the
>> conventional memory; 0xA0000000 == 2560M.
>>
>> However, the ACPI table exported by qemu says
>>
>>   begin32=c0000000 end32=fec00000 begin64=0 end64=0
>>
>> (exclusive end32).
>>
>> Even though this latter range is a proper subset of the former, it
>> breaks the cirrus display in both said guests (didn't test more guests).
>>
>> When I applied this PoC patch on qemu, the displays started to work:
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index edc974e..305b786 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -348,11 +348,11 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>>      /* Set PCI window size the way seabios has always done it. */
>>      /* Power of 2 so bios can cover it with a single MTRR */
>>      if (ram_size <= 0x80000000) {
>>          i440fx->pci_info.w32.begin = 0x80000000;
>>      } else if (ram_size <= 0xc0000000) {
>> -        i440fx->pci_info.w32.begin = 0xc0000000;
>> +        i440fx->pci_info.w32.begin = 0xa0000000;
> You cant just move it for compatibility reasons,
> since similar code is hardcoded in Seabios since forever.

Only since commit b1c35f2b ("pciinit: Align start of PCI memory on i440
chipset."), Nov 26, 2012. The only justification given is "simplify mtrr
ranges".

The problem is, when OVMF forwards qemu's tables to the OS, rather than
creating and installing its own tables for the OS, the video output
breaks in the OS.

Anyway, as qemu owns (actually, qemu *is*) the hardware, and it deems
the above range appropriate, OVMF must accept it. The bug reproduces
with the RHEL-6 guest too, so I hope I can look into it. (Early dmesg on
serial would be a start.)

Thanks
Laszlo

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.

cheers,
  Gerd