On 09/03/2018 19:37, Radim Krčmář wrote:
> 2018-03-05 09:33-0800, Sean Christopherson:
>> Segment registers must be synchronized prior to any code that may
>> trigger a call to emulation_required()/guest_state_valid(), e.g.
>> vmx_set_cr0(). Because preparing vmcs02 writes segmentation fields
>> directly, i.e. doesn't use vmx_set_segment(), emulation_required
>> will not be re-evaluated when synchronizing the segment registers,
>> which can result in L0 incorrectly starting emulation of L2.
>>
>> Fixes: 8665c3f97320 ("KVM: nVMX: initialize descriptor cache fields in prepare_vmcs02_full")
>>
>> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>> ---
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> @@ -10563,11 +10563,8 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>> return 0;
>> }
>>
>> -static void prepare_vmcs02_full(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>> - bool from_vmentry)
>
> Nice catch!
>
> Paolo, was there a reason to that prevented prepare_vmcs02_full at the
> beginning of prepare_vmcs02?
No, I placed it there because initially I wanted to move more work to
prepare_vmcs02_full (such as entry/exit controls, CR0/CR4/EFER, etc.),
but that didn't quite work out.
Sean, do you have a test case for this bug? (And if it can be fixed by
moving prepare_vmcs02_full at the beginning, as suggested by Radim, I'm
all for it).
Paolo
