Re: [RFC] Re: [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

[Jimi Xenidis <jimix@watson.ibm.com>]

Keir Fraser <keir@xensource.com> writes:

> On 21/12/06 5:32 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
>
>>> How about functions long_to_byte_bitmap(bitmap, nr_bits) and
>>> byte_to_long_bitmap(bitmap, nr_bits) defined in asm/bitops.h?
>> 
[snip]
>
> The way you are doing the conversion would be fine. I'm mainly aiming for a
> clean abstraction and formalising the concept of a 'byte-oriented bitmap' is
> the best way of doing this that I can see right now.

Sorry for taking so long with this, please comment on the following:

The following patch adds the ability to convert form "long" based
bitmaps to "byte" base bitmaps for portability. This is only has the
hypervisor side, but the definitions are such that the same prototype
can be use by XC and Xen.

Sadly it adds a copy on the xenctl_cpumap_to_cpumask(), but it minimally impacts the common paths.

-JX


diff -r a6813a652f39 xen/arch/powerpc/domctl.c
--- a/xen/arch/powerpc/domctl.c	Tue Jan 16 11:55:05 2007 -0500
+++ b/xen/arch/powerpc/domctl.c	Tue Jan 16 18:11:13 2007 -0500
@@ -116,3 +116,44 @@ long arch_do_domctl(struct xen_domctl *d
 
     return ret;
 }
+
+/*
+ * these routine implement bitmasks as uint8_t arrays for portability
+ */
+static inline int test_bit8(ulong nr, const uint8_t *addr)
+{
+	return (1U & (addr[nr >> 3] >> (nr & 15)));
+}
+
+static inline void set_bit8(ulong nr, uint8_t *addr)
+{
+	uint8_t bit = 1UL << (nr & 15);
+	uint8_t *p = addr + (nr >> 3);
+
+	*p |= bit;
+}
+
+
+void *long_to_byte_bitmap(uint8_t *bp, const ulong *lp, size_t n)
+{
+    int i;
+
+    memset(bp, 0, n);
+    for (i = 0; i < n * 8; i++) {
+        if (test_bit(i, lp))
+            set_bit8(i, bp);
+    }
+    return bp;
+}
+
+void *byte_to_long_bitmap(ulong *lp, const uint8_t *bp, size_t n)
+{
+    int i;
+
+    memset(lp, 0, n);
+    for (i = 0; i < n * 8; i++) {
+        if (test_bit8(i, bp))
+            __set_bit(i, lp); /* non-atomic */
+    }
+    return lp;
+}
diff -r a6813a652f39 xen/common/domctl.c
--- a/xen/common/domctl.c	Tue Jan 16 11:55:05 2007 -0500
+++ b/xen/common/domctl.c	Tue Jan 16 13:40:01 2007 -0500
@@ -32,6 +32,8 @@ void cpumask_to_xenctl_cpumap(
 {
     unsigned int guest_bytes, copy_bytes, i;
     uint8_t zero = 0;
+    uint8_t local[sizeof (*cpumask)];
+    void *ptr;
 
     if ( guest_handle_is_null(xenctl_cpumap->bitmap) )
         return;
@@ -39,9 +41,9 @@ void cpumask_to_xenctl_cpumap(
     guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
     copy_bytes  = min_t(unsigned int, guest_bytes, (NR_CPUS + 7) / 8);
 
-    copy_to_guest(xenctl_cpumap->bitmap,
-                  (uint8_t *)cpus_addr(*cpumask),
-                  copy_bytes);
+    ptr = long_to_byte_bitmap(local, cpus_addr(*cpumask), copy_bytes);
+
+    copy_to_guest(xenctl_cpumap->bitmap, ptr, copy_bytes);
 
     for ( i = copy_bytes; i < guest_bytes; i++ )
         copy_to_guest_offset(xenctl_cpumap->bitmap, i, &zero, 1);
@@ -51,6 +53,7 @@ void xenctl_cpumap_to_cpumask(
     cpumask_t *cpumask, struct xenctl_cpumap *xenctl_cpumap)
 {
     unsigned int guest_bytes, copy_bytes;
+    uint8_t local[sizeof (*cpumask)];
 
     guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
     copy_bytes  = min_t(unsigned int, guest_bytes, (NR_CPUS + 7) / 8);
@@ -60,9 +63,9 @@ void xenctl_cpumap_to_cpumask(
     if ( guest_handle_is_null(xenctl_cpumap->bitmap) )
         return;
 
-    copy_from_guest((uint8_t *)cpus_addr(*cpumask),
-                    xenctl_cpumap->bitmap,
-                    copy_bytes);
+    copy_from_guest(&local[0], xenctl_cpumap->bitmap, copy_bytes);
+
+    byte_to_long_bitmap(cpus_addr(*cpumask), local, copy_bytes);
 }
 
 static inline int is_free_domid(domid_t dom)
diff -r a6813a652f39 xen/include/public/arch-powerpc.h
--- a/xen/include/public/arch-powerpc.h	Tue Jan 16 11:55:05 2007 -0500
+++ b/xen/include/public/arch-powerpc.h	Tue Jan 16 17:53:30 2007 -0500
@@ -118,4 +118,6 @@ struct arch_vcpu_info {
 #define MAX_VIRT_CPUS 32
 #endif
 
+#define __HAVE_ARCH_BYTE_TO_LONG_BITMAP
+
 #endif
diff -r a6813a652f39 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Tue Jan 16 11:55:05 2007 -0500
+++ b/xen/include/public/domctl.h	Tue Jan 16 18:09:55 2007 -0500
@@ -424,6 +424,16 @@ typedef struct xen_domctl xen_domctl_t;
 typedef struct xen_domctl xen_domctl_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_t);
 
+#ifdef __HAVE_ARCH_BYTE_TO_LONG_BITMAP
+extern void *long_to_byte_bitmap(uint8_t *bp, const ulong *lp,
+                                 size_t n);
+extern void *byte_to_long_bitmap(ulong *lp, const uint8_t *bp,
+                                 size_t n);
+#else
+#define long_to_byte_bitmap(bp, lp, s) ((void)bp, (void)s, lp)
+#define byte_to_long_bitmap(lp, bp, s) memcpy(lp, bp, s)
+#endif
+
 #endif /* __XEN_PUBLIC_DOMCTL_H__ */
 
 /*