Re: [v5][PATCH 06/10] hvm_info_table: introduce nr_reserved_device_memory_map

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

On 2014/9/4 15:17, Jan Beulich wrote:
>>>> On 04.09.14 at 08:55, <tiejun.chen@intel.com> wrote:
>> On 2014/9/4 14:32, Jan Beulich wrote:
>>>>>> On 04.09.14 at 04:07, <tiejun.chen@intel.com> wrote:
>>>> On 2014/9/2 16:34, Jan Beulich wrote:
>>>>>>>> On 26.08.14 at 13:02, <tiejun.chen@intel.com> wrote:
>>>>>> --- a/xen/include/public/hvm/hvm_info_table.h
>>>>>> +++ b/xen/include/public/hvm/hvm_info_table.h
>>>>>> @@ -67,6 +67,9 @@ struct hvm_info_table {
>>>>>>
>>>>>>         /* Bitmap of which CPUs are online at boot time. */
>>>>>>         uint8_t     vcpu_online[(HVM_MAX_VCPUS + 7)/8];
>>>>>> +
>>>>>> +    /* How many reserved device memory does this domain have? */
>>>>>> +    uint32_t    nr_reserved_device_memory_map;
>>>>>>     };
>>>>>
>>>>> This being defacto a private interface between tools and hvmloader
>>>>> I wonder if it wouldn't be better to put this before the (in the future)
>>>>> eventually growing vcpu_online[].
>>>>>
>>>>
>>>> Any latest consideration? Could we push line above 'uint8_t
>>>> vcpu_online[(HVM_MAX_VCPUS + 7)/8];'?
>>>>
>>>> And I just think it may be reasonable and convenient to expose this info
>>>> in hvm_info_table since this is a fixed value that we should record for
>>>> all VMs.
>>>
>>> I think information easily obtained via hypercall doesn't belong here,
>>
>> Yes, but just don't feel better to call twice hypercall in hvmloader
>> since this means we have to reallocate memory to store all entries.
>>
>> This is a little bit different what we did in libxc since we can't get
>> that number of all entries directly in libxc. But here it may be fine.
>
> Hmm, latching the output of a hypercall here seems even less
> reasonable to me. What gets communicated here ought to be
> things that have no other good way of telling the VM - at least
> that's my understanding of this structure.
>
>>> but I'd also prefer to have input from the tools maintainers here.
>>>
>>
>> I think I already include all guys by get_maintainer.pc here, but maybe
>> I need ping them actively, so you think who can give us a ack or nack
>> eventually?
>
> People have been traveling a lot lately, so give them a little more
> time.
>

Jan,

As I understand this point is no a big deal, or even the tools 
maintainer have further comment about this point, but I think that 
should not be difficult to address. I guess it may not bring too much 
more change.

So maybe I can send a new revision rebased on that first patch in 
advance since as you know there are some other good comments from you, 
which actually don't conflict that remaining argument.

Thanks
Tiejun

Thanks
Tiejun