linux-kernel - Re: [PATCH] KVM: vmx: Set msr bitmap correctly if vcpu is in guest mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACzj_yU=pN=8hf2oqmNp0ONggpRec9P6FOF5oa-Pfj_JnBOqOA@mail.gmail.com>
Date:	Wed, 4 Mar 2015 11:27:42 +0800
From:	Wincy Van <fanwenyi0529@...il.com>
To:	Bandan Das <bsd@...hat.com>
Cc:	mtosatti@...hat.com, Paolo Bonzini <pbonzini@...hat.com>,
	"gleb@...nel.org" <gleb@...nel.org>,
	"Zhang, Yang Z" <yang.z.zhang@...el.com>,
	Wanpeng Li <wanpeng.li@...ux.intel.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jan Kiszka <jan.kiszka@....de>,
	Yong Wang <yong.y.wang@...ux.intel.com>
Subject: Re: [PATCH] KVM: vmx: Set msr bitmap correctly if vcpu is in guest mode

On Wed, Mar 4, 2015 at 1:39 AM, Bandan Das <bsd@...hat.com> wrote:
> Wincy Van <fanwenyi0529@...il.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@...il.com>
>> ---
>>  arch/x86/kvm/vmx.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index f7b20b4..f6e3457 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)) {
>
> So, we end up writing the MSR_BITMAP field twice - once when we
> call nested_vmx_merge_msr_bitmap() and another here. Why don't we just
> remove the former since prepare_vmcs02 will call vmx_set_efer anyway ?
>

Yes, setting MSR_BITMAP twice is redundant, but we can not rely on
vmx_set_efer to set that field, this is not vmx_set_efer 's duty.
Consider that someone wants to make some changes on loading
L2's efer, he may be confused about this. We should reduce the
degree of code coupling.

Thanks,
Wincy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/