linux-kernel - Re: [PATCH v3 1/6] KVM: nVMX: Use hardware MSR bitmap

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]

Date:	Wed, 28 Jan 2015 20:45:16 +0800
From:	Wincy Van <fanwenyi0529@...il.com>
To:	"Zhang, Yang Z" <yang.z.zhang@...el.com>
Cc:	Paolo Bonzini <pbonzini@...hat.com>,
	"gleb@...nel.org" <gleb@...nel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Wanpeng Li <wanpeng.li@...ux.intel.com>,
	Jan Kiszka <jan.kiszka@....de>
Subject: Re: [PATCH v3 1/6] KVM: nVMX: Use hardware MSR bitmap

On Wed, Jan 28, 2015 at 8:33 PM, Zhang, Yang Z <yang.z.zhang@...el.com> wrote:
>>>
>>> You are right, but this is not fit for all the cases, we should
>>> custom the nested_msr_bitmap.
>>> e.g.  Currently L0 wants to intercept some of the x2apic msrs' reading:
>>>          if (enable_apicv) {
>>>                 for (msr = 0x800; msr <= 0x8ff; msr++)
>> vmx_disable_intercept_msr_read_x2apic(msr);
>>>                 /* According SDM, in x2apic mode, the whole id reg
>>> is
>> used.
>>>                  * But in KVM, it only use the highest eight bits. Need to
>>>                  * intercept it */
>>>                 vmx_enable_intercept_msr_read_x2apic(0x802); /* TMCCT
>>>                 */ vmx_enable_intercept_msr_read_x2apic(0x839); /* TPR
>>>                 */ vmx_disable_intercept_msr_write_x2apic(0x808); /*
>>> EOI
>> */
>>>                 vmx_disable_intercept_msr_write_x2apic(0x80b); /*
>>>                 SELF-IPI */
>>>                 vmx_disable_intercept_msr_write_x2apic(0x83f);
>>>         }
>>> But L1 may not want this. So I think we are better to deal with the
>>
>> Actually, from L0's point, it is totally unaware of the L2. The only
>> thing L0 aware is that the CPU should follow L0's configuration when
>> VCPU is running. So if L0 wants to trap a msr, then the read operation
>> to this msr should cause vmexit unconditionally no matter who is running(who means L1, L2, L3.....).
>>
>>> msrs seperately, there is not a common way suit for all the cases.
>>> If other features want to intercept a msr in nested entry, they can
>>> put the custom code in nested_vmx_merge_msr_bitmap.
>>
>> Yes, if other features want to do it in 'nested' entry, they can fill
>> nested_vmx_merge_msr_bitmap. But if in non-nested case, it should be
>> our responsibly to handle it correctly, how about add following check:
>>
>> if (type & MSR_TYPE_R && !test_bit(msr, vmcs01_msr_bitmap) &&
>> !test_bit(msr, msr_bitmap_l1 + 0x000 / f))
>>         __clear_bit(msr, msr_bitmap_nested + 0x000 / f);
>
>
> Anyway, this is not necessary for your current patch. We can consider it later if there really have other features will use it.
>

Yep, I know what you mean now, for other msrs which are not forwarded
access by a mechanism like virtual-apic page, we should intercept it
unconditionally. I think we should ensure the msr can be allowed
before call nested_vmx_disable_intercept_for_msr, if L0 want to
intercept it, just do not call nested_vmx_disable_intercept_for_msr.

 !test_bit(msr, vmcs01_msr_bitmap) will introduce a problem that some
of the msrs will be affcted by vmcs01_msr_bitmap, TMCCT and TPR, for
example.
Intercept reading for these msrs is okay, but it is not efficient.

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/