On 12.10.2021 15:18, Oleksandr wrote:
> On 12.10.21 14:49, Jan Beulich wrote:
>> On 12.10.2021 13:28, Oleksandr wrote:
>>> On 12.10.21 12:24, Jan Beulich wrote:
>>>> On 11.10.2021 19:48, Oleksandr Tyshchenko wrote:
>>>>> @@ -150,6 +150,7 @@ struct xen_domctl_getdomaininfo {
>>>>> uint32_t ssidref;
>>>>> xen_domain_handle_t handle;
>>>>> uint32_t cpupool;
>>>>> + uint8_t gpaddr_bits; /* Guest physical address space size. */
>>>>> struct xen_arch_domainconfig arch_config;
>>>> On the basis of the above, please add uint8_t pad[3] (or perhaps
>>>> better pad[7] to be independent of the present
>>>> alignof(struct xen_arch_domainconfig) == 4) and make sure the
>>>> array will always be zero-filled, such that the padding space can
>>>> subsequently be assigned a purpose without needing to bump the
>>>> interface version(s) again.
>>> ok, will do.
>>>
>>>
>>>> Right now the sysctl caller of getdomaininfo() clears the full
>>>> structure (albeit only once, so stale / inapplicable data might get
>>>> reported for higher numbered domains if any field was filled only
>>>> in certain cases), while the domctl one doesn't. Maybe it would be
>>>> best looking forward if the domctl path also cleared the full buffer
>>>> up front, or if the clearing was moved into the function. (In the
>>>> latter case some writes of zeros into the struct could be eliminated
>>>> in return.)
>>> Well, I would be OK either way, with a little preference for the latter.
>>>
>>> Is it close to what you meant?
>> Yes, just that ...
>>
>>> --- a/xen/common/domctl.c
>>> +++ b/xen/common/domctl.c
>>> @@ -69,10 +69,10 @@ void getdomaininfo(struct domain *d, struct
>>> xen_domctl_getdomaininfo *info)
>>> int flags = XEN_DOMINF_blocked;
>>> struct vcpu_runstate_info runstate;
>>>
>>> + memset(info, 0, sizeof(*info));
>>> +
>>> info->domain = d->domain_id;
>>> info->max_vcpu_id = XEN_INVALID_MAX_VCPU_ID;
>>> - info->nr_online_vcpus = 0;
>>> - info->ssidref = 0;
>> ... there are a few more "... = 0" further down iirc.
>
> I didn't spot anything except "info->flags = ..." which probably can now
> be converted into "info->flags |= ..."
Oh, I guess you're right: I've been looking at my own tree, with
"paged_pages field is MEM_PAGING-only" and "shr_pages field is
MEM_SHARING-only" already applied. These sadly are still waiting
to go in, as they depend on earlier patches in their series.
Jan
