[2/2,V2] kvm tools: Fix virt_queue__set_used_elem

Commit Message

Ingo Molnar May 5, 2011, 8:07 a.m. UTC

* Pekka Enberg <[e-mail]> wrote:

> > asm/system.h is one of the messier kernel headers, so i'm not surprised it 
> > has assymetric requirements on 64-bit and 32-bit systems.
> 
> So does including <asm/bitsperlong.h> before <asm/system.h> fix the problem 
> on 32-bit boxes? [...]

No, it's more complex than that.

> [...] If so, I'd prefer we fix it like that for now.

The patch below sorts out the dependencies and makes it build on both 64-bit 
and 32-bit systems. (I have not runtime tested the result though.)

I'm not entirely happy about how it has added dependent includes to virtio.c 
but that's a property of this messy header file. Might be worth adding a 
comment about that.

Thanks,

	Ingo

----------------->
From 51cb9390753c0bb333896fbc951cc53bcae2723d Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[e-mail]>
Date: Thu, 5 May 2011 10:00:45 +0200
Subject: [PATCH] kvm: Fix 32-bit build of the asm/system.h include

Provide wrappers and other environmental dependencies that the asm/system.h 
header file from hell needs to build fine in user-space.

Sidenote: right now alternative() defaults to the compatible, slightly slower 
barrier instructions that work on all x86 systems.

If this ever shows up in profiles then kvm could provide an alternatives 
patching machinery as well. Right now those instructions are emitted into 
special sections and then discarded by the linker harmlessly.

Signed-off-by: Ingo Molnar <[e-mail]>
---
 tools/kvm/include/asm/hweight.h  |    8 ++++++++
 tools/kvm/include/linux/bitops.h |   33 +++++++++++++++++++++++++++++++++
 tools/kvm/virtio.c               |    5 +++++
 3 files changed, 46 insertions(+), 0 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ingo Molnar May 5, 2011, 8:12 a.m. UTC | #1

* Ingo Molnar <[e-mail]> wrote:

> I'm not entirely happy about how it has added dependent includes to virtio.c 
> but that's a property of this messy header file. Might be worth adding a 
> comment about that.

This block:

> +#include <linux/stringify.h>
> +#include <linux/bitops.h>
> +#include <asm/alternative.h>
>  #include <asm/system.h>

Could be put into a new tools/kvm/include/kvm/barrier.h file, with a comment - 
that way the virtio.c inclusion looks very clean.

Note: i'd not put it into linux/barrier.h, to not clash with any possible 
future linux/barrier.h file, and to also make it clear that this is a kvm 
specific wrapper.

Oh, and those 3 lines could be put into upstream arch/x86's system.h as well, 
to make asm/system.h standalone includable. Does anyone want to send a patch 
for that?

Once that header fix is upstream and once tools/kvm/ merges that upstream 
kernel the special wrapper barrier.h can be dropped and virtio.c can include 
asm/system.h for the barriers.

The joys of a clean Git workflow and a unified kernel+tools tree, it actually 
helps fix crap on both sides :-)

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/tools/kvm/include/asm/hweight.h b/tools/kvm/include/asm/hweight.h
new file mode 100644
index 0000000..36cf26d
--- /dev/null
+++ b/tools/kvm/include/asm/hweight.h
@@ -0,0 +1,8 @@ 
+#ifndef PERF_HWEIGHT_H
+#define PERF_HWEIGHT_H
+
+#include <linux/types.h>
+unsigned int hweight32(unsigned int w);
+unsigned long hweight64(__u64 w);
+
+#endif /* PERF_HWEIGHT_H */
diff --git a/tools/kvm/include/linux/bitops.h b/tools/kvm/include/linux/bitops.h
new file mode 100644
index 0000000..305c848
--- /dev/null
+++ b/tools/kvm/include/linux/bitops.h
@@ -0,0 +1,33 @@ 
+#ifndef _PERF_LINUX_BITOPS_H_
+#define _PERF_LINUX_BITOPS_H_
+
+#include <linux/kernel.h>
+#include <linux/compiler.h>
+#include <asm/hweight.h>
+
+#define BITS_PER_LONG __WORDSIZE
+#define BITS_PER_BYTE           8
+#define BITS_TO_LONGS(nr)       DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+
+static inline void set_bit(int nr, unsigned long *addr)
+{
+	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
+}
+
+static inline void clear_bit(int nr, unsigned long *addr)
+{
+	addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
+}
+
+static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
+{
+	return ((1UL << (nr % BITS_PER_LONG)) &
+		(((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;
+}
+
+static inline unsigned long hweight_long(unsigned long w)
+{
+	return sizeof(w) == 4 ? hweight32(w) : hweight64(w);
+}
+
+#endif
diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
index 266a1b6..3a4b1c9 100644
--- a/tools/kvm/virtio.c
+++ b/tools/kvm/virtio.c
@@ -1,7 +1,12 @@ 
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+
+#include <linux/stringify.h>
+#include <linux/bitops.h>
+#include <asm/alternative.h>
 #include <asm/system.h>
+
 #include "kvm/kvm.h"
 #include "kvm/virtio.h"