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
|