| Message ID | 1575627817-24625-1-git-send-email-catherine.hecx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/i386: skip kvm_msr_entry_add when kvm_vmx_basic is 0 | expand |
Hi Paolo and Eduardo I digged into the put msr assertion bug a little more, and seems I found the root cause. Please have a review. Best regards. Catherine On Fri, 6 Dec 2019 at 18:25, Catherine Ho <catherine.hecx@gmail.com> wrote: > > Commit 1389309c811b ("KVM: nVMX: expose VMX capabilities for nested > hypervisors to userspace") expands the msr_based_features with > MSR_IA32_VMX_BASIC and others. Then together with an old kernel before > 1389309c811b, the qemu call KVM_GET_MSR_FEATURE_INDEX_LIST and got the > smaller kvm_feature_msrs. Then in kvm_arch_get_supported_msr_feature(), > searching VMX_BASIC will be failed and return 0. At last kvm_vmx_basic > will be assigned to 0. > > Without this patch, it will cause a qemu crash (host kernel 4.15 > ubuntu 18.04+qemu 4.1): > qemu-system-x86_64: error: failed to set MSR 0x480 to 0x0 > target/i386/kvm.c:2932: kvm_put_msrs: Assertion `ret == > cpu->kvm_msr_buf->nmsrs' failed. > > This fixes it by skipping kvm_msr_entry_add when kvm_vmx_basic is 0 > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com> > --- > target/i386/kvm.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index a8c44bf..8cf84a2 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -2632,8 +2632,13 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f) > f[FEAT_VMX_SECONDARY_CTLS])); > kvm_msr_entry_add(cpu, MSR_IA32_VMX_EPT_VPID_CAP, > f[FEAT_VMX_EPT_VPID_CAPS] | fixed_vmx_ept_vpid); > - kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, > + > + if (kvm_vmx_basic) { > + /* Only add the entry when host supports it */ > + kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, > f[FEAT_VMX_BASIC] | fixed_vmx_basic); > + } > + > kvm_msr_entry_add(cpu, MSR_IA32_VMX_MISC, > f[FEAT_VMX_MISC] | fixed_vmx_misc); > if (has_msr_vmx_vmfunc) { > -- > 1.7.1 >
On 06/12/19 11:23, Catherine Ho wrote: > Commit 1389309c811b ("KVM: nVMX: expose VMX capabilities for nested > hypervisors to userspace") expands the msr_based_features with > MSR_IA32_VMX_BASIC and others. Then together with an old kernel before > 1389309c811b, the qemu call KVM_GET_MSR_FEATURE_INDEX_LIST and got the > smaller kvm_feature_msrs. Then in kvm_arch_get_supported_msr_feature(), > searching VMX_BASIC will be failed and return 0. At last kvm_vmx_basic > will be assigned to 0. > > Without this patch, it will cause a qemu crash (host kernel 4.15 > ubuntu 18.04+qemu 4.1): > qemu-system-x86_64: error: failed to set MSR 0x480 to 0x0 > target/i386/kvm.c:2932: kvm_put_msrs: Assertion `ret == > cpu->kvm_msr_buf->nmsrs' failed. > > This fixes it by skipping kvm_msr_entry_add when kvm_vmx_basic is 0 > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com> > --- > target/i386/kvm.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index a8c44bf..8cf84a2 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -2632,8 +2632,13 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f) > f[FEAT_VMX_SECONDARY_CTLS])); > kvm_msr_entry_add(cpu, MSR_IA32_VMX_EPT_VPID_CAP, > f[FEAT_VMX_EPT_VPID_CAPS] | fixed_vmx_ept_vpid); > - kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, > + > + if (kvm_vmx_basic) { > + /* Only add the entry when host supports it */ > + kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, > f[FEAT_VMX_BASIC] | fixed_vmx_basic); > + } > + > kvm_msr_entry_add(cpu, MSR_IA32_VMX_MISC, > f[FEAT_VMX_MISC] | fixed_vmx_misc); > if (has_msr_vmx_vmfunc) { > Yang Zhong from Intel also sent a similar patch. Thanks very much to both of you. Paolo
Patchew URL: https://patchew.org/QEMU/1575627817-24625-1-git-send-email-catherine.hecx@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] target/i386: skip kvm_msr_entry_add when kvm_vmx_basic is 0 Type: series Message-id: 1575627817-24625-1-git-send-email-catherine.hecx@gmail.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 987441f target/i386: skip kvm_msr_entry_add when kvm_vmx_basic is 0 === OUTPUT BEGIN === ERROR: code indent should never use tabs #38: FILE: target/i386/kvm.c:2637: +^I/* Only add the entry when host supports it */$ total: 1 errors, 0 warnings, 14 lines checked Commit 987441f7424c (target/i386: skip kvm_msr_entry_add when kvm_vmx_basic is 0) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/1575627817-24625-1-git-send-email-catherine.hecx@gmail.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
diff --git a/target/i386/kvm.c b/target/i386/kvm.c index a8c44bf..8cf84a2 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -2632,8 +2632,13 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f) f[FEAT_VMX_SECONDARY_CTLS])); kvm_msr_entry_add(cpu, MSR_IA32_VMX_EPT_VPID_CAP, f[FEAT_VMX_EPT_VPID_CAPS] | fixed_vmx_ept_vpid); - kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, + + if (kvm_vmx_basic) { + /* Only add the entry when host supports it */ + kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC, f[FEAT_VMX_BASIC] | fixed_vmx_basic); + } + kvm_msr_entry_add(cpu, MSR_IA32_VMX_MISC, f[FEAT_VMX_MISC] | fixed_vmx_misc); if (has_msr_vmx_vmfunc) {
Commit 1389309c811b ("KVM: nVMX: expose VMX capabilities for nested hypervisors to userspace") expands the msr_based_features with MSR_IA32_VMX_BASIC and others. Then together with an old kernel before 1389309c811b, the qemu call KVM_GET_MSR_FEATURE_INDEX_LIST and got the smaller kvm_feature_msrs. Then in kvm_arch_get_supported_msr_feature(), searching VMX_BASIC will be failed and return 0. At last kvm_vmx_basic will be assigned to 0. Without this patch, it will cause a qemu crash (host kernel 4.15 ubuntu 18.04+qemu 4.1): qemu-system-x86_64: error: failed to set MSR 0x480 to 0x0 target/i386/kvm.c:2932: kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed. This fixes it by skipping kvm_msr_entry_add when kvm_vmx_basic is 0 Cc: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com> --- target/i386/kvm.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)