* [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
@ 2009-05-13 8:32 Mark McLoughlin
2009-05-13 21:57 ` Arnd Bergmann
2009-05-17 22:18 ` Avi Kivity
0 siblings, 2 replies; 8+ messages in thread
From: Mark McLoughlin @ 2009-05-13 8:32 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, 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 <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-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 \
--
1.6.0.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-13 8:32 [PATCH] kvm: user: include arch specific headers from $(KERNELDIR) Mark McLoughlin
@ 2009-05-13 21:57 ` Arnd Bergmann
2009-05-14 7:52 ` Mark McLoughlin
2009-05-14 8:02 ` Avi Kivity
2009-05-17 22:18 ` Avi Kivity
1 sibling, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2009-05-13 21:57 UTC (permalink / raw)
To: Mark McLoughlin; +Cc: Avi Kivity, kvm
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 <><
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-13 21:57 ` Arnd Bergmann
@ 2009-05-14 7:52 ` Mark McLoughlin
2009-05-14 8:02 ` Avi Kivity
1 sibling, 0 replies; 8+ messages in thread
From: Mark McLoughlin @ 2009-05-14 7:52 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Avi Kivity, kvm
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-13 21:57 ` Arnd Bergmann
2009-05-14 7:52 ` Mark McLoughlin
@ 2009-05-14 8:02 ` Avi Kivity
2009-05-14 15:49 ` Arnd Bergmann
1 sibling, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2009-05-14 8:02 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Mark McLoughlin, kvm
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.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-14 8:02 ` Avi Kivity
@ 2009-05-14 15:49 ` Arnd Bergmann
2009-05-14 15:55 ` Avi Kivity
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2009-05-14 15:49 UTC (permalink / raw)
To: Avi Kivity; +Cc: Mark McLoughlin, kvm
On Thursday 14 May 2009, Avi Kivity wrote:
>
> 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.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/kvm/user/configure b/kvm/user/configure
index efb8705..858a519 100755
--- a/kvm/user/configure
+++ b/kvm/user/configure
@@ -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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-14 15:49 ` Arnd Bergmann
@ 2009-05-14 15:55 ` Avi Kivity
2009-05-14 16:07 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2009-05-14 15:55 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Mark McLoughlin, kvm
Arnd Bergmann wrote:
> On Thursday 14 May 2009, Avi Kivity wrote:
>
>> 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.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/kvm/user/configure b/kvm/user/configure
> index efb8705..858a519 100755
> --- a/kvm/user/configure
> +++ b/kvm/user/configure
> @@ -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)
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-14 15:55 ` Avi Kivity
@ 2009-05-14 16:07 ` Arnd Bergmann
0 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2009-05-14 16:07 UTC (permalink / raw)
To: Avi Kivity; +Cc: Mark McLoughlin, kvm
On Thursday 14 May 2009, Avi Kivity wrote:
> 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 <><
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm: user: include arch specific headers from $(KERNELDIR)
2009-05-13 8:32 [PATCH] kvm: user: include arch specific headers from $(KERNELDIR) Mark McLoughlin
2009-05-13 21:57 ` Arnd Bergmann
@ 2009-05-17 22:18 ` Avi Kivity
1 sibling, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2009-05-17 22:18 UTC (permalink / raw)
To: Mark McLoughlin; +Cc: kvm
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).
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-05-17 22:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-13 8:32 [PATCH] kvm: user: include arch specific headers from $(KERNELDIR) Mark McLoughlin
2009-05-13 21:57 ` Arnd Bergmann
2009-05-14 7:52 ` Mark McLoughlin
2009-05-14 8:02 ` Avi Kivity
2009-05-14 15:49 ` Arnd Bergmann
2009-05-14 15:55 ` Avi Kivity
2009-05-14 16:07 ` Arnd Bergmann
2009-05-17 22:18 ` Avi Kivity
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.