| Message ID | 20190928172323.14663-3-aarcange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM monolithic v2 | expand |
On Sat, Sep 28, 2019 at 01:23:11PM -0400, Andrea Arcangeli wrote: > Linking both vmx and svm into the kernel at the same time isn't > possible anymore or the kvm_x86/kvm_x86_pmu external function names > would collide. > > Reported-by: kbuild test robot <lkp@intel.com> > Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> > --- > arch/x86/kvm/Kconfig | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index 840e12583b85..e1601c54355e 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -59,9 +59,29 @@ config KVM > > If unsure, say N. > > +if KVM=y Hmm, I see why the previous patch left KVM as a tristate. I tried a variety of hacks to let KVM be a bool but nothing worked. > + > +choice > + prompt "To link KVM statically into the kernel you need to choose" > + help > + In order to build a kernel with support for both AMD and Intel > + CPUs, you need to set CONFIG_KVM=m. > + > +config KVM_AMD_STATIC > + select KVM_AMD > + bool "Link KVM AMD statically into the kernel" > + > +config KVM_INTEL_STATIC > + select KVM_INTEL > + bool "Link KVM Intel statically into the kernel" The prompt and choice text is way too long, e.g. in my usual window it cuts off at: To link KVM statically into the kernel you need to choose (Link KVM Intel statically into Without the full text (the -> at the end), it's not obvious it's an option menu (AMD was selected by default for me and it took me a second to figure out what to hit enter on). I think short and sweet is enough for the prompt, with the details of how build both buried in the help text. choice prompt "KVM built-in support" help Here be a long and detailed help text. config KVM_AMD_STATIC select KVM_AMD bool "KVM AMD" config KVM_INTEL_STATIC select KVM_INTEL bool "KVM Intel" endchoice The ends up looking like: <*> Kernel-based Virtual Machine (KVM) support KVM built-in support (KVM Intel) ---> -*- KVM for Intel processors support > + > +endchoice > + > +endif > + > config KVM_INTEL > tristate "KVM for Intel processors support" > - depends on KVM > + depends on (KVM && !KVM_AMD_STATIC) || KVM_INTEL_STATIC > # for perf_guest_get_msrs(): > depends on CPU_SUP_INTEL > ---help--- > @@ -73,7 +93,7 @@ config KVM_INTEL > > config KVM_AMD > tristate "KVM for AMD processors support" > - depends on KVM > + depends on (KVM && !KVM_INTEL_STATIC) || KVM_AMD_STATIC > ---help--- > Provides support for KVM on AMD processors equipped with the AMD-V > (SVM) extensions.
On 15/10/19 05:16, Sean Christopherson wrote: > I think short and sweet is enough for the prompt, with the details of how > build both buried in the help text. > > choice > prompt "KVM built-in support" > help > Here be a long and detailed help text. > > config KVM_AMD_STATIC > select KVM_AMD > bool "KVM AMD" > > config KVM_INTEL_STATIC > select KVM_INTEL > bool "KVM Intel" Or even just bool "AMD" ... bool "Intel" > endchoice > > The ends up looking like: > > <*> Kernel-based Virtual Machine (KVM) support > KVM built-in support (KVM Intel) ---> > -*- KVM for Intel processors support On top of this, it's also nice to hide the KVM_INTEL/KVM_AMD prompts if linking statically. You can achieve that with config KVM_INTEL tristate prompt "KVM for Intel processors support" if KVM=m depends on (KVM=m && m) || KVM_INTEL_STATIC config KVM_AMD tristate prompt "KVM for AMD processors support" if KVM=m depends on (KVM=m && m) || KVM_AMD_STATIC The left side of the "||" ensures that, if KVM=m, you can only choose module build for both KVM_INTEL and KVM_AMD. Having just "depends on KVM" would allow a pre-existing .config to choose the now-invalid combination CONFIG_KVM=y CONFIG_KVM_INTEL=y CONFIG_KVM_AMD=y The right side of the "||" part is just for documentation, to avoid that a selected symbol does not satisfy its dependencies. Thanks, Paolo
On Tue, Oct 15, 2019 at 10:21:59AM +0200, Paolo Bonzini wrote: > On 15/10/19 05:16, Sean Christopherson wrote: > > I think short and sweet is enough for the prompt, with the details of how > > build both buried in the help text. > > > > choice > > prompt "KVM built-in support" > > help > > Here be a long and detailed help text. > > > > config KVM_AMD_STATIC > > select KVM_AMD > > bool "KVM AMD" > > > > config KVM_INTEL_STATIC > > select KVM_INTEL > > bool "KVM Intel" > > Or even just > > bool "AMD" > ... > bool "Intel" Ya. > > endchoice > > > > The ends up looking like: > > > > <*> Kernel-based Virtual Machine (KVM) support > > KVM built-in support (KVM Intel) ---> > > -*- KVM for Intel processors support > > On top of this, it's also nice to hide the KVM_INTEL/KVM_AMD prompts if > linking statically. You can achieve that with > > config KVM_INTEL > tristate > prompt "KVM for Intel processors support" if KVM=m That's painfully obvious now that I see it. I always forget about putting conditionals at the end... > depends on (KVM=m && m) || KVM_INTEL_STATIC > > config KVM_AMD > tristate > prompt "KVM for AMD processors support" if KVM=m > depends on (KVM=m && m) || KVM_AMD_STATIC > > The left side of the "||" ensures that, if KVM=m, you can only choose > module build for both KVM_INTEL and KVM_AMD. Having just "depends on > KVM" would allow a pre-existing .config to choose the now-invalid > combination > > CONFIG_KVM=y > CONFIG_KVM_INTEL=y > CONFIG_KVM_AMD=y > > The right side of the "||" part is just for documentation, to avoid that > a selected symbol does not satisfy its dependencies. > > Thanks, > > Paolo
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 840e12583b85..e1601c54355e 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -59,9 +59,29 @@ config KVM If unsure, say N. +if KVM=y + +choice + prompt "To link KVM statically into the kernel you need to choose" + help + In order to build a kernel with support for both AMD and Intel + CPUs, you need to set CONFIG_KVM=m. + +config KVM_AMD_STATIC + select KVM_AMD + bool "Link KVM AMD statically into the kernel" + +config KVM_INTEL_STATIC + select KVM_INTEL + bool "Link KVM Intel statically into the kernel" + +endchoice + +endif + config KVM_INTEL tristate "KVM for Intel processors support" - depends on KVM + depends on (KVM && !KVM_AMD_STATIC) || KVM_INTEL_STATIC # for perf_guest_get_msrs(): depends on CPU_SUP_INTEL ---help--- @@ -73,7 +93,7 @@ config KVM_INTEL config KVM_AMD tristate "KVM for AMD processors support" - depends on KVM + depends on (KVM && !KVM_INTEL_STATIC) || KVM_AMD_STATIC ---help--- Provides support for KVM on AMD processors equipped with the AMD-V (SVM) extensions.
Linking both vmx and svm into the kernel at the same time isn't possible anymore or the kvm_x86/kvm_x86_pmu external function names would collide. Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> --- arch/x86/kvm/Kconfig | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)