| Message ID | 1363306426-27209-1-git-send-email-khilman@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -
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(-)