[PATCH] KVM: x86: Add EOI_INDUCED exit handlers for Hyper-V SynIC vectors

[PATCH] KVM: x86: Add EOI_INDUCED exit handlers for Hyper-V SynIC vectors

[Wang Guangju]

From: chukaiping <chukaiping@baidu.com>

When EOI virtualization is performed on VMX,
kvm_apic_set_eoi_accelerated() is called upon
EXIT_REASON_EOI_INDUCED but unlike its non-accelerated
apic_set_eoi() sibling, Hyper-V SINT vectors are
left unhandled.

This patch fix it, and add a new helper function to
handle both IOAPIC and Hyper-V SINT vectors.

Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: wangguangju <wangguangju@baidu.com>
---
 arch/x86/kvm/lapic.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index f03facc..e046afe 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1269,6 +1269,16 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
 	kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode);
 }
 
+static inline void apic_set_eoi_vector(struct kvm_lapic *apic, int vector)
+{
+	if (to_hv_vcpu(apic->vcpu) &&
+	    test_bit(vector, to_hv_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);
+}
+
 static int apic_set_eoi(struct kvm_lapic *apic)
 {
 	int vector = apic_find_highest_isr(apic);
@@ -1285,12 +1295,8 @@ static int apic_set_eoi(struct kvm_lapic *apic)
 	apic_clear_isr(vector, apic);
 	apic_update_ppr(apic);
 
-	if (to_hv_vcpu(apic->vcpu) &&
-	    test_bit(vector, to_hv_synic(apic->vcpu)->vec_bitmap))
-		kvm_hv_synic_send_eoi(apic->vcpu, vector);
+	apic_set_eoi_vector(apic, vector);
 
-	kvm_ioapic_send_eoi(apic, vector);
-	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 	return vector;
 }
 
@@ -1304,8 +1310,7 @@ void kvm_apic_set_eoi_accelerated(struct kvm_vcpu *vcpu, int vector)
 
 	trace_kvm_eoi(apic, vector);
 
-	kvm_ioapic_send_eoi(apic, vector);
-	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
+	apic_set_eoi_vector(apic, vector);
 }
 EXPORT_SYMBOL_GPL(kvm_apic_set_eoi_accelerated);
 
-- 
2.9.4

Re: [PATCH] KVM: x86: Add EOI_INDUCED exit handlers for Hyper-V SynIC vectors

[Sean Christopherson]

This isn't adding a handler so much as it's signaling EOI for SynIC vectors. Maybe?

  KVM: x86: Send EOI to SynIC vectors on accelerated EOI-induced VM-Exits

On Thu, Jul 07, 2022, Wang Guangju wrote:
> From: chukaiping <chukaiping@baidu.com>
> 
> When EOI virtualization is performed on VMX,
> kvm_apic_set_eoi_accelerated() is called upon
> EXIT_REASON_EOI_INDUCED but unlike its non-accelerated
> apic_set_eoi() sibling, Hyper-V SINT vectors are
> left unhandled.

Wrap changelogs closer to ~75 chars.

> This patch fix it, and add a new helper function to
> handle both IOAPIC and Hyper-V SINT vectors.

Avoid "this patch" and simply state what change is being made.  E.g.


  Send EOI to Hyper-V SINT vectors when handling acclerated EOI-induced
  VM-Exits.  KVM Hyper-V needs to handle the SINT EOI irrespective of
  whether the EOI is acclerated or not.

Fixes: 5c919412fe61 ("kvm/x86: Hyper-V synthetic interrupt controller")

and probably Cc: stable@vger.kernel.org?

> Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: wangguangju <wangguangju@baidu.com>
> ---
>  arch/x86/kvm/lapic.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index f03facc..e046afe 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1269,6 +1269,16 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
>  	kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode);
>  }
>  
> +static inline void apic_set_eoi_vector(struct kvm_lapic *apic, int vector)
> +{
> +	if (to_hv_vcpu(apic->vcpu) &&
> +	    test_bit(vector, to_hv_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);
> +}

Rather than add a third helper, what about renaming kvm_apic_set_eoi_accelerated()
and having the non-accelerated helper call the "acclerated" version?  That will
document the delta between the non-accelerated patch and the accelerated path.
The only hiccup is tracing, but that's easy to resolve (or we could just not trace
if there's no valid vector to EOI), e.g.


/*
 * Send EOI for a valid vector.  The caller, or hardware when this is invoked
 * after an accelerated EOI VM-Exit, is responsible for updating the vISR and
 * vPPR.
 */
void kvm_apic_set_eoi(struct kvm_lapic *apic, int vector)
{
	trace_kvm_eoi(apic, vector);

	if (to_hv_vcpu(apic->vcpu) &&
	    test_bit(vector, to_hv_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);
}
EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);

static int apic_set_eoi(struct kvm_lapic *apic)
{
	int vector = apic_find_highest_isr(apic);

	/*
	 * Not every write EOI will has corresponding ISR,
	 * one example is when Kernel check timer on setup_IO_APIC
	 */
	if (vector == -1) {
		trace_kvm_eoi(apic, vector);   <---- maybe just drop this?
		return vector;
	}

	apic_clear_isr(vector, apic);
	apic_update_ppr(apic);

	kvm_apic_set_eoi(apic, vector);

	return vector;
}

答复: [PATCH] KVM: x86: Add EOI_INDUCED exit handlers for Hyper-V SynIC vectors

[Wang,Guangju]

> Rather than add a third helper, what about renaming kvm_apic_set_eoi_accelerated() and having the non-accelerated helper call the "acclerated" version?  That will document the delta between the non-accelerated patch and the accelerated path.
> The only hiccup is tracing, but that's easy to resolve (or we could just not trace if there's no valid vector to EOI), e.g.

Yeah, rename the function and integrate two paths looks clearer and easier to understand?
Thanks Sean for the suggestion, I will send a new patch with a new subject and changelog.

--
Wang

[PATCH] KVM: x86: Add EOI_INDUCED_EXIT handlers for Hyper-V SynIC vectors

[Wang Guangju]

From: wangguangju <wangguangju@baidu.com>

When EOI virtualization is performed on VMX,
kvm_apic_set_eoi_accelerated() is called upon
EXIT_REASON_EOI_INDUCED but unlike its non-accelerated
apic_set_eoi() sibling, Hyper-V SINT vectors are
left unhandled.

This patch fix it, and add a new helper function to
handle both IOAPIC and Hyper-V SINT vectors.

Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: wangguangju <wangguangju@baidu.com>
---
 arch/x86/kvm/lapic.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index f03facc..e046afe 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1269,6 +1269,16 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
 	kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode);
 }
 
+static inline void apic_set_eoi_vector(struct kvm_lapic *apic, int vector)
+{
+	if (to_hv_vcpu(apic->vcpu) &&
+	    test_bit(vector, to_hv_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);
+}
+
 static int apic_set_eoi(struct kvm_lapic *apic)
 {
 	int vector = apic_find_highest_isr(apic);
@@ -1285,12 +1295,8 @@ static int apic_set_eoi(struct kvm_lapic *apic)
 	apic_clear_isr(vector, apic);
 	apic_update_ppr(apic);
 
-	if (to_hv_vcpu(apic->vcpu) &&
-	    test_bit(vector, to_hv_synic(apic->vcpu)->vec_bitmap))
-		kvm_hv_synic_send_eoi(apic->vcpu, vector);
+	apic_set_eoi_vector(apic, vector);
 
-	kvm_ioapic_send_eoi(apic, vector);
-	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 	return vector;
 }
 
@@ -1304,8 +1310,7 @@ void kvm_apic_set_eoi_accelerated(struct kvm_vcpu *vcpu, int vector)
 
 	trace_kvm_eoi(apic, vector);
 
-	kvm_ioapic_send_eoi(apic, vector);
-	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
+	apic_set_eoi_vector(apic, vector);
 }
 EXPORT_SYMBOL_GPL(kvm_apic_set_eoi_accelerated);
 
-- 
2.9.4