Re: [Xen-devel] [PATCH 06/13] xen: detect pre-allocated memory interfering with e820 map

[David Vrabel <dvrabel@xxxxxxxxxx>]

On 18/02/2015 06:51, Juergen Gross wrote:
> Currently especially for dom0 guest memory with guest pfns not
> matching host areas populated with RAM are remapped to areas which
> are RAM native as well. This is done to be able to use identity
> mappings (pfn == mfn) for I/O areas.
>
> Up to now it is not checked whether the remapped memory is already
> in use. Remapping used memory will probably result in data corruption,
> as the remapped memory will no longer be reserved. Any memory
> allocation after the remap can claim that memory.
>
> Add an infrastructure to check for conflicts of reserved memory areas
> and in case of a conflict to react via an area specific function.
>
> This function has 3 options:
> - Panic
> - Handle the conflict by moving the data to another memory area.
>    This is indicated by a return value other than 0.
> - Just return 0. This will delay invalidating the conflicting memory
>    area to just before doing the remap. This option will be usable for
>    cases only where the memory will no longer be needed when the remap
>    operation will be started, e.g. for the p2m list, which is already
>    copied then.
>
> When doing the remap, check for not remapping a reserved page.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>   arch/x86/xen/setup.c   | 185 +++++++++++++++++++++++++++++++++++++++++++++++--
>   arch/x86/xen/xen-ops.h |   2 +
>   2 files changed, 182 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 0dda131..a0af554 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -59,6 +59,20 @@ static unsigned long xen_remap_mfn __initdata = 
> INVALID_P2M_ENTRY;
>   static unsigned long xen_remap_pfn;
>   static unsigned long xen_max_pfn;
>
> +/*
> + * Areas with memblock_reserve()d memory to be checked against final E820 map.
> + * Each area has an associated function to handle conflicts (by either
> + * removing the conflict or by just crashing with an appropriate message).
> + * The array has a fixed size as there are only few areas of interest which are
> + * well known: kernel, page tables, p2m, initrd.
> + */
> +#define XEN_N_RESERVED_AREAS   4
> +static struct {
> +       phys_addr_t     start;
> +       phys_addr_t     size;
> +       int             (*func)(phys_addr_t start, phys_addr_t size);
> +} xen_reserved_area[XEN_N_RESERVED_AREAS] __initdata;
> +
>   /*
>    * The maximum amount of extra memory compared to the base size.  The
>    * main scaling factor is the size of struct page.  At extreme ratios
> @@ -365,10 +379,10 @@ static void __init 
> xen_set_identity_and_remap_chunk(unsigned long start_pfn,
>         unsigned long end_pfn, unsigned long *released, unsigned long *remapped)
>   {
>         unsigned long pfn;
> -       unsigned long i = 0;
> +       unsigned long i;
>         unsigned long n = end_pfn - start_pfn;
>
> -       while (i < n) {
> +       for (i = 0; i < n; ) {
>                 unsigned long cur_pfn = start_pfn + i;
>                 unsigned long left = n - i;
>                 unsigned long size = left;
> @@ -411,6 +425,53 @@ static void __init 
> xen_set_identity_and_remap_chunk(unsigned long start_pfn,
>                         (unsigned long)__va(pfn << PAGE_SHIFT),
>                         mfn_pte(pfn, PAGE_KERNEL_IO), 0);
>   }
> +/* Check to be remapped memory area for conflicts with reserved areas.
> + *
> + * Skip regions known to be reserved which are handled later. For these
> + * regions we have to increase the remapped counter in order to reserve
> + * extra memory space.
> + *
> + * In case a memory page already in use is to be remapped, just BUG().
> + */
> +static void __init xen_set_identity_and_remap_chunk_chk(unsigned long 
> start_pfn,
> +       unsigned long end_pfn, unsigned long *released, unsigned long *remapped)
...remap_chunk_checked() ?

> +{
> +       unsigned long pfn;
> +       unsigned long area_start, area_end;
> +       unsigned i;
> +
> +       for (i = 0; i < XEN_N_RESERVED_AREAS; i++) {
> +
> +               if (!xen_reserved_area[i].size)
> +                       break;
> +
> +               area_start = PFN_DOWN(xen_reserved_area[i].start);
> +               area_end = PFN_UP(xen_reserved_area[i].start +
> +                                 xen_reserved_area[i].size);
> +               if (area_start >= end_pfn || area_end <= start_pfn)
> +                       continue;
> +
> +               if (area_start > start_pfn)
> +                       xen_set_identity_and_remap_chunk(start_pfn, area_start,
> +                                                        released, remapped);
> +
> +               if (area_end < end_pfn)
> +                       xen_set_identity_and_remap_chunk(area_end, end_pfn,
> +                                                        released, remapped);
> +
> +               *remapped += min(area_end, end_pfn) -
> +                           max(area_start, start_pfn);
> +
> +               return;
Why not defer the whole chunk that conflicts?  Or for that matter defer 
all this remapping to the last minute.
David

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