KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Fix issue with not starting nesting guests on my system
@ 2020-05-23 16:14 Maxim Levitsky
  2020-05-23 16:14 ` [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities Maxim Levitsky
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-23 16:14 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, H. Peter Anvin, Tao Xu, Sean Christopherson,
	Jim Mattson, linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu, Maxim Levitsky

On my AMD machine I noticed that I can't start any nested guests,\r
because nested KVM (everything from master git branches) complains\r
that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system doesn't support\r
at all anyway.\r
\r
I traced it to the recently added UMWAIT support to qemu and kvm.\r
The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST without\r
checking that it the underlying feature is supported in CPUID.\r
It happened to work when non nested because as a precation kvm,\r
tries to read each MSR on host before adding it to that list,\r
and when read gets a #GP it ignores it.\r
\r
When running nested, the L1 hypervisor can be set to ignore unknown\r
msr read/writes (I need this for some other guests), thus this safety\r
check doesn't work anymore.\r
\r
V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm capability\r
    * dropped the cosmetic fix patch as it is now fixed in kvm/queue\r
\r
Best regards,\r
	Maxim Levitsky\r
\r
Maxim Levitsky (2):\r
  kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities\r
  kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally\r
\r
 arch/x86/kvm/vmx/vmx.c | 3 +++\r
 arch/x86/kvm/x86.c     | 4 ++++\r
 2 files changed, 7 insertions(+)\r
\r
-- \r
2.26.2\r
\r


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
  2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
@ 2020-05-23 16:14 ` Maxim Levitsky
  2020-05-27  1:20   ` Sean Christopherson
  2020-05-23 16:14 ` [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally Maxim Levitsky
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-23 16:14 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, H. Peter Anvin, Tao Xu, Sean Christopherson,
	Jim Mattson, linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu, Maxim Levitsky

Even though we might not allow the guest to use
WAITPKG's new instructions, we should tell KVM
that the feature is supported by the host CPU.

Note that vmx_waitpkg_supported checks that WAITPKG
_can_ be set in secondary execution controls as specified
by VMX capability MSR, rather that we actually enable it for a guest.

Fixes: e69e72faa3a0 KVM: x86: Add support for user wait instructions

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/vmx/vmx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 55712dd86bafa..fca493d4517c5 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7298,6 +7298,9 @@ static __init void vmx_set_cpu_caps(void)
 	/* CPUID 0x80000001 */
 	if (!cpu_has_vmx_rdtscp())
 		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
+
+	if (vmx_waitpkg_supported())
+		kvm_cpu_cap_check_and_set(X86_FEATURE_WAITPKG);
 }
 
 static void vmx_request_immediate_exit(struct kvm_vcpu *vcpu)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
  2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
  2020-05-23 16:14 ` [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities Maxim Levitsky
@ 2020-05-23 16:14 ` Maxim Levitsky
  2020-05-27  1:21   ` Sean Christopherson
  2020-05-27  1:03 ` [PATCH 0/2] Fix issue with not starting nesting guests on my system Krish Sadhukhan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-23 16:14 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, H. Peter Anvin, Tao Xu, Sean Christopherson,
	Jim Mattson, linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu, Maxim Levitsky

This msr is only available when the host supports WAITPKG feature.

This breaks a nested guest, if the L1 hypervisor is set to ignore
unknown msrs, because the only other safety check that the
kernel does is that it attempts to read the msr and
rejects it if it gets an exception.

Fixes: 6e3ba4abce KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/x86.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b226fb8abe41b..4752293312947 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5316,6 +5316,10 @@ static void kvm_init_msr_list(void)
 			    min(INTEL_PMC_MAX_GENERIC, x86_pmu.num_counters_gp))
 				continue;
 			break;
+		case MSR_IA32_UMWAIT_CONTROL:
+			if (!kvm_cpu_cap_has(X86_FEATURE_WAITPKG))
+				continue;
+			break;
 		default:
 			break;
 		}
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
  2020-05-23 16:14 ` [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities Maxim Levitsky
  2020-05-23 16:14 ` [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally Maxim Levitsky
@ 2020-05-27  1:03 ` Krish Sadhukhan
  2020-05-27  1:22   ` Sean Christopherson
  2020-05-27  1:13 ` Sean Christopherson
  2020-05-27 17:00 ` Paolo Bonzini
  4 siblings, 1 reply; 13+ messages in thread
From: Krish Sadhukhan @ 2020-05-27  1:03 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: Paolo Bonzini, H. Peter Anvin, Tao Xu, Sean Christopherson,
	Jim Mattson, linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu


On 5/23/20 9:14 AM, Maxim Levitsky wrote:
> On my AMD machine I noticed that I can't start any nested guests,
> because nested KVM (everything from master git branches) complains
> that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system doesn't support
> at all anyway.
>
> I traced it to the recently added UMWAIT support to qemu and kvm.
> The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST without
> checking that it the underlying feature is supported in CPUID.
> It happened to work when non nested because as a precation kvm,
> tries to read each MSR on host before adding it to that list,
> and when read gets a #GP it ignores it.
>
> When running nested, the L1 hypervisor can be set to ignore unknown
> msr read/writes (I need this for some other guests), thus this safety
> check doesn't work anymore.
>
> V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm capability
>      * dropped the cosmetic fix patch as it is now fixed in kvm/queue
>
> Best regards,
> 	Maxim Levitsky
>
> Maxim Levitsky (2):
>    kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
>    kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
>
>   arch/x86/kvm/vmx/vmx.c | 3 +++
>   arch/x86/kvm/x86.c     | 4 ++++
>   2 files changed, 7 insertions(+)
>
Nit: The added 'break' statement in patch# 2 is not required.

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
                   ` (2 preceding siblings ...)
  2020-05-27  1:03 ` [PATCH 0/2] Fix issue with not starting nesting guests on my system Krish Sadhukhan
@ 2020-05-27  1:13 ` Sean Christopherson
  2020-05-27 15:17   ` Maxim Levitsky
  2020-05-27 15:17   ` Maxim Levitsky
  2020-05-27 17:00 ` Paolo Bonzini
  4 siblings, 2 replies; 13+ messages in thread
From: Sean Christopherson @ 2020-05-27  1:13 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Sat, May 23, 2020 at 07:14:53PM +0300, Maxim Levitsky wrote:
> On my AMD machine I noticed that I can't start any nested guests,
> because nested KVM (everything from master git branches) complains
> that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system doesn't support
> at all anyway.
> 
> I traced it to the recently added UMWAIT support to qemu and kvm.
> The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST without
> checking that it the underlying feature is supported in CPUID.
> It happened to work when non nested because as a precation kvm,
> tries to read each MSR on host before adding it to that list,
> and when read gets a #GP it ignores it.
> 
> When running nested, the L1 hypervisor can be set to ignore unknown
> msr read/writes (I need this for some other guests), thus this safety
> check doesn't work anymore.
> 
> V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm capability
>     * dropped the cosmetic fix patch as it is now fixed in kvm/queue
> 
> Best regards,
> 	Maxim Levitsky
> 
> Maxim Levitsky (2):
>   kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
>   kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally

Standard scoping in the shortlog is "KVM: VMX:" and "KVM: x86:".

> 
>  arch/x86/kvm/vmx/vmx.c | 3 +++
>  arch/x86/kvm/x86.c     | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> -- 
> 2.26.2
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
  2020-05-23 16:14 ` [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities Maxim Levitsky
@ 2020-05-27  1:20   ` Sean Christopherson
  2020-05-27 15:16     ` Maxim Levitsky
  0 siblings, 1 reply; 13+ messages in thread
From: Sean Christopherson @ 2020-05-27  1:20 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Sat, May 23, 2020 at 07:14:54PM +0300, Maxim Levitsky wrote:
> Even though we might not allow the guest to use
> WAITPKG's new instructions, we should tell KVM
> that the feature is supported by the host CPU.
> 
> Note that vmx_waitpkg_supported checks that WAITPKG
> _can_ be set in secondary execution controls as specified
> by VMX capability MSR, rather that we actually enable it for a guest.

These line wraps are quite weird and inconsistent.

> 
> Fixes: e69e72faa3a0 KVM: x86: Add support for user wait instructions

Checkpatch doesn't complain,  but the preferred Fixes format is

  Fixes: e69e72faa3a07 ("KVM: x86: Add support for user wait instructions")

e.g.

  git show -s --pretty='tformat:%h ("%s")'

For the code itself:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 55712dd86bafa..fca493d4517c5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7298,6 +7298,9 @@ static __init void vmx_set_cpu_caps(void)
>  	/* CPUID 0x80000001 */
>  	if (!cpu_has_vmx_rdtscp())
>  		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
> +
> +	if (vmx_waitpkg_supported())
> +		kvm_cpu_cap_check_and_set(X86_FEATURE_WAITPKG);
>  }
>  
>  static void vmx_request_immediate_exit(struct kvm_vcpu *vcpu)
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
  2020-05-23 16:14 ` [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally Maxim Levitsky
@ 2020-05-27  1:21   ` Sean Christopherson
  2020-05-27 15:17     ` Maxim Levitsky
  0 siblings, 1 reply; 13+ messages in thread
From: Sean Christopherson @ 2020-05-27  1:21 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Sat, May 23, 2020 at 07:14:55PM +0300, Maxim Levitsky wrote:
> This msr is only available when the host supports WAITPKG feature.
> 
> This breaks a nested guest, if the L1 hypervisor is set to ignore
> unknown msrs, because the only other safety check that the
> kernel does is that it attempts to read the msr and
> rejects it if it gets an exception.
> 
> Fixes: 6e3ba4abce KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL

Same comments on the line wraps and Fixes tag.

For the code:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  arch/x86/kvm/x86.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b226fb8abe41b..4752293312947 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5316,6 +5316,10 @@ static void kvm_init_msr_list(void)
>  			    min(INTEL_PMC_MAX_GENERIC, x86_pmu.num_counters_gp))
>  				continue;
>  			break;
> +		case MSR_IA32_UMWAIT_CONTROL:
> +			if (!kvm_cpu_cap_has(X86_FEATURE_WAITPKG))
> +				continue;
> +			break;
>  		default:
>  			break;
>  		}
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-27  1:03 ` [PATCH 0/2] Fix issue with not starting nesting guests on my system Krish Sadhukhan
@ 2020-05-27  1:22   ` Sean Christopherson
  0 siblings, 0 replies; 13+ messages in thread
From: Sean Christopherson @ 2020-05-27  1:22 UTC (permalink / raw)
  To: Krish Sadhukhan
  Cc: Maxim Levitsky, kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu,
	Jim Mattson, linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Tue, May 26, 2020 at 06:03:32PM -0700, Krish Sadhukhan wrote:
> 
> On 5/23/20 9:14 AM, Maxim Levitsky wrote:
> >On my AMD machine I noticed that I can't start any nested guests,
> >because nested KVM (everything from master git branches) complains
> >that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system doesn't support
> >at all anyway.
> >
> >I traced it to the recently added UMWAIT support to qemu and kvm.
> >The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST without
> >checking that it the underlying feature is supported in CPUID.
> >It happened to work when non nested because as a precation kvm,
> >tries to read each MSR on host before adding it to that list,
> >and when read gets a #GP it ignores it.
> >
> >When running nested, the L1 hypervisor can be set to ignore unknown
> >msr read/writes (I need this for some other guests), thus this safety
> >check doesn't work anymore.
> >
> >V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm capability
> >     * dropped the cosmetic fix patch as it is now fixed in kvm/queue
> >
> >Best regards,
> >	Maxim Levitsky
> >
> >Maxim Levitsky (2):
> >   kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
> >   kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
> >
> >  arch/x86/kvm/vmx/vmx.c | 3 +++
> >  arch/x86/kvm/x86.c     | 4 ++++
> >  2 files changed, 7 insertions(+)
> >
> Nit: The added 'break' statement in patch# 2 is not required.

It is unless you want to add a fallthrough annotation.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
  2020-05-27  1:20   ` Sean Christopherson
@ 2020-05-27 15:16     ` Maxim Levitsky
  0 siblings, 0 replies; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-27 15:16 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Tue, 2020-05-26 at 18:20 -0700, Sean Christopherson wrote:
> On Sat, May 23, 2020 at 07:14:54PM +0300, Maxim Levitsky wrote:
> > Even though we might not allow the guest to use
> > WAITPKG's new instructions, we should tell KVM
> > that the feature is supported by the host CPU.
> > 
> > Note that vmx_waitpkg_supported checks that WAITPKG
> > _can_ be set in secondary execution controls as specified
> > by VMX capability MSR, rather that we actually enable it for a
> > guest.
> 
> These line wraps are quite weird and inconsistent.
Known issue for me, I usually don't have line wrapping enabled,
and I wrap the lines a bit earlier that 72 character limit. 
I'll re-formatted the commit message to be on 72 line format and I will
try now to pay much more attention to that.

> 
> > Fixes: e69e72faa3a0 KVM: x86: Add support for user wait
> > instructions
> 
> Checkpatch doesn't complain,  but the preferred Fixes format is
> 
>   Fixes: e69e72faa3a07 ("KVM: x86: Add support for user wait
> instructions")


> 
> e.g.
> 
>   git show -s --pretty='tformat:%h ("%s")'

Got it, and added to git aliases :-)

> 
> For the code itself:
> 
> Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Thank you!

> 
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  arch/x86/kvm/vmx/vmx.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 55712dd86bafa..fca493d4517c5 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7298,6 +7298,9 @@ static __init void vmx_set_cpu_caps(void)
> >  	/* CPUID 0x80000001 */
> >  	if (!cpu_has_vmx_rdtscp())
> >  		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
> > +
> > +	if (vmx_waitpkg_supported())
> > +		kvm_cpu_cap_check_and_set(X86_FEATURE_WAITPKG);
> >  }
> >  
> >  static void vmx_request_immediate_exit(struct kvm_vcpu *vcpu)
> > -- 
> > 2.26.2
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
  2020-05-27  1:21   ` Sean Christopherson
@ 2020-05-27 15:17     ` Maxim Levitsky
  0 siblings, 0 replies; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-27 15:17 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Tue, 2020-05-26 at 18:21 -0700, Sean Christopherson wrote:
> On Sat, May 23, 2020 at 07:14:55PM +0300, Maxim Levitsky wrote:
> > This msr is only available when the host supports WAITPKG feature.
> > 
> > This breaks a nested guest, if the L1 hypervisor is set to ignore
> > unknown msrs, because the only other safety check that the
> > kernel does is that it attempts to read the msr and
> > rejects it if it gets an exception.
> > 
> > Fixes: 6e3ba4abce KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
> 
> Same comments on the line wraps and Fixes tag.
I rewrote the commit message and I hope that the new version
is better. I fixed the 'fixes' message as well.

> 
> For the code:
> 
> Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Thank you!

Best regards,
	Maxim Levitsky


> 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  arch/x86/kvm/x86.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index b226fb8abe41b..4752293312947 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5316,6 +5316,10 @@ static void kvm_init_msr_list(void)
> >  			    min(INTEL_PMC_MAX_GENERIC,
> > x86_pmu.num_counters_gp))
> >  				continue;
> >  			break;
> > +		case MSR_IA32_UMWAIT_CONTROL:
> > +			if (!kvm_cpu_cap_has(X86_FEATURE_WAITPKG))
> > +				continue;
> > +			break;
> >  		default:
> >  			break;
> >  		}
> > -- 
> > 2.26.2
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-27  1:13 ` Sean Christopherson
@ 2020-05-27 15:17   ` Maxim Levitsky
  2020-05-27 15:17   ` Maxim Levitsky
  1 sibling, 0 replies; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-27 15:17 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Tue, 2020-05-26 at 18:13 -0700, Sean Christopherson wrote:
> On Sat, May 23, 2020 at 07:14:53PM +0300, Maxim Levitsky wrote:
> > On my AMD machine I noticed that I can't start any nested guests,
> > because nested KVM (everything from master git branches) complains
> > that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system
> > doesn't support
> > at all anyway.
> > 
> > I traced it to the recently added UMWAIT support to qemu and kvm.
> > The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST
> > without
> > checking that it the underlying feature is supported in CPUID.
> > It happened to work when non nested because as a precation kvm,
> > tries to read each MSR on host before adding it to that list,
> > and when read gets a #GP it ignores it.
> > 
> > When running nested, the L1 hypervisor can be set to ignore unknown
> > msr read/writes (I need this for some other guests), thus this
> > safety
> > check doesn't work anymore.
> > 
> > V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm
> > capability
> >     * dropped the cosmetic fix patch as it is now fixed in
> > kvm/queue
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > Maxim Levitsky (2):
> >   kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
> >   kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
> 
> Standard scoping in the shortlog is "KVM: VMX:" and "KVM: x86:".
This another thing I usually mess up in the commit messages.
Fixed and noted for futher patches 

> 
> >  arch/x86/kvm/vmx/vmx.c | 3 +++
> >  arch/x86/kvm/x86.c     | 4 ++++
> >  2 files changed, 7 insertions(+)
> > 
> > -- 
> > 2.26.2
> > 
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-27  1:13 ` Sean Christopherson
  2020-05-27 15:17   ` Maxim Levitsky
@ 2020-05-27 15:17   ` Maxim Levitsky
  1 sibling, 0 replies; 13+ messages in thread
