| Message ID | 20170901160843.9057-3-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <olaf@aepfle.de> > Acked-by: Wei Liu <wei.liu2@citrix.com> > --- > 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, &ctx->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, &ctx->restore.populated_pfns) ) > + { > + ERROR("Failed to realloc populated_pfns bitmap"); > + errno = ENOMEM; > + return -1; > + } > + return 0; > +} Why are these moved? They are still restore specific. ~Andrew > + > 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 ) > - return false; > - return test_bit(pfn, ctx->restore.populated_pfns); > -} > - > -/* > - * Set a pfn as populated, expanding the tracking structures if needed. To > - * avoid realloc()ing too excessively, the size increased to the nearest power > - * of two large enough to contain the required pfn. > - */ > -static int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn) > -{ > - xc_interface *xch = ctx->xch; > - > - if ( pfn > ctx->restore.max_populated_pfn ) > - { > - xen_pfn_t new_max; > - size_t old_sz, new_sz; > - unsigned long *p; > - > - /* Round up to the nearest power of two larger than pfn, less 1. */ > - new_max = pfn; > - 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(ctx->restore.max_populated_pfn + 1); > - new_sz = bitmap_size(new_max + 1); > - p = realloc(ctx->restore.populated_pfns, new_sz); > - if ( !p ) > - { > - ERROR("Failed to realloc populated bitmap"); > - errno = ENOMEM; > - return -1; > - } > - > - memset((uint8_t *)p + old_sz, 0x00, new_sz - old_sz); > - > - ctx->restore.populated_pfns = p; > - ctx->restore.max_populated_pfn = new_max; > - } > - > - assert(!test_bit(pfn, ctx->restore.populated_pfns)); > - set_bit(pfn, ctx->restore.populated_pfns); > - > - return 0; > -} > - > /* > * Given a set of pfns, obtain memory from Xen to fill the physmap for the > * unpopulated subset. If types is NULL, no page type checking is performed > @@ -684,10 +626,8 @@ static int setup(struct xc_sr_context *ctx) > if ( rc ) > goto err; > > - ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; > - ctx->restore.populated_pfns = bitmap_alloc( > - ctx->restore.max_populated_pfn + 1); > - if ( !ctx->restore.populated_pfns ) > + rc = !xc_sr_bitmap_resize(&ctx->restore.populated_pfns, 32 * 1024 / 4); > + if ( rc ) > { > ERROR("Unable to allocate memory for populated_pfns bitmap"); > rc = -1; > @@ -722,7 +662,7 @@ static void cleanup(struct xc_sr_context *ctx) > xc_hypercall_buffer_free_pages(xch, dirty_bitmap, > NRPAGES(bitmap_size(ctx->restore.p2m_size))); > free(ctx->restore.buffered_records); > - free(ctx->restore.populated_pfns); > + xc_sr_bitmap_free(&ctx->restore.populated_pfns); > if ( ctx->restore.ops.cleanup(ctx) ) > PERROR("Failed to clean up"); > }
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
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, &ctx->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, &ctx->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 ) - return false; - return test_bit(pfn, ctx->restore.populated_pfns); -} - -/* - * Set a pfn as populated, expanding the tracking structures if needed. To - * avoid realloc()ing too excessively, the size increased to the nearest power - * of two large enough to contain the required pfn. - */ -static int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn) -{ - xc_interface *xch = ctx->xch; - - if ( pfn > ctx->restore.max_populated_pfn ) - { - xen_pfn_t new_max; - size_t old_sz, new_sz; - unsigned long *p; - - /* Round up to the nearest power of two larger than pfn, less 1. */ - new_max = pfn; - 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(ctx->restore.max_populated_pfn + 1); - new_sz = bitmap_size(new_max + 1); - p = realloc(ctx->restore.populated_pfns, new_sz); - if ( !p ) - { - ERROR("Failed to realloc populated bitmap"); - errno = ENOMEM; - return -1; - } - - memset((uint8_t *)p + old_sz, 0x00, new_sz - old_sz); - - ctx->restore.populated_pfns = p; - ctx->restore.max_populated_pfn = new_max; - } - - assert(!test_bit(pfn, ctx->restore.populated_pfns)); - set_bit(pfn, ctx->restore.populated_pfns); - - return 0; -} - /* * Given a set of pfns, obtain memory from Xen to fill the physmap for the * unpopulated subset. If types is NULL, no page type checking is performed @@ -684,10 +626,8 @@ static int setup(struct xc_sr_context *ctx) if ( rc ) goto err; - ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; - ctx->restore.populated_pfns = bitmap_alloc( - ctx->restore.max_populated_pfn + 1); - if ( !ctx->restore.populated_pfns ) + rc = !xc_sr_bitmap_resize(&ctx->restore.populated_pfns, 32 * 1024 / 4); + if ( rc ) { ERROR("Unable to allocate memory for populated_pfns bitmap"); rc = -1; @@ -722,7 +662,7 @@ static void cleanup(struct xc_sr_context *ctx) xc_hypercall_buffer_free_pages(xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->restore.p2m_size))); free(ctx->restore.buffered_records); - free(ctx->restore.populated_pfns); + xc_sr_bitmap_free(&ctx->restore.populated_pfns); if ( ctx->restore.ops.cleanup(ctx) ) PERROR("Failed to clean up"); }