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 <[email protected]>
Signed-off-by: Kevin Hilman <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Kevin Hilman <[email protected]>
> ---
> 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.
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 <[email protected]>
> Signed-off-by: Kevin Hilman <[email protected]>
> ---
> 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
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 <[email protected]>
> >Signed-off-by: Kevin Hilman <[email protected]>
> >---
> >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.
Gleb Natapov <[email protected]> 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 <[email protected]>
>> >Signed-off-by: Kevin Hilman <[email protected]>
>> >---
>> >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
On 03/21/2013 09:27:14 AM, Kevin Hilman wrote:
> Gleb Natapov <[email protected]> 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 <[email protected]>
> >> >Signed-off-by: Kevin Hilman <[email protected]>
> >> >---
> >> >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
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 <[email protected]> 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 <[email protected]>
> >>> >Signed-off-by: Kevin Hilman <[email protected]>
> >>> >---
> >>> >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.
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 <[email protected]> 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
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 <[email protected]> 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.
Gleb Natapov <[email protected]> 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 <[email protected]> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Kevin Hilman <[email protected]>
---
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
On Thu, Mar 21, 2013 at 05:02:15PM -0700, Kevin Hilman wrote:
> Gleb Natapov <[email protected]> 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 <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Kevin Hilman <[email protected]>
> ---
> 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.
2013/3/21 Gleb Natapov <[email protected]>:
> 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.
On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> 2013/3/21 Gleb Natapov <[email protected]>:
> > 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.
Gleb Natapov <[email protected]> writes:
> On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> 2013/3/21 Gleb Natapov <[email protected]>:
>> > 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Kevin Hilman <[email protected]>
---
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
On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> Gleb Natapov <[email protected]> writes:
>
> > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> 2013/3/21 Gleb Natapov <[email protected]>:
> >> > 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Kevin Hilman <[email protected]>
> ---
> 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.
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 <[email protected]> writes:
>>
>>> On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>>>> 2013/3/21 Gleb Natapov <[email protected]>:
>>>>> 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
On Mon, Mar 25, 2013 at 02:14:20PM -0700, Kevin Hilman wrote:
> Gleb Natapov <[email protected]> writes:
>
> > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> 2013/3/21 Gleb Natapov <[email protected]>:
> >> > 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 <[email protected]>
> 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.
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 <[email protected]> writes:
> >
> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> > >> 2013/3/21 Gleb Natapov <[email protected]>:
> > >> > 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 <[email protected]>
> > 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 <[email protected]>
Date: Thu May 16 01:21:38 2013 +0200
kvm: Move guest entry/exit APIs to context_tracking
Signed-off-by: Frederic Weisbecker <[email protected]>
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;
Frederic Weisbecker <[email protected]> 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 <[email protected]> writes:
>> >
>> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> > >> 2013/3/21 Gleb Natapov <[email protected]>:
>> > >> > 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 <[email protected]>
>> > 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
On Fri, May 17, 2013 at 07:09:42AM -0700, Kevin Hilman wrote:
> Frederic Weisbecker <[email protected]> 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 <[email protected]> writes:
> >> >
> >> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
> >> > >> 2013/3/21 Gleb Natapov <[email protected]>:
> >> > >> > 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 <[email protected]>
> >> > 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>
Frederic Weisbecker <[email protected]> writes:
> On Fri, May 17, 2013 at 07:09:42AM -0700, Kevin Hilman wrote:
>> Frederic Weisbecker <[email protected]> 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 <[email protected]> writes:
>> >> >
>> >> > > On Sun, Mar 24, 2013 at 02:44:26PM +0100, Frederic Weisbecker wrote:
>> >> > >> 2013/3/21 Gleb Natapov <[email protected]>:
>> >> > >> > 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 <[email protected]>
>> >> > 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 <[email protected]>
Kevin