[PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

The new context tracking subsystem unconditionally includes kvm_host.h
headers for the guest enter/exit macros.  This causes a compile
failure when KVM is not enabled.

Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it can
be included/compiled even when KVM is not enabled.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
Applies on v3.9-rc2

 include/linux/kvm_host.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index cad77fe..a942863 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1,6 +1,8 @@
 #ifndef __KVM_HOST_H
 #define __KVM_HOST_H
 
+#if IS_ENABLED(CONFIG_KVM)
+
 /*
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -1055,5 +1057,8 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
 }
 
 #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
+#else
+static inline void __guest_enter(void) { return; }
+static inline void __guest_exit(void) { return; }
+#endif /* IS_ENABLED(CONFIG_KVM) */
 #endif
-
-- 
1.8.1.2

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Marcelo Tosatti]

On Thu, Mar 14, 2013 at 05:13:46PM -0700, Kevin Hilman wrote:
> The new context tracking subsystem unconditionally includes kvm_host.h
> headers for the guest enter/exit macros.  This causes a compile
> failure when KVM is not enabled.
> 
> Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it can
> be included/compiled even when KVM is not enabled.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
> Applies on v3.9-rc2
> 
>  include/linux/kvm_host.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Applied and queued for v3.9, thanks.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Scott Wood]

On 03/14/2013 07:13:46 PM, Kevin Hilman wrote:
> The new context tracking subsystem unconditionally includes kvm_host.h
> headers for the guest enter/exit macros.  This causes a compile
> failure when KVM is not enabled.
> 
> Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it can
> be included/compiled even when KVM is not enabled.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
> Applies on v3.9-rc2
> 
>  include/linux/kvm_host.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

This broke the PPC non-KVM build, which was relying on stub functions  
in kvm_ppc.h, which relies on "struct vcpu" in kvm_host.h.

Why can't the entirety kvm_host.h be included regardless of CONFIG_KVM,  
just like most other feature-specific headers?  Why can't the if/else  
just go around the functions that you want to stub out for non-KVM  
builds?

-Scott

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> On 03/14/2013 07:13:46 PM, Kevin Hilman wrote:
> >The new context tracking subsystem unconditionally includes kvm_host.h
> >headers for the guest enter/exit macros.  This causes a compile
> >failure when KVM is not enabled.
> >
> >Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it can
> >be included/compiled even when KVM is not enabled.
> >
> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >Signed-off-by: Kevin Hilman <khilman@linaro.org>
> >---
> >Applies on v3.9-rc2
> >
> > include/linux/kvm_host.h | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> This broke the PPC non-KVM build, which was relying on stub
> functions in kvm_ppc.h, which relies on "struct vcpu" in kvm_host.h.
> 
> Why can't the entirety kvm_host.h be included regardless of
> CONFIG_KVM, just like most other feature-specific headers?  Why
> can't the if/else just go around the functions that you want to stub
> out for non-KVM builds?
> 
Kevin,

 What compilation failure this patch fixes? I presume something ARM
related.

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

Gleb Natapov <gleb@redhat.com> writes:

> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
>> On 03/14/2013 07:13:46 PM, Kevin Hilman wrote:
>> >The new context tracking subsystem unconditionally includes kvm_host.h
>> >headers for the guest enter/exit macros.  This causes a compile
>> >failure when KVM is not enabled.
>> >
>> >Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it can
>> >be included/compiled even when KVM is not enabled.
>> >
>> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> >Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> >---
>> >Applies on v3.9-rc2
>> >
>> > include/linux/kvm_host.h | 7 ++++++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> This broke the PPC non-KVM build, which was relying on stub
>> functions in kvm_ppc.h, which relies on "struct vcpu" in kvm_host.h.
>> 
>> Why can't the entirety kvm_host.h be included regardless of
>> CONFIG_KVM, just like most other feature-specific headers?  Why
>> can't the if/else just go around the functions that you want to stub
>> out for non-KVM builds?
>> 
> Kevin,
>
>  What compilation failure this patch fixes? I presume something ARM
> related.

Not specficially ARM related, but more context tracking related since
kernel/context_tracking.c pulls in kvm_host.h, which attempts to pull in
<asm/kvm*.h> which may not exist on some platforms.

At least for ARM, KVM support was added in v3.9 so this patch can
probably be dropped since the non-KVM builds on ARM now work.  But any
platform without the <asm/kvm*.h> will still be broken when trying to
build the context tracker.

Kevin

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Scott Wood]

On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> >> On 03/14/2013 07:13:46 PM, Kevin Hilman wrote:
> >> >The new context tracking subsystem unconditionally includes  
> kvm_host.h
> >> >headers for the guest enter/exit macros.  This causes a compile
> >> >failure when KVM is not enabled.
> >> >
> >> >Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so it  
> can
> >> >be included/compiled even when KVM is not enabled.
> >> >
> >> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >> >Signed-off-by: Kevin Hilman <khilman@linaro.org>
> >> >---
> >> >Applies on v3.9-rc2
> >> >
> >> > include/linux/kvm_host.h | 7 ++++++-
> >> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> This broke the PPC non-KVM build, which was relying on stub
> >> functions in kvm_ppc.h, which relies on "struct vcpu" in  
> kvm_host.h.
> >>
> >> Why can't the entirety kvm_host.h be included regardless of
> >> CONFIG_KVM, just like most other feature-specific headers?  Why
> >> can't the if/else just go around the functions that you want to  
> stub
> >> out for non-KVM builds?
> >>
> > Kevin,
> >
> >  What compilation failure this patch fixes? I presume something ARM
> > related.
> 
> Not specficially ARM related, but more context tracking related since
> kernel/context_tracking.c pulls in kvm_host.h, which attempts to pull  
> in
> <asm/kvm*.h> which may not exist on some platforms.
> 
> At least for ARM, KVM support was added in v3.9 so this patch can
> probably be dropped since the non-KVM builds on ARM now work.  But any
> platform without the <asm/kvm*.h> will still be broken when trying to
> build the context tracker.

Maybe other platforms should get empty asm/kvm*.h files.  Is there  
anything from those files that the linux/kvm*.h headers need to build?

-Scott

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Thu, Mar 21, 2013 at 01:42:34PM -0500, Scott Wood wrote:
> On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> >Gleb Natapov <gleb@redhat.com> writes:
> >
> >> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> >>> On 03/14/2013 07:13:46 PM, Kevin Hilman wrote:
> >>> >The new context tracking subsystem unconditionally includes
> >kvm_host.h
> >>> >headers for the guest enter/exit macros.  This causes a compile
> >>> >failure when KVM is not enabled.
> >>> >
> >>> >Fix by adding an IS_ENABLED(CONFIG_KVM) check to kvm_host so
> >it can
> >>> >be included/compiled even when KVM is not enabled.
> >>> >
> >>> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >>> >Signed-off-by: Kevin Hilman <khilman@linaro.org>
> >>> >---
> >>> >Applies on v3.9-rc2
> >>> >
> >>> > include/linux/kvm_host.h | 7 ++++++-
> >>> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> This broke the PPC non-KVM build, which was relying on stub
> >>> functions in kvm_ppc.h, which relies on "struct vcpu" in
> >kvm_host.h.
> >>>
> >>> Why can't the entirety kvm_host.h be included regardless of
> >>> CONFIG_KVM, just like most other feature-specific headers?  Why
> >>> can't the if/else just go around the functions that you want to
> >stub
> >>> out for non-KVM builds?
> >>>
> >> Kevin,
> >>
> >>  What compilation failure this patch fixes? I presume something ARM
> >> related.
> >
> >Not specficially ARM related, but more context tracking related since
> >kernel/context_tracking.c pulls in kvm_host.h, which attempts to
> >pull in
> ><asm/kvm*.h> which may not exist on some platforms.
> >
> >At least for ARM, KVM support was added in v3.9 so this patch can
> >probably be dropped since the non-KVM builds on ARM now work.  But any
> >platform without the <asm/kvm*.h> will still be broken when trying to
> >build the context tracker.
> 
> Maybe other platforms should get empty asm/kvm*.h files.  Is there
> anything from those files that the linux/kvm*.h headers need to
> build?
> 
arch things. kvm_vcpu_arch, kvm_arch_memory_slot, kvm_arch etc.

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Scott Wood]

On 03/21/2013 02:16:00 PM, Gleb Natapov wrote:
> On Thu, Mar 21, 2013 at 01:42:34PM -0500, Scott Wood wrote:
> > On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> > >Gleb Natapov <gleb@redhat.com> writes:
> > >
> > >> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> > >>> Why can't the entirety kvm_host.h be included regardless of
> > >>> CONFIG_KVM, just like most other feature-specific headers?  Why
> > >>> can't the if/else just go around the functions that you want to
> > >stub
> > >>> out for non-KVM builds?
> > >>>
> > >> Kevin,
> > >>
> > >>  What compilation failure this patch fixes? I presume something  
> ARM
> > >> related.
> > >
> > >Not specficially ARM related, but more context tracking related  
> since
> > >kernel/context_tracking.c pulls in kvm_host.h, which attempts to
> > >pull in
> > ><asm/kvm*.h> which may not exist on some platforms.
> > >
> > >At least for ARM, KVM support was added in v3.9 so this patch can
> > >probably be dropped since the non-KVM builds on ARM now work.  But  
> any
> > >platform without the <asm/kvm*.h> will still be broken when trying  
> to
> > >build the context tracker.
> >
> > Maybe other platforms should get empty asm/kvm*.h files.  Is there
> > anything from those files that the linux/kvm*.h headers need to
> > build?
> >
> arch things. kvm_vcpu_arch, kvm_arch_memory_slot, kvm_arch etc.

