Re: [PATCH v2 1/5] hvmloader: Correct bug in low mmio region accounting

[George Dunlap <george.dunlap@eu.citrix.com>]

On 06/18/2013 05:46 PM, George Dunlap wrote:
> When deciding whether to map a device in low MMIO space (<4GiB),
> hvmloader compares it with "mmio_left", which is set to the size of
> the low MMIO range (pci_mem_end - pci_mem_start).  However, even if it
> does map a device in high MMIO space, it still removes the size of its
> BAR from mmio_left.
>
> This patch first changes the name of this variable to "low_mmio_left"
> to distinguish it from generic MMIO, and corrects the logic to only
> subtract the size of the BAR for devices maped in the low MMIO region.
>
> Also make low_mmio_left unsigned, and don't allow it to go negative.
> Since its main use is to be compared to a 64-bit unsigned int, this
> may have undefined (and in practice almost certainly incorrect)
> results.  Not subtracting is OK because if there's not enough room, it
> won't actually be mapped.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

This patch series is a lot longer and more intrusive than I was hoping 
it would be. :-(

However, it still seems to me that this will be a smaller impact on 
people than the alternatives.

If the code is done properly, then none of these changes should have any 
effect on people who are not passing through devices with large BARs: 
the vast majority of our users, who are simply booting plain VMs in HVM 
mode, should see zero effect.

Similarly, the vast majority of qemu-traditional users should also see 
no effect; the 64-bit code should only be invoked when the MMIO hole 
cannot be resized large enough, and by default, qemu-traditional will be 
able to resize the hole up to 2GiB -- far larger than almost any guest 
should need.

qemu-xen users will be affected, but they would have been affected 
anyway.  This changes a certain "will 100% crash at some point" to "will 
probably work most of the time, with a small unforeseen chance of 
something crashing randomly".  I think that's still a step forward.

  -George