Remove the superfluous flush of the current TLB in VMX's handling of
migration of the APIC-access page, as a full TLB flush on all vCPUs will
have already been performed in response to kvm_unmap_gfn_range() *if*
there were SPTEs pointing at the APIC-access page. And if there were no
valid SPTEs, then there can't possibly be TLB entries to flush.
The extra flush was added by commit fb6c81984313 ("kvm: vmx: Flush TLB
when the APIC-access address changes"), with the justification of "because
the SDM says so". The SDM said, and still says:
As detailed in Section xx.x.x, an access to the APIC-access page might
not cause an APIC-access VM exit if software does not properly invalidate
information that may be cached from the EPT paging structures. If EPT was
in use on a logical processor at one time with EPTP X, it is recommended
that software use the INVEPT instruction with the “single-context” INVEPT
type and with EPTP X in the INVEPT descriptor before a VM entry on the
same logical processor that enables EPT with EPTP X and either (a) the
"virtualize APIC accesses" VM- execution control was changed from 0 to 1;
or (b) the value of the APIC-access address was changed.
But the "recommendation" for (b) is predicated on there actually being
a valid EPT translation *and* possible TLB entries for the GPA (or guest
VA when using shadow paging). It's possible that a different vCPU has
established a mapping for the new page, but the current vCPU can't have
entered the guest, i.e. can't have created a TLB entry, between flushing
the old mappings and changing its vmcs.APIC_ACCESS_ADDR.
kvm_unmap_gfn_range() waits for all vCPUs to ack KVM_REQ_APIC_PAGE_RELOAD,
and then flushes remote TLBs (which may or may not also pend a request).
Thus the vCPU is guaranteed to update vmcs.APIC_ACCESS_ADDR before
re-entering the guest and before it can possibly create new TLB entries.
In other words, KVM does flush in this case, it just does so earlier
on while handling the page migration.
Note, VMX also flushes if the vCPU is migrated to a new pCPU, i.e. if
the vCPU is migrated to a pCPU that entered the guest for a different
vCPU.
Suggested-by: Yu Zhang <[email protected]>
Cc: Jim Mattson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/vmx/vmx.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 0ecf4be2c6af..3f868826db7d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6767,8 +6767,10 @@ static void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
vmcs_write64(APIC_ACCESS_ADDR, pfn_to_hpa(pfn));
read_unlock(&vcpu->kvm->mmu_lock);
- vmx_flush_tlb_current(vcpu);
-
+ /*
+ * No need for a manual TLB flush at this point, KVM has already done a
+ * flush if there were SPTEs pointing at the previous page.
+ */
out:
/*
* Do not pin apic access page in memory, the MMU notifier
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
--
2.41.0.487.g6d72f3e995-goog
On Fri, Jul 21, 2023 at 04:38:58PM -0700, Sean Christopherson wrote:
> Remove the superfluous flush of the current TLB in VMX's handling of
> migration of the APIC-access page, as a full TLB flush on all vCPUs will
> have already been performed in response to kvm_unmap_gfn_range() *if*
> there were SPTEs pointing at the APIC-access page. And if there were no
> valid SPTEs, then there can't possibly be TLB entries to flush.
>
> The extra flush was added by commit fb6c81984313 ("kvm: vmx: Flush TLB
> when the APIC-access address changes"), with the justification of "because
> the SDM says so". The SDM said, and still says:
>
> As detailed in Section xx.x.x, an access to the APIC-access page might
> not cause an APIC-access VM exit if software does not properly invalidate
> information that may be cached from the EPT paging structures. If EPT was
> in use on a logical processor at one time with EPTP X, it is recommended
> that software use the INVEPT instruction with the “single-context” INVEPT
> type and with EPTP X in the INVEPT descriptor before a VM entry on the
> same logical processor that enables EPT with EPTP X and either (a) the
> "virtualize APIC accesses" VM- execution control was changed from 0 to 1;
> or (b) the value of the APIC-access address was changed.
>
> But the "recommendation" for (b) is predicated on there actually being
> a valid EPT translation *and* possible TLB entries for the GPA (or guest
> VA when using shadow paging). It's possible that a different vCPU has
> established a mapping for the new page, but the current vCPU can't have
> entered the guest, i.e. can't have created a TLB entry, between flushing
> the old mappings and changing its vmcs.APIC_ACCESS_ADDR.
>
> kvm_unmap_gfn_range() waits for all vCPUs to ack KVM_REQ_APIC_PAGE_RELOAD,
> and then flushes remote TLBs (which may or may not also pend a request).
> Thus the vCPU is guaranteed to update vmcs.APIC_ACCESS_ADDR before
> re-entering the guest and before it can possibly create new TLB entries.
>
> In other words, KVM does flush in this case, it just does so earlier
> on while handling the page migration.
>
> Note, VMX also flushes if the vCPU is migrated to a new pCPU, i.e. if
> the vCPU is migrated to a pCPU that entered the guest for a different
> vCPU.
>
> Suggested-by: Yu Zhang <[email protected]>
> Cc: Jim Mattson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/vmx/vmx.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 0ecf4be2c6af..3f868826db7d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6767,8 +6767,10 @@ static void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
> vmcs_write64(APIC_ACCESS_ADDR, pfn_to_hpa(pfn));
> read_unlock(&vcpu->kvm->mmu_lock);
>
> - vmx_flush_tlb_current(vcpu);
> -
> + /*
> + * No need for a manual TLB flush at this point, KVM has already done a
> + * flush if there were SPTEs pointing at the previous page.
> + */
> out:
> /*
> * Do not pin apic access page in memory, the MMU notifier
>
Reviewed-by: Yu Zhang <[email protected]>
> In other words, KVM does flush in this case, it just does so earlier
> on while handling the page migration.
Reviewed-by: Paolo Bonzini <[email protected]>
but not 6.5 material, so I'm leaving this to you.
Paolo
On Fri, 21 Jul 2023 16:38:58 -0700, Sean Christopherson wrote:
> Remove the superfluous flush of the current TLB in VMX's handling of
> migration of the APIC-access page, as a full TLB flush on all vCPUs will
> have already been performed in response to kvm_unmap_gfn_range() *if*
> there were SPTEs pointing at the APIC-access page. And if there were no
> valid SPTEs, then there can't possibly be TLB entries to flush.
>
> The extra flush was added by commit fb6c81984313 ("kvm: vmx: Flush TLB
> when the APIC-access address changes"), with the justification of "because
> the SDM says so". The SDM said, and still says:
>
> [...]
Applied to kvm-x86 vmx, thanks!
[1/1] KVM: VMX: Drop manual TLB flush when migrating vmcs.APIC_ACCESS_ADDR
https://github.com/kvm-x86/linux/commit/775bc098657b
--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes