On Wed, Mar 4, 2015 at 11:58 PM, Bandan Das <bsd@xxxxxxxxxx> wrote: > Hi Wincy, > > Wincy Van <fanwenyi0529@xxxxxxxxx> 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@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html