[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V6 1/2] xen/arm: Introduce gpaddr_bits field to struct xen_domctl_getdomaininfo


  • To: Oleksandr <olekstysh@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 12 Oct 2021 13:49:17 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=LrXYyPsZMP3IyLVpP4WPdYGaL6Pi4ZtvbWuubEvrdTw=; b=V3HJ0TYN8OqM6cwdMT3LQgyFuMTn4a7Z1VPWgupP9duzG8m1hJqiXbmhwKbibY5FpJYQRxUPonqF1a9MEh9pB5zVddFzvyupKEooSXJmIjdQaLw+J5GkYWN09WruqKD4YVg0Igm7Sh90GaNwjCprsMG4kSBZs+PXuetWnC/7anL5e2GdO4FzAOmVruipKMbaqOXwYBiMSLCHXpq4u6Yg6EgOtfYQDvJip/qhy9JG3+DCNbc1eho7aUOqyxbfEzFHEgWk5U+9c6sC47Iq0l2OMcJ601EpOhRgEYXc2UE+BeS91Bg6wkAv7bxuIqiMlX7J+S7iLK8Y5btX5l9+e487RQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MC8PTaVet+9UB850fQ259X3+mgSZGZ60J7G8KOiIoIuRnCKBpPmy2x8byzuzdG3IWGXb43f4tWy0JHwrjxa6EAQyDWRFl54M3WgXLcR5XMRN3NRWD9uz3Gr1dd6FQTQ1943KJMWCk7TJQnjjYiGk23oJO7fP0bUWKwTBiE2BqSYPGmNLjuVSmFSVgZxm4UkMcIIrxw7uDXJOZTn32UkkyBw40afrooOAgJpkKItZ0ASzWiHvqaiwAewif8iBX3PQIa+AQiZWfLm9h0lTSrXKZT+MSijrOi18MoIxO45qIhl/bbdeCOg3I7DHYCzbRdU3XcpeATpfxiK6B3i9MMdcXw==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Tue, 12 Oct 2021 11:49:42 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.

>> Perhaps, once properly first zero-filling the struct in all cases,
>> the padding field near the start should also be made explicit,
>> clarifying visually that it is an option to use the space there for
>> something that makes sense to live early in the struct (i.e. I
>> wouldn't necessarily recommend putting there the new field you add,
>> albeit - as mentioned near the top - that's certainly an option).
> 
> I read this as a request to also add uint16_t pad after "domid_t domain" 
> at least. Correct?

I guess I should really leave this up to you - that's largely a cosmetic
thing after all once clearing the whole struct up front.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.