diff mbox

kvm: user: include arch specific headers from $(KERNELDIR)

Message ID 1242203541-12959-1-git-send-email-markmc@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mark McLoughlin May 13, 2009, 8:32 a.m. UTC
Currently we only include $(KERNELDIR)/include in CFLAGS,
but we also have $(KERNELDIR)/arch/$(arch)/include or else
we'll get mis-matched headers.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 kvm/user/config-i386.mak       |    1 -
 kvm/user/config-ia64.mak       |    1 +
 kvm/user/config-powerpc.mak    |    1 +
 kvm/user/config-x86-common.mak |    2 ++
 kvm/user/config-x86_64.mak     |    1 -
 5 files changed, 4 insertions(+), 2 deletions(-)

Comments

Arnd Bergmann May 13, 2009, 9:57 p.m. UTC | #1
On Wednesday 13 May 2009 08:32:21 Mark McLoughlin wrote:
> Currently we only include $(KERNELDIR)/include in CFLAGS,
> but we also have $(KERNELDIR)/arch/$(arch)/include or else
> we'll get mis-matched headers.
> 

I think this is fundamentally wrong. User files should never directly
access kernel headers, because they are postprocessed in various
ways in order to get files that are valid in user space, e.g. __user
annotations are removed.

The three possible sources for kernel headers are:

/usr/include 
	- system provided headers, may be older than the running kernel
/lib/modules/$(uname -r)/build/usr/include
	- user space headers for the currently running kernel
$(KERNELDIR)/usr/include
	-  user space headers from a configured kernel tree after 'make headers_install'

	Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark McLoughlin May 14, 2009, 7:52 a.m. UTC | #2
On Wed, 2009-05-13 at 21:57 +0000, Arnd Bergmann wrote:
> On Wednesday 13 May 2009 08:32:21 Mark McLoughlin wrote:
> > Currently we only include $(KERNELDIR)/include in CFLAGS,
> > but we also have $(KERNELDIR)/arch/$(arch)/include or else
> > we'll get mis-matched headers.
> > 
> 
> I think this is fundamentally wrong. User files should never directly
> access kernel headers,

Just to be more clear on the use case for this patch - it's needed to
allow building kvmtrace against the copy of kvm kernel headers carried
in the qemu-kvm-0.10.4 release tarball.

>  because they are postprocessed in various
> ways in order to get files that are valid in user space, e.g. __user
> annotations are removed.
> 
> The three possible sources for kernel headers are:
> 
> /usr/include 
> 	- system provided headers, may be older than the running kernel
> /lib/modules/$(uname -r)/build/usr/include
> 	- user space headers for the currently running kernel
> $(KERNELDIR)/usr/include
> 	-  user space headers from a configured kernel tree after 'make headers_install'

Cheers,
Mark.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity May 14, 2009, 8:02 a.m. UTC | #3
Arnd Bergmann wrote:
> On Wednesday 13 May 2009 08:32:21 Mark McLoughlin wrote:
>   
>> Currently we only include $(KERNELDIR)/include in CFLAGS,
>> but we also have $(KERNELDIR)/arch/$(arch)/include or else
>> we'll get mis-matched headers.
>>
>>     
>
> I think this is fundamentally wrong. User files should never directly
> access kernel headers, because they are postprocessed in various
> ways in order to get files that are valid in user space, e.g. __user
> annotations are removed.
>   

There aren't the real kernel headers, just cheap copies carried in 
qemu-kvm.git which have been appropriately postprocessed.  We do this 
since the kvm external module can run on a much older kernel, so there 
is no natural place to find it headers.

> The three possible sources for kernel headers are:
>
> /usr/include 
> 	- system provided headers, may be older than the running kernel
> /lib/modules/$(uname -r)/build/usr/include
> 	- user space headers for the currently running kernel
> $(KERNELDIR)/usr/include
> 	-  user space headers from a configured kernel tree after 'make headers_install'

