WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH 1/3] xen: allow balloon driver to use more th

To: David Vrabel <david.vrabel@xxxxxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH 1/3] xen: allow balloon driver to use more than one memory region
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 16 Aug 2011 10:48:33 -0400
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 16 Aug 2011 07:50:11 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E4A7CCF.4070209@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1313488838-28809-1-git-send-email-david.vrabel@xxxxxxxxxx> <1313488838-28809-2-git-send-email-david.vrabel@xxxxxxxxxx> <20110816133854.GB30261@xxxxxxxxxxxx> <4E4A7CCF.4070209@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Aug 16, 2011 at 03:21:03PM +0100, David Vrabel wrote:
> On 16/08/11 14:38, Konrad Rzeszutek Wilk wrote:
> > 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 <david.vrabel@xxxxxxxxxx>
> >> ---
> >> 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]?
> 
> No. I probably should have made it clear in the description but this
> patch doesn't change the number of regions.  It only changes the pair of
> variables to a single element array of a structure.

Ok, I ask again - what does this patch solve?

> 
> See XEN_EXTRA_MEM_MAX_REGIONS in include/xen/page.h:
> 
> --- a/include/xen/page.h
> +++ b/include/xen/page.h
> @@ -3,6 +3,13 @@
> 
>  #include <asm/xen/page.h>
> 
> -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 */
> 
> >> @@ -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?
> 
> Again, the patch isn't adding any additional regions.
> 
> 
> >> +  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() is a nop if size == 0.  But I can an
> explicit check (of size) here if that is preferred.
> 
> >> +          balloon_add_memory_region(PFN_UP(xen_extra_mem[r].start),
> >> +                                    PFN_DOWN(xen_extra_mem[r].size));
> >>  
> >>    return 0;
> >>  }
> 
> David
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>