Re: [Xen-devel] [PATCH] xen/setup: Don't relocate p2m/initrd over existing one

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 12/09/2016 09:08 PM, Boris Ostrovsky wrote:
> On 12/09/2016 10:50 AM, Ross Lagerwall wrote:
> > When relocating the p2m/initrd, take special care not to relocate it so
> > that is overlaps with the current location of the p2m/initrd. This is
> > needed since the full extent of the current location is not marked as a
> > reserved region in the e820 (and it shouldn't be since it is about to be
> > moved).
> >
> > This was seen to happen to a dom0 with a large initial p2m and a small
> > reserved region in the middle of the initial p2m.
>
> >  /*
> > - * Find a free area in physical memory not yet reserved and compliant with
> > - * E820 map.
> > + * Find a free area in physical memory not yet reserved, compliant with the
> > + * E820 map and not overlapping with the pre-allocated area.
> >   * Used to relocate pre-allocated areas like initrd or p2m list which are in
> >   * conflict with the to be used E820 map.
> >   * In case no area is found, return 0. Otherwise return the physical address
> >   * of the area which is already reserved for convenience.
> >   */
> > -phys_addr_t __init xen_find_free_area(phys_addr_t size)
> > +phys_addr_t __init xen_find_free_area(phys_addr_t size, phys_addr_t cur_start,
> > +                                     phys_addr_t cur_size)
> >  {
> >         unsigned mapcnt;
> >         phys_addr_t addr, start;
> > @@ -652,7 +653,8 @@ phys_addr_t __init xen_find_free_area(phys_addr_t size)
> >                         continue;
> >                 start = entry->addr;
> >                 for (addr = start; addr < start + size; addr += PAGE_SIZE) {
> > -                       if (!memblock_is_reserved(addr))
> > +                       if (!memblock_is_reserved(addr) &&
> > +                           (addr < cur_start || addr >= cur_start + cur_size))
> >                                 continue;
> >                         start = addr + PAGE_SIZE;
> >                         if (start + size > entry->addr + entry->size)
>
> Wouldn't this in fact make it even more likely to return pointer to the
> currently allocated chunk?
>
> If we are in [cur_start, cur_start+cur_size) then we assign this address
> to 'start' and may well return it, no?
If addr is in [cur_start, cur_start + cur_size) then start is set to 
addr + PAGE_SIZE and we go round the inner loop again. If the new addr 
is still in [cur_start, cur_start + cur_size) then we set start to addr 
+ PAGE_SIZE and go round again... I can't see why it would return it
It is treated the same as if addr is in a reserved memblock.

--
Ross Lagerwall

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