thr3ads.net - Xen devel - [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays [Dec 2006]

If this information is useful, please help other people find it:
Share via:

Jimi Xenidis

2006-Dec-17 17:53 UTC

[Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

On PowerPC (and other big endian and/or RISC architectures) bit
offsets in a bitmap are actually represented by a bit-offset from an
element in an array rather than a bit-offset from the base memory
pointer,  see xen/include/asm-powerpc/bitops.h for a complete
explanation.

This complicates the conversion of cpumask_t from/to xenctl_cpumap.

The following patch allows an architecture to declare that bitops are
"by long" rather than "by bit" and use an alternate scheme
for
encoding.

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>

---

diff -r b04e24db308f xen/common/domctl.c
--- a/xen/common/domctl.c	Sun Dec 17 12:40:10 2006 -0500
+++ b/xen/common/domctl.c	Fri Dec 15 17:03:47 2006 -0500
@@ -30,14 +30,36 @@ void cpumask_to_xenctl_cpumap(
 void cpumask_to_xenctl_cpumap(
     struct xenctl_cpumap *xenctl_cpumap, cpumask_t *cpumask)
 {
-    unsigned int guest_bytes, copy_bytes, i;
+    unsigned int guest_bytes, copy_bytes, xen_bytes, i;
     uint8_t zero = 0;
+    cpumask_t local;
 
     if ( guest_handle_is_null(xenctl_cpumap->bitmap) )
         return;
 
     guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
-    copy_bytes  = min_t(unsigned int, guest_bytes, (NR_CPUS + 7) / 8);
+
+    xen_bytes = (NR_CPUS + 7) / 8;
+    if (bitmap_by_long) {
+        if (((guest_bytes * 8) % BITS_PER_LONG) != 0) {
+            printk("%s: Unable to translate bitmap\n", __func__);
+            return;
+        }
+
+        /* local copy */
+        memcpy(cpus_addr(local), cpus_addr(*cpumask), sizeof (local));
+
+        /* clear unused bits */
+        for (i = NR_CPUS; i < sizeof(local) * 8; i++) {
+            /* non-atomic version */
+            __clear_bit(i, cpus_addr(local));
+        }
+
+        xen_bytes = sizeof(local);
+        cpumask = &local;
+    }
+
+    copy_bytes  = min_t(unsigned int, guest_bytes, xen_bytes);
 
     copy_to_guest(xenctl_cpumap->bitmap,
                   (uint8_t *)cpus_addr(*cpumask),
@@ -50,10 +72,20 @@ void xenctl_cpumap_to_cpumask(
 void xenctl_cpumap_to_cpumask(
     cpumask_t *cpumask, struct xenctl_cpumap *xenctl_cpumap)
 {
-    unsigned int guest_bytes, copy_bytes;
+    unsigned int guest_bytes, copy_bytes, xen_bytes;
 
     guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
-    copy_bytes  = min_t(unsigned int, guest_bytes, (NR_CPUS + 7) / 8);
+    xen_bytes = (NR_CPUS + 7) / 8;
+
+    if (bitmap_by_long) {
+        if (((guest_bytes * 8) % BITS_PER_LONG) != 0) {
+            printk("%s: Unable to translate bitmap\n", __func__);
+            return;
+        }
+        xen_bytes = sizeof(*cpumask);
+    }
+
+    copy_bytes  = min_t(unsigned int, guest_bytes, xen_bytes);
 
     cpus_clear(*cpumask);
 
diff -r b04e24db308f xen/include/asm-powerpc/bitops.h
--- a/xen/include/asm-powerpc/bitops.h	Sun Dec 17 12:40:10 2006 -0500
+++ b/xen/include/asm-powerpc/bitops.h	Fri Dec 15 16:24:26 2006 -0500
@@ -41,6 +41,9 @@
 #define _PPC64_BITOPS_H
 
 #include <asm/memory.h>
+
+/* The following indicates that bitops are implemented as described above */
+#define bitmap_by_long (1)
 
 /*
  * clear_bit doesn''t imply a memory barrier
diff -r b04e24db308f xen/include/xen/bitops.h
--- a/xen/include/xen/bitops.h	Sun Dec 17 12:40:10 2006 -0500
+++ b/xen/include/xen/bitops.h	Fri Dec 15 16:22:32 2006 -0500
@@ -75,6 +75,10 @@ static __inline__ int generic_fls(int x)
  * scope
  */
 #include <asm/bitops.h>
+
+#ifndef bitmap_by_long
+#define bitmap_by_long (0)
+#endif
 
 
 static inline int generic_fls64(__u64 x)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Dec-18 09:26 UTC

head link

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

On 17/12/06 5:53 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
> On PowerPC (and other big endian and/or RISC architectures) bit
> offsets in a bitmap are actually represented by a bit-offset from an
> element in an array rather than a bit-offset from the base memory
> pointer,  see xen/include/asm-powerpc/bitops.h for a complete
> explanation.
> 
> This complicates the conversion of cpumask_t from/to xenctl_cpumap.
> 
> The following patch allows an architecture to declare that bitops are
> "by long" rather than "by bit" and use an alternate
scheme for
> encoding.
If the array-element type matters on big-endian systems then the tools
really need fixing: one libxc operation passes down a uint32_t unit; another
a uint64_t. Perhaps we should fix so they pass down 8-bit units (as the type
of xenctl_cpumap_t would suggest) and then we could have byte_to_long_bitmap
and long_to_byte_bitmap in Xen (which might encourage us to pull proper
endianness headers into Xen).

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2006-Dec-18 16:50 UTC

head link

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

On Dec 18, 2006, at 4:26 AM, Keir Fraser wrote:
> On 17/12/06 5:53 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> On PowerPC (and other big endian and/or RISC architectures) bit
>> offsets in a bitmap are actually represented by a bit-offset from an
>> element in an array rather than a bit-offset from the base memory
>> pointer,  see xen/include/asm-powerpc/bitops.h for a complete
>> explanation.
>>
>> This complicates the conversion of cpumask_t from/to xenctl_cpumap.
>>
>> The following patch allows an architecture to declare that bitops are
>> "by long" rather than "by bit" and use an alternate
scheme for
>> encoding.
>
> If the array-element type matters on big-endian systems then the tools
> really need fixing: one libxc operation passes down a uint32_t unit;
Of the 3 usages only the tracebuffer mask uses uint32_t, maybe that  
could be repaired with the updated tracing work.
> another
> a uint64_t. Perhaps we should fix so they pass down 8-bit units (as  
> the type
> of xenctl_cpumap_t would suggest) and then we could have  
> byte_to_long_bitmap
> and long_to_byte_bitmap in Xen
Are you suggesting to still use bits in the bytes?
If so, then do you want "C" ordering in the byte or arch bit ordering?
Perhaps we simply to us the byte array as a list of CPU numbers, with  
-1 to terminate?
> (which might encourage us to pull proper
> endianness headers into Xen).
I suspect that this is more "RISC thing" than an "Endian
thing" since
it is possible for little endian arch to use the array method (like  
ARM?).

Pulling in endian headers could be avoided.

-JX






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Dec-18 17:55 UTC

head link

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

On 18/12/06 16:50, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
>> another
>> a uint64_t. Perhaps we should fix so they pass down 8-bit units (as
>> the type
>> of xenctl_cpumap_t would suggest) and then we could have
>> byte_to_long_bitmap
>> and long_to_byte_bitmap in Xen
> 
> Are you suggesting to still use bits in the bytes?
> If so, then do you want "C" ordering in the byte or arch bit
ordering?
> Perhaps we simply to us the byte array as a list of CPU numbers, with
> -1 to terminate?
I mean bits 8k to 8k+7 inclusive are contained in byte at offset k in the
cpumap, and in the obvious order (8k is least significant bit; 8k+7 is most
significant). This is the byte-oriented bitmap type that your bitops.h goes
on about. Since xenctl_cpumap_t is defined as a byte array and byte count,
this interpretation would make sense.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2006-Dec-20 19:05 UTC

head link

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

Is this what you were looking for? let me know and I''ll complete it.

-JX


diff -r c145c6638187 tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Tue Dec 19 09:47:54 2006 -0500
+++ b/tools/libxc/xc_domain.c	Wed Dec 20 13:49:05 2006 -0500
@@ -96,16 +96,25 @@ int xc_vcpu_setaffinity(int xc_handle,
 {
     DECLARE_DOMCTL;
     int ret = -1;
+    uint8_t cpumap_array[sizeof (cpumap)];
+    int i;
 
     domctl.cmd = XEN_DOMCTL_setvcpuaffinity;
     domctl.domain = (domid_t)domid;
     domctl.u.vcpuaffinity.vcpu    = vcpu;
 
+    memset(cpumap_array, 0, sizeof(cpumap_array));
+
+    for (i = 0; i < sizeof (cpumap) * 8; i ++) {
+        if (cpumap & (1UL << i))
+            set_bit8(i, cpumap_array);
+    }
+
     set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap,
-                         (uint8_t *)&cpumap);
-    domctl.u.vcpuaffinity.cpumap.nr_cpus = sizeof(cpumap) * 8;
+                         cpumap_array);
+    domctl.u.vcpuaffinity.cpumap.nr_cpus = sizeof(cpumap_array) * 8;
     
-    if ( lock_pages(&cpumap, sizeof(cpumap)) != 0 )
+    if ( lock_pages(cpumap_array, sizeof(cpumap_array)) != 0 )
     {
         PERROR("Could not lock memory for Xen hypercall");
         goto out;
@@ -113,7 +122,7 @@ int xc_vcpu_setaffinity(int xc_handle,
 
     ret = do_domctl(xc_handle, &domctl);
 
-    unlock_pages(&cpumap, sizeof(cpumap));
+    unlock_pages(cpumap_array, sizeof(cpumap_array));
 
  out:
     return ret;
@@ -127,16 +136,18 @@ int xc_vcpu_getaffinity(int xc_handle,
 {
     DECLARE_DOMCTL;
     int ret = -1;
+    uint8_t cpumap_array[sizeof (*cpumap)];
+    int i;
 
     domctl.cmd = XEN_DOMCTL_getvcpuaffinity;
     domctl.domain = (domid_t)domid;
     domctl.u.vcpuaffinity.vcpu = vcpu;
 
     set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap,
-                         (uint8_t *)cpumap);
-    domctl.u.vcpuaffinity.cpumap.nr_cpus = sizeof(*cpumap) * 8;
+                         (uint8_t *)cpumap_array);
+    domctl.u.vcpuaffinity.cpumap.nr_cpus = sizeof(cpumap_array) * 8;
     
-    if ( lock_pages(cpumap, sizeof(*cpumap)) != 0 )
+    if ( lock_pages(cpumap_array, sizeof(cpumap_array)) != 0 )
     {
         PERROR("Could not lock memory for Xen hypercall");
         goto out;
@@ -144,7 +155,13 @@ int xc_vcpu_getaffinity(int xc_handle,
 
     ret = do_domctl(xc_handle, &domctl);
 
-    unlock_pages(cpumap, sizeof(*cpumap));
+    unlock_pages(cpumap_array, sizeof(cpumap_array));
+
+    *cpumap = 0;
+    for (i = 0; i < sizeof (*cpumap) * 8; i ++) {
+        if (test_bit8(i, cpumap_array))
+            *cpumap |= 1UL << i;
+    }
 
  out:
     return ret;
diff -r c145c6638187 xen/common/domctl.c
--- a/xen/common/domctl.c	Tue Dec 19 09:47:54 2006 -0500
+++ b/xen/common/domctl.c	Wed Dec 20 13:36:32 2006 -0500
@@ -28,20 +28,24 @@ extern void arch_getdomaininfo_ctxt(
     struct vcpu *, struct vcpu_guest_context *);
 
 void cpumask_to_xenctl_cpumap(
-    struct xenctl_cpumap *xenctl_cpumap, cpumask_t *cpumask)
+    struct xenctl_cpumap *xenctl_cpumap, const cpumask_t *cpumask)
 {
     unsigned int guest_bytes, copy_bytes, i;
     uint8_t zero = 0;
+    uint8_t local[sizeof (*cpumask)];
 
     if ( guest_handle_is_null(xenctl_cpumap->bitmap) )
         return;
 
+    memset(local, 0, sizeof (local));
+    for_each_cpu_mask(i, *cpumask) {
+        set_bit8(i, local);
+    }
+
     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);
+    copy_to_guest(xenctl_cpumap->bitmap, &local[0], copy_bytes);
 
     for ( i = copy_bytes; i < guest_bytes; i++ )
         copy_to_guest_offset(xenctl_cpumap->bitmap, i, &zero, 1);
@@ -51,6 +55,8 @@ void xenctl_cpumap_to_cpumask(
     cpumask_t *cpumask, struct xenctl_cpumap *xenctl_cpumap)
 {
     unsigned int guest_bytes, copy_bytes;
+    unsigned int i;
+    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 +66,14 @@ 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);
+    memset(local, 0, sizeof(local));
+
+    copy_from_guest(&local[0], xenctl_cpumap->bitmap, copy_bytes);
+
+    for ( i = 0; i < NR_CPUS; i++) {
+        if (test_bit8(i, local))
+            cpu_set(i, *cpumask);
+    }
 }
 
 static inline int is_free_domid(domid_t dom)
diff -r c145c6638187 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Tue Dec 19 09:47:54 2006 -0500
+++ b/xen/include/public/domctl.h	Wed Dec 20 13:35:54 2006 -0500
@@ -424,6 +424,22 @@ typedef struct xen_domctl xen_domctl_t;
 typedef struct xen_domctl xen_domctl_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_t);
 
+/*
+ * 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;
+}
+
 #endif /* __XEN_PUBLIC_DOMCTL_H__ */
 
 /*
diff -r c145c6638187 xen/include/xen/cpumask.h
--- a/xen/include/xen/cpumask.h	Tue Dec 19 09:47:54 2006 -0500
+++ b/xen/include/xen/cpumask.h	Tue Dec 19 10:11:25 2006 -0500
@@ -382,7 +382,7 @@ extern cpumask_t cpu_present_map;
 /* Copy to/from cpumap provided by control tools. */
 struct xenctl_cpumap;
 void cpumask_to_xenctl_cpumap(
-    struct xenctl_cpumap *enctl_cpumap, cpumask_t *cpumask);
+    struct xenctl_cpumap *enctl_cpumap, const cpumask_t *cpumask);
 void xenctl_cpumap_to_cpumask(
     cpumask_t *cpumask, struct xenctl_cpumap *enctl_cpumap);
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Dec-21 10:48 UTC

head link

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

On 20/12/06 19:05, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> Is this what you were looking for? let me know and I''ll complete
it.
> 
> -JX
How about functions long_to_byte_bitmap(bitmap, nr_bits) and
byte_to_long_bitmap(bitmap, nr_bits) defined in asm/bitops.h? These would be
no-op for ia64 and x86, and could perform an in-place endian swap on each
longword on powerpc.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2006-Dec-21 17:32 UTC

head link

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

On Dec 21, 2006, at 5:48 AM, Keir Fraser wrote:
> On 20/12/06 19:05, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> Is this what you were looking for? let me know and I''ll
complete it.
>>
>> -JX
>
> How about functions long_to_byte_bitmap(bitmap, nr_bits) and
> byte_to_long_bitmap(bitmap, nr_bits) defined in asm/bitops.h?
hmm, as I see it we have 3 interfaces that should remain compatible.
  1) The hvcall which uses a byte array
  2) The libxc calls where 2 use uint64_t and one uses uint32_t
  3) The xm/xc.c call which constructs a python list using "C"  
ordering of the uint*_t word

so we would need similar interfaces in libxc and we would need a  
byte_to_uint{64,32}(), unless you want to change the libxc interfaces  
as well.

> These would be
> no-op for ia64 and x86, and could perform an in-place endian swap  
> on each
> longword on powerpc.
I see now (I''m not that familiar with x86) that bitops in x86 are  
"consistent" with "C" shifting so x86 experience no change,
however
is it worth optimizing this?
-JX

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Dec-21 19:51 UTC

head link

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

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?
> 
> hmm, as I see it we have 3 interfaces that should remain compatible.
>   1) The hvcall which uses a byte array
>   2) The libxc calls where 2 use uint64_t and one uses uint32_t
>   3) The xm/xc.c call which constructs a python list using "C"
> ordering of the uint*_t word
> 
> so we would need similar interfaces in libxc and we would need a
> byte_to_uint{64,32}(), unless you want to change the libxc interfaces
> as well.
I am suggesting we change just the hypervisor interface to expect a
byte-oriented bitmap. So the libxc interfaces can stay as they are, but the
libxc functions will need to do conversion from uint32_t/uint64_t to a
byte-oriented bitmap. There is no loss of backward compatibility here for
ia64 or x86 as little-endian systems do not care whether a bitmap is byte-
word- or longword-oriented -- they''re all identical.
> I see now (I''m not that familiar with x86) that bitops in x86 are
> "consistent" with "C" shifting so x86 experience no
change, however
> is it worth optimizing this?
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.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2007-Jan-16 23:26 UTC

head link

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

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__ */
 
 /*

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2007-Jan-17 12:20 UTC

head link

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

On 16/1/07 23:26, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> 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.
Fine interface. On the Xen side the prototypes should go in xen/bitmap.h and
the implementations in common/bitmap.c. I''ve added the Linux byteorder
functions so in bitmap.c you ifdef on __BIG_ENDIAN and then use le32_to_cpu
or le64_to_cpu (depending on BITS_PER_LONG). There''s a question over
whether
we should do this transformation in-place on the bitmap, or use the copying
semantics that you''ve chosen. I guess the latter is cleaner. Actually,
your
''no-op'' long_to_byte_bitmap() implementation does not memcpy
-- why is that?

On the guest side you''ll need to dup the function prototypes and
implementations in libxenctrl. Trying to share prototypes or anything via
the public headers is just skanky. Add a comment to xenctl_cpumap to explain
the format (explain it is ''little endian'' or ''byte
oriented'' with an ascii
diagram) and leave it at that.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2007-Jan-17 12:50 UTC

head link

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

On Jan 17, 2007, at 7:20 AM, Keir Fraser wrote:
> On 16/1/07 23:26, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> 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.
>
> Fine interface. On the Xen side the prototypes should go in xen/ 
> bitmap.h and
> the implementations in common/bitmap.c.
Ack
> I''ve added the Linux byteorder
> functions so in bitmap.c you ifdef on __BIG_ENDIAN and then use  
> le32_to_cpu
> or le64_to_cpu (depending on BITS_PER_LONG).Ack
> There''s a question over whether
> we should do this transformation in-place on the bitmap, or use the  
> copying
> semantics that you''ve chosen. I guess the latter is cleaner.
A copy will almost always be necessary since the source or  
destination (always the long) may actually be active and could have  
come from anywhere, where the byte is most likely local.
> Actually, your
> ''no-op'' long_to_byte_bitmap() implementation does not
memcpy -- why
> is that?
This is a little (possibly dirty) attempt to avoid a copy if no  
conversion is required.
As long as the caller uses the return value all is well.
I''d happy to make it a memcpy() and feel better about it.
>
> On the guest side you''ll need to dup the function prototypes and
> implementations in libxenctrl. Trying to share prototypes or  
> anything via
> the public headers is just skanky. Add a comment to xenctl_cpumap  
> to explain
> the format (explain it is ''little endian'' or
''byte oriented'' with
> an ascii
> diagram) and leave it at that.Ack

-JX


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2007-Jan-17 13:55 UTC

head link

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

On 17/1/07 12:50, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
>> Actually, your
>> ''no-op'' long_to_byte_bitmap() implementation does not
memcpy -- why
>> is that?
> 
> This is a little (possibly dirty) attempt to avoid a copy if no
> conversion is required.
> As long as the caller uses the return value all is well.
> I''d happy to make it a memcpy() and feel better about it.
Yes please.

 K.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2007-Jan-21 16:10 UTC

head link

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

From the patch that you committed, AFAICT this function does not do  
anything:

void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, int  
nbits)
{
	unsigned long l;
	int i, j, b;

	for (i = 0, b = 0; nbits > 0; i++, b += sizeof(l)) {
		l = 0;
		for (j = 0; (j < sizeof(l)) && (nbits > 0); j++) {
			l <<= 8;
			l |= bp[b+j];
			nbits -= 8;
		}
		lp[i] = l;
	}
}

On Jan 17, 2007, at 8:55 AM, Keir Fraser wrote:
>
>
>
> On 17/1/07 12:50, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>>> Actually, your
>>> ''no-op'' long_to_byte_bitmap() implementation does
not memcpy -- why
>>> is that?
>>
>> This is a little (possibly dirty) attempt to avoid a copy if no
>> conversion is required.
>> As long as the caller uses the return value all is well.
>> I''d happy to make it a memcpy() and feel better about it.
>
> Yes please.
>
>  K.
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2007-Jan-21 16:20 UTC

head link

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

Yes looks like I screwed up the source endianness. Perhaps the first two
lines of the inner loop should be replaced by:
 l |= (unsigned long)bp[b+j] << (j*8);

(and a similar change to the similar function) in libxc?

 -- Keir

On 21/1/07 4:10 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
>  From the patch that you committed, AFAICT this function does not do
> anything:
> 
> void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, int
> nbits)
> {
> unsigned long l;
> int i, j, b;
> 
> for (i = 0, b = 0; nbits > 0; i++, b += sizeof(l)) {
> l = 0;
> for (j = 0; (j < sizeof(l)) && (nbits > 0); j++) {
> l <<= 8;
> l |= bp[b+j];
> nbits -= 8;
> }
> lp[i] = l;
> }
> }

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jimi Xenidis

2007-Jan-21 21:43 UTC

head link

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

Ack.. I saw it hit Xen-staging.. Thanks

-JX
On Jan 21, 2007, at 11:20 AM, Keir Fraser wrote:
> Yes looks like I screwed up the source endianness. Perhaps the  
> first two
> lines of the inner loop should be replaced by:
>  l |= (unsigned long)bp[b+j] << (j*8);
>
> (and a similar change to the similar function) in libxc?
>
>  -- Keir
>
> On 21/1/07 4:10 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>>  From the patch that you committed, AFAICT this function does not do
>> anything:
>>
>> void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, int
>> nbits)
>> {
>> unsigned long l;
>> int i, j, b;
>>
>> for (i = 0, b = 0; nbits > 0; i++, b += sizeof(l)) {
>> l = 0;
>> for (j = 0; (j < sizeof(l)) && (nbits > 0); j++) {
>> l <<= 8;
>> l |= bp[b+j];
>> nbits -= 8;
>> }
>> lp[i] = l;
>> }
>> }
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Dec 2006 - [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

[Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

Re: [Xen-devel] [PATCH] fix xenctl_cpumap translation to handle bitops accessed like arrays

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

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

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

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

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

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

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

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

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

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

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