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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.14 at 07:24, <kevin.tian@xxxxxxxxx> wrote:
>>  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.

I don't think that would work, as you can't predict where the holes
are, and limiting allocations to e.g. just the largest region between
any two holes may end up being too restrictive (without having
checked how much memory may get allocated this way in the
worst case). Of course there's also the problem to address that
the whole region used so far overlaps with an enforced hole.

Jan


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