Re: [PATCH v2 4/7] bitmap: move to/from xenctl_bitmap conversion helpers

[Julien Grall <julien@xxxxxxx>]

Hi,

On 07/08/2020 12:33, Jan Beulich wrote:
> A subsequent change will exclude domctl.c from getting built for a
> particular configuration, yet the two functions get used from elsewhere.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

> ---
> v2: Move function decls to xen/bitmap.h.
>
> --- a/xen/common/bitmap.c
> +++ b/xen/common/bitmap.c
> @@ -9,6 +9,8 @@
>   #include <xen/errno.h>
>   #include <xen/bitmap.h>
>   #include <xen/bitops.h>
> +#include <xen/cpumask.h>
> +#include <xen/guest_access.h>
>   #include <asm/byteorder.h>
>   
>   /*
> @@ -384,3 +386,87 @@ void bitmap_byte_to_long(unsigned long *
>   }
>   
>   #endif
> +
> +int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap,
> +                            const unsigned long *bitmap, unsigned int nbits)
> +{
> +    unsigned int guest_bytes, copy_bytes, i;
> +    uint8_t zero = 0;
> +    int err = 0;
> +    uint8_t *bytemap = xmalloc_array(uint8_t, (nbits + 7) / 8);
> +
> +    if ( !bytemap )
> +        return -ENOMEM;
> +
> +    guest_bytes = (xenctl_bitmap->nr_bits + 7) / 8;
> +    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
> +
> +    bitmap_long_to_byte(bytemap, bitmap, nbits);
> +
> +    if ( copy_bytes != 0 )
> +        if ( 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) )
> +            err = -EFAULT;
> +
> +    xfree(bytemap);
> +
> +    return err;
> +}
> +
> +int xenctl_bitmap_to_bitmap(unsigned long *bitmap,
> +                            const struct xenctl_bitmap *xenctl_bitmap,
> +                            unsigned int nbits)
> +{
> +    unsigned int guest_bytes, copy_bytes;
> +    int err = 0;
> +    uint8_t *bytemap = xzalloc_array(uint8_t, (nbits + 7) / 8);
> +
> +    if ( !bytemap )
> +        return -ENOMEM;
> +
> +    guest_bytes = (xenctl_bitmap->nr_bits + 7) / 8;
> +    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
> +
> +    if ( copy_bytes != 0 )
> +    {
> +        if ( copy_from_guest(bytemap, xenctl_bitmap->bitmap, copy_bytes) )
> +            err = -EFAULT;
> +        if ( (xenctl_bitmap->nr_bits & 7) && (guest_bytes == copy_bytes) )
> +            bytemap[guest_bytes-1] &= ~(0xff << (xenctl_bitmap->nr_bits & 7));
> +    }
> +
> +    if ( !err )
> +        bitmap_byte_to_long(bitmap, bytemap, nbits);
> +
> +    xfree(bytemap);
> +
> +    return err;
> +}
> +
> +int cpumask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_cpumap,
> +                             const cpumask_t *cpumask)
> +{
> +    return bitmap_to_xenctl_bitmap(xenctl_cpumap, cpumask_bits(cpumask),
> +                                   nr_cpu_ids);
> +}
> +
> +int xenctl_bitmap_to_cpumask(cpumask_var_t *cpumask,
> +                             const struct xenctl_bitmap *xenctl_cpumap)
> +{
> +    int err = 0;
> +
> +    if ( alloc_cpumask_var(cpumask) ) {
> +        err = xenctl_bitmap_to_bitmap(cpumask_bits(*cpumask), xenctl_cpumap,
> +                                      nr_cpu_ids);
> +        /* In case of error, cleanup is up to us, as the caller won't care! */
> +        if ( err )
> +            free_cpumask_var(*cpumask);
> +    }
> +    else
> +        err = -ENOMEM;
> +
> +    return err;
> +}
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -34,91 +34,6 @@
>   
>   static DEFINE_SPINLOCK(domctl_lock);
>   
> -static int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap,
> -                                   const unsigned long *bitmap,
> -                                   unsigned int nbits)
> -{
> -    unsigned int guest_bytes, copy_bytes, i;
> -    uint8_t zero = 0;
> -    int err = 0;
> -    uint8_t *bytemap = xmalloc_array(uint8_t, (nbits + 7) / 8);
> -
> -    if ( !bytemap )
> -        return -ENOMEM;
> -
> -    guest_bytes = (xenctl_bitmap->nr_bits + 7) / 8;
> -    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
> -
> -    bitmap_long_to_byte(bytemap, bitmap, nbits);
> -
> -    if ( copy_bytes != 0 )
> -        if ( 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) )
> -            err = -EFAULT;
> -
> -    xfree(bytemap);
> -
> -    return err;
> -}
> -
> -int xenctl_bitmap_to_bitmap(unsigned long *bitmap,
> -                            const struct xenctl_bitmap *xenctl_bitmap,
> -                            unsigned int nbits)
> -{
> -    unsigned int guest_bytes, copy_bytes;
> -    int err = 0;
> -    uint8_t *bytemap = xzalloc_array(uint8_t, (nbits + 7) / 8);
> -
> -    if ( !bytemap )
> -        return -ENOMEM;
> -
> -    guest_bytes = (xenctl_bitmap->nr_bits + 7) / 8;
> -    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
> -
> -    if ( copy_bytes != 0 )
> -    {
> -        if ( copy_from_guest(bytemap, xenctl_bitmap->bitmap, copy_bytes) )
> -            err = -EFAULT;
> -        if ( (xenctl_bitmap->nr_bits & 7) && (guest_bytes == copy_bytes) )
> -            bytemap[guest_bytes-1] &= ~(0xff << (xenctl_bitmap->nr_bits & 7));
> -    }
> -
> -    if ( !err )
> -        bitmap_byte_to_long(bitmap, bytemap, nbits);
> -
> -    xfree(bytemap);
> -
> -    return err;
> -}
> -
> -int cpumask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_cpumap,
> -                             const cpumask_t *cpumask)
> -{
> -    return bitmap_to_xenctl_bitmap(xenctl_cpumap, cpumask_bits(cpumask),
> -                                   nr_cpu_ids);
> -}
> -
> -int xenctl_bitmap_to_cpumask(cpumask_var_t *cpumask,
> -                             const struct xenctl_bitmap *xenctl_cpumap)
> -{
> -    int err = 0;
> -
> -    if ( alloc_cpumask_var(cpumask) ) {
> -        err = xenctl_bitmap_to_bitmap(cpumask_bits(*cpumask), xenctl_cpumap,
> -                                      nr_cpu_ids);
> -        /* In case of error, cleanup is up to us, as the caller won't care! */
> -        if ( err )
> -            free_cpumask_var(*cpumask);
> -    }
> -    else
> -        err = -ENOMEM;
> -
> -    return err;
> -}
> -
>   static int nodemask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_nodemap,
>                                        const nodemask_t *nodemask)
>   {
> --- a/xen/include/xen/bitmap.h
> +++ b/xen/include/xen/bitmap.h
> @@ -273,6 +273,13 @@ static inline void bitmap_clear(unsigned
>   void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, int nbits);
>   void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, int nbits);
>   
> +struct xenctl_bitmap;
> +int xenctl_bitmap_to_bitmap(unsigned long *bitmap,
> +                            const struct xenctl_bitmap *xenctl_bitmap,
> +                            unsigned int nbits);
> +int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap,
> +                            const unsigned long *bitmap, unsigned int nbits);
> +
>   #endif /* __ASSEMBLY__ */
>   
>   #endif /* __XEN_BITMAP_H */
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -27,9 +27,6 @@ struct xen_domctl_getdomaininfo;
>   void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
>   void arch_get_domain_info(const struct domain *d,
>                             struct xen_domctl_getdomaininfo *info);
> -int xenctl_bitmap_to_bitmap(unsigned long *bitmap,
> -                            const struct xenctl_bitmap *xenctl_bitmap,
> -                            unsigned int nbits);
>   
>   /*
>    * Arch-specifics.
--
Julien Grall