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

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

> From: Chen, Tiejun
> Sent: Monday, December 01, 2014 5:24 PM
> 
> 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.

OK, seems you meant to use this patch to address opregion confliction.
when it works, I think it's better to still add opregion detection in e820,
as a modulo suggestion. It's not good to make assumption in one module
about how other module works. Now opregion is allocated dynamically,
but it may be fixed somewhere in the future. So you always need to
detect confliction, purely in e820 world, regardless of how a range is
actually allocated.

> 
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> ---
>  tools/firmware/hvmloader/util.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index 8767897..f3723c7 100644
> --- 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,
> +                                     rdm_start, rdm_end -
> rdm_start) )
> +        {
> +            alloc_end = rdm_start;
> +            alloc_start = alloc_end - (nr_mfns << PAGE_SHIFT);
> +            BUG_ON(alloc_up >= alloc_start);
> +        }
> +    }
> +
>      BUG_ON(alloc_up >= alloc_down);
> -    return alloc_down >> PAGE_SHIFT;
> +    return alloc_start >> PAGE_SHIFT;
>  }
> 

this patch is required, but I'd prefer to have an initialization phase check
to have a sane alloc_up/down, so you don't bother detection for every
run-time call.

>  void *mem_alloc(uint32_t size, uint32_t align)
> --
> 1.9.1


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