None of these are sufficiently up-to-date.
Avi Kivity May 17, 2009, 10:18 p.m. UTC | #4
Mark McLoughlin wrote:
> Currently we only include $(KERNELDIR)/include in CFLAGS,
> but we also have $(KERNELDIR)/arch/$(arch)/include or else
> we'll get mis-matched headers.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  kvm/user/config-i386.mak       |    1 -
>  kvm/user/config-ia64.mak       |    1 +
>  kvm/user/config-powerpc.mak    |    1 +
>  kvm/user/config-x86-common.mak |    2 ++
>  kvm/user/config-x86_64.mak     |    1 -
>  5 files changed, 4 insertions(+), 2 deletions(-)
>
>  
> diff --git a/kvm/user/config-ia64.mak b/kvm/user/config-ia64.mak
> index c4c639e..e8803a0 100644
> --- a/kvm/user/config-ia64.mak
> +++ b/kvm/user/config-ia64.mak
> @@ -2,6 +2,7 @@ bits = 64
>  CFLAGS += -m64
>  CFLAGS += -D__ia64__
>  CFLAGS += -I $(KERNELDIR)/include
> +CFLAGS += -I $(KERNELDIR)/arch/ia64/include
> (TEST_DIR)/stringio.flat \
>   

Can we have arch/$(ARCH)/include in some central place instead?  I can't 
bear the thought of touching this again when we merge ARM.

btw, I'd like to see the kvm/user/test stuff ported to load with qemu 
-kernel instead of the custom bootstrap.  We lose the simplicity of 
kvmctl, but we don't really need it now (kvmctl predates qemu-kvm; it 
was how we ran virtual machines until we noticed we didn't have a device 
model).
diff mbox

Patch

diff --git a/kvm/user/config-i386.mak b/kvm/user/config-i386.mak
index 09175d5..eebb9de 100644
--- a/kvm/user/config-i386.mak
+++ b/kvm/user/config-i386.mak
@@ -3,7 +3,6 @@  cstart.o = $(TEST_DIR)/cstart.o
 bits = 32
 ldarch = elf32-i386
 CFLAGS += -D__i386__
-CFLAGS += -I $(KERNELDIR)/include
 
 tests=
 
diff --git a/kvm/user/config-ia64.mak b/kvm/user/config-ia64.mak
index c4c639e..e8803a0 100644
--- a/kvm/user/config-ia64.mak
+++ b/kvm/user/config-ia64.mak
@@ -2,6 +2,7 @@  bits = 64
 CFLAGS += -m64
 CFLAGS += -D__ia64__
 CFLAGS += -I $(KERNELDIR)/include
+CFLAGS += -I $(KERNELDIR)/arch/ia64/include
 
 all:
 
diff --git a/kvm/user/config-powerpc.mak b/kvm/user/config-powerpc.mak
index dd7ef54..589aa61 100644
--- a/kvm/user/config-powerpc.mak
+++ b/kvm/user/config-powerpc.mak
@@ -1,4 +1,5 @@ 
 CFLAGS += -I $(KERNELDIR)/include
+CFLAGS += -I $(KERNELDIR)/arch/powerpc/include
 CFLAGS += -Wa,-mregnames -I test/lib
 CFLAGS += -ffreestanding
 
diff --git a/kvm/user/config-x86-common.mak b/kvm/user/config-x86-common.mak
index e789fd4..8d8fadf 100644
--- a/kvm/user/config-x86-common.mak
+++ b/kvm/user/config-x86-common.mak
@@ -12,6 +12,8 @@  cflatobjs += \
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I test/lib
 
+CFLAGS += -I $(KERNELDIR)/include
+CFLAGS += -I $(KERNELDIR)/arch/x86/include
 CFLAGS += -m$(bits)
 
 FLATLIBS = test/lib/libcflat.a $(libgcc)
diff --git a/kvm/user/config-x86_64.mak b/kvm/user/config-x86_64.mak
index b50b540..d88f54c 100644
--- a/kvm/user/config-x86_64.mak
+++ b/kvm/user/config-x86_64.mak
@@ -3,7 +3,6 @@  cstart.o = $(TEST_DIR)/cstart64.o
 bits = 64
 ldarch = elf64-x86-64
 CFLAGS += -D__x86_64__
-CFLAGS += -I $(KERNELDIR)/include
 
 tests = $(TEST_DIR)/access.flat $(TEST_DIR)/irq.flat $(TEST_DIR)/sieve.flat \
       $(TEST_DIR)/simple.flat $(TEST_DIR)/stringio.flat \