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

Re: [PATCH for-4.22 v2 2/5] tools/macros: adjust ROUNDUP() interface to match hypervisor


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 5 Jun 2026 17:38:27 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=+PGVV9U+zysbOTQjOgeQumWu5m47aboUeJSvnOGH1P0=; b=b23vAvLUVCqHqrwLX0LFghnzA3sShD9q+St66c0mFoWTXtUY6p6CHbpWO+kKXslqANjzj+glXn2XbXBvxNwtdCxlkSYq4gtqsXoJe9B77cJHjrlTfIJqiaJz/GYoXik0FSJNWMzhb5lof5LrOkWN9iaIntyC0k7fEa1XdLeEsZHmHQthGkRl1SP8VqTFGfkM6C2l7qOcYzZqQkJDk+FZ1jj4PYRjrWAIX4Q24/sd8mo9JYUBTQvlmFcvxxQpQurBSKNMpSfZAO0T4lqLk4JkXBuT22ITiOwuSl6niVtBY+Nu+WR1a6bTcRbEi0/fIOG8Et9W2x7g8Lk7m4YDJD3AXQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=gWbOKgVHwDGDXZtkpv2SGglJgeB/e1riRi+48ZzbbsKsmjC2EFIN9wRD34gleh+GdJEsqD1gGtstC/2d47Xl/Ms3MCgryCpy2RwLf4Iv8y/jOEgIwXl5Ex7jMnYkJNgJ0WtulFGpI40icAwjWpENDKCkddibPJpsh7+PMQ3AHLJ0LGweZ5Wv2yj3gh4lKr0IprMaxjfWWircRYiPv+eMRKPr14xzSuwo9BTy+N/tNzE5xelqOEKwpX+dTSJhcXlc4iJ/HLzcuHuzt6w++6ChJFU5dAF+2U1FzL03kyjUB2U5CItGaZAPY6J6CqOmpsEyU19c5DHUasWQJGjIYLTvbA==
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=citrix.com header.i="@citrix.com" header.h="From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck"
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Julien Grall <julien@xxxxxxx>
  • Delivery-date: Fri, 05 Jun 2026 15:38:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Jun 05, 2026 at 04:24:27PM +0100, Andrew Cooper wrote:
