| Message ID | 1425450716-2216-1-git-send-email-fanwenyi0529@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Wincy, Wincy Van <fanwenyi0529@gmail.com> writes: > In commit 3af18d9c5fe9 ("KVM: nVMX: Prepare for using hardware MSR bitmap"), > we are setting MSR_BITMAP in prepare_vmcs02 if we should use hardware. This > is not enough since the field will be modified by following vmx_set_efer. > > Fix this by setting vmx_msr_bitmap_nested in vmx_set_msr_bitmap if vcpu is > in guest mode. > > Signed-off-by: Wincy Van <fanwenyi0529@gmail.com> > --- > arch/x86/kvm/vmx.c | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index f7b20b4..10a481b 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2168,7 +2168,10 @@ static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu) > { > unsigned long *msr_bitmap; > > - if (irqchip_in_kernel(vcpu->kvm) && apic_x2apic_mode(vcpu->arch.apic)) { > + if (is_guest_mode(vcpu)) > + msr_bitmap = vmx_msr_bitmap_nested; Do you think this should be (is_guest_mode(vcpu) & (exec_control & CPU_BASED_USE_MSR_BITMAPS)) ? > + else if (irqchip_in_kernel(vcpu->kvm) && > + apic_x2apic_mode(vcpu->arch.apic)) { > if (is_long_mode(vcpu)) > msr_bitmap = vmx_msr_bitmap_longmode_x2apic; > else > @@ -9218,9 +9221,9 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) > } > > if (cpu_has_vmx_msr_bitmap() && > - exec_control & CPU_BASED_USE_MSR_BITMAPS && > - nested_vmx_merge_msr_bitmap(vcpu, vmcs12)) { > - vmcs_write64(MSR_BITMAP, __pa(vmx_msr_bitmap_nested)); > + exec_control & CPU_BASED_USE_MSR_BITMAPS) { > + nested_vmx_merge_msr_bitmap(vcpu, vmcs12); > + /* MSR_BITMAP will be set by following vmx_set_efer. */ > } else > exec_control &= ~CPU_BASED_USE_MSR_BITMAPS; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 4, 2015 at 11:58 PM, Bandan Das <bsd@redhat.com> wrote: > Hi Wincy, > > Wincy Van <fanwenyi0529@gmail.com> writes: > >> In commit 3af18d9c5fe9 ("KVM: nVMX: Prepare for using hardware MSR bitmap"), >> we are setting MSR_BITMAP in prepare_vmcs02 if we should use hardware. This >> is not enough since the field will be modified by following vmx_set_efer. >> >> Fix this by setting vmx_msr_bitmap_nested in vmx_set_msr_bitmap if vcpu is >> in guest mode. >> >> Signed-off-by: Wincy Van <fanwenyi0529@gmail.com> >> --- >> arch/x86/kvm/vmx.c | 11 +++++++---- >> 1 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index f7b20b4..10a481b 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2168,7 +2168,10 @@ static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu) >> { >> unsigned long *msr_bitmap; >> >> - if (irqchip_in_kernel(vcpu->kvm) && apic_x2apic_mode(vcpu->arch.apic)) { >> + if (is_guest_mode(vcpu)) >> + msr_bitmap = vmx_msr_bitmap_nested; > Do you think this should be (is_guest_mode(vcpu) & > (exec_control & CPU_BASED_USE_MSR_BITMAPS)) ? > We don't need to do that, because if we don't use the hardware, we will disable hardware msr bitmap: if (cpu_has_vmx_msr_bitmap() && exec_control & CPU_BASED_USE_MSR_BITMAPS) { nested_vmx_merge_msr_bitmap(vcpu, vmcs12); /* MSR_BITMAP will be set by following vmx_set_efer. */ } else exec_control &= ~CPU_BASED_USE_MSR_BITMAPS; Then, the hardware will ignore MSR_BITMAP. Setting MSR_BITMAP without enabling it may be unnecessary, but it is no harm, and we can reduce the code complexity of vmx_set_msr_bitmap. Thanks, Wincy -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 04, 2015 at 02:31:56PM +0800, Wincy Van wrote: > In commit 3af18d9c5fe9 ("KVM: nVMX: Prepare for using hardware MSR bitmap"), > we are setting MSR_BITMAP in prepare_vmcs02 if we should use hardware. This > is not enough since the field will be modified by following vmx_set_efer. > > Fix this by setting vmx_msr_bitmap_nested in vmx_set_msr_bitmap if vcpu is > in guest mode. > > Signed-off-by: Wincy Van <fanwenyi0529@gmail.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index f7b20b4..10a481b 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2168,7 +2168,10 @@ static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu) { unsigned long *msr_bitmap; - if (irqchip_in_kernel(vcpu->kvm) && apic_x2apic_mode(vcpu->arch.apic)) { + if (is_guest_mode(vcpu)) + msr_bitmap = vmx_msr_bitmap_nested; + else if (irqchip_in_kernel(vcpu->kvm) && + apic_x2apic_mode(vcpu->arch.apic)) { if (is_long_mode(vcpu)) msr_bitmap = vmx_msr_bitmap_longmode_x2apic; else @@ -9218,9 +9221,9 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) } if (cpu_has_vmx_msr_bitmap() && - exec_control & CPU_BASED_USE_MSR_BITMAPS && - nested_vmx_merge_msr_bitmap(vcpu, vmcs12)) { - vmcs_write64(MSR_BITMAP, __pa(vmx_msr_bitmap_nested)); + exec_control & CPU_BASED_USE_MSR_BITMAPS) { + nested_vmx_merge_msr_bitmap(vcpu, vmcs12); + /* MSR_BITMAP will be set by following vmx_set_efer. */ } else exec_control &= ~CPU_BASED_USE_MSR_BITMAPS;
In commit 3af18d9c5fe9 ("KVM: nVMX: Prepare for using hardware MSR bitmap"), we are setting MSR_BITMAP in prepare_vmcs02 if we should use hardware. This is not enough since the field will be modified by following vmx_set_efer. Fix this by setting vmx_msr_bitmap_nested in vmx_set_msr_bitmap if vcpu is in guest mode. Signed-off-by: Wincy Van <fanwenyi0529@gmail.com> --- arch/x86/kvm/vmx.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)