----- pbonz...@redhat.com wrote:

> MSR_IA32_SPEC_CTRL is not available unless CPU[7,0].EDX[26] is 1.
> Check that against host CPUID or guest CPUID, respectively for
> host-initiated and guest-initiated accesses.
> 
> Suggested-by: Jim Mattson <jmatt...@google.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
>       This is for after X86_FEATURE_SPEC_CTRL is added to Linux, but
>       I still wanted to ack Jim's improvement.
> 
>  arch/x86/kvm/svm.c | 8 ++++++++
>  arch/x86/kvm/vmx.c | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 97126c2bd663..3a646580d7c5 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3648,6 +3648,10 @@ static int svm_get_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
>               msr_info->data = svm->nested.vm_cr_msr;
>               break;
>       case MSR_IA32_SPEC_CTRL:
> +             if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> +                 (!msr_info->host_initiated &&
> +                          !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
> +                     return 1;
>               msr_info->data = svm->spec_ctrl;
>               break;
>       case MSR_IA32_UCODE_REV:
> @@ -3806,6 +3810,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr)
>               vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", 
> ecx,
> data);
>               break;
>       case MSR_IA32_SPEC_CTRL:
> +             if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> +                 (!msr_info->host_initiated &&
> +                          !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
> +                     return 1;
>               svm->spec_ctrl = data;
>               break;
>       case MSR_IA32_APICBASE:
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 49b4a2d61603..42bc7ee293e4 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3368,6 +3368,10 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
>               msr_info->data = guest_read_tsc(vcpu);
>               break;
>       case MSR_IA32_SPEC_CTRL:
> +             if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> +                 (!msr_info->host_initiated &&
> +                  !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
> +                     return 1;
>               msr_info->data = to_vmx(vcpu)->spec_ctrl;
>               break;
>       case MSR_IA32_SYSENTER_CS:
> @@ -3510,6 +3514,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
>               kvm_write_tsc(vcpu, msr_info);
>               break;
>       case MSR_IA32_SPEC_CTRL:
> +             if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> +                 (!msr_info->host_initiated &&
> +                  !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
> +                     return 1;
>               to_vmx(vcpu)->spec_ctrl = data;
>               break;
>       case MSR_IA32_CR_PAT:
> -- 
> 1.8.3.1

Reviewed-by: Liran Alon <liran.a...@oracle.com>

The only thing I a bit dislike is that currently these MSRs are always 
pass-through to guest and therefore
there is no case vmx_set_msr() is called with !msr_info->host_initiated.
Don't you think we should BUG_ON(!msr_info->host_initiated)?

-Liran

Reply via email to