kvm: user: include arch specific headers from $(KERNELDIR)
PATCH 1/1 kvm: user: include arch specific headers from $(KERNELDIR)
Currently we only include $(KERNELDIR)/include in CFLAGS, but we also have $(KERNELDIR)/arch/$(arch)/include or else we'll get mis-matched headers.
--- 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(-)
kvm/user/config-i386.mak +0 -1
@@ -3,7 +3,6 @@ cstart.o = $(TEST_DIR)/cstart.o bits = 32 ldarch = elf32-i386 CFLAGS += -D__i386__ -CFLAGS += -I $(KERNELDIR)/include tests=
kvm/user/config-ia64.mak +1 -0
@@ -2,6 +2,7 @@ bits = 64 CFLAGS += -m64 CFLAGS += -D__ia64__ CFLAGS += -I $(KERNELDIR)/include +CFLAGS += -I $(KERNELDIR)/arch/ia64/include all:
kvm/user/config-powerpc.mak +1 -0
@@ -1,4 +1,5 @@ CFLAGS += -I $(KERNELDIR)/include +CFLAGS += -I $(KERNELDIR)/arch/powerpc/include CFLAGS += -Wa,-mregnames -I test/lib CFLAGS += -ffreestanding
kvm/user/config-x86-common.mak +2 -0
@@ -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)
kvm/user/config-x86_64.mak +0 -1
@@ -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 \
Signature · 1 line
1.6.0.6
Quoted 4 lines · 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 <><
Quoted 8 lines · Arnd Bergmann
Quoted 4 lines · 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.
Quoted 4 lines
because they are postprocessed in various
ways in order to get files that are valid in user space, e.g.__userannotations are removed.
Quoted 8 lines
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 'makeheaders_install'
Cheers,
Mark.
Quoted 8 lines · Arnd Bergmann
Quoted 5 lines · 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.
Quoted 5 lines
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.
__userannotations 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.
Quoted 8 lines
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 'makeheaders_install'
None of these are sufficiently up-to-date.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Quoted 6 lines · Avi Kivity
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.
Sorry for the confusion on my part. I was aware of the sanitized kernel headers, but was mislead by the line
kerneldir=/lib/modules/$(uname -r)/build
in kvm/user/configure. What I didn't realize is that this always gets overridden by kvm/configure.
Maybe we can change the default in kvm/user/configure to something more sensible:
---
[PATCH] kvm: user: fix default kerneldir
calling ./configure in kvm/user sets the kerneldir to the currently running kernel, which is incorrect for user code. This changes the default to the sanitized header files from the kvm/kernel directory.
kvm/user/configure +1 -1
@@ -1,7 +1,7 @@ #!/bin/bash prefix=/usr/local -kerneldir=/lib/modules/$(uname -r)/build +kerneldir="$(dirname $0)/../kernel" cc=gcc ld=ld objcopy=objcopy
Quoted 30 lines · Arnd Bergmann
Quoted 2 lines · Avi Kivity
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.
Sorry for the confusion on my part. I was aware of the sanitized kernel headers, but was mislead by the line
kerneldir=/lib/modules/$(uname -r)/build
in kvm/user/configure. What I didn't realize is that this always gets overridden by kvm/configure.
Maybe we can change the default in kvm/user/configure to something more sensible:
---
[PATCH] kvm: user: fix default kerneldircalling ./configure in kvm/user sets the kerneldir to the currently running kernel, which is incorrect for user code. This changes the default to the sanitized header files from the kvm/kernel directory.
Signed-off-by Arnd Bergmann <[email protected]>kvm/user/configure +0
-0
kvm/user/configure line 7
Quoted 8 lines
kvm/user/configure +1
-1@@ -1,7 +1,7 @@ #!/bin/bash prefix=/usr/local -kerneldir=/lib/modules/$(uname -r)/build +kerneldir="$(dirname $0)/../kernel" cc=gcc ld=ld objcopy=objcopy
I usually add a readlink -f in there due to my innate fear of relative directories and cd.
btw, these are my plans for kvm/user:
- convert the tests to be loadable with qemu -kernel; we lose the simplicity of kvmctl so I'm not 100% sure it's a good idea. On the other hand some of the tests are useful for tcg.
- kill kvmtrace (replaced by the standard ftrace tools, whatever they are; maybe create a new repo if kvm specific tools are needed)
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 2 lines · Avi Kivity
I usually add a readlink -f in there due to my innate fear of relative directories and cd.
There is one already in the only place where this gets used:
KERNELDIR=$(readlink -f $kerneldir)
It also gets shown in the configure --help output, but I suppose showing the relative path there may be helpful because of its brevity.
Arnd <><
Quoted 19 lines · Mark McLoughlin
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 <[email protected]>--- 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(-)kvm/user/config-ia64.mak +0
-0
kvm/user/config-ia64.mak bits = 64 · line 7
Quoted 6 lines
kvm/user/config-ia64.mak +1
-0@@ -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).
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.