From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 1/3] xen: allow balloon driver to use more than one memory region Date: Tue, 16 Aug 2011 09:38:54 -0400 Message-ID: <20110816133854.GB30261@dumpdata.com> References: <1313488838-28809-1-git-send-email-david.vrabel@citrix.com> <1313488838-28809-2-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1313488838-28809-2-git-send-email-david.vrabel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: David Vrabel Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Tue, Aug 16, 2011 at 11:00:36AM +0100, David Vrabel wrote: > Allow the xen balloon driver to populate its list of extra pages from > more than one region of memory. This will allow platforms to provide > (for example) a region of low memory and a region of high memory. What does this solve? Is this a requirement for another patch? If so please specify the name of it. > > Signed-off-by: David Vrabel > --- > Is there a better way of passing the memory information to the balloon > driver? I think the way you have it is OK. > --- > arch/x86/xen/setup.c | 20 ++++++++++---------- > drivers/xen/balloon.c | 48 ++++++++++++++++++++++++++++-------------------- > include/xen/page.h | 9 ++++++++- > 3 files changed, 46 insertions(+), 31 deletions(-) > > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c > index df118a8..30d0015 100644 > --- a/arch/x86/xen/setup.c > +++ b/arch/x86/xen/setup.c > @@ -37,7 +37,7 @@ extern void xen_syscall_target(void); > extern void xen_syscall32_target(void); > > /* Amount of extra memory space we add to the e820 ranges */ > -phys_addr_t xen_extra_mem_start, xen_extra_mem_size; > +struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS]; > > /* > * The maximum amount of extra memory compared to the base size. The > @@ -56,7 +56,7 @@ static void __init xen_add_extra_mem(unsigned long pages) > unsigned long pfn; > > u64 size = (u64)pages * PAGE_SIZE; > - u64 extra_start = xen_extra_mem_start + xen_extra_mem_size; > + u64 extra_start = xen_extra_mem[0].start + xen_extra_mem[0].size; Wouldn't this be for [1]? > > if (!pages) > return; > @@ -66,7 +66,7 @@ static void __init xen_add_extra_mem(unsigned long pages) > > memblock_x86_reserve_range(extra_start, extra_start + size, "XEN EXTRA"); > > - xen_extra_mem_size += size; > + xen_extra_mem[0].size += size; > > xen_max_p2m_pfn = PFN_DOWN(extra_start + size); > > @@ -226,7 +226,7 @@ char * __init xen_memory_setup(void) > > memcpy(map_raw, map, sizeof(map)); > e820.nr_map = 0; > - xen_extra_mem_start = mem_end; > + xen_extra_mem[0].start = mem_end; > for (i = 0; i < memmap.nr_entries; i++) { > unsigned long long end; > > @@ -254,8 +254,8 @@ char * __init xen_memory_setup(void) > e820_add_region(end, delta, E820_UNUSABLE); > } > > - if (map[i].size > 0 && end > xen_extra_mem_start) > - xen_extra_mem_start = end; > + if (map[i].size > 0 && end > xen_extra_mem[0].start) > + xen_extra_mem[0].start = end; > > /* Add region if any remains */ > if (map[i].size > 0) > @@ -263,10 +263,10 @@ char * __init xen_memory_setup(void) > } > /* Align the balloon area so that max_low_pfn does not get set > * to be at the _end_ of the PCI gap at the far end (fee01000). > - * Note that xen_extra_mem_start gets set in the loop above to be > - * past the last E820 region. */ > - if (xen_initial_domain() && (xen_extra_mem_start < (1ULL<<32))) > - xen_extra_mem_start = (1ULL<<32); > + * Note that the start of balloon area gets set in the loop above > + * to be past the last E820 region. */ > + if (xen_initial_domain() && (xen_extra_mem[0].start < (1ULL<<32))) > + xen_extra_mem[0].start = (1ULL<<32); So what about the highmem memory? Should there a be a check to move the lowmem to highmem count? > > /* > * In domU, the ISA region is normal, usable memory, but we > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 5dfd8f8..de7278a 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -555,11 +555,35 @@ void free_xenballooned_pages(int nr_pages, struct page** pages) > } > EXPORT_SYMBOL(free_xenballooned_pages); > > -static int __init balloon_init(void) > +static void __init balloon_add_memory_region(unsigned long start_pfn, unsigned long pages) > { > unsigned long pfn, extra_pfn_end; > struct page *page; > > + /* > + * Initialise the balloon with excess memory space. We need > + * to make sure we don't add memory which doesn't exist or > + * logically exist. The E820 map can be trimmed to be smaller > + * than the amount of physical memory due to the mem= command > + * line parameter. And if this is a 32-bit non-HIGHMEM kernel > + * on a system with memory which requires highmem to access, > + * don't try to use it. > + */ > + extra_pfn_end = min(min(max_pfn, e820_end_of_ram_pfn()), > + start_pfn + pages); > + for (pfn = start_pfn; pfn < extra_pfn_end; pfn++) { > + page = pfn_to_page(pfn); > + /* totalram_pages and totalhigh_pages do not > + include the boot-time balloon extension, so > + don't subtract from it. */ > + __balloon_append(page); > + } > +} > + > +static int __init balloon_init(void) > +{ > + int r; > + > if (!xen_domain()) > return -ENODEV; > > @@ -583,25 +607,9 @@ static int __init balloon_init(void) > register_memory_notifier(&xen_memory_nb); > #endif > > - /* > - * Initialise the balloon with excess memory space. We need > - * to make sure we don't add memory which doesn't exist or > - * logically exist. The E820 map can be trimmed to be smaller > - * than the amount of physical memory due to the mem= command > - * line parameter. And if this is a 32-bit non-HIGHMEM kernel > - * on a system with memory which requires highmem to access, > - * don't try to use it. > - */ > - extra_pfn_end = min(min(max_pfn, e820_end_of_ram_pfn()), > - (unsigned long)PFN_DOWN(xen_extra_mem_start + xen_extra_mem_size)); > - for (pfn = PFN_UP(xen_extra_mem_start); > - pfn < extra_pfn_end; > - pfn++) { > - page = pfn_to_page(pfn); > - /* totalram_pages and totalhigh_pages do not include the boot-time > - balloon extension, so don't subtract from it. */ > - __balloon_append(page); > - } > + for (r = 0; r < XEN_EXTRA_MEM_MAX_REGIONS; r++) You probably should also check to make sure that the values are actually valid. Like if (!xedn_extra_mem[r].start) continue; > + balloon_add_memory_region(PFN_UP(xen_extra_mem[r].start), > + PFN_DOWN(xen_extra_mem[r].size)); > > return 0; > } > diff --git a/include/xen/page.h b/include/xen/page.h > index 0be36b9..b01f6ac 100644 > --- a/include/xen/page.h > +++ b/include/xen/page.h > @@ -3,6 +3,13 @@ > > #include > > -extern phys_addr_t xen_extra_mem_start, xen_extra_mem_size; > +struct xen_memory_region { > + phys_addr_t start; > + phys_addr_t size; > +}; > + > +#define XEN_EXTRA_MEM_MAX_REGIONS 1 > + > +extern struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS]; > > #endif /* _XEN_PAGE_H */ > -- > 1.7.4.1