Re: [PATCH] x86/kvm/hyper-v: Don't deactivate APICv unconditionally when Hyper-V SynIC enabled

[Vitaly Kuznetsov <vkuznets@redhat.com>]

junjiehua0xff@gmail.com writes:

> From: Junjie Hua <junjiehua@tencent.com>
>
> The current implementation of Hyper-V SynIC[1] request to deactivate 
> APICv when SynIC is enabled, since the AutoEOI feature of SynIC is not 
> compatible with APICv[2].
>
> Actually, windows doesn't use AutoEOI if deprecating AutoEOI bit is set 
> (CPUID.40000004H:EAX[bit 9], HyperV-TLFS v6.0b section 2.4.5), we don't 
> need to disable APICv in this case.
>

Thank you for the patch, the fact that we disable APICv every time we
enable SynIC is nothing to be proud of. I'm, however, not sure we can
treat 'Recommend deprecating AutoEOI' as 'AutoEOI must not be
used.'. Could you please clarify which Windows versions you've tested
with with?

> [1] commit 5c919412fe61 ("kvm/x86: Hyper-V synthetic interrupt controller")
> [2] https://patchwork.kernel.org/patch/7486761/
>
> Signed-off-by: Junjie Hua <junjiehua@tencent.com>
> ---
>  arch/x86/kvm/hyperv.c | 18 +++++++++++++++++-
>  arch/x86/kvm/lapic.c  |  3 +++
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 5c7c406..9eee2da 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -899,6 +899,19 @@ void kvm_hv_vcpu_postcreate(struct kvm_vcpu *vcpu)
>  	hv_vcpu->vp_index = kvm_vcpu_get_idx(vcpu);
>  }
>  
> +static bool kvm_hv_is_synic_autoeoi_deprecated(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpuid_entry2 *entry;
> +
> +	entry = kvm_find_cpuid_entry(vcpu,
> +				HYPERV_CPUID_ENLIGHTMENT_INFO,
> +				0);
> +	if (!entry)
> +		return false;
> +
> +	return entry->eax & HV_DEPRECATING_AEOI_RECOMMENDED;
> +}

I think we should complement (replace?) this with checking that no SINTx
was configured with AutoEOI (and immeditely inhibit APICv if the
situation changes).

> +
>  int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages)
>  {
>  	struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
> @@ -908,7 +921,10 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages)
>  	 * not compatible with APICV, so request
>  	 * to deactivate APICV permanently.
>  	 */
> -	kvm_request_apicv_update(vcpu->kvm, false, APICV_INHIBIT_REASON_HYPERV);
> +	if (!kvm_hv_is_synic_autoeoi_deprecated(vcpu))
> +		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;
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 105e785..0bb431f 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1263,6 +1263,9 @@ void kvm_apic_set_eoi_accelerated(struct kvm_vcpu *vcpu, int vector)
>  
>  	trace_kvm_eoi(apic, vector);
>  
> +	if (test_bit(vector, vcpu_to_synic(apic->vcpu)->vec_bitmap))
> +		kvm_hv_synic_send_eoi(apic->vcpu, vector);
> +
>  	kvm_ioapic_send_eoi(apic, vector);
>  	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
>  }

-- 
Vitaly