Could define them as empty structs.

-Scott

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Thu, Mar 21, 2013 at 02:33:13PM -0500, Scott Wood wrote:
> On 03/21/2013 02:16:00 PM, Gleb Natapov wrote:
> >On Thu, Mar 21, 2013 at 01:42:34PM -0500, Scott Wood wrote:
> >> On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> >> >Gleb Natapov <gleb@redhat.com> writes:
> >> >
> >> >> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> >> >>> Why can't the entirety kvm_host.h be included regardless of
> >> >>> CONFIG_KVM, just like most other feature-specific headers?  Why
> >> >>> can't the if/else just go around the functions that you want to
> >> >stub
> >> >>> out for non-KVM builds?
> >> >>>
> >> >> Kevin,
> >> >>
> >> >>  What compilation failure this patch fixes? I presume
> >something ARM
> >> >> related.
> >> >
> >> >Not specficially ARM related, but more context tracking related
> >since
> >> >kernel/context_tracking.c pulls in kvm_host.h, which attempts to
> >> >pull in
> >> ><asm/kvm*.h> which may not exist on some platforms.
> >> >
> >> >At least for ARM, KVM support was added in v3.9 so this patch can
> >> >probably be dropped since the non-KVM builds on ARM now work.
> >But any
> >> >platform without the <asm/kvm*.h> will still be broken when
> >trying to
> >> >build the context tracker.
> >>
> >> Maybe other platforms should get empty asm/kvm*.h files.  Is there
> >> anything from those files that the linux/kvm*.h headers need to
> >> build?
> >>
> >arch things. kvm_vcpu_arch, kvm_arch_memory_slot, kvm_arch etc.
> 
> Could define them as empty structs.
> 
Isn't is simpler for kernel/context_tracking.c to define empty
__guest_enter()/__guest_exit() if !CONFIG_KVM.

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

Gleb Natapov <gleb@redhat.com> writes:

> On Thu, Mar 21, 2013 at 02:33:13PM -0500, Scott Wood wrote:
>> On 03/21/2013 02:16:00 PM, Gleb Natapov wrote:
>> >On Thu, Mar 21, 2013 at 01:42:34PM -0500, Scott Wood wrote:
>> >> On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
>> >> >Gleb Natapov <gleb@redhat.com> writes:
>> >> >
>> >> >> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
>> >> >>> Why can't the entirety kvm_host.h be included regardless of
>> >> >>> CONFIG_KVM, just like most other feature-specific headers?  Why
>> >> >>> can't the if/else just go around the functions that you want to
>> >> >stub
>> >> >>> out for non-KVM builds?
>> >> >>>
>> >> >> Kevin,
>> >> >>
>> >> >>  What compilation failure this patch fixes? I presume
>> >something ARM
>> >> >> related.
>> >> >
>> >> >Not specficially ARM related, but more context tracking related
>> >since
>> >> >kernel/context_tracking.c pulls in kvm_host.h, which attempts to
>> >> >pull in
>> >> ><asm/kvm*.h> which may not exist on some platforms.
>> >> >
>> >> >At least for ARM, KVM support was added in v3.9 so this patch can
>> >> >probably be dropped since the non-KVM builds on ARM now work.
>> >But any
>> >> >platform without the <asm/kvm*.h> will still be broken when
>> >trying to
>> >> >build the context tracker.
>> >>
>> >> Maybe other platforms should get empty asm/kvm*.h files.  Is there
>> >> anything from those files that the linux/kvm*.h headers need to
>> >> build?
>> >>
>> >arch things. kvm_vcpu_arch, kvm_arch_memory_slot, kvm_arch etc.
>> 
>> Could define them as empty structs.
>> 
> Isn't is simpler for kernel/context_tracking.c to define empty
> __guest_enter()/__guest_exit() if !CONFIG_KVM.

I proposed something like that in an earlier version but Frederic asked
me to propose a fix to the KVM headers instead.

Just in case fixing the context tracking subsystem is preferred, 
the patch below fixes the problem also.

Kevin

>From f22995a262144d0d61705fa72134694d911283eb Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 21 Mar 2013 16:57:14 -0700
Subject: [PATCH] context_tracking: fix !CONFIG_KVM compile: add stub guest
 enter/exit

When KVM is not enabled, or not available on a platform, the KVM
headers should not be included.  Instead, just define stub
__guest_[enter|exit] functions.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 65349f0..64b0f80 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -15,12 +15,18 @@
  */
 
 #include <linux/context_tracking.h>
-#include <linux/kvm_host.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
 #include <linux/export.h>
 
