Re: [RFC PATCH v2 22/25] KVM: x86/mmu: Refactor kvm_mmu_invlpg() to allow skipping the gva flush

[Yosry Ahmed]

> Actually, isn't there a pre-existing over-flush when handling kvm_mmu_invpcid_gva()?
> Oof, and a missed flush?
>
> To fix the over-flush, I think we want this?
>
> diff --git arch/x86/kvm/mmu/mmu.c arch/x86/kvm/mmu/mmu.c
> index 6c13da942bfc..7f3e0eb33b29 100644
> --- arch/x86/kvm/mmu/mmu.c
> +++ arch/x86/kvm/mmu/mmu.c
> @@ -6672,7 +6672,8 @@ void kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_pagewalk *w,
>                 if (is_noncanonical_invlpg_address(addr, vcpu))
>                         return;
>
> -               kvm_x86_call(flush_tlb_gva)(vcpu, addr);
> +               if (roots & KVM_MMU_ROOT_CURRENT)
> +                       kvm_x86_call(flush_tlb_gva)(vcpu, addr);

Hmmm why?

AFAICT, if the gva has a shadow mapping, then the rest of
kvm_mmu_invalidate_addr() will make sure the shadow mapping is
up-to-date and do any necessary TLB flushes in the process. So I am
assuming the flush here is intended to cover some cases where the gva
does not have a shadow mapping. Not sure how this could happen, I
assume we always flush the TLB when a shadow PTE is freed.

If there are cases where we can have TLB entries but no shadow
mappings, then shouldn't we flush regardless of which cached roots
match the target PCID? I assume if none of the cached roots match the
PCID it's not an issue because KVM will create a new root and flush
the TLB before switching into the new PCID.

>
>                 if (tdp_enabled)
>                         return;
>
>
> And that highlights the missed flush: if the PCID isn't the current PCID, then
> flush_tlb_gva() neglects to flush the hardware TLB for the target PCID, which
> could leave a stale entry in the TLB if the guest switches to the new PCID with
> MOV CR3 + X86_CR3_PCID_NOFLUSH.

I also don't follow this part flush_tlb_gva() ends up executing
INVVPID or INVLPGA. In both cases, all PCIDs for the VPID/ASID are
flushed, right?