> On 7 Oct 2018, at 23:14, Liran Alon <liran.alon@xxxxxxxxxx> wrote:
>
>
>
>> 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
I have an idea on how to preserve the wanted functionality of this patch:
We could modify handle_invvpid() to emulate INVVPID of all types besides individual-address by executing INVEPT(eptp02) when vmcs12 uses EPT.
This will prevent L1 from invalidating TLB entries of other guests by executing INVVPID.
This will also result in every INVVPID of non-individual-address type to invalidate all L2 TLB entries of given eptp02 of all various L2 VPIDs.
However, it is not different than today in that regard, as today all L2 TLB entries are tagged with a single vmx->nested.vpid02.
Paolo, if you agree to this, I will create relevant patches for v2 of this series.
-Liran
