target/i386/kvm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Commit 20a78b02d315 ("target/i386: add VMX features") unconditionally
add vmx msr entry although older host kernels don't include them.
But old host kernel + newest qemu will cause a qemu crash as follows:
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 relaxing the condition.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
target/i386/kvm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index bf16556..a8c44bf 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2936,7 +2936,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
(uint32_t)e->index, (uint64_t)e->data);
}
- assert(ret == cpu->kvm_msr_buf->nmsrs);
+ assert(ret <= cpu->kvm_msr_buf->nmsrs);
return 0;
}
--
1.7.1
On 04/12/19 09:50, Catherine Ho wrote: > Commit 20a78b02d315 ("target/i386: add VMX features") unconditionally > add vmx msr entry although older host kernels don't include them. > > But old host kernel + newest qemu will cause a qemu crash as follows: > 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 relaxing the condition. This is intentional. The VMX MSR entries should not have been added. What combination of host kernel/QEMU are you using, and what QEMU command line? Paolo
Hi Paolo [sorry to resend it, seems to reply it incorrectly] On Wed, 4 Dec 2019 at 19:23, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 04/12/19 09:50, Catherine Ho wrote: > > Commit 20a78b02d315 ("target/i386: add VMX features") unconditionally > > add vmx msr entry although older host kernels don't include them. > > > > But old host kernel + newest qemu will cause a qemu crash as follows: > > 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 relaxing the condition. > > This is intentional. The VMX MSR entries should not have been added. > What combination of host kernel/QEMU are you using, and what QEMU > command line? > > > Host kernel: 4.15.0 (ubuntu 18.04) Qemu: https://gitlab.com/virtio-fs/qemu/tree/virtio-fs-dev cmdline: qemu-system-x86_64 -M pc -cpu host --enable-kvm -smp 8 \ -m 4G,maxmem=4G But before 20a78b02d315, the older kernel + latest qemu can boot guest successfully. Best Regards, Catherine
On 04/12/19 14:33, Catherine Ho wrote: > Hi Paolo > [sorry to resend it, seems to reply it incorrectly] > > On Wed, 4 Dec 2019 at 19:23, Paolo Bonzini <pbonzini@redhat.com > <mailto:pbonzini@redhat.com>> wrote: > > On 04/12/19 09:50, Catherine Ho wrote: > > Commit 20a78b02d315 ("target/i386: add VMX features") unconditionally > > add vmx msr entry although older host kernels don't include them. > > > > But old host kernel + newest qemu will cause a qemu crash as follows: > > 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 relaxing the condition. > > This is intentional. The VMX MSR entries should not have been added. > What combination of host kernel/QEMU are you using, and what QEMU > command line? > > > Host kernel: 4.15.0 (ubuntu 18.04) > Qemu: https://gitlab.com/virtio-fs/qemu/tree/virtio-fs-dev > cmdline: qemu-system-x86_64 -M pc -cpu host --enable-kvm -smp 8 \ > -m 4G,maxmem=4G > > But before 20a78b02d315, the older kernel + latest qemu can boot guest > successfully. Ok, so the problem is that some MSR didn't exist in that version. Which one it is? Can you make it conditional, similar to MSR_IA32_VMX_VMFUNC? Thanks, Paolo
Hi Paolo On Wed, 4 Dec 2019 at 21:53, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 04/12/19 14:33, Catherine Ho wrote: > > Hi Paolo > > [sorry to resend it, seems to reply it incorrectly] > > > > On Wed, 4 Dec 2019 at 19:23, Paolo Bonzini <pbonzini@redhat.com > > <mailto:pbonzini@redhat.com>> wrote: > > > > On 04/12/19 09:50, Catherine Ho wrote: > > > Commit 20a78b02d315 ("target/i386: add VMX features") unconditionally > > > add vmx msr entry although older host kernels don't include them. > > > > > > But old host kernel + newest qemu will cause a qemu crash as follows: > > > 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 relaxing the condition. > > > > This is intentional. The VMX MSR entries should not have been added. > > What combination of host kernel/QEMU are you using, and what QEMU > > command line? > > > > > > Host kernel: 4.15.0 (ubuntu 18.04) > > Qemu: https://gitlab.com/virtio-fs/qemu/tree/virtio-fs-dev > > cmdline: qemu-system-x86_64 -M pc -cpu host --enable-kvm -smp 8 \ > > -m 4G,maxmem=4G > > > > But before 20a78b02d315, the older kernel + latest qemu can boot guest > > successfully. > > Ok, so the problem is that some MSR didn't exist in that version. Which I thought in my platform, the only MSR didn't exist is MSR_IA32_VMX_BASIC (0x480). If I remove this kvm_msr_entry_add(), everything is ok, the guest can be boot up successfully. > one it is? Can you make it conditional, similar to MSR_IA32_VMX_VMFUNC? Ok, I will. Thanks for the suggestion Best regards Catherine
On 04/12/19 16:07, Catherine Ho wrote: >> Ok, so the problem is that some MSR didn't exist in that version. Which > I thought in my platform, the only MSR didn't exist is MSR_IA32_VMX_BASIC > (0x480). If I remove this kvm_msr_entry_add(), everything is ok, the guest can > be boot up successfully. > MSR_IA32_VMX_BASIC was added in kvm-4.10. Maybe the issue is the _value_ that is being written to the VM is not valid? Can you check what's happening in vmx_restore_vmx_basic? Paolo
On Wed, Dec 04, 2019 at 04:34:45PM +0100, Paolo Bonzini wrote: > On 04/12/19 16:07, Catherine Ho wrote: > >> Ok, so the problem is that some MSR didn't exist in that version. Which > > I thought in my platform, the only MSR didn't exist is MSR_IA32_VMX_BASIC > > (0x480). If I remove this kvm_msr_entry_add(), everything is ok, the guest can > > be boot up successfully. > > > > MSR_IA32_VMX_BASIC was added in kvm-4.10. Maybe the issue is the > _value_ that is being written to the VM is not valid? Can you check > what's happening in vmx_restore_vmx_basic? I believe env->features[FEAT_VMX_BASIC] will be initialized to 0 if the host kernel doesn't have KVM_CAP_GET_MSR_FEATURES. -- Eduardo
On 04/12/19 16:47, Eduardo Habkost wrote: > On Wed, Dec 04, 2019 at 04:34:45PM +0100, Paolo Bonzini wrote: >> On 04/12/19 16:07, Catherine Ho wrote: >>>> Ok, so the problem is that some MSR didn't exist in that version. Which >>> I thought in my platform, the only MSR didn't exist is MSR_IA32_VMX_BASIC >>> (0x480). If I remove this kvm_msr_entry_add(), everything is ok, the guest can >>> be boot up successfully. >>> >> >> MSR_IA32_VMX_BASIC was added in kvm-4.10. Maybe the issue is the >> _value_ that is being written to the VM is not valid? Can you check >> what's happening in vmx_restore_vmx_basic? > > I believe env->features[FEAT_VMX_BASIC] will be initialized to 0 > if the host kernel doesn't have KVM_CAP_GET_MSR_FEATURES. But the host must have MSR features if the MSRs are added: if (kvm_feature_msrs && cpu_has_vmx(env)) { kvm_msr_entry_add_vmx(cpu, env->features); } Looks like feature MSRs were backported to 4.14, but 1389309c811b0c954bf3b591b761d79b1700283d and the previous commit weren't. Paolo
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
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
© 2016 - 2024 Red Hat, Inc.