+#if IS_ENABLED(CONFIG_KVM)
+#include <linux/kvm_host.h>
+#else
+#define __guest_enter()
+#define __guest_exit()
+#endif
+
 DEFINE_PER_CPU(struct context_tracking, context_tracking) = {
 #ifdef CONFIG_CONTEXT_TRACKING_FORCE
 	.active = true,
-- 
1.8.2

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Thu, Mar 21, 2013 at 05:02:15PM -0700, Kevin Hilman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Thu, Mar 21, 2013 at 02:33:13PM -0500, Scott Wood wrote:
> >> On 03/21/2013 02:16:00 PM, Gleb Natapov wrote:
> >> >On Thu, Mar 21, 2013 at 01:42:34PM -0500, Scott Wood wrote:
> >> >> On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> >> >> >Gleb Natapov <gleb@redhat.com> writes:
> >> >> >
> >> >> >> On Wed, Mar 20, 2013 at 06:58:41PM -0500, Scott Wood wrote:
> >> >> >>> Why can't the entirety kvm_host.h be included regardless of
> >> >> >>> CONFIG_KVM, just like most other feature-specific headers?  Why
> >> >> >>> can't the if/else just go around the functions that you want to
> >> >> >stub
> >> >> >>> out for non-KVM builds?
> >> >> >>>
> >> >> >> Kevin,
> >> >> >>
> >> >> >>  What compilation failure this patch fixes? I presume
> >> >something ARM
> >> >> >> related.
> >> >> >
> >> >> >Not specficially ARM related, but more context tracking related
> >> >since
> >> >> >kernel/context_tracking.c pulls in kvm_host.h, which attempts to
> >> >> >pull in
> >> >> ><asm/kvm*.h> which may not exist on some platforms.
> >> >> >
> >> >> >At least for ARM, KVM support was added in v3.9 so this patch can
> >> >> >probably be dropped since the non-KVM builds on ARM now work.
> >> >But any
> >> >> >platform without the <asm/kvm*.h> will still be broken when
> >> >trying to
> >> >> >build the context tracker.
> >> >>
> >> >> Maybe other platforms should get empty asm/kvm*.h files.  Is there
> >> >> anything from those files that the linux/kvm*.h headers need to
> >> >> build?
> >> >>
> >> >arch things. kvm_vcpu_arch, kvm_arch_memory_slot, kvm_arch etc.
> >> 
> >> Could define them as empty structs.
> >> 
> > Isn't is simpler for kernel/context_tracking.c to define empty
> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> 
> I proposed something like that in an earlier version but Frederic asked
> me to propose a fix to the KVM headers instead.
> 
> Just in case fixing the context tracking subsystem is preferred, 
> the patch below fixes the problem also.
> 
The patch that broke PPC build was reverted. Frederic can you get the
patch below?

> Kevin
> 
> >From f22995a262144d0d61705fa72134694d911283eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 21 Mar 2013 16:57:14 -0700
> Subject: [PATCH] context_tracking: fix !CONFIG_KVM compile: add stub guest
>  enter/exit
> 
> When KVM is not enabled, or not available on a platform, the KVM
> headers should not be included.  Instead, just define stub
> __guest_[enter|exit] functions.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 65349f0..64b0f80 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -15,12 +15,18 @@
>   */
>  
>  #include <linux/context_tracking.h>
> -#include <linux/kvm_host.h>
>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
>  #include <linux/export.h>
>  
> +#if IS_ENABLED(CONFIG_KVM)
> +#include <linux/kvm_host.h>
> +#else
> +#define __guest_enter()
> +#define __guest_exit()
> +#endif
> +
>  DEFINE_PER_CPU(struct context_tracking, context_tracking) = {
>  #ifdef CONFIG_CONTEXT_TRACKING_FORCE
>  	.active = true,
> -- 
> 1.8.2

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Frederic Weisbecker]

2013/3/21 Gleb Natapov <gleb@redhat.com>:
> Isn't is simpler for kernel/context_tracking.c to define empty
> __guest_enter()/__guest_exit() if !CONFIG_KVM.

That doesn't look right. Off-cases are usually handled from the
headers, right? So that we avoid iffdeffery ugliness in core code.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
> > Isn't is simpler for kernel/context_tracking.c to define empty
> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> 
> That doesn't look right. Off-cases are usually handled from the
> headers, right? So that we avoid iffdeffery ugliness in core code.
Lets put it in linux/context_tracking.h header then.

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

Gleb Natapov <gleb@redhat.com> writes:

> On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
>> > Isn't is simpler for kernel/context_tracking.c to define empty
>> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
>> 
>> That doesn't look right. Off-cases are usually handled from the
>> headers, right? So that we avoid iffdeffery ugliness in core code.
> Lets put it in linux/context_tracking.h header then.

Here's a version to do that.

Kevin

>From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Mon, 25 Mar 2013 14:12:41 -0700
Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
 enter/exit

When KVM is not enabled, or not available on a platform, the KVM
headers should not be included.  Instead, just define stub
__guest_[enter|exit] functions.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 include/linux/context_tracking.h | 7 +++++++
 kernel/context_tracking.c        | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index 365f4a6..9d0f242 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,13 @@
 
 #include <linux/sched.h>
 #include <linux/percpu.h>
+#if IS_ENABLED(CONFIG_KVM)
+#include <linux/kvm_host.h>
+#else
+#define __guest_enter()
+#define __guest_exit()
+#endif
+
 #include <asm/ptrace.h>
 
 struct context_tracking {
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 65349f0..85bdde1 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -15,7 +15,6 @@
  */
 
 #include <linux/context_tracking.h>
-#include <linux/kvm_host.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
-- 
1.8.2

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Gleb Natapov]

On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
> >> > Isn't is simpler for kernel/context_tracking.c to define empty
> >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> >> 
> >> That doesn't look right. Off-cases are usually handled from the
> >> headers, right? So that we avoid iffdeffery ugliness in core code.
> > Lets put it in linux/context_tracking.h header then.
> 
> Here's a version to do that.
> 
Frederic, are you OK with this version?


> Kevin
> 
> >From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Mon, 25 Mar 2013 14:12:41 -0700
> Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
>  enter/exit
> 
> When KVM is not enabled, or not available on a platform, the KVM
> headers should not be included.  Instead, just define stub
> __guest_[enter|exit] functions.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  include/linux/context_tracking.h | 7 +++++++
>  kernel/context_tracking.c        | 1 -
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index 365f4a6..9d0f242 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/sched.h>
>  #include <linux/percpu.h>
> +#if IS_ENABLED(CONFIG_KVM)
> +#include <linux/kvm_host.h>
> +#else
> +#define __guest_enter()
> +#define __guest_exit()
> +#endif
> +
>  #include <asm/ptrace.h>
>  
>  struct context_tracking {
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 65349f0..85bdde1 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -15,7 +15,6 @@
>   */
>  
>  #include <linux/context_tracking.h>
> -#include <linux/kvm_host.h>
>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
> -- 
> 1.8.2

--
			Gleb.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Alexander Graf]

On 02.04.2013, at 13:56, Gleb Natapov wrote:

> On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
>> Gleb Natapov <gleb@redhat.com> writes:
>> 
>>> On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>>>> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
>>>>> Isn't is simpler for kernel/context_tracking.c to define empty
>>>>> __guest_enter()/__guest_exit() if !CONFIG_KVM.
>>>> 
>>>> That doesn't look right. Off-cases are usually handled from the
>>>> headers, right? So that we avoid iffdeffery ugliness in core code.
>>> Lets put it in linux/context_tracking.h header then.
>> 
>> Here's a version to do that.
>> 
> Frederic, are you OK with this version?

Did anything happen on this? The tree is still broken...


Alex

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Frederic Weisbecker]

