Re: xen: memory initialization/balloon fixes (#3)

[David Vrabel <david.vrabel@citrix.com>]

On 24/09/11 03:08, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 15, 2011 at 01:29:21PM +0100, David Vrabel wrote:
>> This set of patches fixes some bugs in the memory initialization under
>> Xen and in Xen's memory balloon driver.  They can make 100s of MB of
>> additional RAM available (depending on the system/configuration).
>>
>> Patch 1 is already applied.
>>
>> Patch 2 fixes a bug in patch 1 and should be queued for 3.1 (and along
>> with patch 1 considered for 3.0 stable).
>>
>> Patch 3 is a bug fix and should be queued for 3.1 and possibly
>> queued for the 3.0 stable tree.
>>
>> Patches 5 & 6 increase the amount of low memory in 32 bit domains
>> started with < 1 GiB of RAM.  Please queue for 3.2
>>
>> Patch 7 releases all pages in the initial allocation with PFNs that
>> lie within a 1-1 mapping.  This seems correct to me as I think that
>> once the 1-1 mapping is set the MFN of the original page is lost so
>> it's no longer accessible by the kernel (and it cannot be used by
>> another domain
>>
>> Changes since #2:
>>
>> - New patch: xen: avoid adding non-existant memory if the reservation
>>   is unlimited
>> - Avoid using a hypercall to get the current number of pages in the
>>   ballon driver.  Apparently the hypercall won't return the right
>>   value if paging is used.
>> - Addresses Konrad's review comments.
> 
> They don't work on AMD boxes:

It's not specific to AMD boxes.

> (XEN) Xen-e820 RAM map:
> (XEN)  0000000000000000 - 000000000009d800 (usable)

It's because it's not correctly handling the half-page of RAM at the end
of this region.

I don't have access to any test boxes with a dodgy BIOS like this so can
you test this patch?  If it works I'll fold it in and post an updated
series.

Can you remember why this page alignment was required?  I'd like to
update the comment with the reason because the bare-metal x86 memory
init code doesn't appear to fixup the memory map in this way.

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 986661b..e473c4c 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -178,6 +178,19 @@ static unsigned long __init xen_get_max_pages(void)
 	return min(max_pages, MAX_DOMAIN_PAGES);
 }

+static void xen_e820_add_region(u64 start, u64 size, int type)
+{
+	u64 end = start + size;
+
+	/* Align RAM regions to page boundaries. */
+	if (type == E820_RAM || type == E820_UNUSABLE) {
+		start = PAGE_ALIGN(start);
+		end &= ~((u64)PAGE_SIZE - 1);
+	}
+
+	e820_add_region(start, end - start, type);
+}
+
 /**
  * machine_specific_memory_setup - Hook for machine specific memory setup.
  **/
@@ -253,10 +266,6 @@ char * __init xen_memory_setup(void)
 		u32 type = map[i].type;

 		if (type == E820_RAM) {
-			/* RAM regions must be page aligned. */
-			size -= (addr + size) % PAGE_SIZE;
-			addr = PAGE_ALIGN(addr);
-
 			if (addr < mem_end) {
 				size = min(size, mem_end - addr);
 			} else if (extra_pages) {
@@ -267,7 +276,7 @@ char * __init xen_memory_setup(void)
 				type = E820_UNUSABLE;
 		}

-		e820_add_region(addr, size, type);
+		xen_e820_add_region(addr, size, type);

 		map[i].addr += size;
 		map[i].size -= size;


David