[PATCH] Fix KVM includes in helper.c

[PATCH] Fix KVM includes in helper.c

[Alexander Graf]

By default target-i386/helper.c does not include config.h, so
no code in there knows if we're enabling KVM or not.
This breaks the nested activation, as that's depending on the
config options.

Of course, with this patch applied the CPUID KVM checks are
applied twice, as now kvm_enabled() does not default to true
for the cpuid helper functions. Maybe it's a good idea to
remove the qemu-kvm-x86.c cpuid code?

This patch fixes compilation broken thanks to my nested SVM
patches.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 qemu/qemu-kvm.h           |    1 +
 qemu/target-i386/helper.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 154ff63..7224820 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -161,6 +161,7 @@ int qemu_kvm_has_sync_mmu(void);
 void kvm_init_vcpu(CPUState *env);
 #else
 #define kvm_enabled() (0)
+#define kvm_nested 0
 #define qemu_kvm_irqchip_in_kernel() (0)
 #define qemu_kvm_pit_in_kernel() (0)
 #define kvm_has_sync_mmu() (0)
diff --git a/qemu/target-i386/helper.c b/qemu/target-i386/helper.c
index 1c5b85c..04da2b0 100644
--- a/qemu/target-i386/helper.c
+++ b/qemu/target-i386/helper.c
@@ -30,6 +30,7 @@
 #include "qemu-common.h"
 #include "kvm.h"
 
+#include "config.h"
 #include "qemu-kvm.h"
 
 //#define DEBUG_MMU
-- 
1.5.6

Re: [PATCH] Fix KVM includes in helper.c

[Avi Kivity]

Alexander Graf wrote:
> By default target-i386/helper.c does not include config.h, so
> no code in there knows if we're enabling KVM or not.
> This breaks the nested activation, as that's depending on the
> config options.
>
> Of course, with this patch applied the CPUID KVM checks are
> applied twice, as now kvm_enabled() does not default to true
> for the cpuid helper functions. Maybe it's a good idea to
> remove the qemu-kvm-x86.c cpuid code?
>
> This patch fixes compilation broken thanks to my nested SVM
> patches.
>
> @@ -30,6 +30,7 @@
>  #include "qemu-common.h"
>  #include "kvm.h"
>  
> +#include "config.h"
>  #include "qemu-kvm.h"
>  

Isn't this a problem with upstream qemu as well?

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

Re: [PATCH] Fix KVM includes in helper.c

[Alexander Graf]

On 24.12.2008, at 11:38, Avi Kivity wrote:

> Alexander Graf wrote:
>> By default target-i386/helper.c does not include config.h, so
>> no code in there knows if we're enabling KVM or not.
>> This breaks the nested activation, as that's depending on the
>> config options.
>>
>> Of course, with this patch applied the CPUID KVM checks are
>> applied twice, as now kvm_enabled() does not default to true
>> for the cpuid helper functions. Maybe it's a good idea to
>> remove the qemu-kvm-x86.c cpuid code?
>>
>> This patch fixes compilation broken thanks to my nested SVM
>> patches.
>>
>> @@ -30,6 +30,7 @@
>> #include "qemu-common.h"
>> #include "kvm.h"
>> +#include "config.h"
>> #include "qemu-kvm.h"
>>
>
> Isn't this a problem with upstream qemu as well?

kvm.h includes config.h - I'm puzzled.

I'm pretty sure when I did that patch USE_KVM was not set in helper.c.

Alex

Re: [PATCH] Fix KVM includes in helper.c

[Alexander Graf]

On 24.12.2008, at 11:38, Avi Kivity wrote:

> Alexander Graf wrote:
>> By default target-i386/helper.c does not include config.h, so
>> no code in there knows if we're enabling KVM or not.
>> This breaks the nested activation, as that's depending on the
>> config options.
>>
>> Of course, with this patch applied the CPUID KVM checks are
>> applied twice, as now kvm_enabled() does not default to true
>> for the cpuid helper functions. Maybe it's a good idea to
>> remove the qemu-kvm-x86.c cpuid code?
>>
>> This patch fixes compilation broken thanks to my nested SVM
>> patches.
>>
>> @@ -30,6 +30,7 @@
>> #include "qemu-common.h"
>> #include "kvm.h"
>> +#include "config.h"
>> #include "qemu-kvm.h"
>>
>
> Isn't this a problem with upstream qemu as well?

Nevertheless, if you only apply the second chunk with the #else  
#define kvm_nested 0 we should be fine off. The only place where  
kvm_enabled() is called in helper.c is the cpuid check and that's a  
logic AND, meaning the SVM bit wouldn't get stripped if kvm_enabled()  
isn't set. We'd do that check in the second KVM CPUID parsing again  
later on anyways.

Alex

Re: [PATCH] Fix KVM includes in helper.c

[Avi Kivity]

Alexander Graf wrote:
>
> kvm.h includes config.h - I'm puzzled.

Oh, it doesn't in kvm-userspace.  I'll fix it.

Qemu should probably move to -include "config.h" instead of relying on 
all users to include it, since it will fail silently instead of complaining.

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

Re: [PATCH] Fix KVM includes in helper.c

[Avi Kivity]

Alexander Graf wrote:
>
> Nevertheless, if you only apply the second chunk with the #else 
> #define kvm_nested 0 we should be fine off. The only place where 
> kvm_enabled() is called in helper.c is the cpuid check and that's a 
> logic AND, meaning the SVM bit wouldn't get stripped if kvm_enabled() 
> isn't set.

Did just that.


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