Re: [Xen-devel] [PATCH v9 2/3] tools/libxc: add API for bitmap access for restore

[Olaf Hering]

On Wed, Sep 06, Andrew Cooper wrote:

> On 01/09/17 17:08, Olaf Hering wrote:
> > +static inline bool pfn_is_populated(struct xc_sr_context *ctx, xen_pfn_t 
> > pfn)
> > +static inline int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t 
> > pfn)
> Why are these moved?  They are still restore specific.

There is no tools/libxc/xc_sr_restore.h, should I create one?

Olaf


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v9 2/3] tools/libxc: add API for bitmap access for restore

[Andrew Cooper]

On 01/09/17 17:08, Olaf Hering wrote:
> Extend API for managing bitmaps. Each bitmap is now represented by a
> generic struct xc_sr_bitmap.
> Switch the existing populated_pfns to this API.
>
> Signed-off-by: Olaf Hering 
> Acked-by: Wei Liu 
> ---
>  tools/libxc/xc_sr_common.c  | 41 ++
>  tools/libxc/xc_sr_common.h  | 72 
> +++--
>  tools/libxc/xc_sr_restore.c | 66 ++---
>  3 files changed, 114 insertions(+), 65 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
> index 79b9c3e940..4d221ca90c 100644
> --- a/tools/libxc/xc_sr_common.c
> +++ b/tools/libxc/xc_sr_common.c
> @@ -155,6 +155,47 @@ static void __attribute__((unused)) 
> build_assertions(void)
>  BUILD_BUG_ON(sizeof(struct xc_sr_rec_hvm_params)!= 8);
>  }
>  
> +/*
> + * Expand the tracking structures as needed.
> + * To avoid realloc()ing too excessively, the size increased to the nearest 
> power
> + * of two large enough to contain the required number of bits.
> + */
> +bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long bits)
> +{
> +if (bits > bm->bits)

Libxc uses the hypervisor coding style, and xc_sr_* currently have a
consistent style.

> +{
> +size_t new_max;
> +size_t old_sz, new_sz;
> +void *p;
> +
> +/* Round up to the nearest power of two larger than bit, less 1. */
> +new_max = bits;
> +new_max |= new_max >> 1;
> +new_max |= new_max >> 2;
> +new_max |= new_max >> 4;
> +new_max |= new_max >> 8;
> +new_max |= new_max >> 16;
> +#ifdef __x86_64__
> +new_max |= new_max >> 32;
> +#endif
> +
> +old_sz = bitmap_size(bm->bits + 1);
> +new_sz = bitmap_size(new_max + 1);
> +p = realloc(bm->p, new_sz);
> +if (!p)
> +return false;
> +
> +if (bm->p)
> +memset(p + old_sz, 0, new_sz - old_sz);
> +else
> +memset(p, 0, new_sz);
> +
> +bm->p = p;
> +bm->bits = new_max;
> +}
> +return true;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index a83f22af4e..734320947a 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -172,6 +172,12 @@ struct xc_sr_x86_pv_restore_vcpu
>  size_t basicsz, extdsz, xsavesz, msrsz;
>  };
>  
> +struct xc_sr_bitmap
> +{
> +void *p;
> +unsigned long bits;
> +};
> +
>  struct xc_sr_context
>  {
>  xc_interface *xch;
> @@ -255,8 +261,7 @@ struct xc_sr_context
>  domid_t  xenstore_domid,  console_domid;
>  
>  /* Bitmap of currently populated PFNs during restore. */
> -unsigned long *populated_pfns;
> -xen_pfn_t max_populated_pfn;
> +struct xc_sr_bitmap populated_pfns;
>  
>  /* Sender has invoked verify mode on the stream. */
>  bool verify;
> @@ -343,6 +348,69 @@ extern struct xc_sr_save_ops save_ops_x86_hvm;
>  extern struct xc_sr_restore_ops restore_ops_x86_pv;
>  extern struct xc_sr_restore_ops restore_ops_x86_hvm;
>  
> +extern bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long 
> bits);

No need for extern on function prototypes.

