[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 Tyshchenko <olekstysh@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 12 Oct 2021 11:24:20 +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=YJZGcpXtDh02n9Dttd375ZEN/OhO4vsMLJThZyN/6xA=; b=T8MLz7TsJ0GY2cwNb/QgPTyjyKycVnOgq8+rCFU0r82J3kuSBd0Jc0gj0fRXuB2qHFSZjJkX//hDwZ4LaPB3tXkygY89OQZtIXs8yODwdDvilzcdgUe+vxccOvfaY5T0l9tAmzB4F8Yk9z11wNUxHE99ZZCWCMs5pk0+Y8SGeIAn6WjsNbIdV/G7gGhu/1LIZmod9uzWMDBhvx9dTDuJdYrj9rw6bAHtsGNnCs0GdT4P9GuxoHyn9dAW0WaVATvleNwqG/TRU967B/EcYuVZ1uaarJeA++14fp0Ce6VKljbsaFDqLXaDRQT0rSDrZYJdyVnnYz42ssbvWUb7+GpBYw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Aq1sQFcEMdeRPHlrGJaaYv0tiXDnxgNnRTrgnq02Z4Ah4xvEDKHKEnSNyZb58oBtveX2H2Li8Zm7B2JR7OqSsQ/SdK4ztNsZLGO3sb1H9J/QIA3C/Mc6Wgu5vS+vT6wNY4qxl7RifeNyAGgjh6v9Z/FApczv2K+AR2N6kkYwnWpiLa+K5iNPeePkSx7TpwjPPttIa2wYwBPKxd2dO/+lbxaepuJzeAtxGovFkQhqOPHAubnFi9P7RUyam8jLeWZCTKUnkPHrmCdvj4+ESO2U8REbm1JCM+y0RBrk3XE+Owsp2it5UMywUUjL63tXazz48xEYbCMYe1PTsn7Au8XdAA==
  • 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 09:24:39 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 11.10.2021 19:48, Oleksandr Tyshchenko wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -38,7 +38,7 @@
>  #include "hvm/save.h"
>  #include "memory.h"
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000015

So this bump would not have been needed if the rule of making padding
fields explicit in the public interface had been followed by earlier
changes, as you could have fit the 8-bit field in the 16-bit gap
after domid.

Furthermore this bump is only going to be necessary if your patch
doesn't make 4.16. 4.15 uses 0x13 here, i.e. a bump has already
occurred in this release cycle.

Otoh, because of the re-use of the struct in a sysctl, you do need
to bump XEN_SYSCTL_INTERFACE_VERSION here (unless you fit the new
field in the existing padding slot, which for the sysctl has been
guaranteed to be zero; see also below).

> @@ -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.

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.)

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).




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