Re: [RFC PATCH 07/12] hvmloader: allocate MMCONFIG area in the MMIO hole + minor code refactoring

[Alexey G <x1917x@gmail.com>]

On Wed, 28 Mar 2018 10:03:29 +0000
Paul Durrant <Paul.Durrant@citrix.com> wrote:
>> >IMO a single entity should be in control of the memory layout, and
>> >that's the toolstack.
>> >
>> >Ideally we should not allow the firmware to change the layout at
>> >all.  
>> 
>> This approach is terribly wrong, I don't know why opinions like this
>> so common at Citrix. The toolstack is a least informed side. If
>> MMIO/memory layout should be immutable, it must be calculated
>> considering all factors, like chipset-specific MMIO ranges or ranges
>> which cannot be used for the MMIO hole.
>>   
>
>Why is this approach wrong? Code running in the guest is
>non-privileged and we really don't want it messing around with memory
>layout. We really want to be in a position to e.g. build ACPI tables
>in the toolstack and we cannot do this until the layout becomes
>immutable.

Only firmware code in the guest can correctly determine guest's MMIO
hole requirements (BIOS typically, but hvmloader in our case).

It is impossible to do in the toolstack at the moment, because it
doesn't know at least
- MMIO BAR sizes of device-model's PCI devices
- chipset-specific MMIO ranges the DM emulates for a chosen machine
- the way these ranges are allocated to the MMIO hole by guest firmware

Even providing some interface to query all related information from a
device model won't cover the problem how firmware will be allocating
these ranges to the MMIO hole. Any code (or version) change can make
the toolstack's expectations wrong ->

>> We need to know all resource requirements of device-model's and PT
>> PCI devices, all chipset-specific MMIO ranges (which belong to a
>> device model), all RMRRs (host's property) and all device-model
>> invisible ranges like VRAM backing store (another device model's
>> property).  
>
>Yes, indeed we do.
>
>> And we need to know in which manner hvmloader will be allocating BARs
>> to the MMIO hole -- eg. either in a forward direction starting from
>> some base or moving backwards from the end of 4Gb (minus hardcoded
>> ranges).  
>
>Eventually we want to get rid of hvmloader.

...especially if BAR allocation will be delegated from hvmloader to
other firmware like SeaBIOS/OVMF.

> Why do we need to know
>anything about its enumeration of BARs? After all they could be
>completely re-enumerated by the guest OS during or after boot (and
>indeed Windows does precisely that).

You probably confuse BAR assignment with BAR values enumeration.

Windows reallocates all PCI BARs in specific conditions only, they call
this feature 'PCI resources rebalancing'. Normally it sticks to the PCI
BAR allocation setup provided by firmware (hvmloader in our case). BAR
reallocation doesn't really matter as long as we have a correct MMIO
hole size.

The very last thing a user needs to do is guessing the correct value of
the mmio_hole_size parameter, which value will be ok for all his PT
devices while being not too large at the same time to leave more RAM
for 32-bit guests.

Those 32-bit guests are most problematic for MMIO hole sizing. We should
try to keep the MMIO hole size as small as possible to reduce RAM losses
while at the same time we are not permitted to allocate any BARs to the
high MMIO hole -- moving 64-bit BARs above 4Gb will automatically make
such devices non-functional for 32-bit guests.

This means we need to calculate the precise MMIO hole size, according
to all factors. And only firmware code in the guest can do it right.

>> Basically this means that we have to depend on hvmloader code/version
>> too in the toolstack, which is wrong on its own -- we should have a
>> freedom to modify the BAR allocation algo in hvmloader at any time.
>>   
>
>It should be irrelevant. The toolstack should decide on the sizes and
>locations of the MMIO holes and they should remain fixed, and be
>enforced by Xen. This avoids issues that we currently have such as
>guests populating RAM inside MMIO holes.

The toolstack can't do it. There should be some one-time way to
communicate the MMIO hole setup between Xen and hvmloader. And after
that we can make it immutable.

Write-once interface via emulated platform registers (either designated
for this purpose or any arbitrarily chosen) is a safe approach. We have
full control of what can be provided or allowed to guest firmware via
this interface. A dedicated hypercall should be ok too, but it's a bit
overkill.

What is definitely not safe is to allow hvmloader to use the
add_to_physmap hypercall.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel