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 -
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.
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
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.
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
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
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.
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
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.
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,
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.
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.
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.
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>
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.
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
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.
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;
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
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>
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
linaro-kernel@lists.linaro.org
-
Alexander Graf -
Frederic Weisbecker -
Gleb Natapov -
Kevin Hilman -
Marcelo Tosatti -
Scott Wood