> On 25 Jun 2019, at 14:15, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > 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 Oh oops sure. Yes you are right. I forgot about the larger context here for a moment. Sorry for the confusion. :) -Liran
