Skip to contentSkip to conversation

kvm tools: Drop ALIGN from bios.h

PATCH In review Sasha Levin 5 participants 20 messages May 3, 2011 – May 5, 2011 KVM

lore
Share & tools
lore

Sasha Levin

author
#0 lore raw

PATCH v2 1/2 kvm tools: Drop ALIGN from bios.h

Drops align from bios.h, fixes related code to use <linux/kernel.h> instead.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/include/kvm/bios.h |   13 -------------
 tools/kvm/mptable.c          |    1 +
 2 files changed, 1 insertions(+), 13 deletions(-)
tools/kvm/include/kvm/bios.h +0 -13
index 914720b..7586e2a 100644
@@ -51,19 +51,6 @@
 #define MB_BIOS_SS			0xfff7
 #define MB_BIOS_SP			0x40
 
-#ifndef ALIGN
-#define ALIGN(x, a)	\
-	(((x) + ((a) - 1)) & ~((a) - 1))
-#endif
-
-/*
- * note we use 16 bytes alignment which makes segment based
- * addressing easy to compute, dont change it otherwise you
- * may break local variables offsets in BIOS irq routines
- */
-#define BIOS_NEXT_IRQ_ADDR(addr, size)	\
-	ALIGN((addr + size + 1), 16)
-
 /*
  * When interfere with assembler code we need to be sure how
  * arguments are passed in real mode.
tools/kvm/mptable.c +1 -0
index 5bbe7ea..b74c491 100644
@@ -4,6 +4,7 @@
 #include "kvm/mptable.h"
 #include "kvm/util.h"
 
+#include <linux/kernel.h>
 #include <string.h>
 
 /*
Signature · 1 line
1.7.5.rc3
author
#1 lore raw

PATCH v2 2/2 kvm tools: Fix virt_queue__set_used_elem

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +18 -1
index 6249521..266a1b6 100644
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>
 #include "kvm/kvm.h"
 #include "kvm/virtio.h"
 
 struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, uint32_t head, uint32_t len)
 {
 	struct vring_used_elem *used_elem;
-	used_elem	= &queue->vring.used->ring[queue->vring.used->idx++ % queue->vring.num];
+	used_elem	= &queue->vring.used->ring[queue->vring.used->idx % queue->vring.num];
 	used_elem->id	= head;
 	used_elem->len	= len;
+
+	/*
+	 * Use wmb to assure that used elem was updated with head and len.
+	 * We need a wmb here since we can't advance idx unless we're ready
+	 * to pass the used element to the guest.
+	 */
+	wmb();
+	queue->vring.used->idx++;
+
+	/*
+	 * Use wmb to assure used idx has been increased before we signal the guest.
+	 * Without a wmb here the guest may ignore the queue since it won't see
+	 * an updated idx.
+	 */
+	wmb();
+
 	return used_elem;
 }
Signature · 1 line
1.7.5.rc3
Acked-by on v2 2/2
#2 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 14 lines · Sasha Levin

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +0 -0
index 6249521..266a1b6 100644

tools/kvm/virtio.c line 4

Quoted 4 lines
tools/kvm/virtio.c +1 -0
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>

