Re: [Xen-devel] [v8][PATCH 10/17] hvmloader/mem_hole_alloc: skip any overlap with reserved device memory

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, December 05, 2014 12:29 AM
> 
> >>> On 01.12.14 at 10:24, <tiejun.chen@xxxxxxxxx> wrote:
> > In some cases like igd_opregion_pgbase, guest will use mem_hole_alloc
> > to allocate some memory to use in runtime cycle, so we alsoe need to
> > make sure all reserved device memory don't overlap such a region.
> 
> While ideally this would get switched to the model outlined for
> the previous two patches too, it's at least reasonable to keep
> this simple allocator simple for the time being.
> 
> > --- a/tools/firmware/hvmloader/util.c
> > +++ b/tools/firmware/hvmloader/util.c
> > @@ -416,9 +416,29 @@ static uint32_t alloc_down =
> RESERVED_MEMORY_DYNAMIC_END;
> >
> >  xen_pfn_t mem_hole_alloc(uint32_t nr_mfns)
> >  {
> > +    unsigned int i, num = hvm_get_reserved_device_memory_map();
> > +    uint64_t rdm_start, rdm_end;
> > +    uint32_t alloc_start, alloc_end;
> > +
> >      alloc_down -= nr_mfns << PAGE_SHIFT;
> > +    alloc_start = alloc_down;
> > +    alloc_end = alloc_start + (nr_mfns << PAGE_SHIFT);
> > +    for ( i = 0; i < num; i++ )
> > +    {
> > +        rdm_start = (uint64_t)rdm_map[i].start_pfn << PAGE_SHIFT;
> > +        rdm_end = rdm_start + ((uint64_t)rdm_map[i].nr_pages <<
> PAGE_SHIFT);
> > +        if ( check_rdm_hole_conflict((uint64_t)alloc_start,
> > +                                     (uint64_t)alloc_end,
> 
> Pointless casts.
> 
> > +                                     rdm_start, rdm_end -
> rdm_start) )
> > +        {
> > +            alloc_end = rdm_start;
> > +            alloc_start = alloc_end - (nr_mfns << PAGE_SHIFT);
> > +            BUG_ON(alloc_up >= alloc_start);
> 
> This is redundant with the BUG_ON() below afaict. Or at least it
> would be, if you would properly update allow_down (if you don't
> you may end up returning the same PFN for two allocations).
> 

I'd like this being done once at init time. Once alloc_up/down is
verified/adjusted, no need to add run-time overhead here.

Thanks
Kevin

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