On Tue, Dec 20, 2016 at 9:47 AM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote:
> 2016-12-20 16:24+0100, Radim Krčmář:
>> 2016-12-19 19:15-0800, Kyle Huey:
>> > On Mon, Dec 19, 2016 at 12:48 PM, David Matlack <dmatlack@xxxxxxxxxx> wrote:
>> >> kvm_skip_emulated_instruction() should not be called after emulating
>> >> a VM-entry failure during or after loading guest state
>> >> (nested_vmx_entry_failure()). Otherwise the L1 hypervisor is resumed
>> >> some number of bytes past vmcs->host_rip.
>> >
>> > Ah, I see. Sorry for that regression.
>> >
>> > These paths are supposed to trigger TF-induced singlestep exceptions
>> > though. Quoting from the Intel SDM (Vol 3, Chapter 26)
>> >
>> > "EFLAGS.TF = 1 causes a VM-entry instruction to generate a single-step
>> > debug exception only if failure of one of the checks in Section 26.1
>> > and Section 26.2 causes control to pass to the following instruction.
>> > A VM-entry does not generate a single-step debug exception in any of
>> > the following cases: (1) the instruction generates a fault; (2)
>> > failure of one of the checks in Section 26.3 or in loading MSRs causes
>> > processor state to be loaded from the hoststate area of the VMCS; or
>> > (3) the instruction passes all checks in Section 26.1, Section 26.2,
>> > and Section 26.3 and there is no failure in loading MSRs"
>>
>> Changed cases are in section 26.3 => not generating #DB is correct,
>>
>> Reviewed-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>>
>> Sorry for missing that while applying, I wonder if there is a reason why
>> we didn't check them after enter_guest_mode() ...
I think this would be a nice cleanup.
>>
>> >> @@ -10517,7 +10517,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
>> >> ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
>> >> nested_vmx_entry_failure(vcpu, vmcs12,
>> >> EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
>> >> - goto out;
>> >> + return 1;
>
> My attention is in shambles these days ... this one looks like it is in
> section 26.2.2 and should therefore be
>
> nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
> goto out;
I agree. Good catch.
>
> Not a problem of this patch though, I'll go for a minor refactoring
> after a beer.
--
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
