Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On 10/02/16 10:18, Ian Campbell wrote:
> On Wed, 2016-02-10 at 10:07 +, Andrew Cooper wrote:
>> On 08/02/16 16:36, Ian Campbell wrote:
>>> On Mon, 2016-02-08 at 16:23 +, Tim Deegan wrote:
At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote:
> The type of the pointer to a bitmap is not interesting; it does not
> affect the
> representation of the block of bits being pointed to.
It does affect the alignment, though. Is this safe on ARM?
>>> Good point. These constructs in the patch:
>>>
>>> +const unsigned long *addr = _addr;
>>>
>>> Would be broken if _addr were not suitably aligned for an unsigned
>>> long.
>>>
>>> That probably rules out this approach unfortunately.
>> What about reworking libxc bitops in terms of unsigned char? That
>> should cover all alignment issues.
> Assuming any asm or calls to __builtin_foo backends were adjusted to suite,
> that would be ok, would that be compatible with the Xen side though?
It is all plain C. What I mean is
-static inline int test_bit(int nr, unsigned long *addr)
+static inline int test_bit(int nr, const void *_addr)
{
+const char *addr = _addr;
return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1;
}
and changing BITMAP_{ENTRY,SHIFT}() to use char rather than unsigned long.
The prototypes still have void *, but char* is used internally which
will match the minimum alignment of any object passed.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On Wed, Feb 10, 2016 at 10:07:12AM +, Andrew Cooper wrote: > On 08/02/16 16:36, Ian Campbell wrote: > > On Mon, 2016-02-08 at 16:23 +, Tim Deegan wrote: > >> At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote: > >>> The type of the pointer to a bitmap is not interesting; it does not > >>> affect the > >>> representation of the block of bits being pointed to. > >> It does affect the alignment, though. Is this safe on ARM? > > Good point. These constructs in the patch: > > > > +const unsigned long *addr = _addr; > > > > Would be broken if _addr were not suitably aligned for an unsigned long. > > > > That probably rules out this approach unfortunately. > > What about reworking libxc bitops in terms of unsigned char? That > should cover all alignment issues. See 3cab67ac83b1d56c3daedd9c4adfed497a114246 "+/* + * xc_bitops.h has macros that do this as well - however they assume that + * the bitmask is word aligned but xc_cpumap_t is only guaranteed to be + * byte aligned and so we need byte versions for architectures which do + * not support misaligned accesses (which is basically everyone + * but x86, although even on x86 it can be inefficient). + */ " > > ~Andrew > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On Wed, 2016-02-10 at 10:07 +, Andrew Cooper wrote: > On 08/02/16 16:36, Ian Campbell wrote: > > On Mon, 2016-02-08 at 16:23 +, Tim Deegan wrote: > > > At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote: > > > > The type of the pointer to a bitmap is not interesting; it does not > > > > affect the > > > > representation of the block of bits being pointed to. > > > It does affect the alignment, though. Is this safe on ARM? > > Good point. These constructs in the patch: > > > > +const unsigned long *addr = _addr; > > > > Would be broken if _addr were not suitably aligned for an unsigned > > long. > > > > That probably rules out this approach unfortunately. > > What about reworking libxc bitops in terms of unsigned char? That > should cover all alignment issues. Assuming any asm or calls to __builtin_foo backends were adjusted to suite, that would be ok, would that be compatible with the Xen side though? Or you could make things a bit cleverer to do the const unsigned long *addr = _addr; in a way which also forces the alignment and adjusts nr to compensate (I don't see that 4 bytes of align is going to make addr point to e.g. a different sort of memory to _addr). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On 08/02/16 16:36, Ian Campbell wrote: > On Mon, 2016-02-08 at 16:23 +, Tim Deegan wrote: >> At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote: >>> The type of the pointer to a bitmap is not interesting; it does not >>> affect the >>> representation of the block of bits being pointed to. >> It does affect the alignment, though. Is this safe on ARM? > Good point. These constructs in the patch: > > +const unsigned long *addr = _addr; > > Would be broken if _addr were not suitably aligned for an unsigned long. > > That probably rules out this approach unfortunately. What about reworking libxc bitops in terms of unsigned char? That should cover all alignment issues. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On Mon, 2016-02-08 at 16:23 +, Tim Deegan wrote: > At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote: > > The type of the pointer to a bitmap is not interesting; it does not > > affect the > > representation of the block of bits being pointed to. > > It does affect the alignment, though. Is this safe on ARM? Good point. These constructs in the patch: +const unsigned long *addr = _addr; Would be broken if _addr were not suitably aligned for an unsigned long. That probably rules out this approach unfortunately. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
At 13:42 + on 05 Feb (1454679737), Andrew Cooper wrote: > The type of the pointer to a bitmap is not interesting; it does not affect the > representation of the block of bits being pointed to. It does affect the alignment, though. Is this safe on ARM? Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On 05/02/16 16:12, Wei Liu wrote: > On Fri, Feb 05, 2016 at 01:42:17PM +, Andrew Cooper wrote: >> The type of the pointer to a bitmap is not interesting; it does not affect >> the >> representation of the block of bits being pointed to. >> >> Make the libxc functions consistent with those in Xen, so they can work just >> as well with 'unsigned int *' based bitmaps. > I'm not sure I understand this, the bitmap functions in > xen/include/bitmap.h seem to take unsigned long *. > > Not saying that I object to this patch, just this comment looks wrong. The lower level bit operations are all in terms of void*, for both x86 and arm ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
On Fri, Feb 05, 2016 at 01:42:17PM +, Andrew Cooper wrote: > The type of the pointer to a bitmap is not interesting; it does not affect the > representation of the block of bits being pointed to. > > Make the libxc functions consistent with those in Xen, so they can work just > as well with 'unsigned int *' based bitmaps. I'm not sure I understand this, the bitmap functions in xen/include/bitmap.h seem to take unsigned long *. Not saying that I object to this patch, just this comment looks wrong. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 24/30] tools/libxc: Modify bitmap operations to take void pointers
The type of the pointer to a bitmap is not interesting; it does not affect the
representation of the block of bits being pointed to.
Make the libxc functions consistent with those in Xen, so they can work just
as well with 'unsigned int *' based bitmaps.
Signed-off-by: Andrew Cooper
---
CC: Ian Campbell
CC: Ian Jackson
CC: Wei Liu
New in v2
---
tools/libxc/xc_bitops.h | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h
index cd749f4..2a1710f 100644
--- a/tools/libxc/xc_bitops.h
+++ b/tools/libxc/xc_bitops.h
@@ -26,48 +26,53 @@ static inline unsigned long *bitmap_alloc(int nr_bits)
return calloc(1, bitmap_size(nr_bits));
}
-static inline void bitmap_set(unsigned long *addr, int nr_bits)
+static inline void bitmap_set(void *addr, int nr_bits)
{
memset(addr, 0xff, bitmap_size(nr_bits));
}
-static inline void bitmap_clear(unsigned long *addr, int nr_bits)
+static inline void bitmap_clear(void *addr, int nr_bits)
{
memset(addr, 0, bitmap_size(nr_bits));
}
-static inline int test_bit(int nr, unsigned long *addr)
+static inline int test_bit(int nr, const void *_addr)
{
+const unsigned long *addr = _addr;
return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1;
}
-static inline void clear_bit(int nr, unsigned long *addr)
+static inline void clear_bit(int nr, void *_addr)
{
+unsigned long *addr = _addr;
BITMAP_ENTRY(nr, addr) &= ~(1UL << BITMAP_SHIFT(nr));
}
-static inline void set_bit(int nr, unsigned long *addr)
+static inline void set_bit(int nr, void *_addr)
{
+unsigned long *addr = _addr;
BITMAP_ENTRY(nr, addr) |= (1UL << BITMAP_SHIFT(nr));
}
-static inline int test_and_clear_bit(int nr, unsigned long *addr)
+static inline int test_and_clear_bit(int nr, void *addr)
{
int oldbit = test_bit(nr, addr);
clear_bit(nr, addr);
return oldbit;
}
-static inline int test_and_set_bit(int nr, unsigned long *addr)
+static inline int test_and_set_bit(int nr, void *addr)
{
int oldbit = test_bit(nr, addr);
set_bit(nr, addr);
return oldbit;
}
-static inline void bitmap_or(unsigned long *dst, const unsigned long *other,
+static inline void bitmap_or(void *_dst, const void *_other,
int nr_bits)
{
+unsigned long *dst = _dst;
+const unsigned long *other = _other;
int i, nr_longs = (bitmap_size(nr_bits) / sizeof(unsigned long));
for ( i = 0; i < nr_longs; ++i )
dst[i] |= other[i];
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
