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

[Laszlo Ersek]

On 11/19/13 01:04, Igor Mammedov wrote:
> On Tue, 19 Nov 2013 00:34:31 +0100
> Laszlo Ersek <address@hidden> 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