diff mbox

KVM: Avoid using CONFIG_ in userspace visible headers

Message ID 1232370050-7434-1-git-send-email-avi@redhat.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Avi Kivity Jan. 19, 2009, 1 p.m. UTC
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(-)

Comments

Ingo Molnar Jan. 19, 2009, 1:09 p.m. UTC | #1
* 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
--
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 Jan. 19, 2009, 1:12 p.m. UTC | #2
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.
Ingo Molnar Jan. 19, 2009, 1:22 p.m. UTC | #3
* 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
--
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 Jan. 19, 2009, 1:31 p.m. UTC | #4
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.
Avi Kivity Jan. 19, 2009, 4:35 p.m. UTC | #5
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.
diff mbox

Patch

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