| Message ID | alpine.DEB.2.22.394.2503182002160.2000798@ubuntu-linux-20-04-desktop (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen: simplify bitmap_to_xenctl_bitmap for little endian | expand |
On 19.03.2025 04:03, Stefano Stabellini wrote: > --- a/xen/common/bitmap.c > +++ b/xen/common/bitmap.c > @@ -384,21 +384,26 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, > uint8_t zero = 0; > int err = 0; > unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); > - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > - > - if ( !bytemap ) > - return -ENOMEM; > + uint8_t *bytemap = (uint8_t *)bitmap; Not only Misra dislikes casting away of const-ness. > guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); > copy_bytes = min(guest_bytes, xen_bytes); > > - bitmap_long_to_byte(bytemap, bitmap, nbits); > + if ( IS_ENABLED(__BIG_ENDIAN) ) > + { > + bytemap = xmalloc_array(uint8_t, xen_bytes); > + if ( !bytemap ) > + return -ENOMEM; > + > + bitmap_long_to_byte(bytemap, bitmap, nbits); > + } Where did the clamp_last_byte() go in the little-endian case? > if ( copy_bytes && > copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) > err = -EFAULT; > > - xfree(bytemap); > + if ( IS_ENABLED(__BIG_ENDIAN) ) > + xfree(bytemap); > > for ( i = copy_bytes; !err && i < guest_bytes; i++ ) > if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) ) What about xenctl_bitmap_to_bitmap()? Jan
On Wed, 19 Mar 2025, Jan Beulich wrote: > What about xenctl_bitmap_to_bitmap()? Let me see first if I managed to handle bitmap_to_xenctl_bitmap well. --- [PATCH v2] xen: simplify bitmap_to_xenctl_bitmap for little endian The little endian implementation of bitmap_to_xenctl_bitmap leads to unnecessary xmallocs and xfrees. Given that Xen only supports little endian architectures, it is worth optimizing. This patch removes the need for the xmalloc on little endian architectures. Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> --- Changes in v2: - don't remove const - handle clamp_last_byte for little endian diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c index 3da63a32a6..e9876ee5a6 100644 --- a/xen/common/bitmap.c +++ b/xen/common/bitmap.c @@ -384,21 +384,33 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, uint8_t zero = 0; int err = 0; unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); - - if ( !bytemap ) - return -ENOMEM; + bool alloc = (bitmap[nbits/8] & ((1U << (nbits % 8)) - 1)) || + IS_ENABLED(__BIG_ENDIAN); guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); copy_bytes = min(guest_bytes, xen_bytes); - bitmap_long_to_byte(bytemap, bitmap, nbits); + if ( alloc ) + { + uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); + if ( !bytemap ) + return -ENOMEM; - if ( copy_bytes && - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) - err = -EFAULT; + bitmap_long_to_byte(bytemap, bitmap, nbits); - xfree(bytemap); + if ( copy_bytes && + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) + err = -EFAULT; + + xfree(bytemap); + } + else + { + const uint8_t *bytemap = (const uint8_t *)bitmap; + if ( copy_bytes && + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) + err = -EFAULT; + } for ( i = copy_bytes; !err && i < guest_bytes; i++ ) if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) )
On 20.03.2025 01:57, Stefano Stabellini wrote: > On Wed, 19 Mar 2025, Jan Beulich wrote: >> What about xenctl_bitmap_to_bitmap()? > > Let me see first if I managed to handle bitmap_to_xenctl_bitmap well. Well, the code looks correct to me, but the description now has gone stale. I also wonder whether with that extra restriction the optimization is then actually worth it. Just one further nit: > --- a/xen/common/bitmap.c > +++ b/xen/common/bitmap.c > @@ -384,21 +384,33 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, > uint8_t zero = 0; > int err = 0; > unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); > - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > - > - if ( !bytemap ) > - return -ENOMEM; > + bool alloc = (bitmap[nbits/8] & ((1U << (nbits % 8)) - 1)) || Blanks missing around / here. Jan
On Thu, 20 Mar 2025, Jan Beulich wrote: > On 20.03.2025 01:57, Stefano Stabellini wrote: > > On Wed, 19 Mar 2025, Jan Beulich wrote: > >> What about xenctl_bitmap_to_bitmap()? > > > > Let me see first if I managed to handle bitmap_to_xenctl_bitmap well. > > Well, the code looks correct to me, but the description now has gone > stale. I also wonder whether with that extra restriction the optimization > is then actually worth it. Just one further nit: Hi Jan, you make a good point. I tried to come up with a better approach. What do you think of this? diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c index 3da63a32a6..2f448693c3 100644 --- a/xen/common/bitmap.c +++ b/xen/common/bitmap.c @@ -52,7 +52,7 @@ static void clamp_last_byte(uint8_t *bp, unsigned int nbits) unsigned int remainder = nbits % 8; if (remainder) - bp[nbits/8] &= (1U << remainder) - 1; + *bp &= (1U << remainder) - 1; } int __bitmap_empty(const unsigned long *bitmap, unsigned int bits) @@ -338,7 +338,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, nbits -= 8; } } - clamp_last_byte(bp, nbits); } static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, @@ -363,7 +362,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, unsigned int nbits) { memcpy(bp, lp, DIV_ROUND_UP(nbits, BITS_PER_BYTE)); - clamp_last_byte(bp, nbits); } static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, @@ -384,21 +382,40 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, uint8_t zero = 0; int err = 0; unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); - - if ( !bytemap ) - return -ENOMEM; + uint8_t last; guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); copy_bytes = min(guest_bytes, xen_bytes); - bitmap_long_to_byte(bytemap, bitmap, nbits); + if ( IS_ENABLED(__BIG_ENDIAN) ) + { + uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); - if ( copy_bytes && - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) - err = -EFAULT; + if ( !bytemap ) + return -ENOMEM; - xfree(bytemap); + bitmap_long_to_byte(bytemap, bitmap, nbits); + last = bytemap[nbits/8]; + + if ( copy_bytes && + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) + err = -EFAULT; + + xfree(bytemap); + } + else + { + const uint8_t *bytemap = (const uint8_t *)bitmap; + last = bytemap[nbits/8]; + + if ( copy_bytes && + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) + err = -EFAULT; + } + + clamp_last_byte(&last, nbits); + if ( copy_to_guest_offset(xenctl_bitmap->bitmap, copy_bytes - 1, &last, 1) ) + err = -EFAULT; for ( i = copy_bytes; !err && i < guest_bytes; i++ ) if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) )
On 22.03.2025 00:09, Stefano Stabellini wrote: > On Thu, 20 Mar 2025, Jan Beulich wrote: >> On 20.03.2025 01:57, Stefano Stabellini wrote: >>> On Wed, 19 Mar 2025, Jan Beulich wrote: >>>> What about xenctl_bitmap_to_bitmap()? >>> >>> Let me see first if I managed to handle bitmap_to_xenctl_bitmap well. >> >> Well, the code looks correct to me, but the description now has gone >> stale. I also wonder whether with that extra restriction the optimization >> is then actually worth it. Just one further nit: > > Hi Jan, you make a good point. I tried to come up with a better > approach. What do you think of this? > > > diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c > index 3da63a32a6..2f448693c3 100644 > --- a/xen/common/bitmap.c > +++ b/xen/common/bitmap.c > @@ -52,7 +52,7 @@ static void clamp_last_byte(uint8_t *bp, unsigned int nbits) > unsigned int remainder = nbits % 8; > > if (remainder) > - bp[nbits/8] &= (1U << remainder) - 1; > + *bp &= (1U << remainder) - 1; > } > > int __bitmap_empty(const unsigned long *bitmap, unsigned int bits) > @@ -338,7 +338,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, > nbits -= 8; > } > } > - clamp_last_byte(bp, nbits); > } > > static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, > @@ -363,7 +362,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, > unsigned int nbits) > { > memcpy(bp, lp, DIV_ROUND_UP(nbits, BITS_PER_BYTE)); > - clamp_last_byte(bp, nbits); > } > > static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, > @@ -384,21 +382,40 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, > uint8_t zero = 0; > int err = 0; > unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); > - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > - > - if ( !bytemap ) > - return -ENOMEM; > + uint8_t last; > > guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); > copy_bytes = min(guest_bytes, xen_bytes); > > - bitmap_long_to_byte(bytemap, bitmap, nbits); > + if ( IS_ENABLED(__BIG_ENDIAN) ) > + { > + uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > > - if ( copy_bytes && > - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) > - err = -EFAULT; > + if ( !bytemap ) > + return -ENOMEM; > > - xfree(bytemap); > + bitmap_long_to_byte(bytemap, bitmap, nbits); > + last = bytemap[nbits/8]; Same style nit as before. > + if ( copy_bytes && copy_bytes > 1 > + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > + err = -EFAULT; > + > + xfree(bytemap); > + } > + else > + { > + const uint8_t *bytemap = (const uint8_t *)bitmap; > + last = bytemap[nbits/8]; > + > + if ( copy_bytes && > + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > + err = -EFAULT; The two identical instances would imo better stay common, even if this may require another function-scope variable (to invoke xfree() on after the copy-out). > + } > + > + clamp_last_byte(&last, nbits); > + if ( copy_to_guest_offset(xenctl_bitmap->bitmap, copy_bytes - 1, &last, 1) ) > + err = -EFAULT; Careful here in particular when copy_bytes == 0. Jan
On Mon, 24 Mar 2025, Jan Beulich wrote: > On 22.03.2025 00:09, Stefano Stabellini wrote: > > On Thu, 20 Mar 2025, Jan Beulich wrote: > >> On 20.03.2025 01:57, Stefano Stabellini wrote: > >>> On Wed, 19 Mar 2025, Jan Beulich wrote: > >>>> What about xenctl_bitmap_to_bitmap()? > >>> > >>> Let me see first if I managed to handle bitmap_to_xenctl_bitmap well. > >> > >> Well, the code looks correct to me, but the description now has gone > >> stale. I also wonder whether with that extra restriction the optimization > >> is then actually worth it. Just one further nit: > > > > Hi Jan, you make a good point. I tried to come up with a better > > approach. What do you think of this? > > > > > > diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c > > index 3da63a32a6..2f448693c3 100644 > > --- a/xen/common/bitmap.c > > +++ b/xen/common/bitmap.c > > @@ -52,7 +52,7 @@ static void clamp_last_byte(uint8_t *bp, unsigned int nbits) > > unsigned int remainder = nbits % 8; > > > > if (remainder) > > - bp[nbits/8] &= (1U << remainder) - 1; > > + *bp &= (1U << remainder) - 1; > > } > > > > int __bitmap_empty(const unsigned long *bitmap, unsigned int bits) > > @@ -338,7 +338,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, > > nbits -= 8; > > } > > } > > - clamp_last_byte(bp, nbits); > > } > > > > static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, > > @@ -363,7 +362,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, > > unsigned int nbits) > > { > > memcpy(bp, lp, DIV_ROUND_UP(nbits, BITS_PER_BYTE)); > > - clamp_last_byte(bp, nbits); > > } > > > > static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, > > @@ -384,21 +382,40 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, > > uint8_t zero = 0; > > int err = 0; > > unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); > > - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > > - > > - if ( !bytemap ) > > - return -ENOMEM; > > + uint8_t last; > > > > guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); > > copy_bytes = min(guest_bytes, xen_bytes); > > > > - bitmap_long_to_byte(bytemap, bitmap, nbits); > > + if ( IS_ENABLED(__BIG_ENDIAN) ) > > + { > > + uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); > > > > - if ( copy_bytes && > > - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) > > - err = -EFAULT; > > + if ( !bytemap ) > > + return -ENOMEM; > > > > - xfree(bytemap); > > + bitmap_long_to_byte(bytemap, bitmap, nbits); > > + last = bytemap[nbits/8]; > > Same style nit as before. > > > + if ( copy_bytes && > > copy_bytes > 1 > > > + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > > + err = -EFAULT; > > + > > + xfree(bytemap); > > + } > > + else > > + { > > + const uint8_t *bytemap = (const uint8_t *)bitmap; > > + last = bytemap[nbits/8]; > > + > > + if ( copy_bytes && > > + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > > + err = -EFAULT; > > The two identical instances would imo better stay common, even if this may > require another function-scope variable (to invoke xfree() on after the > copy-out). That's not possible because bytemap is defined differently in the two cases so it has to be defined within the if block. I addressed everything else, I'll send v3 as a separate patch > > + } > > + > > + clamp_last_byte(&last, nbits); > > + if ( copy_to_guest_offset(xenctl_bitmap->bitmap, copy_bytes - 1, &last, 1) ) > > + err = -EFAULT; > > Careful here in particular when copy_bytes == 0. > > Jan >
On 28.03.2025 00:33, Stefano Stabellini wrote: > On Mon, 24 Mar 2025, Jan Beulich wrote: >> On 22.03.2025 00:09, Stefano Stabellini wrote: >>> @@ -384,21 +382,40 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, >>> uint8_t zero = 0; >>> int err = 0; >>> unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); >>> - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); >>> - >>> - if ( !bytemap ) >>> - return -ENOMEM; >>> + uint8_t last; >>> >>> guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); >>> copy_bytes = min(guest_bytes, xen_bytes); >>> >>> - bitmap_long_to_byte(bytemap, bitmap, nbits); >>> + if ( IS_ENABLED(__BIG_ENDIAN) ) >>> + { >>> + uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); >>> >>> - if ( copy_bytes && >>> - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) >>> - err = -EFAULT; >>> + if ( !bytemap ) >>> + return -ENOMEM; >>> >>> - xfree(bytemap); >>> + bitmap_long_to_byte(bytemap, bitmap, nbits); >>> + last = bytemap[nbits/8]; >> >> Same style nit as before. >> >>> + if ( copy_bytes && >> >> copy_bytes > 1 >> >>> + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) >>> + err = -EFAULT; >>> + >>> + xfree(bytemap); >>> + } >>> + else >>> + { >>> + const uint8_t *bytemap = (const uint8_t *)bitmap; >>> + last = bytemap[nbits/8]; >>> + >>> + if ( copy_bytes && >>> + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) >>> + err = -EFAULT; >> >> The two identical instances would imo better stay common, even if this may >> require another function-scope variable (to invoke xfree() on after the >> copy-out). > > That's not possible because bytemap is defined differently in the two > cases so it has to be defined within the if block. Hence why I said "even if this may require another function-scope variable". Jan
On Fri, 28 Mar 2025, Jan Beulich wrote: > >>> + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > >>> + err = -EFAULT; > >>> + > >>> + xfree(bytemap); > >>> + } > >>> + else > >>> + { > >>> + const uint8_t *bytemap = (const uint8_t *)bitmap; > >>> + last = bytemap[nbits/8]; > >>> + > >>> + if ( copy_bytes && > >>> + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) > >>> + err = -EFAULT; > >> > >> The two identical instances would imo better stay common, even if this may > >> require another function-scope variable (to invoke xfree() on after the > >> copy-out). > > > > That's not possible because bytemap is defined differently in the two > > cases so it has to be defined within the if block. > > Hence why I said "even if this may require another function-scope variable". Sorry Jan, I don't understand in practice how to make your suggestion work. Do you mean to do this? #ifdef __BIG_ENDIAN uint8_t *bytemap; #else const uint8_t *bytemap; #endif If so, I get this build error: common/bitmap.c: In function ‘bitmap_to_xenctl_bitmap’: common/bitmap.c:402:29: error: passing argument 1 of ‘bitmap_long_to_byte’ discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 402 | bitmap_long_to_byte(bytemap, bitmap, nbits); | ^~~~~~~ I am trying to guess what you have in mind, but it might be more effective if you could write it down. For reference, I am including the patch I was trying to get to work. diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c index 3da63a32a6..b213e29ca5 100644 --- a/xen/common/bitmap.c +++ b/xen/common/bitmap.c @@ -52,7 +52,7 @@ static void clamp_last_byte(uint8_t *bp, unsigned int nbits) unsigned int remainder = nbits % 8; if (remainder) - bp[nbits/8] &= (1U << remainder) - 1; + *bp &= (1U << remainder) - 1; } int __bitmap_empty(const unsigned long *bitmap, unsigned int bits) @@ -338,7 +338,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, nbits -= 8; } } - clamp_last_byte(bp, nbits); } static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, @@ -363,7 +362,6 @@ static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, unsigned int nbits) { memcpy(bp, lp, DIV_ROUND_UP(nbits, BITS_PER_BYTE)); - clamp_last_byte(bp, nbits); } static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, @@ -384,21 +382,43 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, uint8_t zero = 0; int err = 0; unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); - - if ( !bytemap ) - return -ENOMEM; + uint8_t last; +#ifdef __BIG_ENDIAN + uint8_t *bytemap; +#else + const uint8_t *bytemap; +#endif guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); copy_bytes = min(guest_bytes, xen_bytes); - bitmap_long_to_byte(bytemap, bitmap, nbits); + if ( IS_ENABLED(__BIG_ENDIAN) ) + { + bytemap = xmalloc_array(uint8_t, xen_bytes); + + if ( !bytemap ) + return -ENOMEM; - if ( copy_bytes && - copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) + bitmap_long_to_byte(bytemap, bitmap, nbits); + last = bytemap[nbits / 8]; + } + else + { + bytemap = (const uint8_t *)bitmap; + last = bytemap[nbits / 8]; + } + + if ( copy_bytes > 1 && + copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) ) err = -EFAULT; - xfree(bytemap); + if ( IS_ENABLED(__BIG_ENDIAN) ) + xfree(bytemap); + + clamp_last_byte(&last, nbits); + if ( copy_bytes > 0 && + copy_to_guest_offset(xenctl_bitmap->bitmap, copy_bytes - 1, &last, 1) ) + err = -EFAULT; for ( i = copy_bytes; !err && i < guest_bytes; i++ ) if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) )
diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c index 3da63a32a6..38d77fc876 100644 --- a/xen/common/bitmap.c +++ b/xen/common/bitmap.c @@ -384,21 +384,26 @@ int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap, uint8_t zero = 0; int err = 0; unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE); - uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes); - - if ( !bytemap ) - return -ENOMEM; + uint8_t *bytemap = (uint8_t *)bitmap; guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE); copy_bytes = min(guest_bytes, xen_bytes); - bitmap_long_to_byte(bytemap, bitmap, nbits); + if ( IS_ENABLED(__BIG_ENDIAN) ) + { + bytemap = xmalloc_array(uint8_t, xen_bytes); + if ( !bytemap ) + return -ENOMEM; + + bitmap_long_to_byte(bytemap, bitmap, nbits); + } if ( copy_bytes && copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) ) err = -EFAULT; - xfree(bytemap); + if ( IS_ENABLED(__BIG_ENDIAN) ) + xfree(bytemap); for ( i = copy_bytes; !err && i < guest_bytes; i++ ) if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) )
The little endian implementation of bitmap_to_xenctl_bitmap leads to unnecessary xmallocs and xfrees. Given that Xen only supports little endian architectures, it is worth optimizing. This patch removes the need for the xmalloc on little endian architectures. Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>