|
From: | Montes, Julio |
Subject: | Re: [PATCH] target/i386: do not set unsupported VMX secondary execution controls |
Date: | Tue, 31 Mar 2020 16:56:09 +0000 |
Hi Vitaly
thanks for raising this, unfortunately this patch didn't work for me, I still get the same error:
my qemu command line:
From: Qemu-devel <qemu-devel-bounces+julio.montes=address@hidden> on behalf of Paolo Bonzini <address@hidden>
Sent: Tuesday, March 31, 2020 10:32 AM To: Vitaly Kuznetsov <address@hidden>; address@hidden <address@hidden> Cc: Marcelo Tosatti <address@hidden>; Eduardo Habkost <address@hidden>; Dr . David Alan Gilbert <address@hidden>; Richard Henderson <address@hidden> Subject: Re: [PATCH] target/i386: do not set unsupported VMX secondary execution controls 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; > } > } > } > |
[Prev in Thread] | Current Thread | [Next in Thread] |