> +
> +static inline bool xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned 
> long bits)
> +{
> +if (bits > bm->bits)
> +return _xc_sr_bitmap_resize(bm, bits);
> +return true;
> +}
> +
> +static inline void xc_sr_bitmap_free(struct xc_sr_bitmap *bm)
> +{
> +free(bm->p);
> +bm->p = NULL;

bm->bits = 0, or a subsequent test/set/clear will fall over a NULL pointer.

> +}
> +
> +static inline bool xc_sr_set_bit(unsigned long bit, struct xc_sr_bitmap *bm)
> +{
> +if (!xc_sr_bitmap_resize(bm, bit))

There's a boundary condition here trying to test bit 0 of an empty bitmap.

> +return false;
> +
> +set_bit(bit, bm->p);
> +return true;
> +}
> +
> +static inline bool xc_sr_test_bit(unsigned long bit, struct xc_sr_bitmap *bm)
> +{
> +if (bit > bm->bits)
> +return false;
> +return !!test_bit(bit, bm->p);
> +}
> +
> +static inline bool xc_sr_test_and_clear_bit(unsigned long bit, struct 
> xc_sr_bitmap *bm)
> +{
> +if (bit > bm->bits)
> +return false;
> +return !!test_and_clear_bit(bit, bm->p);
> +}
> +
> +static inline bool xc_sr_test_and_set_bit(unsigned long bit, struct 
> xc_sr_bitmap *bm)
> +{
> +if (bit > bm->bits)
> +return false;
> +return !!test_and_set_bit(bit, bm->p);
> +}
> +
> +static inline bool pfn_is_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
> +{
> +return xc_sr_test_bit(pfn, >restore.populated_pfns);
> +}
> +
> +static inline int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
> +{
> +xc_interface *xch = ctx->xch;
> +
> +if ( !xc_sr_set_bit(pfn, 

[Xen-devel] [PATCH v9 2/3] tools/libxc: add API for bitmap access for restore

[Olaf Hering]

Extend API for managing bitmaps. Each bitmap is now represented by a
generic struct xc_sr_bitmap.
Switch the existing populated_pfns to this API.

Signed-off-by: Olaf Hering 
Acked-by: Wei Liu 
---
 tools/libxc/xc_sr_common.c  | 41 ++
 tools/libxc/xc_sr_common.h  | 72 +++--
 tools/libxc/xc_sr_restore.c | 66 ++---
 3 files changed, 114 insertions(+), 65 deletions(-)

diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
index 79b9c3e940..4d221ca90c 100644
--- a/tools/libxc/xc_sr_common.c
+++ b/tools/libxc/xc_sr_common.c
@@ -155,6 +155,47 @@ static void __attribute__((unused)) build_assertions(void)
 BUILD_BUG_ON(sizeof(struct xc_sr_rec_hvm_params)!= 8);
 }
 
+/*
+ * Expand the tracking structures as needed.
+ * To avoid realloc()ing too excessively, the size increased to the nearest 
power
+ * of two large enough to contain the required number of bits.
+ */
+bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long bits)
+{
+if (bits > bm->bits)
+{
+size_t new_max;
+size_t old_sz, new_sz;
+void *p;
+
+/* Round up to the nearest power of two larger than bit, less 1. */
+new_max = bits;
+new_max |= new_max >> 1;
+new_max |= new_max >> 2;
+new_max |= new_max >> 4;
+new_max |= new_max >> 8;
+new_max |= new_max >> 16;
+#ifdef __x86_64__
+new_max |= new_max >> 32;
+#endif
+
+old_sz = bitmap_size(bm->bits + 1);
+new_sz = bitmap_size(new_max + 1);
+p = realloc(bm->p, new_sz);
+if (!p)
+return false;
+
+if (bm->p)
+memset(p + old_sz, 0, new_sz - old_sz);
+else
+memset(p, 0, new_sz);
+
+bm->p = p;
+bm->bits = new_max;
+}
+return true;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index a83f22af4e..734320947a 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -172,6 +172,12 @@ struct xc_sr_x86_pv_restore_vcpu
 size_t basicsz, extdsz, xsavesz, msrsz;
 };
 
+struct xc_sr_bitmap
+{
+void *p;
+unsigned long bits;
+};
+
 struct xc_sr_context
 {
 xc_interface *xch;
@@ -255,8 +261,7 @@ struct xc_sr_context
 domid_t  xenstore_domid,  console_domid;
 
 /* Bitmap of currently populated PFNs during restore. */
-unsigned long *populated_pfns;
-xen_pfn_t max_populated_pfn;
+struct xc_sr_bitmap populated_pfns;
 
 /* Sender has invoked verify mode on the stream. */
 bool verify;
@@ -343,6 +348,69 @@ extern struct xc_sr_save_ops save_ops_x86_hvm;
 extern struct xc_sr_restore_ops restore_ops_x86_pv;
 extern struct xc_sr_restore_ops restore_ops_x86_hvm;
 
+extern bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long bits);
+
+static inline bool xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long 
bits)
+{
+if (bits > bm->bits)
+return _xc_sr_bitmap_resize(bm, bits);
+return true;
+}
+
+static inline void xc_sr_bitmap_free(struct xc_sr_bitmap *bm)
+{
+free(bm->p);
+bm->p = NULL;
+}
+
+static inline bool xc_sr_set_bit(unsigned long bit, struct xc_sr_bitmap *bm)
+{
+if (!xc_sr_bitmap_resize(bm, bit))
+return false;
+
+set_bit(bit, bm->p);
+return true;
+}
+
+static inline bool xc_sr_test_bit(unsigned long bit, struct xc_sr_bitmap *bm)
+{
+if (bit > bm->bits)
+return false;
+return !!test_bit(bit, bm->p);
+}
+
+static inline bool xc_sr_test_and_clear_bit(unsigned long bit, struct 
xc_sr_bitmap *bm)
+{
+if (bit > bm->bits)
+return false;
+return !!test_and_clear_bit(bit, bm->p);
+}
+
+static inline bool xc_sr_test_and_set_bit(unsigned long bit, struct 
xc_sr_bitmap *bm)
+{
+if (bit > bm->bits)
+return false;
+return !!test_and_set_bit(bit, bm->p);
+}
+
+static inline bool pfn_is_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
+{
+return xc_sr_test_bit(pfn, >restore.populated_pfns);
+}
+
+static inline int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
+{
+xc_interface *xch = ctx->xch;
+
+if ( !xc_sr_set_bit(pfn, >restore.populated_pfns) )
+{
+ERROR("Failed to realloc populated_pfns bitmap");
+errno = ENOMEM;
+return -1;
+}
+return 0;
+}
+
 struct xc_sr_record
 {
 uint32_t type;
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index a016678332..d53948e1a6 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -68,64 +68,6 @@ static int read_headers(struct xc_sr_context *ctx)
 return 0;
 }
 
-/*
- * Is a pfn populated?
- */
-static bool pfn_is_populated(const struct xc_sr_context *ctx, xen_pfn_t pfn)
-{
-if ( pfn > ctx->restore.max_populated_pfn )
-