Re: [Qemu-devel] [PATCH] q35: split memory at 2G

[Laszlo Ersek]

On 05/29/19 06:47, Gerd Hoffmann wrote:
> On Tue, May 28, 2019 at 10:49:55PM -0400, Michael S. Tsirkin wrote:
>> On Wed, May 29, 2019 at 03:21:16AM +0200, Paolo Bonzini wrote:
>>> On 28/05/19 22:48, Gerd Hoffmann wrote:
>>>> Original q35 behavior was to split memory 2.75 GB, leaving space for the
>>>> mmconfig bar at 0xb000000 and pci I/O window starting at 0xc0000000.
>>>>
>>>> Note: Those machine types have been removed from the qemu codebase
>>>> meanwhile because they could not be live-migrated so there was little
>>>> value in keeping them around.
>>>>
>>>> With the effort to allow for gigabyte-alignment of guest memory that
>>>> behavior was changed:  The split was moved to 2G, but only in case the
>>>> memory didn't fit below 2.75 GB.
>>>>
>>>> So today the address space between 2G and 2,75G is not used for guest
>>>> memory in typical use cases, where the guest memory sized at a power of
>>>> two or a gigabyte number.  But if you configure your guest with some odd
>>>> amout of memory (such as 2.5G) the address space is used.
>>>
>>> Wasn't it done to ensure pre-PAE OSes could use as much memory as
>>> possible?  (If you run pre-PAE OSes with more RAM than can fit below 4G,
>>> you can just reduce the amount of memory and get all the 2.75G).
>>>
>>> Paolo
>>
>> Absolutely. Gerd is just saying the configuration is rare enough that
>> it's not worth worrying about. I don't know myself - why do
>> we bother making this change? What's the advantage?
> 
> Some ovmf versions place the mmconfig @ 2G.  Which works fine in 99% of
> the cases, but with memory sizes between 2G and 2.75G it doesn't.

Here's the stages of PCIEXBAR placement in OVMF:

#1 Commit 7b8fe63561b4 ("OvmfPkg: PlatformPei: enable PCIEXBAR (aka
MMCONFIG / ECAM) on Q35", 2016-03-10): places the PCIEXBAR at 2GB.
Behaves according to your description.

#2 Commit 75136b29541b ("OvmfPkg/PlatformPei: reorder the 32-bit PCI
window vs. the PCIEXBAR on q35", 2019-05-16): made for the 1% of cases
(according to your description) where the previous logic would fail,
namely with RAM between 2G and 2.75G. Places the PCIEXBAR at
0xE000_0000, and the 32-bit MMIO window below it. Unfortunately, this
causes a regression for end-users, because this ordering of areas
triggers a bug in QEMU's ACPI generator somewhere. The 32-bit MMIO
window will be clamped *above* 0xF000_0000, when it should actually end
at 0xE000_0000. Causes confusion for some guest OSes (the mildest
symptom is PCI resource reassignment).

#3 Patch series linked in
<https://bugzilla.tianocore.org/show_bug.cgi?id=1859#c1>: restores the
original order (so as to pacify the ACPI generator in QEMU). Places the
PCIEXBAR at 0xB000_0000. Places the 32-bit MMIO window at 0xC000_0000.
This wastes a bit of 32-bit MMIO space, but it has the best
compatibility. The cases under which the waste occurs are: (a) pre-4.1
Q35 machine types with low RAM side *outside* of 2GB..2.75GB, and (b)
4.1+ Q35 machine types (with this patch applied), regardless of low RAM
size. Effectively this patch set returns to the first variant, except it
bumps the PCIEXBAR base from 2GB to 2.75GB.

Once all pre-4.1 Q35 machine types can be considered obsolete, we can
return OVMF to option#1 again. Until then, it's best to fix the host
side and the guest side both, for best compatibility for end-users.
(This is generally what we do, i.e. fix both sides, when there is a
host-guest disagreement.)

So, for this patch:

Acked-by: Laszlo Ersek <address@hidden>

I'll also push (soon) the edk2 patches for option#3 above.

(

Things I haven't discussed here: (a) why we'd like to continue
specifying the PXIEXBAR base in OVMF as a build-time constant (because
making it dynamic might introduce complications for module dispatch
order), (b) MTRR aspects (both the PCIEXBAR and the 32-bit MMIO window
should be marked UC through variable MTRRs, and while SeaBIOS currently
ignores the first, I wouldn't like to, in OVMF). All of options #1
through #3 can be made work correctly wrt. variable MTRRs, but *some*
OVMF patches are necessary for either of those.

)

Thanks
Laszlo