[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/i386: do not set unsupported VMX secondary execution
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH] target/i386: do not set unsupported VMX secondary execution controls |
Date: |
Tue, 31 Mar 2020 18:32:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 31/03/20 18:27, Vitaly Kuznetsov wrote:
> Commit 048c95163b4 ("target/i386: work around KVM_GET_MSRS bug for
> secondary execution controls") added a workaround for KVM pre-dating
> commit 6defc591846d ("KVM: nVMX: include conditional controls in /dev/kvm
> KVM_GET_MSRS") which wasn't setting certain available controls. The
> workaround uses generic CPUID feature bits to set missing VMX controls.
>
> It was found that in some cases it is possible to observe hosts which
> have certain CPUID features but lack the corresponding VMX control.
>
> In particular, it was reported that Azure VMs have RDSEED but lack
> VMX_SECONDARY_EXEC_RDSEED_EXITING; attempts to enable this feature
> bit result in QEMU abort.
>
> Resolve the issue but not applying the workaround when we don't have
> to. As there is no good way to find out if KVM has the fix itself, use
> 95c5c7c77c ("KVM: nVMX: list VMX MSRs in KVM_GET_MSR_INDEX_LIST") instead
> as these [are supposed to] come together.
>
> Fixes: 048c95163b4 ("target/i386: work around KVM_GET_MSRS bug for secondary
> execution controls")
> Suggested-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Vitaly Kuznetsov <address@hidden>
Queued, thanks.
Paolo
> ---
> target/i386/kvm.c | 41 ++++++++++++++++++++++++++---------------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 69eb43d796e6..4901c6dd747d 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -106,6 +106,7 @@ static bool has_msr_arch_capabs;
> static bool has_msr_core_capabs;
> static bool has_msr_vmx_vmfunc;
> static bool has_msr_ucode_rev;
> +static bool has_msr_vmx_procbased_ctls2;
>
> static uint32_t has_architectural_pmu_version;
> static uint32_t num_architectural_pmu_gp_counters;
> @@ -490,21 +491,28 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState
> *s, uint32_t index)
> value = msr_data.entries[0].data;
> switch (index) {
> case MSR_IA32_VMX_PROCBASED_CTLS2:
> - /* KVM forgot to add these bits for some time, do this ourselves. */
> - if (kvm_arch_get_supported_cpuid(s, 0xD, 1, R_ECX) &
> CPUID_XSAVE_XSAVES) {
> - value |= (uint64_t)VMX_SECONDARY_EXEC_XSAVES << 32;
> - }
> - if (kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX) & CPUID_EXT_RDRAND)
> {
> - value |= (uint64_t)VMX_SECONDARY_EXEC_RDRAND_EXITING << 32;
> - }
> - if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) &
> CPUID_7_0_EBX_INVPCID) {
> - value |= (uint64_t)VMX_SECONDARY_EXEC_ENABLE_INVPCID << 32;
> - }
> - if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) &
> CPUID_7_0_EBX_RDSEED) {
> - value |= (uint64_t)VMX_SECONDARY_EXEC_RDSEED_EXITING << 32;
> - }
> - if (kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX) &
> CPUID_EXT2_RDTSCP) {
> - value |= (uint64_t)VMX_SECONDARY_EXEC_RDTSCP << 32;
> + if (!has_msr_vmx_procbased_ctls2) {
> + /* KVM forgot to add these bits for some time, do this
> ourselves. */
> + if (kvm_arch_get_supported_cpuid(s, 0xD, 1, R_ECX) &
> + CPUID_XSAVE_XSAVES) {
> + value |= (uint64_t)VMX_SECONDARY_EXEC_XSAVES << 32;
> + }
> + if (kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX) &
> + CPUID_EXT_RDRAND) {
> + value |= (uint64_t)VMX_SECONDARY_EXEC_RDRAND_EXITING << 32;
> + }
> + if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) &
> + CPUID_7_0_EBX_INVPCID) {
> + value |= (uint64_t)VMX_SECONDARY_EXEC_ENABLE_INVPCID << 32;
> + }
> + if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) &
> + CPUID_7_0_EBX_RDSEED) {
> + value |= (uint64_t)VMX_SECONDARY_EXEC_RDSEED_EXITING << 32;
> + }
> + if (kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX) &
> + CPUID_EXT2_RDTSCP) {
> + value |= (uint64_t)VMX_SECONDARY_EXEC_RDTSCP << 32;
> + }
> }
> /* fall through */
> case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> @@ -2060,6 +2068,9 @@ static int kvm_get_supported_msrs(KVMState *s)
> case MSR_IA32_UCODE_REV:
> has_msr_ucode_rev = true;
> break;
> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> + has_msr_vmx_procbased_ctls2 = true;
> + break;
> }
> }
> }
>