Liran Alon <liran.alon@xxxxxxxxxx> writes: >> On 25 Jun 2019, at 11:51, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: >> >> Liran Alon <liran.alon@xxxxxxxxxx> writes: >> >>>> On 24 Jun 2019, at 16:30, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: >>>> >>>> >>>> +bool nested_enlightened_vmentry(struct kvm_vcpu *vcpu, u64 *evmptr) >>> >>> I prefer to rename evmptr to evmcs_ptr. I think it’s more readable and sufficiently short. >>> In addition, I think you should return either -1ull or assist_page.current_nested_vmcs. >>> i.e. Don’t return evmcs_ptr by pointer but instead as a return-value >>> and get rid of the bool. >> >> Actually no, sorry, I'm having second thoughts here: in handle_vmclear() >> we don't care about the value of evmcs_ptr, we only want to check that >> enlightened vmentry bit is enabled in assist page. If we switch to >> checking evmcs_ptr against '-1', for example, we will make '-1' a magic >> value which is not in the TLFS. Windows may decide to use it for >> something else - and we will get a hard-to-debug bug again. > > I’m not sure I understand. > You are worried that when guest have setup a valid assist-page and set > enlighten_vmentry to true, > that assist_page.current_nested_vmcs can be -1ull and still be considered a valid eVMCS? > I don't think that's reasonable. No, -1ull is not a valid eVMCS - but this shouldn't change VMCLEAR semantics as VMCLEAR has it's own argument. It's perfectly valid to try to put a eVMCS which was previously used on a different vCPU (and thus which is 'active') to non-active state. The fact that we don't have an active eVMCS on the vCPU doing VMCLEAR shouldn't matter at all. -- Vitaly