From: Maxim Levitsky @ 2020-05-27 15:17 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, Paolo Bonzini, H. Peter Anvin, Tao Xu, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On Tue, 2020-05-26 at 18:13 -0700, Sean Christopherson wrote:
> On Sat, May 23, 2020 at 07:14:53PM +0300, Maxim Levitsky wrote:
> > On my AMD machine I noticed that I can't start any nested guests,
> > because nested KVM (everything from master git branches) complains
> > that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system
> > doesn't support
> > at all anyway.
> > 
> > I traced it to the recently added UMWAIT support to qemu and kvm.
> > The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST
> > without
> > checking that it the underlying feature is supported in CPUID.
> > It happened to work when non nested because as a precation kvm,
> > tries to read each MSR on host before adding it to that list,
> > and when read gets a #GP it ignores it.
> > 
> > When running nested, the L1 hypervisor can be set to ignore unknown
> > msr read/writes (I need this for some other guests), thus this
> > safety
> > check doesn't work anymore.
> > 
> > V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm
> > capability
> >     * dropped the cosmetic fix patch as it is now fixed in kvm/queue
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > Maxim Levitsky (2):
> >   kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
> >   kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
> 
> Standard scoping in the shortlog is "KVM: VMX:" and "KVM: x86:".
Noted and I will use it from now on.
Thanks!

Best regards,
	Maxim Levitsky

> 
> >  arch/x86/kvm/vmx/vmx.c | 3 +++
> >  arch/x86/kvm/x86.c     | 4 ++++
> >  2 files changed, 7 insertions(+)
> > 
> > -- 
> > 2.26.2
> > 
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fix issue with not starting nesting guests on my system
  2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
                   ` (3 preceding siblings ...)
  2020-05-27  1:13 ` Sean Christopherson
@ 2020-05-27 17:00 ` Paolo Bonzini
  4 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2020-05-27 17:00 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: H. Peter Anvin, Tao Xu, Sean Christopherson, Jim Mattson,
	linux-kernel, Joerg Roedel,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Wanpeng Li, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Vitaly Kuznetsov, Jingqi Liu

On 23/05/20 18:14, Maxim Levitsky wrote:
> On my AMD machine I noticed that I can't start any nested guests,
> because nested KVM (everything from master git branches) complains
> that it can't find msr MSR_IA32_UMWAIT_CONTROL which my system doesn't support
> at all anyway.
> 
> I traced it to the recently added UMWAIT support to qemu and kvm.
> The kvm portion exposed the new MSR in KVM_GET_MSR_INDEX_LIST without
> checking that it the underlying feature is supported in CPUID.
> It happened to work when non nested because as a precation kvm,
> tries to read each MSR on host before adding it to that list,
> and when read gets a #GP it ignores it.
> 
> When running nested, the L1 hypervisor can be set to ignore unknown
> msr read/writes (I need this for some other guests), thus this safety
> check doesn't work anymore.
> 
> V2: * added a patch to setup correctly the X86_FEATURE_WAITPKG kvm capability
>     * dropped the cosmetic fix patch as it is now fixed in kvm/queue
> 
> Best regards,
> 	Maxim Levitsky
> 
> Maxim Levitsky (2):
>   kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities
>   kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally
> 
>  arch/x86/kvm/vmx/vmx.c | 3 +++
>  arch/x86/kvm/x86.c     | 4 ++++
>  2 files changed, 7 insertions(+)
> 

Queued for 5.7, thanks (with cosmetic touches to the commit message, and
moving the "case" earlier to avoid conflicts).

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-23 16:14 [PATCH 0/2] Fix issue with not starting nesting guests on my system Maxim Levitsky
2020-05-23 16:14 ` [PATCH 1/2] kvm/x86/vmx: enable X86_FEATURE_WAITPKG in KVM capabilities Maxim Levitsky
2020-05-27  1:20   ` Sean Christopherson
2020-05-27 15:16     ` Maxim Levitsky
2020-05-23 16:14 ` [PATCH 2/2] kvm/x86: don't expose MSR_IA32_UMWAIT_CONTROL unconditionally Maxim Levitsky
2020-05-27  1:21   ` Sean Christopherson
2020-05-27 15:17     ` Maxim Levitsky
2020-05-27  1:03 ` [PATCH 0/2] Fix issue with not starting nesting guests on my system Krish Sadhukhan
2020-05-27  1:22   ` Sean Christopherson
2020-05-27  1:13 ` Sean Christopherson
2020-05-27 15:17   ` Maxim Levitsky
2020-05-27 15:17   ` Maxim Levitsky
2020-05-27 17:00 ` Paolo Bonzini

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git