[PATCH 1/2 V2] kvm tools: Drop ALIGN from bios.h

[PATCH 1/2 V2] kvm tools: Drop ALIGN from bios.h

[Sasha Levin]

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

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/bios.h |   13 -------------
 tools/kvm/mptable.c          |    1 +
 2 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/tools/kvm/include/kvm/bios.h b/tools/kvm/include/kvm/bios.h
index 914720b..7586e2a 100644
--- a/tools/kvm/include/kvm/bios.h
+++ b/tools/kvm/include/kvm/bios.h
@@ -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.
diff --git a/tools/kvm/mptable.c b/tools/kvm/mptable.c
index 5bbe7ea..b74c491 100644
--- a/tools/kvm/mptable.c
+++ b/tools/kvm/mptable.c
@@ -4,6 +4,7 @@
 #include "kvm/mptable.h"
 #include "kvm/util.h"
 
+#include <linux/kernel.h>
 #include <string.h>
 
 /*
-- 
1.7.5.rc3

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

[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 <levinsasha928@gmail.com>
---
 tools/kvm/virtio.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
index 6249521..266a1b6 100644
--- a/tools/kvm/virtio.c
+++ b/tools/kvm/virtio.c
@@ -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;
 }
 
-- 
1.7.5.rc3

Re: [PATCH 1/2 V2] kvm tools: Drop ALIGN from bios.h

[Cyrill Gorcunov]

On 05/04/2011 12:28 AM, Sasha Levin wrote:
> Drops align from bios.h, fixes related code to use
> <linux/kernel.h> instead.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
...

Looks good to me, thanks Sasha. (I suspect it still builds and run, right? :)

-- 
Thanks,
  Cyrill

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

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> 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 <levinsasha928@gmail.com>
> ---
>  tools/kvm/virtio.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
> index 6249521..266a1b6 100644
> --- a/tools/kvm/virtio.c
> +++ b/tools/kvm/virtio.c
> @@ -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 <mingo@elte.hu>

Thanks,

	Ingo

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

[Sasha Levin]

On Tue, 2011-05-03 at 22:47 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
> 
> > 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 <levinsasha928@gmail.com>
> > ---
> >  tools/kvm/virtio.c |   19 ++++++++++++++++++-
> >  1 files changed, 18 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
> > index 6249521..266a1b6 100644
> > --- a/tools/kvm/virtio.c
> > +++ b/tools/kvm/virtio.c
> > @@ -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 <mingo@elte.hu>
> 

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.
> Thanks,
> 
> 	Ingo

-- 

Sasha.

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

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> On Tue, 2011-05-03 at 22:47 +0200, Ingo Molnar wrote:
> > * Sasha Levin <levinsasha928@gmail.com> wrote:
> > 
> > > 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 <levinsasha928@gmail.com>
> > > ---
> > >  tools/kvm/virtio.c |   19 ++++++++++++++++++-
> > >  1 files changed, 18 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
> > > index 6249521..266a1b6 100644
> > > --- a/tools/kvm/virtio.c
> > > +++ b/tools/kvm/virtio.c
> > > @@ -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 <mingo@elte.hu>
> > 
> 
> 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

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

[Asias He]

On 05/04/2011 12:41 PM, Sasha Levin wrote:
> On Tue, 2011-05-03 at 22:47 +0200, Ingo Molnar wrote:
>> * Sasha Levin <levinsasha928@gmail.com> wrote:
>>
>>> 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 <levinsasha928@gmail.com>
>>> ---
>>>  tools/kvm/virtio.c |   19 ++++++++++++++++++-
>>>  1 files changed, 18 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
>>> index 6249521..266a1b6 100644
>>> --- a/tools/kvm/virtio.c
>>> +++ b/tools/kvm/virtio.c
>>> @@ -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 <mingo@elte.hu>
>>
> 
> 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.
>> 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


-- 
Best Regards,
Asias He

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

[Sasha Levin]

On Thu, 2011-05-05 at 07:47 +0800, Asias He wrote:
> On 05/04/2011 12:41 PM, Sasha Levin wrote:
> > On Tue, 2011-05-03 at 22:47 +0200, Ingo Molnar wrote:
> >> * Sasha Levin <levinsasha928@gmail.com> wrote:
> >>
> >>> 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 <levinsasha928@gmail.com>
> >>> ---
> >>>  tools/kvm/virtio.c |   19 ++++++++++++++++++-
> >>>  1 files changed, 18 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
> >>> index 6249521..266a1b6 100644
> >>> --- a/tools/kvm/virtio.c
> >>> +++ b/tools/kvm/virtio.c
> >>> @@ -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 <mingo@elte.hu>
> >>
> > 
> > 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.
> >> 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
> 
> 

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?

-- 

Sasha.

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

[Ingo Molnar]

* Asias He <asias.hejun@gmail.com> wrote:

> 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

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

[Pekka Enberg]

On Thu, May 5, 2011 at 10:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Asias He <asias.hejun@gmail.com> wrote:
>
>> 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.

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

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

[Sasha Levin]

On Thu, 2011-05-05 at 10:13 +0300, Pekka Enberg wrote:
> On Thu, May 5, 2011 at 10:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Asias He <asias.hejun@gmail.com> wrote:
> >
> >> 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.
> 
> 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.

-- 

Sasha.

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

[Pekka Enberg]

On Thu, May 5, 2011 at 10:18 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Thu, 2011-05-05 at 10:13 +0300, Pekka Enberg wrote:
>> On Thu, May 5, 2011 at 10:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
>> >
>> > * Asias He <asias.hejun@gmail.com> wrote:
>> >
>> >> 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.
>>
>> 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.

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

[Ingo Molnar]

* Pekka Enberg <penberg@kernel.org> wrote:

> On Thu, May 5, 2011 at 10:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Asias He <asias.hejun@gmail.com> wrote:
> >
> >> 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.
> 
> 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

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

[Pekka Enberg]

On Thu, May 5, 2011 at 10:47 AM, Ingo Molnar <mingo@elte.hu> wrote:
>> > 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.

> 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

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

[Sasha Levin]

On Thu, 2011-05-05 at 10:52 +0300, Pekka Enberg wrote:
> On Thu, May 5, 2011 at 10:47 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >> > 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.
> 
> > 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.

-- 

Sasha.

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

[Ingo Molnar]

* Pekka Enberg <penberg@kernel.org> 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 <mingo@elte.hu>
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 <mingo@elte.hu>
---
 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(-)

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"
 

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

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> 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

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

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> * Ingo Molnar <mingo@elte.hu> 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.

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

Thanks,

	Ingo

------------------->
>From 5c196bbbe1d5e8c5de8ea5db1f0b274d7f8b7aac Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
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 <mingo@elte.hu>
---
 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(-)

diff --git a/tools/kvm/include/asm/hweight.h b/tools/kvm/include/asm/hweight.h
new file mode 100644
index 0000000..1a43977
--- /dev/null
+++ b/tools/kvm/include/asm/hweight.h
@@ -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_ */
diff --git a/tools/kvm/include/kvm/barrier.h b/tools/kvm/include/kvm/barrier.h
new file mode 100644
index 0000000..c11a239
--- /dev/null
+++ b/tools/kvm/include/kvm/barrier.h
@@ -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_ */
diff --git a/tools/kvm/include/linux/bitops.h b/tools/kvm/include/linux/bitops.h
new file mode 100644
index 0000000..56448b7
--- /dev/null
+++ b/tools/kvm/include/linux/bitops.h
@@ -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
diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c
index 266a1b6..4dcd092 100644
--- a/tools/kvm/virtio.c
+++ b/tools/kvm/virtio.c
@@ -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"
 

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

[Pekka Enberg]

On Thu, 2011-05-05 at 10:22 +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Ingo Molnar <mingo@elte.hu> 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.
> 
> 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.

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

[Asias He]

On 05/05/2011 10:45 PM, Pekka Enberg wrote:
> On Thu, 2011-05-05 at 10:22 +0200, Ingo Molnar wrote:
>> * Ingo Molnar <mingo@elte.hu> wrote:
>>
>>> * Ingo Molnar <mingo@elte.hu> 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.
>>
>> 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.


-- 
Best Regards,
Asias He