On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
> >> > Isn't is simpler for kernel/context_tracking.c to define empty
> >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> >> 
> >> That doesn't look right. Off-cases are usually handled from the
> >> headers, right? So that we avoid iffdeffery ugliness in core code.
> > Lets put it in linux/context_tracking.h header then.
> 
> Here's a version to do that.
> 
> Kevin
> 
> From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Mon, 25 Mar 2013 14:12:41 -0700
> Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
>  enter/exit

Sorry for my very delayed response...

> 
> When KVM is not enabled, or not available on a platform, the KVM
> headers should not be included.  Instead, just define stub
> __guest_[enter|exit] functions.

May be it would be cleaner to move guest_enter/exit definitions altogether
in linux/context_tracking.h

After all that's where the implementation mostly belong to.

Let me see if I can get that in shape.

Thanks.

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Frederic Weisbecker]

On Thu, May 16, 2013 at 12:52:03AM +0200, Frederic Weisbecker wrote:
> On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> > Gleb Natapov <gleb@redhat.com> writes:
> > 
> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> > >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
> > >> > Isn't is simpler for kernel/context_tracking.c to define empty
> > >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> > >> 
> > >> That doesn't look right. Off-cases are usually handled from the
> > >> headers, right? So that we avoid iffdeffery ugliness in core code.
> > > Lets put it in linux/context_tracking.h header then.
> > 
> > Here's a version to do that.
> > 
> > Kevin
> > 
> > From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
> > From: Kevin Hilman <khilman@linaro.org>
> > Date: Mon, 25 Mar 2013 14:12:41 -0700
> > Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
> >  enter/exit
> 
> Sorry for my very delayed response...
> 
> > 
> > When KVM is not enabled, or not available on a platform, the KVM
> > headers should not be included.  Instead, just define stub
> > __guest_[enter|exit] functions.
> 
> May be it would be cleaner to move guest_enter/exit definitions altogether
> in linux/context_tracking.h
> 
> After all that's where the implementation mostly belong to.
> 
> Let me see if I can get that in shape.

