| Message ID | 20221001005915.2041642-25-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: AVIC and local APIC fixes+cleanups | expand |
On Sat, 2022-10-01 at 00:59 +0000, Sean Christopherson wrote: > Inhibit APICv/AVIC if the optimized physical map is disabled so that KVM > KVM provides consistent APIC behavior if xAPIC IDs are aliased due to > vcpu_id being truncated and the x2APIC hotplug hack isn't enabled. If > the hotplug hack is disabled, events that are emulated by KVM will follow > architectural behavior (all matching vCPUs receive events, even if the > "match" is due to truncation), whereas APICv and AVIC will deliver events > only to the first matching vCPU, i.e. the vCPU that matches without > truncation. > > Note, the "extra" inhibit is needed because KVM deliberately ignores > mismatches due to truncation when applying the APIC_ID_MODIFIED inhibit > so that large VMs (>255 vCPUs) can run with APICv/AVIC. > > Fixes: TDB Do you mean Trade & Development Bank of Mongolia ? ;-) > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/include/asm/kvm_host.h | 6 ++++++ > arch/x86/kvm/lapic.c | 13 ++++++++++++- > arch/x86/kvm/svm/avic.c | 1 + > arch/x86/kvm/vmx/vmx.c | 1 + > 4 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index ac28bbfbf0e3..171e38b94c89 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1104,6 +1104,12 @@ enum kvm_apicv_inhibit { > */ > APICV_INHIBIT_REASON_BLOCKIRQ, > > + /* > + * APICv is disabled because not all vCPUs have a 1:1 mapping between > + * APIC ID and vCPU, _and_ KVM is not applying its x2APIC hotplug hack. > + */ > + APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED, > + > /* > * For simplicity, the APIC acceleration is inhibited > * first time either APIC ID or APIC base are changed by the guest > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 340c2d3e781b..f6f328d36ae2 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -381,6 +381,16 @@ void kvm_recalculate_apic_map(struct kvm *kvm) > cluster[ldr] = apic; > } > out: > + /* > + * The optimized map is effectively KVM's internal version of APICv, Nitpick: APICv/AVIC? > + * and all unwanted aliasing that results in disabling the optimized > + * map also applies to APICv. > + */ > + if (!new) > + kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED); > + else > + kvm_clear_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED); > + > old = rcu_dereference_protected(kvm->arch.apic_map, > lockdep_is_held(&kvm->arch.apic_map_lock)); > rcu_assign_pointer(kvm->arch.apic_map, new); > @@ -2153,7 +2163,8 @@ static void kvm_lapic_xapic_id_updated(struct kvm_lapic *apic) > /* > * Deliberately truncate the vCPU ID when detecting a modified APIC ID > * to avoid false positives if the vCPU ID, i.e. x2APIC ID, is a 32-bit > - * value. > + * value. If the wrap/truncation results in unwatned aliasing, APICv ^^ typo > + * will be inhibited as part of updating KVM's optimized APIC maps. > */ > if (kvm_xapic_id(apic) == (u8)apic->vcpu->vcpu_id) > return; > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index dd0e41d454a7..2908adc79ea6 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -965,6 +965,7 @@ bool avic_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason) > BIT(APICV_INHIBIT_REASON_PIT_REINJ) | > BIT(APICV_INHIBIT_REASON_BLOCKIRQ) | > BIT(APICV_INHIBIT_REASON_SEV) | > + BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) | > BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) | > BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED) | > BIT(APICV_INHIBIT_REASON_X2APIC); > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 974d9a366d5d..5920166d7260 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -7955,6 +7955,7 @@ static bool vmx_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason) > BIT(APICV_INHIBIT_REASON_ABSENT) | > BIT(APICV_INHIBIT_REASON_HYPERV) | > BIT(APICV_INHIBIT_REASON_BLOCKIRQ) | > + BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) | > BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) | > BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED); > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
On Thu, Dec 08, 2022, Maxim Levitsky wrote: > On Sat, 2022-10-01 at 00:59 +0000, Sean Christopherson wrote: > > Inhibit APICv/AVIC if the optimized physical map is disabled so that KVM > > KVM provides consistent APIC behavior if xAPIC IDs are aliased due to > > vcpu_id being truncated and the x2APIC hotplug hack isn't enabled. If > > the hotplug hack is disabled, events that are emulated by KVM will follow > > architectural behavior (all matching vCPUs receive events, even if the > > "match" is due to truncation), whereas APICv and AVIC will deliver events > > only to the first matching vCPU, i.e. the vCPU that matches without > > truncation. > > > > Note, the "extra" inhibit is needed because KVM deliberately ignores > > mismatches due to truncation when applying the APIC_ID_MODIFIED inhibit > > so that large VMs (>255 vCPUs) can run with APICv/AVIC. > > > > Fixes: TDB > Do you mean Trade & Development Bank of Mongolia ? ;-) Heh, this fixes a patch/commit earlier in the series, hence the placeholder. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/include/asm/kvm_host.h | 6 ++++++ > > arch/x86/kvm/lapic.c | 13 ++++++++++++- > > arch/x86/kvm/svm/avic.c | 1 + > > arch/x86/kvm/vmx/vmx.c | 1 + > > 4 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index ac28bbfbf0e3..171e38b94c89 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -1104,6 +1104,12 @@ enum kvm_apicv_inhibit { > > */ > > APICV_INHIBIT_REASON_BLOCKIRQ, > > > > + /* > > + * APICv is disabled because not all vCPUs have a 1:1 mapping between > > + * APIC ID and vCPU, _and_ KVM is not applying its x2APIC hotplug hack. > > + */ > > + APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED, > > + > > /* > > * For simplicity, the APIC acceleration is inhibited > > * first time either APIC ID or APIC base are changed by the guest > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index 340c2d3e781b..f6f328d36ae2 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -381,6 +381,16 @@ void kvm_recalculate_apic_map(struct kvm *kvm) > > cluster[ldr] = apic; > > } > > out: > > + /* > > + * The optimized map is effectively KVM's internal version of APICv, > Nitpick: APICv/AVIC? Hmm. I think my preference for common code that doesn't need to differentiate between Intel and AMD (or AVIC vs. x2AVIC) is to use just APICv. "APICv" is a mostly a KVM term, e.g. Intel uses "APIC-virtualization" as an umbrella term for a massive pile of features, so I think bending the term to cover Intel+AMD is ok? Typing APICv/AVIC everywhere will get tedious and creates its own flavor of confusion, e.g. the inhibits all use APICV.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index ac28bbfbf0e3..171e38b94c89 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1104,6 +1104,12 @@ enum kvm_apicv_inhibit { */ APICV_INHIBIT_REASON_BLOCKIRQ, + /* + * APICv is disabled because not all vCPUs have a 1:1 mapping between + * APIC ID and vCPU, _and_ KVM is not applying its x2APIC hotplug hack. + */ + APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED, + /* * For simplicity, the APIC acceleration is inhibited * first time either APIC ID or APIC base are changed by the guest diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 340c2d3e781b..f6f328d36ae2 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -381,6 +381,16 @@ void kvm_recalculate_apic_map(struct kvm *kvm) cluster[ldr] = apic; } out: + /* + * The optimized map is effectively KVM's internal version of APICv, + * and all unwanted aliasing that results in disabling the optimized + * map also applies to APICv. + */ + if (!new) + kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED); + else + kvm_clear_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED); + old = rcu_dereference_protected(kvm->arch.apic_map, lockdep_is_held(&kvm->arch.apic_map_lock)); rcu_assign_pointer(kvm->arch.apic_map, new); @@ -2153,7 +2163,8 @@ static void kvm_lapic_xapic_id_updated(struct kvm_lapic *apic) /* * Deliberately truncate the vCPU ID when detecting a modified APIC ID * to avoid false positives if the vCPU ID, i.e. x2APIC ID, is a 32-bit - * value. + * value. If the wrap/truncation results in unwatned aliasing, APICv + * will be inhibited as part of updating KVM's optimized APIC maps. */ if (kvm_xapic_id(apic) == (u8)apic->vcpu->vcpu_id) return; diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index dd0e41d454a7..2908adc79ea6 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -965,6 +965,7 @@ bool avic_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason) BIT(APICV_INHIBIT_REASON_PIT_REINJ) | BIT(APICV_INHIBIT_REASON_BLOCKIRQ) | BIT(APICV_INHIBIT_REASON_SEV) | + BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) | BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) | BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED) | BIT(APICV_INHIBIT_REASON_X2APIC); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 974d9a366d5d..5920166d7260 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7955,6 +7955,7 @@ static bool vmx_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason) BIT(APICV_INHIBIT_REASON_ABSENT) | BIT(APICV_INHIBIT_REASON_HYPERV) | BIT(APICV_INHIBIT_REASON_BLOCKIRQ) | + BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) | BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) | BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
Inhibit APICv/AVIC if the optimized physical map is disabled so that KVM KVM provides consistent APIC behavior if xAPIC IDs are aliased due to vcpu_id being truncated and the x2APIC hotplug hack isn't enabled. If the hotplug hack is disabled, events that are emulated by KVM will follow architectural behavior (all matching vCPUs receive events, even if the "match" is due to truncation), whereas APICv and AVIC will deliver events only to the first matching vCPU, i.e. the vCPU that matches without truncation. Note, the "extra" inhibit is needed because KVM deliberately ignores mismatches due to truncation when applying the APIC_ID_MODIFIED inhibit so that large VMs (>255 vCPUs) can run with APICv/AVIC. Fixes: TDB Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/include/asm/kvm_host.h | 6 ++++++ arch/x86/kvm/lapic.c | 13 ++++++++++++- arch/x86/kvm/svm/avic.c | 1 + arch/x86/kvm/vmx/vmx.c | 1 + 4 files changed, 20 insertions(+), 1 deletion(-)