Re: [Xen-devel] [PATCH 5/7] xen/setup: Transfer MFNs from non-RAM E820 entries and gaps to E820 RAM

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 30/03/12 21:37, Konrad Rzeszutek Wilk wrote:
> We will now get:
> 
> -Released 283999 pages of unused memory
> +Exchanged 283999 pages
> .. snip..
> -Memory: 6487732k/9208688k available (5817k kernel code, 1136060k absent, 
> 1584896k reserved, 2900k data, 692k init)
> +Memory: 6503888k/8072692k available (5817k kernel code, 1136060k absent, 
> 432744k reserved, 2900k data, 692k init)

This isn't correct.  You've have lost ~1 GB of memory which are the
pages that were supposed to be moved.  The additional 1GB of reserved
memory in the old case is the balloon.

In xen_memory_setup() where it loops through the e820 to clip the RAM
regions you need to factor in the additional memory you've moved.  In
this loop you may need to count the pages in the RAM region instead of
the simple (addr < mem_end) test.  Take care with RAM regions with
partial pages and the like.

> which is more in line with classic XenOLinux.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  arch/x86/xen/setup.c |   85 
> ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 82 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 1ba8dff..2a12143 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -120,12 +120,89 @@ static unsigned long __init xen_release_chunk(unsigned 
> long start,
>       return len;
>  }
>  
> +static unsigned long __init xen_exchange_chunk(unsigned long start_pfn,
> +     unsigned long end_pfn, unsigned long nr_pages, unsigned long exchanged,
> +     unsigned long *pages_left, const struct e820entry *list,
> +     size_t map_size)
> +{
[...]
> +
> +             for (pfn = start_pfn; pfn < start_pfn + nr; pfn++) {
> +                     unsigned long mfn = pfn_to_mfn(pfn);
> +
> +                     if (mfn == INVALID_P2M_ENTRY || mfn_to_pfn(mfn) != pfn)
> +                             break;
> +
> +                     if (!early_set_phys_to_machine(dest_pfn, mfn))
> +                             break;
> +
> +                     /* You would think we should do 
> HYPERVISOR_update_va_mapping
> +                      * but we don't need to as the hypervisor only sets up 
> the
> +                      * initial pagetables up to nr_pages, and we stick the 
> MFNs
> +                      * past that.
> +                      */

Hmmm. Are you sure this is safe?  What happens if Linux tries to use
these pages before creating new page tables?  e.g., via some early boot
allocator before the final page tables are setup? (This might not be a
problem, I haven't checked).

You've may have gotten away with it for now because the moved MFNs are
marked as unusable.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel