On Fri, Mar 8, 2019 at 7:32 AM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Thu, Mar 07, 2019 at 03:43:02PM -0800, Sean Christopherson wrote:
> > The CPUID flag ARCH_CAPABILITIES is unconditioinally exposed to host
> > userspace for all x86 hosts, i.e. KVM advertises ARCH_CAPABILITIES
> > regardless of hardware support under the pretense that KVM fully
> > emulates MSR_IA32_ARCH_CAPABILITIES. Unfortunately, only VMX hosts
> > handle accesses to MSR_IA32_ARCH_CAPABILITIES (despite KVM_GET_MSRS
> > also reporting MSR_IA32_ARCH_CAPABILITIES for all hosts).
> >
> > Move the MSR_IA32_ARCH_CAPABILITIES handling to common x86 code so
> > that it's emulated on AMD hosts.
> >
> > Fixes: 1eaafe91a0df4 ("kvm: x86: IA32_ARCH_CAPABILITIES is always supported")
>
> And this one should have:
>
> Cc: stable@xxxxxxxxxxxxxxx
>
> > Reported-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxxxxxxxx>
> > Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > ---
> > arch/x86/include/asm/kvm_host.h | 1 +
> > arch/x86/kvm/vmx/vmx.c | 14 --------------
> > arch/x86/kvm/vmx/vmx.h | 1 -
> > arch/x86/kvm/x86.c | 13 +++++++++++++
> > 4 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 7712b4ed8aa1..3d10ff38cbdb 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -568,6 +568,7 @@ struct kvm_vcpu_arch {
> > bool tpr_access_reporting;
> > u64 ia32_xss;
> > u64 microcode_version;
> > + u64 arch_capabilities;
> >
> > /*
> > * Paging state of the vcpu
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 2a86d296c90f..02cf8a551bd1 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1682,12 +1682,6 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >
> > msr_info->data = to_vmx(vcpu)->spec_ctrl;
> > break;
> > - case MSR_IA32_ARCH_CAPABILITIES:
> > - if (!msr_info->host_initiated &&
> > - !guest_cpuid_has(vcpu, X86_FEATURE_ARCH_CAPABILITIES))
> > - return 1;
> > - msr_info->data = to_vmx(vcpu)->arch_capabilities;
> > - break;
> > case MSR_IA32_SYSENTER_CS:
> > msr_info->data = vmcs_read32(GUEST_SYSENTER_CS);
> > break;
> > @@ -1894,12 +1888,6 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > vmx_disable_intercept_for_msr(vmx->vmcs01.msr_bitmap, MSR_IA32_PRED_CMD,
> > MSR_TYPE_W);
> > break;
> > - case MSR_IA32_ARCH_CAPABILITIES:
> > - if (!msr_info->host_initiated ||
> > - (data & ~kvm_get_arch_capabilities()))
> > - return 1;
> > - vmx->arch_capabilities = data;
> > - break;
> > case MSR_IA32_CR_PAT:
> > if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
> > if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
> > @@ -4088,8 +4076,6 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
> > ++vmx->nmsrs;
> > }
> >
> > - vmx->arch_capabilities = kvm_get_arch_capabilities();
> > -
> > vm_exit_controls_init(vmx, vmx_vmexit_ctrl());
> >
> > /* 22.2.1, 20.8.1 */
> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > index 1554cb45b393..a1e00d0a2482 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -190,7 +190,6 @@ struct vcpu_vmx {
> > u64 msr_guest_kernel_gs_base;
> > #endif
> >
> > - u64 arch_capabilities;
> > u64 spec_ctrl;
> >
> > u32 vm_entry_controls_shadow;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index d90f011f3e32..7bfeebc482c4 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2443,6 +2443,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > if (msr_info->host_initiated)
> > vcpu->arch.microcode_version = data;
> > break;
> > + case MSR_IA32_ARCH_CAPABILITIES:
> > + if (!msr_info->host_initiated ||
> > + (data & ~kvm_get_arch_capabilities()))
> > + return 1;
As I mentioned in the PATCH 1 thread, we'd still like to be able to
set IA32_ARCH_CAPABILITIES.RSBA from userspace, even if it's not
enumerated on the host.
Aside from that, this patch looks good to me. Thanks for the fix. I'll
try to be more cognizant of AMD in the future.
