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.
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
|
Home