| Message ID | 20190307234302.4092-2-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: fix ARCH_CAPBILITIES emulation | expand |
On Thu, Mar 07, 2019 at 03:43:01PM -0800, Sean Christopherson wrote: > For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID > feature leaf. Now that it is emulated, give it the same treatment we > give CPUID leafs with a mixture of emulated and hardware-only features > and reject attempts by host userspace to expose features to the guest > that are not supported by KVM, i.e. are not supported in hardware and > are not emulated by KVM. > > Fixes: 1eaafe91a0df4 ("kvm: x86: IA32_ARCH_CAPABILITIES is always supported") Doh, this should be: Fixes: 28c1c9fabf48 ("KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES") > Cc: Jim Mattson <jmattson@google.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/vmx/vmx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 7aade2dd1da8..2a86d296c90f 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -1895,7 +1895,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > MSR_TYPE_W); > break; > case MSR_IA32_ARCH_CAPABILITIES: > - if (!msr_info->host_initiated) > + if (!msr_info->host_initiated || > + (data & ~kvm_get_arch_capabilities())) > return 1; > vmx->arch_capabilities = data; > break; > -- > 2.21.0 >
On Thu, Mar 7, 2019 at 3:43 PM Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID > feature leaf. Now that it is emulated, give it the same treatment we > give CPUID leafs with a mixture of emulated and hardware-only features > and reject attempts by host userspace to expose features to the guest > that are not supported by KVM, i.e. are not supported in hardware and > are not emulated by KVM. I didn't think we rejected attempts by host userspace to expose CPUID features to the guest that aren't supported by kvm. I know we do so for VMX capability MSRs, but is this overkill for this particular MSR?
On Fri, Mar 08, 2019 at 09:29:15AM -0800, Jim Mattson wrote: > On Thu, Mar 7, 2019 at 3:43 PM Sean Christopherson > <sean.j.christopherson@intel.com> wrote: > > > > For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID > > feature leaf. Now that it is emulated, give it the same treatment we > > give CPUID leafs with a mixture of emulated and hardware-only features > > and reject attempts by host userspace to expose features to the guest > > that are not supported by KVM, i.e. are not supported in hardware and > > are not emulated by KVM. > > I didn't think we rejected attempts by host userspace to expose CPUID > features to the guest that aren't supported by kvm. I know we do so > for VMX capability MSRs, but is this overkill for this particular MSR? Argh, you're right, I got my brain twisted around again, I keep thinking that masking bits in KVM_GET_CPUID2 means KVM prevents setting them via KVM_SET_CPUID2.
On Fri, 2019-03-08 at 09:41 -0800, Sean Christopherson wrote: > On Fri, Mar 08, 2019 at 09:29:15AM -0800, Jim Mattson wrote: > > On Thu, Mar 7, 2019 at 3:43 PM Sean Christopherson > > <sean.j.christopherson@intel.com> wrote: > > > > > > For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID > > > feature leaf. Now that it is emulated, give it the same treatment we > > > give CPUID leafs with a mixture of emulated and hardware-only features > > > and reject attempts by host userspace to expose features to the guest > > > that are not supported by KVM, i.e. are not supported in hardware and > > > are not emulated by KVM. > > > > I didn't think we rejected attempts by host userspace to expose CPUID > > features to the guest that aren't supported by kvm. I know we do so > > for VMX capability MSRs, but is this overkill for this particular MSR? > > Argh, you're right, I got my brain twisted around again, I keep thinking > that masking bits in KVM_GET_CPUID2 means KVM prevents setting them via > KVM_SET_CPUID2. I think masking bits in KVM_GET_CPUID2 only means KVM doesn't support the feature for guest, but not prevents setting them via KVM_SET_CPUID2. Because userspace may have the ability to emulate that feature.
Refusing to let userspace set IA32_ARCH_CAPABILITIES.RSBA is a problem for us. Following Intel's guidance, our userspace sets this bit to indicate that a VM might be scheduled to run on a vulnerable processor. On Fri, Mar 8, 2019 at 6:29 PM Xiaoyao Li <xiaoyao.li@linux.intel.com> wrote: > > On Fri, 2019-03-08 at 09:41 -0800, Sean Christopherson wrote: > > On Fri, Mar 08, 2019 at 09:29:15AM -0800, Jim Mattson wrote: > > > On Thu, Mar 7, 2019 at 3:43 PM Sean Christopherson > > > <sean.j.christopherson@intel.com> wrote: > > > > > > > > For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID > > > > feature leaf. Now that it is emulated, give it the same treatment we > > > > give CPUID leafs with a mixture of emulated and hardware-only features > > > > and reject attempts by host userspace to expose features to the guest > > > > that are not supported by KVM, i.e. are not supported in hardware and > > > > are not emulated by KVM. > > > > > > I didn't think we rejected attempts by host userspace to expose CPUID > > > features to the guest that aren't supported by kvm. I know we do so > > > for VMX capability MSRs, but is this overkill for this particular MSR? > > > > Argh, you're right, I got my brain twisted around again, I keep thinking > > that masking bits in KVM_GET_CPUID2 means KVM prevents setting them via > > KVM_SET_CPUID2. > > I think masking bits in KVM_GET_CPUID2 only means KVM doesn't support the > feature for guest, but not prevents setting them via KVM_SET_CPUID2. Because > userspace may have the ability to emulate that feature. >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 7aade2dd1da8..2a86d296c90f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -1895,7 +1895,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) MSR_TYPE_W); break; case MSR_IA32_ARCH_CAPABILITIES: - if (!msr_info->host_initiated) + if (!msr_info->host_initiated || + (data & ~kvm_get_arch_capabilities())) return 1; vmx->arch_capabilities = data; break;
For all intents and purposes, MSR_IA32_ARCH_CAPABILITIES is a CPUID feature leaf. Now that it is emulated, give it the same treatment we give CPUID leafs with a mixture of emulated and hardware-only features and reject attempts by host userspace to expose features to the guest that are not supported by KVM, i.e. are not supported in hardware and are not emulated by KVM. Fixes: 1eaafe91a0df4 ("kvm: x86: IA32_ARCH_CAPABILITIES is always supported") Cc: Jim Mattson <jmattson@google.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)