Skip to contentSkip to conversation

Fix busy-spin of lapic_timer_int_injected with APIC-v

PATCH In review Marcelo Tosatti 2 participants 6 messages Feb 2, 2015

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

cover Fix busy-spin of lapic_timer_int_injected with APIC-v

See patches for details.

author
#1 lore raw

PATCH 1/2 KVM: x86: fix lapic_timer_int_injected with APIC-v

With APICv, LAPIC timer interrupt is always delivered via IRR:
apic_find_highest_irr syncs PIR to IRR.

Signed-off-by Marcelo Tosatti <[email protected]>
---
 arch/x86/kvm/lapic.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
arch/x86/kvm/lapic.c +6 -6
Index: kvm/arch/x86/kvm/lapic.c
===================================================================
@@ -1086,13 +1086,13 @@
 
 	if (kvm_apic_hw_enabled(apic)) {
 		int vec = reg & APIC_VECTOR_MASK;
+		void *bitmap = apic->regs + APIC_ISR;
 
-		if (kvm_x86_ops->test_posted_interrupt)
-			return kvm_x86_ops->test_posted_interrupt(vcpu, vec);
-		else {
-			if (apic_test_vector(vec, apic->regs + APIC_ISR))
-				return true;
-		}
+		if (kvm_x86_ops->deliver_posted_interrupt)
+			bitmap = apic->regs + APIC_IRR;
+
+		if (apic_test_vector(vec, bitmap))
+			return true;
 	}
 	return false;
 }
author
#2 lore raw

PATCH 2/2 KVM: x86: revert "add method to test PIR bitmap vector"

In reply to Marcelo Tosatti

Revert 7c6a98dfa1ba9dc64a62e73624ecea9995736bbd, given that testing PIR is not necessary anymore.

Signed-off-by Marcelo Tosatti <[email protected]>
---
 arch/x86/include/asm/kvm_host.h |    1 -
 arch/x86/kvm/vmx.c              |   14 --------------
 2 files changed, 15 deletions(-)
arch/x86/include/asm/kvm_host.h +0 -1
Index: kvm/arch/x86/include/asm/kvm_host.h
===================================================================
@@ -767,7 +767,6 @@
 	void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
 	void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
 	void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
-	bool (*test_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
 	void (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
 	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
 	int (*get_tdp_level)(void);
arch/x86/kvm/vmx.c +0 -14
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
@@ -438,11 +438,6 @@
 	return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
 }
 
-static int pi_test_pir(int vector, struct pi_desc *pi_desc)
-{
-	return test_bit(vector, (unsigned long *)pi_desc->pir);
-}
-
 struct vcpu_vmx {
 	struct kvm_vcpu       vcpu;
 	unsigned long         host_rsp;

@@ -5908,7 +5903,6 @@
 		kvm_x86_ops->hwapic_irr_update = NULL;
 		kvm_x86_ops->hwapic_isr_update = NULL;
 		kvm_x86_ops->deliver_posted_interrupt = NULL;
-		kvm_x86_ops->test_posted_interrupt = NULL;
 		kvm_x86_ops->sync_pir_to_irr = vmx_sync_pir_to_irr_dummy;
 	}
 

@@ -6994,13 +6988,6 @@
 	return 1;
 }
 
-static bool vmx_test_pir(struct kvm_vcpu *vcpu, int vector)
-{
-	struct vcpu_vmx *vmx = to_vmx(vcpu);
-
-	return pi_test_pir(vector, &vmx->pi_desc);
-}
-
 static int handle_pml_full(struct kvm_vcpu *vcpu)
 {
 	unsigned long exit_qualification;

@@ -9753,7 +9740,6 @@
 	.hwapic_isr_update = vmx_hwapic_isr_update,
 	.sync_pir_to_irr = vmx_sync_pir_to_irr,
 	.deliver_posted_interrupt = vmx_deliver_posted_interrupt,
-	.test_posted_interrupt = vmx_test_pir,
 
 	.set_tss_addr = vmx_set_tss_addr,
 	.get_tdp_level = get_ept_level,
author
#4 lore raw
Quoted 8 lines · Paolo Bonzini
Quoted 1 line · Marcelo Tosatti

See patches for details.

Nicer indeed. It would be good to know what bug made you do this, though.

Paolo

kvm-unit-test tscdeadline-latency shows negative delta.

Do you want me to resend with the modified changelog?

maintainer
#5 lore raw
Quoted 5 lines · Marcelo Tosatti
Quoted 1 line

Nicer indeed. It would be good to know what bug made you do this, though.

kvm-unit-test tscdeadline-latency shows negative delta.

Do you want me to resend with the modified changelog?

No, it's okay.

Thanks,

Paolo