It looks like this was fixed as part of commit d4667ca142610 ("Merge
branch 'x86-pti-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip"). Until then,
it was possible to leak the control from L0's desires.
On Thu, May 10, 2018 at 9:39 AM, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> Hi Radim,
>
> You're right: this commit is unnecessary. I do think it's helpful,
> though, after initializing exec_control to "L0's desires," to
> explicitly clear those desires which are not relevant.
>
> I like your version.
>
>
>
> On Thu, May 10, 2018 at 8:46 AM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote:
>> 2018-05-07 10:55-0700, Jim Mattson:
>>> If vmcs12 doesn't specify the "use MSR bitmaps" VM-execution control,
>>> then vmcs02 should not specify this control either. When the MSR
>>> bitmaps are not used, all executions of RDMSR and WRMSR cause
>>> VM-exits.
>>
>> We already clear it at the end of nested_get_vmcs12_pages() in this
>> case.
>>
>> I don't think that adding it here improves readability of the code.
>> Maybe if we made sure that vmcs02 always begins with disabled MSR
>> bitmaps and only set it afterwards, i.e.
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c7668806163f..3938573cfb19 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -10435,9 +10435,6 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
>> if (nested_vmx_prepare_msr_bitmap(vcpu, vmcs12))
>> vmcs_set_bits(CPU_BASED_VM_EXEC_CONTROL,
>> CPU_BASED_USE_MSR_BITMAPS);
>> - else
>> - vmcs_clear_bits(CPU_BASED_VM_EXEC_CONTROL,
>> - CPU_BASED_USE_MSR_BITMAPS);
>> }
>>
>> static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
>> @@ -11139,6 +11136,9 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>> exec_control &= ~CPU_BASED_USE_IO_BITMAPS;
>> exec_control |= CPU_BASED_UNCOND_IO_EXITING;
>>
>> + /* MSR bitmaps are potentially enabled after recomputing the bitmap. */
>> + exec_control &= ~CPU_BASED_USE_MSR_BITMAPS;
>> +
>> vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, exec_control);
>>
>> /* EXCEPTION_BITMAP and CR0_GUEST_HOST_MASK should basically be the
