Re: [PATCH 2/3] xen: allow extra memory to be two regions

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Aug 16, 2011 at 03:33:19PM +0100, David Vrabel wrote:
> On 16/08/11 14:48, Konrad Rzeszutek Wilk wrote:
> > On Tue, Aug 16, 2011 at 11:00:37AM +0100, David Vrabel wrote:
> >> Allow the extra memory (used by the balloon driver) to be in two
> >> regions (typically low and high memory).  This allows the balloon
> >> driver to increase the number of available low pages (if the initial
> >> number if pages is small).
> >>
> >> As a side effect, the algorithm for building the e820 memory map is
> >> simpler and more obviously correct as the map supplied by the
> >> hypervisor is (almost) used as is.
> > 
> > Hm, which is not always good. The setting of 'E820_RESERVED' and 'E820_UNUSABLE',
> > and realigning of start of balloon space at 4GB (if necessary) changes
> > need to be preserved. You can look up the why if you run 'git annotate'
> > and look at those lines - we had lots of time getting those right.
> 
> My understanding of the history is that the problems were caused by not
> paying attention to the reserved regions reported in the machine memory

That might have been a problem too, but this is specific to RAM regions.
> map.  This proposed algorithm is careful to only alter RAM regions --
> all reserved regions and gaps are preserved as-is.  I should add some
> comments explaining this.

We cut RAM regions down and the Linux code thought that they were "gap" spaces
and used it as PCI I/O space.  Hence we marked them as unusable. We need
that behavior.
> 
> For example, should a BIOS reserve memory above 4 GiB then the current
> code will place the balloon memory over the top of it but the proposed
> code will not.

The problem with the 4GB was that the Local IOAPIC was not enumerated
in the E820 but was in the ACPI boot table. This meant that E820 had
a "gap" right before the 4GB limit and we thought it was the start of RAM
and marked it to be used as balloon space.

> 
> > You also need to provide the virgin copy of the E820 to the xen_set_identity
> > and not use the same version that is modified - which with the above setting
> > won't work.
> 
> Because we don't alter any reserved regions the resulting map is fine
> for this.
> 
> > I am curious - with the patch to the hypervisor - and with just a newly
> > implemented xen_get_max_pages() code path added to query the new
> > truncated amount of how many pages we need - wont that solve the problem?
> 
> I'll address this in another email.
> 
> David