fix for "xen: use maximum reservation to limit amount of usable RAM" - patch titled: "xen/e820: if there is not dom0_mem, don't tweak extra_pages."

fix for "xen: use maximum reservation to limit amount of usable RAM" - patch titled: "xen/e820: if there is not dom0_mem, don't tweak extra_pages."

[Konrad Rzeszutek Wilk]

.breaks one of my boxes (Core i3-2100), with Xen 4.1.1 (with and w/out the
23790 changset in it).

I've traced it down to the fact that I booted my dom0 without
dom0_mem=X flag with a machine that has more than 8GB. Weirdly enough
I can only reproduce this under Intel boxes.

Anyhow this patch fixes it for me.

commit e4297f5719e982d95788cd53e284a7a389eedb45
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date:   Mon Sep 12 15:58:25 2011 -0400

    xen/e820: if there is not dom0_mem, don't tweak extra_pages.
    
    The patch "xen: use maximum reservation to limit amount of usable RAM"
    (d312ae878b6aed3912e1acaaf5d0b2a9d08a4f11) breaks machines that
    do not use 'dom0_mem=' argument with:
    
    reserve RAM buffer: 000000133f2e2000 - 000000133fffffff
    (XEN) mm.c:4976:d0 Global bit is set to kernel page fffff8117e
    (XEN) domain_crash_sync called from entry.S
    (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
    ...
    The reason being that the last E820 entry is created using the
    'extra_pages' (which is based on how many pages have been freed).
    The mentioned git commit sets the initial value of 'extra_pages'
    using a hypercall which returns the number of pages (if dom0_mem
    has been used) or -1 otherwise. If the later we return with
    MAX_DOMAIN_PAGES as basis for calculation:
    
        return min(max_pages, MAX_DOMAIN_PAGES);
    
    and use it:
    
         extra_limit = xen_get_max_pages();
         if (extra_limit >= max_pfn)
                 extra_pages = extra_limit - max_pfn;
         else
                 extra_pages = 0;
    
    which means we end up with extra_pages = 128GB in PFNs (33554432)
    - 8GB in PFNs (2097152, on this specific box, can be larger or smaller),
    and then we add that value to the E820 making it:
    
      Xen: 00000000ff000000 - 0000000100000000 (reserved)
      Xen: 0000000100000000 - 000000133f2e2000 (usable)
    
    which is clearly wrong. It should look as so:
    
      Xen: 00000000ff000000 - 0000000100000000 (reserved)
      Xen: 0000000100000000 - 000000027fbda000 (usable)
    
    Naturally this problem does not present itself if dom0_mem=max:X
    is used.
    
    CC: David Vrabel <david.vrabel@citrix.com>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index c3b8d44..0632de1 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -185,16 +185,18 @@ static unsigned long __init xen_set_identity(const struct e820entry *list,
 	return identity;
 }
 
-static unsigned long __init xen_get_max_pages(void)
+static bool __init xen_get_max_pages(unsigned long *max_pages)
 {
-	unsigned long max_pages = MAX_DOMAIN_PAGES;
 	domid_t domid = DOMID_SELF;
 	int ret;
 
 	ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
-	if (ret > 0)
-		max_pages = ret;
-	return min(max_pages, MAX_DOMAIN_PAGES);
+	/* If dom0_mem=X is not used, it will return -1. */
+	if (ret > 0) {
+		*max_pages = (unsigned long)min(ret, MAX_DOMAIN_PAGES);
+		return true;
+	}
+	return false;
 }
 
 /**
@@ -210,7 +212,7 @@ char * __init xen_memory_setup(void)
 	int rc;
 	struct xen_memory_map memmap;
 	unsigned long extra_pages = 0;
-	unsigned long extra_limit;
+	unsigned long extra_limit = 0;
 	unsigned long identity_pages = 0;
 	int i;
 	int op;
@@ -305,11 +307,12 @@ char * __init xen_memory_setup(void)
 
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
 
-	extra_limit = xen_get_max_pages();
-	if (extra_limit >= max_pfn)
-		extra_pages = extra_limit - max_pfn;
-	else
-		extra_pages = 0;
+	if (xen_get_max_pages(&extra_limit)) {
+		if (extra_limit >= max_pfn)
+			extra_pages = extra_limit - max_pfn;
+		else
+			extra_pages = 0;
+	}
 
 	extra_pages += xen_return_unused_memory(xen_start_info->nr_pages, &e820);

Re: fix for "xen: use maximum reservation to limit amount of usable RAM" - patch titled: "xen/e820: if there is not dom0_mem, don't tweak extra_pages."

[David Vrabel]

On 12/09/11 21:13, Konrad Rzeszutek Wilk wrote:
> .breaks one of my boxes (Core i3-2100), with Xen 4.1.1 (with and w/out the
> 23790 changset in it).
> 
> I've traced it down to the fact that I booted my dom0 without
> dom0_mem=X flag with a machine that has more than 8GB. Weirdly enough
> I can only reproduce this under Intel boxes.
> 
> Anyhow this patch fixes it for me.

I think this patch is simpler.  Does it fix the issue?

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index c3b8d44..46d6d21 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -306,10 +306,12 @@ char * __init xen_memory_setup(void)
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
 
 	extra_limit = xen_get_max_pages();
-	if (extra_limit >= max_pfn)
-		extra_pages = extra_limit - max_pfn;
-	else
-		extra_pages = 0;
+	if (max_pfn + extra_pages > extra_limit) {
+		if (extra_limit > max_pfn)
+			extra_pages = extra_limit - max_pfn;
+		else
+			extra_pages = 0;
+	}
 
 	extra_pages += xen_return_unused_memory(xen_start_info->nr_pages, &e820);
 
David

Re: fix for "xen: use maximum reservation to limit amount of usable RAM" - patch titled: "xen/e820: if there is not dom0_mem, don't tweak extra_pages."

[Konrad Rzeszutek Wilk]

On Tue, Sep 13, 2011 at 11:01:50AM +0100, David Vrabel wrote:
> On 12/09/11 21:13, Konrad Rzeszutek Wilk wrote:
> > .breaks one of my boxes (Core i3-2100), with Xen 4.1.1 (with and w/out the
> > 23790 changset in it).
> > 
> > I've traced it down to the fact that I booted my dom0 without
> > dom0_mem=X flag with a machine that has more than 8GB. Weirdly enough
> > I can only reproduce this under Intel boxes.
> > 
> > Anyhow this patch fixes it for me.
> 
> I think this patch is simpler.  Does it fix the issue?

Yes. Let me use that instead.
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index c3b8d44..46d6d21 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -306,10 +306,12 @@ char * __init xen_memory_setup(void)
>  	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
>  
>  	extra_limit = xen_get_max_pages();
> -	if (extra_limit >= max_pfn)
> -		extra_pages = extra_limit - max_pfn;
> -	else
> -		extra_pages = 0;
> +	if (max_pfn + extra_pages > extra_limit) {
> +		if (extra_limit > max_pfn)
> +			extra_pages = extra_limit - max_pfn;
> +		else
> +			extra_pages = 0;
> +	}
>  
>  	extra_pages += xen_return_unused_memory(xen_start_info->nr_pages, &e820);
>  
> David