[PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Avi Kivity]

Kconfig symbols are not available in userspace, and are not stripped by
headers-install.  Avoid their use by adding #defines in <asm/kvm.h> to
suit each architecture.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/ia64/include/asm/kvm.h |    4 ++++
 arch/x86/include/asm/kvm.h  |    7 +++++++
 include/linux/kvm.h         |   10 +++++-----
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/include/asm/kvm.h b/arch/ia64/include/asm/kvm.h
index 68aa6da..bfa86b6 100644
--- a/arch/ia64/include/asm/kvm.h
+++ b/arch/ia64/include/asm/kvm.h
@@ -25,6 +25,10 @@
 
 #include <linux/ioctl.h>
 
+/* Select x86 specific features in <linux/kvm.h> */
+#define __KVM_HAVE_IOAPIC
+#define __KVM_HAVE_DEVICE_ASSIGNMENT
+
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256
 
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index b95162a..c4c5c69 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -9,6 +9,13 @@
 #include <asm/types.h>
 #include <linux/ioctl.h>
 
+/* Select x86 specific features in <linux/kvm.h> */
+#define __KVM_HAVE_PIT
+#define __KVM_HAVE_IOAPIC
+#define __KVM_HAVE_DEVICE_ASSIGNMENT
+#define __KVM_HAVE_MSI
+#define __KVM_HAVE_USER_NMI
+
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256
 
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 5715f19..0424326 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -58,10 +58,10 @@ struct kvm_irqchip {
 	__u32 pad;
         union {
 		char dummy[512];  /* reserving space */
-#ifdef CONFIG_X86
+#ifdef __KVM_HAVE_PIT
 		struct kvm_pic_state pic;
 #endif
-#if defined(CONFIG_X86) || defined(CONFIG_IA64)
+#ifdef __KVM_HAVE_IOAPIC
 		struct kvm_ioapic_state ioapic;
 #endif
 	} chip;
@@ -384,16 +384,16 @@ struct kvm_trace_rec {
 #define KVM_CAP_MP_STATE 14
 #define KVM_CAP_COALESCED_MMIO 15
 #define KVM_CAP_SYNC_MMU 16  /* Changes to host mmap are reflected in guest */
-#if defined(CONFIG_X86)||defined(CONFIG_IA64)
+#ifdef __KVM_HAVE_DEVICE_ASSIGNMENT
 #define KVM_CAP_DEVICE_ASSIGNMENT 17
 #endif
 #define KVM_CAP_IOMMU 18
-#if defined(CONFIG_X86)
+#ifdef __KVM_HAVE_MSI
 #define KVM_CAP_DEVICE_MSI 20
 #endif
 /* Bug in KVM_SET_USER_MEMORY_REGION fixed: */
 #define KVM_CAP_DESTROY_MEMORY_REGION_WORKS 21
-#if defined(CONFIG_X86)
+#ifdef __KVM_HAVE_USER_NMI
 #define KVM_CAP_USER_NMI 22
 #endif
 
-- 
1.6.0.6

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Ingo Molnar]

* Avi Kivity <avi@redhat.com> wrote:

> Kconfig symbols are not available in userspace, and are not stripped by
> headers-install.  Avoid their use by adding #defines in <asm/kvm.h> to
> suit each architecture.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/ia64/include/asm/kvm.h |    4 ++++
>  arch/x86/include/asm/kvm.h  |    7 +++++++
>  include/linux/kvm.h         |   10 +++++-----
>  3 files changed, 16 insertions(+), 5 deletions(-)

looks good - you will push this via the KVM tree, right?

	Ingo

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Avi Kivity]

Ingo Molnar wrote:
> * Avi Kivity <avi@redhat.com> wrote:
>
>   
>> Kconfig symbols are not available in userspace, and are not stripped by
>> headers-install.  Avoid their use by adding #defines in <asm/kvm.h> to
>> suit each architecture.
>>
>>     
> looks good - you will push this via the KVM tree, right?
>   

Yes.  Thanks for the review.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Ingo Molnar]