Does the following work for you?

---
commit b1633c075527653a99df6fd54d2611cf8c8047f5
Author: Frederic Weisbecker <fweisbec@gmail.com>
Date:   Thu May 16 01:21:38 2013 +0200

    kvm: Move guest entry/exit APIs to context_tracking
    
    Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>

diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index 365f4a6..fc09d7b 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h>
 #include <linux/percpu.h>
+#include <linux/vtime.h>
 #include <asm/ptrace.h>
 
 struct context_tracking {
@@ -19,6 +20,26 @@ struct context_tracking {
 	} state;
 };
 
+static inline void __guest_enter(void)
+{
+	/*
+	 * This is running in ioctl context so we can avoid
+	 * the call to vtime_account() with its unnecessary idle check.
+	 */
+	vtime_account_system(current);
+	current->flags |= PF_VCPU;
+}
+
+static inline void __guest_exit(void)
+{
+	/*
+	 * This is running in ioctl context so we can avoid
+	 * the call to vtime_account() with its unnecessary idle check.
+	 */
+	vtime_account_system(current);
+	current->flags &= ~PF_VCPU;
+}
+
 #ifdef CONFIG_CONTEXT_TRACKING
 DECLARE_PER_CPU(struct context_tracking, context_tracking);
 
@@ -35,6 +56,9 @@ static inline bool context_tracking_active(void)
 extern void user_enter(void);
 extern void user_exit(void);
 