If this system.h is included from the current kernel (and not from the system's) then:

Acked-by Ingo Molnar <[email protected]>

Thanks,

Ingo

author
#3 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 24 lines · Ingo Molnar
Quoted 14 lines · Sasha Levin

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +0 -0
index 6249521..266a1b6 100644

tools/kvm/virtio.c line 4

Quoted 4 lines
tools/kvm/virtio.c +1 -0
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>

If this system.h is included from the current kernel (and not from the system's) then:

Quoted 2 lines
Acked-by Ingo Molnar <[email protected]>

The system.h that'll get picked up is '../../arch/$(ARCH)/include/asm/system.h' within the current kernel tree and not the system one.

Quoted 3 lines

Thanks,

Ingo

Signature · 1 line
Sasha.
#4 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 25 lines · Sasha Levin
Quoted 24 lines · Ingo Molnar
Quoted 14 lines · Sasha Levin

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +0 -0
index 6249521..266a1b6 100644

tools/kvm/virtio.c line 4

Quoted 4 lines
tools/kvm/virtio.c +1 -0
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>

If this system.h is included from the current kernel (and not from the system's) then:

Quoted 6 lines
Quoted 2 lines
Acked-by Ingo Molnar <[email protected]>

The system.h that'll get picked up is '../../arch/$(ARCH)/include/asm/system.h' within the current kernel tree and not the system one.

ok, that's great - so my Ack stands.

Thanks,

Ingo

#5 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

In reply to Sasha Levin
Quoted 25 lines · Sasha Levin
Quoted 24 lines · Ingo Molnar
Quoted 14 lines · Sasha Levin

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +0 -0
index 6249521..266a1b6 100644

tools/kvm/virtio.c line 4

Quoted 4 lines
tools/kvm/virtio.c +1 -0
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>

If this system.h is included from the current kernel (and not from the system's) then:

Quoted 10 lines
Quoted 2 lines
Acked-by Ingo Molnar <[email protected]>

The system.h that'll get picked up is '../../arch/$(ARCH)/include/asm/system.h' within the current kernel tree and not the system one.

Quoted 3 lines

Thanks,

Ingo

Hi,

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
../../include/linux/bitops.h:133:2: error: #error BITS_PER_LONG not 32 or 64
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
../../include/linux/bitops.h:133:2: error: #error BITS_PER_LONG not 32 or 64
make: *** No rule to make target `virtio.d', needed by `kvm'.  Stop.

With 'make V=1', I found this one triggered the error:

cc -M -MT virtio.o -DCONFIG_X86_32 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -Iinclude -I../../include -I../../arch/x86/include/ -Os -g -Werror -Wall -Wcast-align -Wformat=2 -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wredundant-decls -Wsign-compare -Wstrict-prototypes -Wundef -Wvolatile-register-var -Wwrite-strings virtio.c -o virtio.d

Signature · 2 lines
Best Regards,
Asias He
author
#6 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 63 lines · Asias He
Quoted 25 lines · Sasha Levin
Quoted 24 lines · Ingo Molnar
Quoted 14 lines · Sasha Levin

Increase idx only after updating the used element. Not doing so may mark a buffer as used without having it's head and length updated.

Signed-off-by Sasha Levin <[email protected]>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
tools/kvm/virtio.c +0 -0
index 6249521..266a1b6 100644

tools/kvm/virtio.c line 4

Quoted 4 lines
tools/kvm/virtio.c +1 -0
@@ -1,15 +1,32 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
+#include <asm/system.h>

If this system.h is included from the current kernel (and not from the system's) then:

Quoted 10 lines
Quoted 2 lines
Acked-by Ingo Molnar <[email protected]>

The system.h that'll get picked up is '../../arch/$(ARCH)/include/asm/system.h' within the current kernel tree and not the system one.

Quoted 3 lines

Thanks,

Ingo

Hi,

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
../../include/linux/bitops.h:133:2: error: #error BITS_PER_LONG not 32 or 64
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
../../include/linux/bitops.h:133:2: error: #error BITS_PER_LONG not 32 or 64
make: *** No rule to make target `virtio.d', needed by `kvm'.  Stop.

With 'make V=1', I found this one triggered the error:

cc -M -MT virtio.o -DCONFIG_X86_32 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE

(quoted diff) +0 -5
-Iinclude -I../../include -I../../arch/x86/include/ -Os -g -Werror -Wall
-Wcast-align -Wformat=2 -Winit-self -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wno-system-headers
-Wold-style-definition -Wredundant-decls -Wsign-compare
-Wstrict-prototypes -Wundef -Wvolatile-register-var -Wwrite-strings
Quoted 3 lines

virtio.c -o virtio.d

It looks like gcc is trying to pull kernel headers instead the headers we have in include/linux. I'm not really sure why it happens though, I've tried building again with a clean kvm dir (in case I forgot to add a file to the commit) but it worked fine.

Can you compare your include/linux dir to the one located on the master git tree?

Signature · 1 line
Sasha.
#7 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

In reply to Asias He
Quoted 9 lines · Asias He

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Thanks,

Ingo

#8 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 6 lines · Ingo Molnar
Quoted 3 lines · Asias He

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

Quoted 8 lines
Quoted 6 lines

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

author
#9 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 14 lines · Pekka Enberg
Quoted 13 lines · Ingo Molnar
Quoted 9 lines · Asias He

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
Quoted 5 lines
Quoted 1 line

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

Pekka, Are you on a 64bit box?
I suspect it's not working on 32bit boxes.

Signature · 1 line
Sasha.
#10 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 15 lines · Sasha Levin
Quoted 14 lines · Pekka Enberg
Quoted 13 lines · Ingo Molnar
Quoted 9 lines · Asias He

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
Quoted 8 lines
Quoted 5 lines
Quoted 1 line

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

Pekka, Are you on a 64bit box?
I suspect it's not working on 32bit boxes.

Yes, I am.

#11 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

In reply to Pekka Enberg
Quoted 14 lines · Pekka Enberg
Quoted 13 lines · Ingo Molnar
Quoted 9 lines · Asias He

With commit d7f0c07afeefa2d20739437306e4b8bb2853cf83 in master (kvm tools: Fix virt_queue__set_used_elem)

I see this:

asias@hj:~/qemu-stuff/pekka.git/tools/kvm$ make
  GEN      include/common-cmds.h
../../include/asm-generic/bitops/fls64.h:33:2: error: #error
BITS_PER_LONG not 32 or 64
Quoted 5 lines
Quoted 1 line

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

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.

We could fix it (provide those dependencies), or we could pick tools/perf/perf.h's mb() definitions. That's much cruder than the nice kernel memory barriers though ...

Thanks,

Ingo

#12 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 8 lines · Ingo Molnar
Quoted 5 lines
Quoted 1 line

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

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? If so, I'd prefer we fix it like that for now.

Quoted 3 lines

We could fix it (provide those dependencies), or we could pick tools/perf/perf.h's mb() definitions. That's much cruder than the nice kernel memory barriers though ...

That's the second best option, I think.

Pekka

author
#13 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 14 lines · Pekka Enberg
Quoted 8 lines · Ingo Molnar
Quoted 5 lines
Quoted 1 line

The build fails here too, in a similar way, on a 32-bit Fedora 14 box.

Curious. It works fine on my box. I think it's missing #include <asm/bitsperlong.h>. I wonder why system.h isn't pulling that itself...

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? If so, I'd prefer we fix it like that for now.

Quoted 5 lines
Quoted 3 lines

We could fix it (provide those dependencies), or we could pick tools/perf/perf.h's mb() definitions. That's much cruder than the nice kernel memory barriers though ...

That's the second best option, I think.

I agree with Pekka, For the same reasons we provided the dependencies for including <linux/list.h> instead of just taking the source.

Signature · 1 line
Sasha.
#14 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

In reply to Pekka Enberg
Quoted 5 lines · Pekka Enberg
Quoted 2 lines

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.

Quoted 1 line

[...] 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

----------------->

Quoted 1 line

From 51cb9390753c0bb333896fbc951cc53bcae2723d Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
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 <[email protected]>
---
 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(-)
tools/kvm/include/asm/hweight.h new file +8 -0
new file mode 100644
index 0000000..36cf26d
@@ -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 */
tools/kvm/include/linux/bitops.h new file +33 -0
new file mode 100644
index 0000000..305c848
@@ -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
tools/kvm/virtio.c +5 -0
index 266a1b6..3a4b1c9 100644
@@ -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"
#15 lore raw

Re: [PATCH 2/2 V2] kvm tools: Fix virt_queue__set_used_elem

Quoted 3 lines · Ingo Molnar

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:

tools/kvm/virtio.c line 8

Quoted 4 lines
tools/kvm/virtio.c +3 -0
+#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

#16 lore raw

PATCH 1/1 kvm: Fix 32-bit build of the asm/system.h include

Quoted 16 lines · Ingo Molnar
Quoted 3 lines · Ingo Molnar

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:

Review

Quoted 4 lines
(quoted diff) +3 -0
+#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.

Quoted 3 lines

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.

Like the more complete patch below. Build-tested on 32-bit and 64-bit systems, boot tested on a 64-bit box.

Thanks,

Ingo

------------------->

Quoted 1 line

From 5c196bbbe1d5e8c5de8ea5db1f0b274d7f8b7aac Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
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 <[email protected]>
---
 tools/kvm/include/asm/hweight.h  |    8 ++++++++
 tools/kvm/include/kvm/barrier.h  |   15 +++++++++++++++
 tools/kvm/include/linux/bitops.h |   33 +++++++++++++++++++++++++++++++++
 tools/kvm/virtio.c               |    4 +++-
 4 files changed, 59 insertions(+), 1 deletions(-)
tools/kvm/include/asm/hweight.h new file +8 -0
new file mode 100644
index 0000000..1a43977
@@ -0,0 +1,8 @@
+#ifndef _KVM_ASM_HWEIGHT_H_
+#define _KVM_ASM_HWEIGHT_H_
+
+#include <linux/types.h>
+unsigned int hweight32(unsigned int w);
+unsigned long hweight64(__u64 w);
+
+#endif /* _KVM_ASM_HWEIGHT_H_ */
tools/kvm/include/kvm/barrier.h new file +15 -0
new file mode 100644
index 0000000..c11a239
@@ -0,0 +1,15 @@
+#ifndef _KVM_BARRIER_H_
+#define _KVM_BARRIER_H_
+
+/*
+ * asm/system.h cannot be #included standalone on 32-bit x86 yet.
+ *
+ * Provide the dependencies here - we can drop these wrappers once
+ * the header is fixed upstream:
+ */
+#include <linux/stringify.h>
+#include <linux/bitops.h>
+#include <asm/alternative.h>
+#include <asm/system.h>
+
+#endif /* _KVM_BARRIER_H_ */
tools/kvm/include/linux/bitops.h new file +33 -0
new file mode 100644
index 0000000..56448b7
@@ -0,0 +1,33 @@
+#ifndef _KVM_LINUX_BITOPS_H_
+#define _KVM_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
tools/kvm/virtio.c +3 -1
index 266a1b6..4dcd092 100644
@@ -1,7 +1,9 @@
 #include <linux/virtio_ring.h>
 #include <stdint.h>
 #include <sys/uio.h>
-#include <asm/system.h>
+
+#include "kvm/barrier.h"
+
 #include "kvm/kvm.h"
 #include "kvm/virtio.h"
#17 lore raw

Re: [PATCH] kvm: Fix 32-bit build of the asm/system.h include

Quoted 18 lines · Ingo Molnar
Quoted 16 lines · Ingo Molnar
Quoted 3 lines · Ingo Molnar

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:

tools/kvm/include/kvm/barrier.h

Quoted 4 lines
tools/kvm/include/kvm/barrier.h new file +3 -0
+#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.

Quoted 6 lines
Quoted 3 lines

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.

Like the more complete patch below. Build-tested on 32-bit and 64-bit systems, boot tested on a 64-bit box.

Applied, thanks Ingo! Asias, please let me know if master doesn't build for you still.

#18 lore raw

Re: [PATCH] kvm: Fix 32-bit build of the asm/system.h include

Quoted 23 lines · Pekka Enberg
Quoted 22 lines · Ingo Molnar
Quoted 19 lines · Ingo Molnar
Quoted 3 lines · Ingo Molnar

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:

tools/kvm/include/kvm/barrier.h

Quoted 4 lines
tools/kvm/include/kvm/barrier.h new file +3 -0
+#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.

Quoted 5 lines
Quoted 2 lines

Like the more complete patch below. Build-tested on 32-bit and 64-bit systems, boot tested on a 64-bit box.

Applied, thanks Ingo! Asias, please let me know if master doesn't build for you still.

Works for me now.

Signature · 2 lines
Best Regards,
Asias He