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

[Igor Mammedov <imammedo@redhat.com>]

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
> 


-- 
Regards,
  Igor