Message ID | 20210802183329.2309921-10-mlevitsk@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | My AVIC patch queue | expand |
On 02/08/21 20:33, Maxim Levitsky wrote: > Maxim: > - always set HV_DEPRECATING_AEOI_RECOMMENDED in kvm_get_hv_cpuid, > since this feature can be used regardless of AVIC I tend to agree here. While AutoEOI does have a minor performance improvement, I suspect that APICv will be available in most cases where Windows guest performance matters to that point. Paolo
On 02/08/21 20:33, Maxim Levitsky wrote: > From: Vitaly Kuznetsov <vkuznets@redhat.com> > > APICV_INHIBIT_REASON_HYPERV is currently unconditionally forced upon > SynIC activation as SynIC's AutoEOI is incompatible with APICv/AVIC. It is, > however, possible to track whether the feature was actually used by the > guest and only inhibit APICv/AVIC when needed. > > TLFS suggests a dedicated 'HV_DEPRECATING_AEOI_RECOMMENDED' flag to let > Windows know that AutoEOI feature should be avoided. While it's up to > KVM userspace to set the flag, KVM can help a bit by exposing global > APICv/AVIC enablement. > > Maxim: > - always set HV_DEPRECATING_AEOI_RECOMMENDED in kvm_get_hv_cpuid, > since this feature can be used regardless of AVIC > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 3 +++ > arch/x86/kvm/hyperv.c | 27 +++++++++++++++++++++------ > 2 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 80323b5fb20a..55b1f79d9c43 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -988,6 +988,9 @@ struct kvm_hv { > /* How many vCPUs have VP index != vCPU index */ > atomic_t num_mismatched_vp_indexes; > > + /* How many SynICs use 'AutoEOI' feature */ > + atomic_t synic_auto_eoi_used; > + > struct hv_partition_assist_pg *hv_pa_pg; > struct kvm_hv_syndbg hv_syndbg; > }; > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index b07592ca92f0..638f3c559623 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -88,6 +88,10 @@ static bool synic_has_vector_auto_eoi(struct kvm_vcpu_hv_synic *synic, > static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, > int vector) > { > + struct kvm_vcpu *vcpu = hv_synic_to_vcpu(synic); > + struct kvm_hv *hv = to_kvm_hv(vcpu->kvm); > + int auto_eoi_old, auto_eoi_new; > + > if (vector < HV_SYNIC_FIRST_VALID_VECTOR) > return; > > @@ -96,10 +100,25 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, > else > __clear_bit(vector, synic->vec_bitmap); > > + auto_eoi_old = bitmap_weight(synic->auto_eoi_bitmap, 256); > + > if (synic_has_vector_auto_eoi(synic, vector)) > __set_bit(vector, synic->auto_eoi_bitmap); > else > __clear_bit(vector, synic->auto_eoi_bitmap); > + > + auto_eoi_new = bitmap_weight(synic->auto_eoi_bitmap, 256); > + > + /* Hyper-V SynIC auto EOI SINTs are not compatible with APICV */ > + if (!auto_eoi_old && auto_eoi_new) { > + if (atomic_inc_return(&hv->synic_auto_eoi_used) == 1) > + kvm_request_apicv_update(vcpu->kvm, false, > + APICV_INHIBIT_REASON_HYPERV); > + } else if (!auto_eoi_new && auto_eoi_old) { > + if (atomic_dec_return(&hv->synic_auto_eoi_used) == 0) > + kvm_request_apicv_update(vcpu->kvm, true, > + APICV_INHIBIT_REASON_HYPERV); > + } > } > > static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, int sint, > @@ -933,12 +952,6 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages) > > synic = to_hv_synic(vcpu); > > - /* > - * Hyper-V SynIC auto EOI SINT's are > - * not compatible with APICV, so request > - * to deactivate APICV permanently. > - */ > - kvm_request_apicv_update(vcpu->kvm, false, APICV_INHIBIT_REASON_HYPERV); > synic->active = true; > synic->dont_zero_synic_pages = dont_zero_synic_pages; > synic->control = HV_SYNIC_CONTROL_ENABLE; > @@ -2466,6 +2479,8 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid, > ent->eax |= HV_X64_ENLIGHTENED_VMCS_RECOMMENDED; > if (!cpu_smt_possible()) > ent->eax |= HV_X64_NO_NONARCH_CORESHARING; > + > + ent->eax |= HV_DEPRECATING_AEOI_RECOMMENDED; > /* > * Default number of spinlock retry attempts, matches > * HyperV 2016. > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
On 02/08/21 20:33, Maxim Levitsky wrote: > + if (!auto_eoi_old && auto_eoi_new) { > + if (atomic_inc_return(&hv->synic_auto_eoi_used) == 1) > + kvm_request_apicv_update(vcpu->kvm, false, > + APICV_INHIBIT_REASON_HYPERV); > + } else if (!auto_eoi_new && auto_eoi_old) { > + if (atomic_dec_return(&hv->synic_auto_eoi_used) == 0) > + kvm_request_apicv_update(vcpu->kvm, true, > + APICV_INHIBIT_REASON_HYPERV); Hmm no, Reviewed-by rescinded. This is racy, you cannot use atomics. The check for zero needs to happen within the lock. The easiest way is to have a __kvm_request_apicv_update function that leaves it to the caller to take the lock. Then synic_update_vector can do if (!!auto_eoi_old == !!auto_eoi_new) return; mutex_lock(&kvm->apicv_update_lock); bool was_active = hv->synic_auto_eoi_used; if (auto_eoi_new) hv->synic_auto_eoi_used++; else hv->synic_auto_eoi_used--; if (!!hv->synic_auto_eoi_used != !!was_active) __kvm_request_apicv_update(vcpu->kvm, !!hv->synic_auto_eoi_used, APICV_INHIBIT_REASON_HYPERV); mutex_unlock(&kvm->apicv_update_lock); Please add a note to synic_auto_eoi_used that it is protected by apicv_update_lock. Thanks, Paolo
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 80323b5fb20a..55b1f79d9c43 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -988,6 +988,9 @@ struct kvm_hv { /* How many vCPUs have VP index != vCPU index */ atomic_t num_mismatched_vp_indexes; + /* How many SynICs use 'AutoEOI' feature */ + atomic_t synic_auto_eoi_used; + struct hv_partition_assist_pg *hv_pa_pg; struct kvm_hv_syndbg hv_syndbg; }; diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index b07592ca92f0..638f3c559623 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -88,6 +88,10 @@ static bool synic_has_vector_auto_eoi(struct kvm_vcpu_hv_synic *synic, static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, int vector) { + struct kvm_vcpu *vcpu = hv_synic_to_vcpu(synic); + struct kvm_hv *hv = to_kvm_hv(vcpu->kvm); + int auto_eoi_old, auto_eoi_new; + if (vector < HV_SYNIC_FIRST_VALID_VECTOR) return; @@ -96,10 +100,25 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, else __clear_bit(vector, synic->vec_bitmap); + auto_eoi_old = bitmap_weight(synic->auto_eoi_bitmap, 256); + if (synic_has_vector_auto_eoi(synic, vector)) __set_bit(vector, synic->auto_eoi_bitmap); else __clear_bit(vector, synic->auto_eoi_bitmap); + + auto_eoi_new = bitmap_weight(synic->auto_eoi_bitmap, 256); + + /* Hyper-V SynIC auto EOI SINTs are not compatible with APICV */ + if (!auto_eoi_old && auto_eoi_new) { + if (atomic_inc_return(&hv->synic_auto_eoi_used) == 1) + kvm_request_apicv_update(vcpu->kvm, false, + APICV_INHIBIT_REASON_HYPERV); + } else if (!auto_eoi_new && auto_eoi_old) { + if (atomic_dec_return(&hv->synic_auto_eoi_used) == 0) + kvm_request_apicv_update(vcpu->kvm, true, + APICV_INHIBIT_REASON_HYPERV); + } } static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, int sint, @@ -933,12 +952,6 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages) synic = to_hv_synic(vcpu); - /* - * Hyper-V SynIC auto EOI SINT's are - * not compatible with APICV, so request - * to deactivate APICV permanently. - */ - kvm_request_apicv_update(vcpu->kvm, false, APICV_INHIBIT_REASON_HYPERV); synic->active = true; synic->dont_zero_synic_pages = dont_zero_synic_pages; synic->control = HV_SYNIC_CONTROL_ENABLE; @@ -2466,6 +2479,8 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid, ent->eax |= HV_X64_ENLIGHTENED_VMCS_RECOMMENDED; if (!cpu_smt_possible()) ent->eax |= HV_X64_NO_NONARCH_CORESHARING; + + ent->eax |= HV_DEPRECATING_AEOI_RECOMMENDED; /* * Default number of spinlock retry attempts, matches * HyperV 2016.