| Message ID | 1518130410-6583-3-git-send-email-karahmed@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 8, 2018 at 2:53 PM, KarimAllah Ahmed <karahmed@amazon.de> wrote: > ... otherwise we will just be running with the L1 MSR BITMAP! > > It does not seem that we ever update the MSR_BITMAP when the nested guest > is running. The only place where we update the MSR_BITMAP field in VMCS is > for the L1 guest! > > Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > arch/x86/kvm/vmx.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 91e3539..f40be10 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -10589,6 +10589,9 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > vmcs_writel(GUEST_GDTR_BASE, vmcs12->guest_gdtr_base); > vmcs_writel(GUEST_IDTR_BASE, vmcs12->guest_idtr_base); > > + if (cpu_has_vmx_msr_bitmap()) > + vmcs_write64(MSR_BITMAP, __pa(vmx->nested.vmcs02.msr_bitmap)); > + This is already in David's "[PATCH 5/9] KVM: VMX: make MSR bitmaps per-VCPU," isn't it? > if (from_vmentry && > (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) { > kvm_set_dr(vcpu, 7, vmcs12->guest_dr7); > -- > 2.7.4 >
On 02/10/2018 12:26 AM, Jim Mattson wrote: > On Thu, Feb 8, 2018 at 2:53 PM, KarimAllah Ahmed <karahmed@amazon.de> wrote: >> ... otherwise we will just be running with the L1 MSR BITMAP! >> >> It does not seem that we ever update the MSR_BITMAP when the nested guest >> is running. The only place where we update the MSR_BITMAP field in VMCS is >> for the L1 guest! >> >> Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Radim Krčmář <rkrcmar@redhat.com> >> Cc: kvm@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> --- >> arch/x86/kvm/vmx.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 91e3539..f40be10 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -10589,6 +10589,9 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, >> vmcs_writel(GUEST_GDTR_BASE, vmcs12->guest_gdtr_base); >> vmcs_writel(GUEST_IDTR_BASE, vmcs12->guest_idtr_base); >> >> + if (cpu_has_vmx_msr_bitmap()) >> + vmcs_write64(MSR_BITMAP, __pa(vmx->nested.vmcs02.msr_bitmap)); >> + > > This is already in David's "[PATCH 5/9] KVM: VMX: make MSR bitmaps > per-VCPU," isn't it? I assume you are referring to this: https://patchwork.kernel.org/patch/10194819/ .. which is now: commit 904e14fb7cb9 ("KVM: VMX: make MSR bitmaps per-VCPU") right? If this is the case, then I do not see where the MSR_BITMAP is being updated here. In fact, would not this be the commit that actually broke it? Now MSR_BITMAP is only set in vmx_vcpu_setup: if (cpu_has_vmx_msr_bitmap()) vmcs_write64(MSR_BITMAP, __pa(vmx->vmcs01.msr_bitmap)); > >> if (from_vmentry && >> (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) { >> kvm_set_dr(vcpu, 7, vmcs12->guest_dr7); >> -- >> 2.7.4 >> > Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
On Fri, Feb 9, 2018 at 3:41 PM, KarimAllah Ahmed <karahmed@amazon.com> wrote: > I assume you are referring to this: > > https://patchwork.kernel.org/patch/10194819/ > > .. which is now: > > commit 904e14fb7cb9 ("KVM: VMX: make MSR bitmaps per-VCPU") > > right? > > If this is the case, then I do not see where the MSR_BITMAP is being > updated here. In fact, would not this be the commit that actually broke > it? I'm referring to <1517938181-15317-6-git-send-email-dwmw@amazon.co.uk>, which has: @@ -10043,6 +9954,9 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) if (kvm_has_tsc_control) decache_tsc_multiplier(vmx); + if (cpu_has_vmx_msr_bitmap()) + vmcs_write64(MSR_BITMAP, __pa(vmx->nested.vmcs02.msr_bitmap)); + if (enable_vpid) { /* * There is no direct mapping between vpid02 and vpid12, the
On 02/10/2018 12:57 AM, Jim Mattson wrote: > On Fri, Feb 9, 2018 at 3:41 PM, KarimAllah Ahmed <karahmed@amazon.com> wrote: > >> I assume you are referring to this: >> >> https://patchwork.kernel.org/patch/10194819/ >> >> .. which is now: >> >> commit 904e14fb7cb9 ("KVM: VMX: make MSR bitmaps per-VCPU") >> >> right? >> >> If this is the case, then I do not see where the MSR_BITMAP is being >> updated here. In fact, would not this be the commit that actually broke >> it? > > I'm referring to > <1517938181-15317-6-git-send-email-dwmw@amazon.co.uk>, which has: > > @@ -10043,6 +9954,9 @@ static void prepare_vmcs02(struct kvm_vcpu > *vcpu, struct vmcs12 *vmcs12) > if (kvm_has_tsc_control) > decache_tsc_multiplier(vmx); > > + if (cpu_has_vmx_msr_bitmap()) > + vmcs_write64(MSR_BITMAP, __pa(vmx->nested.vmcs02.msr_bitmap)); > + > if (enable_vpid) { > /* > * There is no direct mapping between vpid02 and vpid12, the > Ooops, My bad! I must have ingested and old version of this commit that did not have this hunk! Now actually looking at the upstream commit and the backports from David, it is indeed there. Sorry for the noise, please ignore this patch :) Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 91e3539..f40be10 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -10589,6 +10589,9 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, vmcs_writel(GUEST_GDTR_BASE, vmcs12->guest_gdtr_base); vmcs_writel(GUEST_IDTR_BASE, vmcs12->guest_idtr_base); + if (cpu_has_vmx_msr_bitmap()) + vmcs_write64(MSR_BITMAP, __pa(vmx->nested.vmcs02.msr_bitmap)); + if (from_vmentry && (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) { kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
... otherwise we will just be running with the L1 MSR BITMAP! It does not seem that we ever update the MSR_BITMAP when the nested guest is running. The only place where we update the MSR_BITMAP field in VMCS is for the L1 guest! Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Radim Krčmář <rkrcmar@redhat.com> Cc: kvm@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- arch/x86/kvm/vmx.c | 3 +++ 1 file changed, 3 insertions(+)