On 18/03/20 18:26, Sean Christopherson wrote:
>>>
>>> if (!nested_ept)
>>> kvm_mmu_new_cr3(vcpu, cr3, enable_ept ||
>>> nested_cpu_has_vpid(vmcs12));
>>
>> ... which is exactly nested_has_guest_tlb_tag(vcpu). Well, not exactly
>> but it's a bug in your code above. :)
>
> I don't think it's a bug, it's intentionally different. When enable_ept=0,
> nested_has_guest_tlb_tag() returns true if and only if L1 has enabled VPID
> for L2 *and* L2 has been assigned a unique VPID by L0.
>
> For sync purposes, whether or not L2 has been assigned a unique VPID is
> irrelevant. L0 needs to invalidate TLB entries to prevent resuing L1's
> entries (assuming L1 has been assigned a VPID), but L0 doesn't need to sync
> SPTEs because L2 doesn't expect them to be refreshed.
^^
L1
Yes you're right. So I would say keep your code, but we can simplify
the comment. Something like:
/*
* We can skip the TLB flush if we have EPT enabled (because...) and
* also if L1 is using VPID, because then it doesn't expect SPTEs for L2
* to be refreshed.
*
* This is almost the same as nested_has_guest_tlb_tag(vcpu), but here
* we don't care if L2 has been assigned a unique VPID; L1 doesn't know,
* and will nevertheless do INVVPID to avoid reuse of stale page
* table entries.
*/
Nevertheless it's scary in that this is a potential attack vector for
reusing stale L0 SPTEs, so we should make sure it's all properly commented.
Thanks,
Paolo
>> It completely makes sense to use that as the third argument, and while a
>> comment is still needed it will be much smaller.
> Ya, agreed.
>