> On 03/06/2026 8:18 pm, Roger Pau Monne wrote:
> > Adjust user-space callers to use the new interface.  No functional change
> > intended.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> Yeah, libxl's choice to use order was always bizarre...  I'm glad we're
> getting rid of this.
> 
> > diff --git a/tools/libs/guest/xg_dom_x86.c b/tools/libs/guest/xg_dom_x86.c
> > index 268936efe25b..9af83535944a 100644
> > --- a/tools/libs/guest/xg_dom_x86.c
> > +++ b/tools/libs/guest/xg_dom_x86.c
> > @@ -678,7 +678,7 @@ static int alloc_magic_pages_hvm(struct xc_dom_image 
> > *dom)
> >      {
> >          if ( dom->cmdline )
> >          {
> > -            dom->cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 3);
> > +            dom->cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 1U << 3);
> >              start_info_size += dom->cmdline_size;
> >          }
> >      }
> 
> I think this would be better as a literal 8.
> 
> > diff --git a/tools/libs/guest/xg_sr_common.c 
> > b/tools/libs/guest/xg_sr_common.c
> > index 7ccdc3b1f6aa..54dde924a7c0 100644
> > --- a/tools/libs/guest/xg_sr_common.c
> > +++ b/tools/libs/guest/xg_sr_common.c
> > @@ -56,11 +56,11 @@ const char *rec_type_to_str(uint32_t type)
> >  int write_split_record(struct xc_sr_context *ctx, struct xc_sr_record *rec,
> >                         void *buf, size_t sz)
> >  {
> > -    static const char zeroes[(1u << REC_ALIGN_ORDER) - 1] = { 0 };
> > +    static const char zeroes[REC_ALIGN - 1] = { 0 };
> 
> = {} as you're editing the line.
> 
> I have no idea why this is 7 in libxg, but 8 in libxl, but dropping the
> -1 here is probably wise.

I've noticed this oddity, but was cautious with doing any change
there, given the code freeze status.  I can do the adjustment.

> > diff --git a/tools/libs/guest/xg_sr_stream_format.h 
> > b/tools/libs/guest/xg_sr_stream_format.h
> > index 8a0da26f7543..4310f4311e65 100644
> > --- a/tools/libs/guest/xg_sr_stream_format.h
> > +++ b/tools/libs/guest/xg_sr_stream_format.h
> > @@ -53,7 +53,7 @@ struct xc_sr_rhdr
> >  };
> >  
> >  /* All records must be aligned up to an 8 octet boundary */
> > -#define REC_ALIGN_ORDER               (3U)
> > +#define REC_ALIGN                     (1U << 3)
> 
> This really does want to be 8 rather than a shift.
> 
> >  /* Somewhat arbitrary - 128MB */
> >  #define REC_LENGTH_MAX                (128U << 20)
> >  
> > diff --git a/tools/libs/light/libxl_arm_acpi.c 
> > b/tools/libs/light/libxl_arm_acpi.c
> > index ba874c3d3224..ac8165de15b6 100644
> > --- a/tools/libs/light/libxl_arm_acpi.c
> > +++ b/tools/libs/light/libxl_arm_acpi.c
> > @@ -107,12 +107,12 @@ int libxl__get_acpi_size(libxl__gc *gc,
> >      if (rc < 0)
> >          goto out;
> >  
> > -    *out = ROUNDUP(size, 3) +
> > -           ROUNDUP(sizeof(struct acpi_table_rsdp), 3) +
> > -           ROUNDUP(sizeof(struct acpi_table_xsdt), 3) +
> > -           ROUNDUP(sizeof(struct acpi_table_gtdt), 3) +
> > -           ROUNDUP(sizeof(struct acpi_table_fadt), 3) +
> > -           ROUNDUP(sizeof(dsdt_anycpu_arm_len), 3);
> > +    *out = ROUNDUP(size, 1U << 3) +
> > +           ROUNDUP(sizeof(struct acpi_table_rsdp), 1U << 3) +
> > +           ROUNDUP(sizeof(struct acpi_table_xsdt), 1U << 3) +
> > +           ROUNDUP(sizeof(struct acpi_table_gtdt), 1U << 3) +
> > +           ROUNDUP(sizeof(struct acpi_table_fadt), 1U << 3) +
> > +           ROUNDUP(sizeof(dsdt_anycpu_arm_len), 1U << 3);
> >  
> >  out:
> >      return rc;
> > @@ -128,7 +128,7 @@ static int libxl__allocate_acpi_tables(libxl__gc *gc,
> >  
> >      acpitables[RSDP].addr = GUEST_ACPI_BASE;
> >      acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[RSDP].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[RSDP].size, 1U << 3);
> >  
> >      acpitables[XSDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
> >      /*
> > @@ -137,11 +137,11 @@ static int libxl__allocate_acpi_tables(libxl__gc *gc,
> >       */
> >      acpitables[XSDT].size = sizeof(struct acpi_table_xsdt) +
> >                              sizeof(uint64_t) * 2;
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[XSDT].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[XSDT].size, 1U << 3);
> >  
> >      acpitables[GTDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
> >      acpitables[GTDT].size = sizeof(struct acpi_table_gtdt);
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[GTDT].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[GTDT].size, 1U << 3);
> >  
> >      acpitables[MADT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
> >  
> > @@ -150,15 +150,15 @@ static int libxl__allocate_acpi_tables(libxl__gc *gc,
> >          goto out;
> >  
> >      acpitables[MADT].size = size;
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[MADT].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[MADT].size, 1U << 3);
> >  
> >      acpitables[FADT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
> >      acpitables[FADT].size = sizeof(struct acpi_table_fadt);
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[FADT].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[FADT].size, 1U << 3);
> >  
> >      acpitables[DSDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
> >      acpitables[DSDT].size = dsdt_anycpu_arm_len;
> > -    dom->acpi_modules[0].length += ROUNDUP(acpitables[DSDT].size, 3);
> > +    dom->acpi_modules[0].length += ROUNDUP(acpitables[DSDT].size, 1U << 3);
> 
> And all of these.
> 
> > diff --git a/tools/libs/light/libxl_sr_stream_format.h 
> > b/tools/libs/light/libxl_sr_stream_format.h
> > index f8f4723c2e91..b02c954a388e 100644
> > --- a/tools/libs/light/libxl_sr_stream_format.h
> > +++ b/tools/libs/light/libxl_sr_stream_format.h
> > @@ -29,7 +29,7 @@ typedef struct libxl__sr_rec_hdr
> >  } libxl__sr_rec_hdr;
> >  
> >  /* All records must be aligned up to an 8 octet boundary */
> > -#define REC_ALIGN_ORDER              3U
> > +#define REC_ALIGN                    (1U << 3)
> >  
> >  #define REC_TYPE_END                    0x00000000U
> >  #define REC_TYPE_LIBXC_CONTEXT          0x00000001U
> > diff --git a/tools/libs/light/libxl_stream_write.c 
> > b/tools/libs/light/libxl_stream_write.c
> > index 98d44597a732..9ea64369352f 100644
> > --- a/tools/libs/light/libxl_stream_write.c
> > +++ b/tools/libs/light/libxl_stream_write.c
> > @@ -119,7 +119,7 @@ static void setup_generic_write(libxl__egc *egc,
> >                                  void *body,
> >                                  sws_record_done_cb cb)
> >  {
> > -    static const uint8_t zero_padding[1U << REC_ALIGN_ORDER] = { 0 };
> > +    static const uint8_t zero_padding[REC_ALIGN] = { 0 };
> 
> These want the same adjustments as the libxg side.
> 
> > diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
> > index d6d462b7bc82..86c86b3e9a77 100644
> > --- a/tools/xenstored/core.c
> > +++ b/tools/xenstored/core.c
> > @@ -3067,7 +3067,7 @@ static int dump_state_node(const void *ctx, struct 
> > connection *conn,
> >     head.length += node->hdr.num_perms * sizeof(*sn.perms);
> >     head.length += pathlen;
> >     head.length += node->hdr.datalen;
> > -   head.length = ROUNDUP(head.length, 3);
> > +   head.length = ROUNDUP(head.length, 1U << 3);
> >  
> >     if (fwrite(&head, sizeof(head), 1, fp) != 1)
> >             return dump_state_node_err(data, "Dump node head error");
> > diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> > index 2db452144dd4..a880ff678ef9 100644
> > --- a/tools/xenstored/domain.c
> > +++ b/tools/xenstored/domain.c
> > @@ -2159,7 +2159,7 @@ const char *dump_state_connections(FILE *fp)
> >             if (ret)
> >                     return ret;
> >             head.length += sc.data_in_len + sc.data_out_len;
> > -           head.length = ROUNDUP(head.length, 3);
> > +           head.length = ROUNDUP(head.length, 1U << 3);
> >             if (c->domain) {
> >                     sc.fields |= XS_STATE_CONN_FIELDS_UNIQ_ID;
> >                     head.length += sizeof(uint64_t);
> > @@ -2232,7 +2232,8 @@ void read_state_connection(const void *ctx, const 
> > void *state)
> >             unsigned long off;
> >  
> >             off = sizeof(*sc) + sc->data_in_len + sc->data_out_len;
> > -           domain->unique_id = *(uint64_t *)(state + ROUNDUP(off, 3));
> > +           domain->unique_id =
> > +               *(uint64_t *)(state + ROUNDUP(off, 1U << 3));
> >     }
> >  }
> >  
> > @@ -2308,7 +2309,7 @@ static int dump_state_domain(const void *k, void *v, 
> > void *arg)
> >     n_quota = get_quota_size(domain->acc, &rec_len);
> >     rec_len += n_quota * sizeof(sd->quota_val[0]);
> >     rec_len += sizeof(*sd);
> > -   rec_len = ROUNDUP(rec_len, 3);
> > +   rec_len = ROUNDUP(rec_len, 1U << 3);
> >  
> >     record = talloc_size(NULL, rec_len + sizeof(*head));
> >     if (!record)
> > @@ -2372,7 +2373,7 @@ const char *dump_state_glb_quota(FILE *fp)
> >     n_quota = get_quota_size(quotas, &rec_len);
> >     rec_len += n_quota * sizeof(glb->quota_val[0]);
> >     rec_len += sizeof(*glb);
> > -   rec_len = ROUNDUP(rec_len, 3);
> > +   rec_len = ROUNDUP(rec_len, 1U << 3);
> >  
> >     record = talloc_size(NULL, rec_len + sizeof(*head));
> >     if (!record)
> > diff --git a/tools/xenstored/watch.c b/tools/xenstored/watch.c
> > index a9a06e9e4816..309c5bb66bef 100644
> > --- a/tools/xenstored/watch.c
> > +++ b/tools/xenstored/watch.c
> > @@ -349,7 +349,7 @@ const char *dump_state_watches(FILE *fp, struct 
> > connection *conn,
> >             }
> >  
> >             head.length += path_len + token_len;
> > -           head.length = ROUNDUP(head.length, 3);
> > +           head.length = ROUNDUP(head.length, 1U << 3);
> >             if (fwrite(&head, sizeof(head), 1, fp) != 1)
> >                     return "Dump watch state error";
> >  
> 
> And these want to be 8 as well I think.
> 
> Definitely with the migration formation adjustments, and preferably with
> the others too, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks, I've done all the adjustments.

Roger.



 


Rackspace

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