+extern void guest_enter(void);
+extern void guest_exit(void);
+
 static inline enum ctx_state exception_enter(void)
 {
 	enum ctx_state prev_ctx;
@@ -57,6 +81,17 @@ extern void context_tracking_task_switch(struct task_struct *prev,
 static inline bool context_tracking_in_user(void) { return false; }
 static inline void user_enter(void) { }
 static inline void user_exit(void) { }
+
+static inline void guest_enter(void)
+{
+	__guest_enter();
+}
+
+static inline void guest_exit(void)
+{
+	__guest_exit();
+}
+
 static inline enum ctx_state exception_enter(void) { return 0; }
 static inline void exception_exit(enum ctx_state prev_ctx) { }
 static inline void context_tracking_task_switch(struct task_struct *prev,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f0eea07..8db53cf 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -23,6 +23,7 @@
 #include <linux/ratelimit.h>
 #include <linux/err.h>
 #include <linux/irqflags.h>
+#include <linux/context_tracking.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -760,42 +761,6 @@ static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
 }
 #endif
 
-static inline void __guest_enter(void)
-{
-	/*
-	 * This is running in ioctl context so we can avoid
-	 * the call to vtime_account() with its unnecessary idle check.
-	 */
-	vtime_account_system(current);
-	current->flags |= PF_VCPU;
-}
-
-static inline void __guest_exit(void)
-{
-	/*
-	 * This is running in ioctl context so we can avoid
-	 * the call to vtime_account() with its unnecessary idle check.
-	 */
-	vtime_account_system(current);
-	current->flags &= ~PF_VCPU;
-}
-
-#ifdef CONFIG_CONTEXT_TRACKING
-extern void guest_enter(void);
-extern void guest_exit(void);
-
-#else /* !CONFIG_CONTEXT_TRACKING */
-static inline void guest_enter(void)
-{
-	__guest_enter();
-}
-
-static inline void guest_exit(void)
-{
-	__guest_exit();
-}
-#endif /* !CONFIG_CONTEXT_TRACKING */
-
 static inline void kvm_guest_enter(void)
 {
 	unsigned long flags;

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

Frederic Weisbecker <fweisbec@gmail.com> writes:

> On Thu, May 16, 2013 at 12:52:03AM +0200, Frederic Weisbecker wrote:
>> On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
>> > Gleb Natapov <gleb@redhat.com> writes:
>> > 
>> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> > >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
>> > >> > Isn't is simpler for kernel/context_tracking.c to define empty
>> > >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
>> > >> 
>> > >> That doesn't look right. Off-cases are usually handled from the
>> > >> headers, right? So that we avoid iffdeffery ugliness in core code.
>> > > Lets put it in linux/context_tracking.h header then.
>> > 
>> > Here's a version to do that.
>> > 
>> > Kevin
>> > 
>> > From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
>> > From: Kevin Hilman <khilman@linaro.org>
>> > Date: Mon, 25 Mar 2013 14:12:41 -0700
>> > Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
>> >  enter/exit
>> 
>> Sorry for my very delayed response...
>> 
>> > 
>> > When KVM is not enabled, or not available on a platform, the KVM
>> > headers should not be included.  Instead, just define stub
>> > __guest_[enter|exit] functions.
>> 
>> May be it would be cleaner to move guest_enter/exit definitions altogether
>> in linux/context_tracking.h
>> 
>> After all that's where the implementation mostly belong to.
>> 
>> Let me see if I can get that in shape.
>
> Does the following work for you?

Nope. 

Since it still includs kvm_host.h on non-KVM builds, there is potential
for problems.  For example, on ARM (v3.10-rc1 + this patch) has this
build error:

  CC      kernel/context_tracking.o
In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
                 from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
                 from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
/work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:38:6: warning: "CONFIG_KVM_ARM_MAX_VCPUS" is not defined [-Wundef]
In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
                 from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
                 from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
/work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:59:11: error: 'CONFIG_KVM_ARM_MAX_VCPUS' undeclared here (not in a function)

Kevin

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Frederic Weisbecker]

On Fri, May 17, 2013 at 07:09:42AM -0700, Kevin Hilman wrote:
> Frederic Weisbecker <fweisbec@gmail.com> writes:
> 
> > On Thu, May 16, 2013 at 12:52:03AM +0200, Frederic Weisbecker wrote:
> >> On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> >> > Gleb Natapov <gleb@redhat.com> writes:
> >> > 
> >> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> > >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
> >> > >> > Isn't is simpler for kernel/context_tracking.c to define empty
> >> > >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
> >> > >> 
> >> > >> That doesn't look right. Off-cases are usually handled from the
> >> > >> headers, right? So that we avoid iffdeffery ugliness in core code.
> >> > > Lets put it in linux/context_tracking.h header then.
> >> > 
> >> > Here's a version to do that.
> >> > 
> >> > Kevin
> >> > 
> >> > From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
> >> > From: Kevin Hilman <khilman@linaro.org>
> >> > Date: Mon, 25 Mar 2013 14:12:41 -0700
> >> > Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
> >> >  enter/exit
> >> 
> >> Sorry for my very delayed response...
> >> 
> >> > 
> >> > When KVM is not enabled, or not available on a platform, the KVM
> >> > headers should not be included.  Instead, just define stub
> >> > __guest_[enter|exit] functions.
> >> 
> >> May be it would be cleaner to move guest_enter/exit definitions altogether
> >> in linux/context_tracking.h
> >> 
> >> After all that's where the implementation mostly belong to.
> >> 
> >> Let me see if I can get that in shape.
> >
> > Does the following work for you?
> 
> Nope. 
> 
> Since it still includs kvm_host.h on non-KVM builds, there is potential
> for problems.  For example, on ARM (v3.10-rc1 + this patch) has this
> build error:
> 
>   CC      kernel/context_tracking.o
> In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
>                  from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
>                  from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
> /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:38:6: warning: "CONFIG_KVM_ARM_MAX_VCPUS" is not defined [-Wundef]
> In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
>                  from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
>                  from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
> /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:59:11: error: 'CONFIG_KVM_ARM_MAX_VCPUS' undeclared here (not in a function)

Sorry I forgot to remove the include to kvm_host.h in context_tracking.c,
here's the fixed patch:

diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index 365f4a6..fc09d7b 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h>
 #include <linux/percpu.h>
+#include <linux/vtime.h>
 #include <asm/ptrace.h>
 
 struct context_tracking {
@@ -19,6 +20,26 @@ struct context_tracking {
 	} state;
 };
 
+static inline void __guest_enter(void)
+{
+	/*
+	 * This is running in ioctl context so we can avoid
+	 * the call to vtime_account() with its unnecessary idle check.
+	 */
+	vtime_account_system(current);
+	current->flags |= PF_VCPU;
+}
+
+static inline void __guest_exit(void)
+{
+	/*
+	 * This is running in ioctl context so we can avoid
+	 * the call to vtime_account() with its unnecessary idle check.
+	 */
+	vtime_account_system(current);
+	current->flags &= ~PF_VCPU;
+}
+
 #ifdef CONFIG_CONTEXT_TRACKING
 DECLARE_PER_CPU(struct context_tracking, context_tracking);
 
@@ -35,6 +56,9 @@ static inline bool context_tracking_active(void)
 extern void user_enter(void);
 extern void user_exit(void);
 
+extern void guest_enter(void);
+extern void guest_exit(void);
+
 static inline enum ctx_state exception_enter(void)
 {
 	enum ctx_state prev_ctx;
@@ -57,6 +81,17 @@ extern void context_tracking_task_switch(struct task_struct *prev,
 static inline bool context_tracking_in_user(void) { return false; }
 static inline void user_enter(void) { }
 static inline void user_exit(void) { }
+
+static inline void guest_enter(void)
+{
+	__guest_enter();
+}
+
+static inline void guest_exit(void)
+{
+	__guest_exit();
+}
+
 static inline enum ctx_state exception_enter(void) { return 0; }
 static inline void exception_exit(enum ctx_state prev_ctx) { }
 static inline void context_tracking_task_switch(struct task_struct *prev,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f0eea07..8db53cf 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -23,6 +23,7 @@
 #include <linux/ratelimit.h>
 #include <linux/err.h>
 #include <linux/irqflags.h>
+#include <linux/context_tracking.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -760,42 +761,6 @@ static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
 }
 #endif
 
-static inline void __guest_enter(void)
-{
-	/*
-	 * This is running in ioctl context so we can avoid
-	 * the call to vtime_account() with its unnecessary idle check.
-	 */
-	vtime_account_system(current);
-	current->flags |= PF_VCPU;
-}
-
-static inline void __guest_exit(void)
-{
-	/*
-	 * This is running in ioctl context so we can avoid
-	 * the call to vtime_account() with its unnecessary idle check.
-	 */
-	vtime_account_system(current);
-	current->flags &= ~PF_VCPU;
-}
-
-#ifdef CONFIG_CONTEXT_TRACKING
-extern void guest_enter(void);
-extern void guest_exit(void);
-
-#else /* !CONFIG_CONTEXT_TRACKING */
-static inline void guest_enter(void)
-{
-	__guest_enter();
-}
-
-static inline void guest_exit(void)
-{
-	__guest_exit();
-}
-#endif /* !CONFIG_CONTEXT_TRACKING */
-
 static inline void kvm_guest_enter(void)
 {
 	unsigned long flags;
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 65349f0..85bdde1 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -15,7 +15,6 @@
  */
 
 #include <linux/context_tracking.h>
-#include <linux/kvm_host.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>

Re: [PATCH] KVM: allow host header to be included even for !CONFIG_KVM

[Kevin Hilman]

Frederic Weisbecker <fweisbec@gmail.com> writes:

> On Fri, May 17, 2013 at 07:09:42AM -0700, Kevin Hilman wrote:
>> Frederic Weisbecker <fweisbec@gmail.com> writes:
>> 
>> > On Thu, May 16, 2013 at 12:52:03AM +0200, Frederic Weisbecker wrote:
>> >> On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
>> >> > Gleb Natapov <gleb@redhat.com> writes:
>> >> > 
>> >> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> >> > >> 2013/3/21 Gleb Natapov <gleb@redhat.com>:
>> >> > >> > Isn't is simpler for kernel/context_tracking.c to define empty
>> >> > >> > __guest_enter()/__guest_exit() if !CONFIG_KVM.
>> >> > >> 
>> >> > >> That doesn't look right. Off-cases are usually handled from the
>> >> > >> headers, right? So that we avoid iffdeffery ugliness in core code.
>> >> > > Lets put it in linux/context_tracking.h header then.
>> >> > 
>> >> > Here's a version to do that.
>> >> > 
>> >> > Kevin
>> >> > 
>> >> > From d9d909394479dd7ff90b7bddb95a564945406719 Mon Sep 17 00:00:00 2001
>> >> > From: Kevin Hilman <khilman@linaro.org>
>> >> > Date: Mon, 25 Mar 2013 14:12:41 -0700
>> >> > Subject: [PATCH v2] ontext_tracking: fix !CONFIG_KVM compile: add stub guest
>> >> >  enter/exit
>> >> 
>> >> Sorry for my very delayed response...
>> >> 
>> >> > 
>> >> > When KVM is not enabled, or not available on a platform, the KVM
>> >> > headers should not be included.  Instead, just define stub
>> >> > __guest_[enter|exit] functions.
>> >> 
>> >> May be it would be cleaner to move guest_enter/exit definitions altogether
>> >> in linux/context_tracking.h
>> >> 
>> >> After all that's where the implementation mostly belong to.
>> >> 
>> >> Let me see if I can get that in shape.
>> >
>> > Does the following work for you?
>> 
>> Nope. 
>> 
>> Since it still includs kvm_host.h on non-KVM builds, there is potential
>> for problems.  For example, on ARM (v3.10-rc1 + this patch) has this
>> build error:
>> 
>>   CC      kernel/context_tracking.o
>> In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
>>                  from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
>>                  from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
>> /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:38:6: warning: "CONFIG_KVM_ARM_MAX_VCPUS" is not defined [-Wundef]
>> In file included from /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_host.h:41:0,
>>                  from /work/kernel/linaro/nohz/include/linux/kvm_host.h:34,
>>                  from /work/kernel/linaro/nohz/kernel/context_tracking.c:18:
>> /work/kernel/linaro/nohz/arch/arm/include/asm/kvm_vgic.h:59:11: error: 'CONFIG_KVM_ARM_MAX_VCPUS' undeclared here (not in a function)
>
> Sorry I forgot to remove the include to kvm_host.h in context_tracking.c,
> here's the fixed patch:

Yup, that one builds just fine.

Reviewed-and-Tested-by: Kevin Hilman <khilman@linaro.org>

Kevin