Re: [v6][PATCH 5/7] hvmloader: introduce hypercall for xc_reserved_device_memory_map

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2014/9/11 5:41, Tian, Kevin wrote:
>> From: Chen, Tiejun
>> Sent: Tuesday, September 09, 2014 10:50 PM
>>
>> We will introduce that hypercall xc_reserved_device_memory_map
>> to hvmloader.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>
>> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
>> index 80d822f..90dbb6e 100644
>> --- a/tools/firmware/hvmloader/util.c
>> +++ b/tools/firmware/hvmloader/util.c
>> @@ -828,6 +828,28 @@ int hpet_exists(unsigned long hpet_base)
>>       return ((hpet_id >> 16) == 0x8086);
>>   }
>>
>> +int get_reserved_device_memory_map(struct
>> xen_mem_reserved_device_memory entries[],
>> +                                   uint32_t max_entries)
>
> usually claim as a pointer instead of array as the parameter.

But I see there are some existing similar things like,

tools/libxc/xenctrl.h:

int xc_domain_set_memory_map(xc_interface *xch,
                                uint32_t domid,
                                struct e820entry entries[],
                                uint32_t nr_entries);

int xc_get_machine_memory_map(xc_interface *xch,
                               struct e820entry entries[],
                               uint32_t max_entries);

>
>> +{
>> +    static int map_done = 0;
>
> don't see much value of the static variable here. Better for the caller
> to judge whether multiple calls are required.

Originally this intends to indicate if we call this hypercall in case 
when we already know that appropriate entries number as we introduce 
that new field nr_reserved_device_memory_map in hvm_info_table. If so, 
we just need to call one time.

But looks you guys think we shouldn't live with 
nr_reserved_device_memory_map, so this should be gone.

Thanks
Tiejun

>
>> +    struct xen_mem_reserved_device_memory_map memmap = {
>> +        .nr_entries = max_entries
>> +    };
>> +
>> +    if ( map_done )
>> +        return 0;
>> +
>> +    set_xen_guest_handle(memmap.buffer, entries);
>> +
>> +    if ( hypercall_memory_op(XENMEM_reserved_device_memory_map,
>> +                             &memmap) != 0 )
>> +        BUG();
>> +
>> +    map_done = 1;
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/tools/firmware/hvmloader/util.h
>> b/tools/firmware/hvmloader/util.h
>> index a70e4aa..cca0d5b 100644
>> --- a/tools/firmware/hvmloader/util.h
>> +++ b/tools/firmware/hvmloader/util.h
>> @@ -241,6 +241,9 @@ int build_e820_table(struct e820entry *e820,
>>                        unsigned int bios_image_base);
>>   void dump_e820_table(struct e820entry *e820, unsigned int nr);
>>
>> +#include <xen/memory.h>
>> +int get_reserved_device_memory_map(struct
>> xen_mem_reserved_device_memory entries[],
>> +                                   uint32_t max_entries);
>>   #ifndef NDEBUG
>>   void perform_tests(void);
>>   #else
>> --
>> 1.9.1
>
>