* Avi Kivity <avi@redhat.com> wrote:

> Ingo Molnar wrote:
>> * Avi Kivity <avi@redhat.com> wrote:
>>
>>   
>>> Kconfig symbols are not available in userspace, and are not stripped by
>>> headers-install.  Avoid their use by adding #defines in <asm/kvm.h> to
>>> suit each architecture.
>>>
>>>     
>> looks good - you will push this via the KVM tree, right?
>>   
>
> Yes.  Thanks for the review.

btw., would be nice to somehow untangle consciously-exported interface 
definitions from kernel side bits, and standardize these 
feature/capability flags like __KVM_HAVE_IOAPIC, etc.

Right now we have this body of 75,000 lines of code spread out in 600+ 
header files that are so-called 'exported' to user-space, but 95% of that 
interface definition code is never being relied on by any user-space bit! 
They are exported due to cargo-cult mentality or due to dependencies.

It would be far better to have an explicit place for such bits, 
include/syscall-ABI/ or so - and not export _any_ other kernel headers. 
But that's a much larger project.

	Ingo

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Avi Kivity]

Ingo Molnar wrote:
> btw., would be nice to somehow untangle consciously-exported interface 
> definitions from kernel side bits, and standardize these 
> feature/capability flags like __KVM_HAVE_IOAPIC, etc.
>
> Right now we have this body of 75,000 lines of code spread out in 600+ 
> header files that are so-called 'exported' to user-space, but 95% of that 
> interface definition code is never being relied on by any user-space bit! 
> They are exported due to cargo-cult mentality or due to dependencies.
>
> It would be far better to have an explicit place for such bits, 
> include/syscall-ABI/ or so - and not export _any_ other kernel headers. 
> But that's a much larger project.
>   

Yes, kvm has non-interface stuff in <linux/kvm_host.h>, so there's at 
least some separation.  All of <linux/kvm.h> is needed by userspace.

What's more painful to me in this area is advertising what's exported.  
Right now userspace has to rely on the kernel version to see what 
features are supported, but that doesn't work if a feature is backported 
or removed.  KVM has KVM_CHECK_EXTENSION which I'd love to see standardized.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Sam Ravnborg]

> 
> btw., would be nice to somehow untangle consciously-exported interface 
> definitions from kernel side bits, and standardize these 
> feature/capability flags like __KVM_HAVE_IOAPIC, etc.
> 
> Right now we have this body of 75,000 lines of code spread out in 600+ 
> header files that are so-called 'exported' to user-space, but 95% of that 
> interface definition code is never being relied on by any user-space bit! 
> They are exported due to cargo-cult mentality or due to dependencies.

With the exported headers we at least have a clear definition
what part of the headers are actually exported.
So what we have now is a much better base to cut off from.

And there is plenty of room for improvements but it requires
someone with understanding of the kernel side and the user side to do
this.

I for once find it hard to judge what is really used by user space and
not. Diving into glibc does not help as userspace is much more than
glibc although glibc is an important 'customer' here.

	Sam

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Sam Ravnborg]

On Mon, Jan 19, 2009 at 03:00:50PM +0200, Avi Kivity wrote:
> Kconfig symbols are not available in userspace, and are not stripped by
> headers-install.  Avoid their use by adding #defines in <asm/kvm.h> to
> suit each architecture.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Looked through the patch and everything looked fine.

	Sam

Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers

[Avi Kivity]

Sam Ravnborg wrote:
> With the exported headers we at least have a clear definition
> what part of the headers are actually exported.
> So what we have now is a much better base to cut off from.
>
> And there is plenty of room for improvements but it requires
> someone with understanding of the kernel side and the user side to do
> this.
>
> I for once find it hard to judge what is really used by user space and
> not. Diving into glibc does not help as userspace is much more than
> glibc although glibc is an important 'customer' here.
>   

If you set up an abi directory with clear guidelines and kbuild support, 
actively maintained pieces could be moved by the maintainers.  The rest 
will continue to suck, but we will have improved the kernel a little bit.

-- 
error compiling committee.c: too many arguments to function