Re: [Xen-devel] [RFC][PATCH 4/5] tools:firmware:hvmloader: reserve RMRR mappings in e820

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.08.14 at 09:30, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/8/8 14:42, Jan Beulich wrote:
>> You never allocate memory for the map, i.e. you invoke the
>> hypercall with a NULL second argument. This just happens to work,
>> but is very unlikely what you intended to do.
>>
> 
> Looks scratch_alloc() should be used to allocate in hvmloader, so what 
> about this?
> 
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -766,6 +766,16 @@ struct shared_info *get_shared_info(void)
>       return shared_info;
>   }
> 
> +struct e820map *get_rmrr_map_info(void)
> +{
> +    struct e820map *e820_map = scratch_alloc(sizeof(struct e820map), 0);
> +
> +    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, e820_map) != 0 )
> +        BUG();
> +
> +    return e820_map;
> +}

More reasonable, albeit now you lost the fetch-just-once behavior.
Furthermore - do you really need this to be a dynamic allocation?
The structure you use right now is fixed size. Which gets me to
another point though: Is it said in any part of the VT-d spec that
there is an upper limit to the number of RMRRs? The re-use of the
E820 interface structure looks pretty odd here (I simply didn't get
to looking at your patches in full yet).

Jan


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