> On 1 Oct 2018, at 19:49, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> On Mon, Oct 1, 2018 at 5:56 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>> On 06/09/2018 18:06, Jim Mattson wrote:
>>> On Thu, Sep 6, 2018 at 6:32 AM, Liran Alon <liran.alon@xxxxxxxxxx> wrote:
>>>> If CPU use both VPID and EPT, TLB entries populated by CPU are tagged
>>>> with both EPTP and VPID. Therefore, if L1 uses EPT, L2 TLB entries
>>>> are separated from L1 TLB entries by the EPTP tags as vmcs02 use
>>>> EPTP02 while vmcs01 use EPTP01.
>>>>
>>>> Thus, we don't need to make sure that vmcs02->vpid != vmcs01->vpid.
>>>> Therefore, we can just set vmcs02->vpid to vmcs12->vpid.
>>>>
>>>> Reviewed-by: Mihai Carabas <mihai.carabas@xxxxxxxxxx>
>>>> Reviewed-by: Darren Kenny <darren.kenny@xxxxxxxxxx>
>>>> Reviewed-by: Nikita Leshchenko <nikita.leshchenko@xxxxxxxxxx>
>>>> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
>>>
>>> I suggested this back in July, but Paolo didn't like it. I still like it. :-)
>>>
>>> Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
>>
>> The problem with this is still the same as in July, namely that if all
>> guests (at any level) share the VPID space, then L1 can force an
>> invalidation of any VPID (and thus slow down execution of other guests,
>> including siblings of L1) through INVVPID.
Funny fact: This discussion have lead me to look at handle_invvpid() which L0 use to
emulate INVVPID for L1 and it seems to me to contain a serious issue.
All calls to __vmx_flush_tlb() is called with invalidate_gpa set to true which actually results
in L0 executing INVEPT(eptp01) instead of INVVPID(vmx->nested.vpid02).
(Bug was introduced by c2ba05ccfde2 ("KVM: X86: introduce invalidate_gpa argument to tlb flush”)).
I will send a very simple patch to fix this issue :)
In addition, it seems that the patch I have submitted here also lacks changing handle_invvpid()
to execute INVVPID(vmcs12->vpid) instead of INVVPID(vmx->nested.vpid02) in case vmcs12 use EPT.
I can fix this in v2 of this patch if we will decide it should be applied… (See below)
I understand the concern raised here as even a non-malicious L1 guest will likely hurt other guests
performance when executing a non-individual-address type INVVPID.
What we really wish to have here is a new Intel VMX instruction which allows invalidating TLB entries
by given EPTP *and* VPID. Too bad there isn’t such an instruction…
>
> Can't L1 already do this by filling the TLB with its own entries,
> thereby evicting its siblings' entries?
Yes, but I agree that with this change, it is much easier for a L1 to just invalidate combined mappings used by other guests.
So I tend to agree with Paolo that maybe this patch should be dropped…
-Liran
