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

[Michael S. Tsirkin]

On Wed, Aug 07, 2013 at 04:21:44PM +0200, Gerd Hoffmann wrote:
> 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.
> 

We need to fix it not to panic, but we put the user in control since
windows size can now come from user.

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

The logic is in hw/i386/pc_piix.c and always was.

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

That's exactly what we have. However QEMU is the one that knows
how hardware is configured (e.g. where's the
PCI hole), so we report that to bios.

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

Only for some hardware (Q35).

> 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 we don't need them in pci-config, just stick constant in ACPI.

> 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

That's not the only issue.
Another issue is where to start/end the 32 bit window for PIIX.
That's really a PV thing for QEMU, hardcoded at the moment in bios
and QEMU.

-- 
MST