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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.08.14 at 04:11, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/8/7 20:03, Andrew Cooper wrote:
>> On 07/08/14 12:02, Tiejun Chen wrote:
>>> --- a/tools/firmware/hvmloader/e820.h
>>> +++ b/tools/firmware/hvmloader/e820.h
>>> @@ -15,6 +15,12 @@ struct e820entry {
>>>       uint32_t type;
>>>   } __attribute__((packed));
>>>
>>> +#define E820MAX 128
>>> +
>>> +struct e820map {
>>> +    int nr_map;
>>
>> This value should be unsigned.
> 
> I'm not sure if we need to change this since here I just copy this from 
> the xen/include/asm-x86/e820.h file,
> 
> struct e820map {
>      int nr_map;
>      struct e820entry map[E820MAX];
> };

While it be welcome for you to (in a separate patch) also change this
one, it is not considered okay to copy existing mistakes: We've been
slowly switching over to put more attention on correct signed-ness
(and const-ness) - variables/fields that can't ever be negative
shouldn't have signed types.

>>> --- a/tools/firmware/hvmloader/util.c
>>> +++ b/tools/firmware/hvmloader/util.c
>>> @@ -766,6 +766,19 @@ struct shared_info *get_shared_info(void)
>>>       return shared_info;
>>>   }
>>>
>>> +struct e820map *get_rmrr_map_info(void)
>>> +{
>>> +    static struct e820map *e820_map = NULL;
>>> +
>>> +    if ( e820_map != NULL )
>>> +        return e820_map;
>>> +
>>> +    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, e820_map) != 0 )
>>> +        BUG();
>>
>> This instructs Xen to clobber the memory starting at 0, and works
>> because HVMLoader is in protected, non-paging mode at this point.  I
>> don't think this is what you meant to do, and will repeatedly make the
> 
> Sorry I can't understand this explicitly. Here I just want a way to get 
> RMRR mapping info under hvmloader circumstance.
> 
> Could you elaborate what you mean? Or show me a proper way I should do 
> as you expect.

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.

Jan


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