[SeaBIOS] [PATCH] acpi: hide 64-bit PCI hole for Windows XP

[Gerd Hoffmann]

On 08/07/13 14:35, Michael S. Tsirkin wrote:
> On Wed, Aug 07, 2013 at 02:15:14PM +0200, Gerd Hoffmann wrote:
>> qemu doesn't error out though, so what is the point?
> 
> The point is management can set the size according
> to the type of guest that will be installed.
> You can misconfigure qemu in a variety of ways
> and not all of them cause it to error out.

Then seabios will still panic.  I fail to see how pci-info makes things
more user-friendly as you've claimed a few mails back.

No doubt about the need to configure the 64bit window size.

>>> The one setting the PCI hole addresses :)
>>
>> That wasn't duplicated before you've added pci-info.
> 
> It was always duplicated. With pci-info it's *not* duplicated anymore:
> qemu is the single source of info.

Huh?  The 32bit window is sized according to the installed memory.  That
logic is in seabios and you'll try to move it to qemu, using pci-info.
It wasn't in qemu before ...

>>> If we do this, we can take the time with designing the
>>> proper fix for winXP - this just shows
>>> that pci-info is the right approach.
>>
>> This only proves that you can fix the bug introduced by adding pci-info.
>>
>> Current seabios which completely ignores pci-info works equally well
>> with WinXP.
> 
> It doesn't really, and the issue is not WinXP.
> 
> Here's a bug, it's easy to reproduce:
> 	1. boot qemu with existing seabios, linux guest
> 	2. add device with large BAR by hotplug (e.g. ivshmem)

Yes, to enable large bar hotplug we need some way to configure the size
of the 64bit pci window.  I didn't question that.

>>> Yes.  It does exactly what's needed to make sure PCI windows
>>> match the addresses that hardware listens on.
>>
>> seabios handles this just fine today.
> 
> Only because qemu and seabios have identical code
> for PCI window. But that code has no basis in reality,
> if we fix QEMU to be closer to real hardware it will break.

What exactly?  q35 has fixed addresses for the 32bit window, do you mean
that?  If needed we can make it look at the installed low (below 4g)
memory instead, like piix4 does.

>> The only missing bit is some way to request seabios add a 64bit window
>> (of a certain size) unconditionally, so it is there even in case it
>> isn't needed for the devices present at boot time.  A simple
>> etc/pci-64bit-size integer would do the job.
>>
>> cheers,
>>   Gerd
> 
> And then it will have exactly the same issue if you misconfigure it.
> The 64bit hole size is the only thing that crashes old windows.
> So if you want that, then what's your problem with pci-info really?

pci-info it has stuff which shouldn't be there IMO.


It is the job of the firmware to initialize the hardware.  IMO it should
work that way on qemu too to mimic real hardware.

The firmware can initialize the hardware as it likes.  For the most part
the OS can simply read the configuration from the pci config space.

Some hardware addresses are in the acpi tables though.  Current seabios
generates/patches acpi tables to handle that.

With qemu providing the acpi tables we need some other way to handle
that.  You are trying to tackle that by sticking the addresses into
pci-info, then expecting the firmware using them.  I think we should
handle that by extending COMMAND_ADD_POINTER, so the firmware can
continue to pick the addresses as it pleases.

The addresses we are talking about here are:

  (1) 32bit pci window start + end + size (for PCI0._CRS)
  (2) 64bit pci window start + end + size (likewise)
  (3) mmconf xbar start (MCFG, q35 only, at 0xb0000000 now).
  (4) pmbase (FADT, at 0xb000 now).

Especially 3+4 tend to be compile-time constants in the firmware as they
are needed very early in the setup process.  So making them runtime
configurable via fw_cfg (say by adding them to pci-info) isn't easy.
Thats why I think we better should do it the other way around, let
firmware pick the addresses and define some way to fixup the acpi tables.

pci-info has the side effect that you can configure the pci window size
that way, so we'll need a replacement for that part when dropping pci-info.

cheers,
  Gerd