linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT
@ 2016-12-19  9:47 Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing Paolo Bonzini
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

This is the result of cleaning up the places that set REQ_EVENT unnecessarily.
The savings on self-IPI kvm-unit-tests are:

    self_ipi_sti_nop                ~300 clock cycles
    self_ipi_sti_hlt                ~300 clock cycles
    self_ipi_tpr                    ~400 clock cycles
    self_ipi_tpr_sti_nop            ~700 clock cycles
    self_ipi_tpr_sti_hlt            ~700 clock cycles
 
which is in the 5-10% range.  Please help me comparing this series with
"[PATCH v2] KVM: x86: avoid redundant REQ_EVENT" on the real workload!

Thanks,

Paolo

Paolo Bonzini (6):
  KVM: x86: add VCPU stat for KVM_REQ_EVENT processing
  KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI
  KVM: vmx: speed up TPR below threshold vmexits
  KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update
  KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update
  KVM: lapic: do not scan IRR when delivering an interrupt

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/lapic.c            | 58 +++++++++++++++++++++++++----------------
 arch/x86/kvm/lapic.h            |  1 +
 arch/x86/kvm/vmx.c              |  2 +-
 arch/x86/kvm/x86.c              |  2 ++
 5 files changed, 41 insertions(+), 23 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
       [not found]   ` <20161220092101.GE2081@rkaganb.sw.ru>
  2016-12-19  9:47 ` [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI Paolo Bonzini
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/x86.c              | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 994d8ed9fc6c..08cfd45a9452 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -850,6 +850,7 @@ struct kvm_vcpu_stat {
 	u64 hypercalls;
 	u64 irq_injections;
 	u64 nmi_injections;
+	u64 req_event;
 };
 
 struct x86_instruction_info;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e95d94edbdc3..e9b512090865 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -180,6 +180,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
 	{ "irq_injections", VCPU_STAT(irq_injections) },
 	{ "nmi_injections", VCPU_STAT(nmi_injections) },
+	{ "req_event", VCPU_STAT(req_event) },
 	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
 	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
 	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
@@ -6691,6 +6692,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	}
 
 	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
+		++vcpu->stat.req_event;
 		kvm_apic_accept_events(vcpu);
 		if (vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED) {
 			r = 1;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
  2016-12-23 12:08   ` Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 3/6] KVM: vmx: speed up TPR below threshold vmexits Paolo Bonzini
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

On EOI, there is no need to set KVM_REQ_EVENT unconditionally.  The PPR
update is already setting it if resetting the ISR bit causes PPR to
decrease.  Even a level-triggered IOAPIC interrupt will set KVM_REQ_EVENT
on a reinjection (ioapic_service -> kvm_irq_delivery_to_apic and from
there to __apic_accept_irq).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e35cbd44c505..70c7428b7a57 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1066,7 +1066,6 @@ static int apic_set_eoi(struct kvm_lapic *apic)
 		kvm_hv_synic_send_eoi(apic->vcpu, vector);
 
 	kvm_ioapic_send_eoi(apic, vector);
-	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 	return vector;
 }
 
@@ -1081,7 +1080,6 @@ 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);
 }
 EXPORT_SYMBOL_GPL(kvm_apic_set_eoi_accelerated);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] KVM: vmx: speed up TPR below threshold vmexits
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 4/6] KVM: lapic: avoid unnecessary KVM_REQ_EVENT on IRR scan Paolo Bonzini
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

Since we're already in VCPU context, all we have to do here is recompute
the PPR value.  That will in turn generate a KVM_REQ_EVENT if necessary.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 6 ++++++
 arch/x86/kvm/lapic.h | 1 +
 arch/x86/kvm/vmx.c   | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 70c7428b7a57..ae59e655cd6d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -595,6 +595,12 @@ static void apic_update_ppr(struct kvm_lapic *apic)
 	}
 }
 
+void kvm_apic_update_ppr(struct kvm_vcpu *vcpu)
+{
+	apic_update_ppr(vcpu->arch.apic);
+}
+EXPORT_SYMBOL_GPL(kvm_apic_update_ppr);
+
 static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
 {
 	kvm_lapic_set_reg(apic, APIC_TASKPRI, tpr);
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index cb16e6fd2330..5b5b1ba644cb 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -73,6 +73,7 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 
 void __kvm_apic_update_irr(u32 *pir, void *regs);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
+void kvm_apic_update_ppr(struct kvm_vcpu *vcpu);
 int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
 		     struct dest_map *dest_map);
 int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 24db5fb6f575..5fb54e3de061 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6158,7 +6158,7 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu)
 
 static int handle_tpr_below_threshold(struct kvm_vcpu *vcpu)
 {
-	kvm_make_request(KVM_REQ_EVENT, vcpu);
+	kvm_apic_update_ppr(vcpu);
 	return 1;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] KVM: lapic: avoid unnecessary KVM_REQ_EVENT on IRR scan
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
                   ` (2 preceding siblings ...)
  2016-12-19  9:47 ` [PATCH 3/6] KVM: vmx: speed up TPR below threshold vmexits Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 5/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update Paolo Bonzini
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

PPR needs to be updated whenever on every IRR read because we may have
missed TPR writes that _increased_ PPR.  However, these writes need not
generate KVM_REQ_EVENT, because KVM_REQ_EVENT has been set already in
__apic_accept_irq, by apic_update_ppr when TPR was lowered, or again by
apic_update_ppr when EOI cleared a bit in ISR.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index ae59e655cd6d..f936644d8011 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -570,7 +570,15 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
 	__clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention);
 }
 
-static void apic_update_ppr(struct kvm_lapic *apic)
+static int apic_has_interrupt_for_ppr(struct kvm_lapic *apic, u32 ppr)
+{
+	int highest_irr = apic_find_highest_irr(apic);
+	if (highest_irr == -1 || (highest_irr & 0xF0) <= ppr)
+		return -1;
+	return highest_irr;
+}
+
+static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr)
 {
 	u32 tpr, isrv, ppr, old_ppr;
 	int isr;
@@ -588,11 +596,19 @@ static void apic_update_ppr(struct kvm_lapic *apic)
 	apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x",
 		   apic, ppr, isr, isrv);
 
-	if (old_ppr != ppr) {
+	*new_ppr = ppr;
+	if (old_ppr != ppr)
 		kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
-		if (ppr < old_ppr)
-			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
-	}
+
+	return ppr < old_ppr;
+}
+
+static void apic_update_ppr(struct kvm_lapic *apic)
+{
+	u32 ppr;
+
+	if (__apic_update_ppr(apic, &ppr))
+		kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 }
 
 void kvm_apic_update_ppr(struct kvm_vcpu *vcpu)
@@ -2051,17 +2067,13 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu)
 int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
-	int highest_irr;
+	u32 ppr;
 
 	if (!apic_enabled(apic))
 		return -1;
 
-	apic_update_ppr(apic);
-	highest_irr = apic_find_highest_irr(apic);
-	if ((highest_irr == -1) ||
-	    ((highest_irr & 0xF0) <= kvm_lapic_get_reg(apic, APIC_PROCPRI)))
-		return -1;
-	return highest_irr;
+	__apic_update_ppr(apic, &ppr);
+	return apic_has_interrupt_for_ppr(apic, ppr);
 }
 
 int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
                   ` (3 preceding siblings ...)
  2016-12-19  9:47 ` [PATCH 4/6] KVM: lapic: avoid unnecessary KVM_REQ_EVENT on IRR scan Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
  2016-12-19  9:47 ` [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt Paolo Bonzini
  2016-12-19 13:05 ` [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Denis Plotnikov
  6 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

On PPR update, we set KVM_REQ_EVENT unconditionally anytime PPR is lowered.
But we can take into account IRR here already.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index f936644d8011..dc4ea8bdea9c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -607,7 +607,8 @@ static void apic_update_ppr(struct kvm_lapic *apic)
 {
 	u32 ppr;
 
-	if (__apic_update_ppr(apic, &ppr))
+	if (__apic_update_ppr(apic, &ppr) &&
+	    apic_has_interrupt_for_ppr(apic, ppr) != -1)
 		kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
                   ` (4 preceding siblings ...)
  2016-12-19  9:47 ` [PATCH 5/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update Paolo Bonzini
@ 2016-12-19  9:47 ` Paolo Bonzini
  2016-12-20 17:11   ` Paolo Bonzini
  2016-12-19 13:05 ` [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Denis Plotnikov
  6 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19  9:47 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov

On interrupt delivery the PPR can only grow, so it is impossible
that interrupt delivery results in KVM_REQ_EVENT.  Make this
clear by using __apic_update_ppr, and by not using apic_*_isr
for Hyper-V auto-EOI interrupts.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index dc4ea8bdea9c..4dc02482faf7 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2110,6 +2110,7 @@ int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
 	struct kvm_lapic *apic = vcpu->arch.apic;
+	u32 ppr;
 
 	if (vector == -1)
 		return -1;
@@ -2121,15 +2122,11 @@ int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 	 * because the process would deliver it through the IDT.
 	 */
 
-	apic_set_isr(vector, apic);
-	apic_update_ppr(apic);
-	apic_clear_irr(vector, apic);
-
-	if (test_bit(vector, vcpu_to_synic(vcpu)->auto_eoi_bitmap)) {
-		apic_clear_isr(vector, apic);
-		apic_update_ppr(apic);
-	}
+	if (!test_bit(vector, vcpu_to_synic(vcpu)->auto_eoi_bitmap))
+		apic_set_isr(vector, apic);
 
+	apic_clear_irr(vector, apic);
+	__apic_update_ppr(apic, &ppr);
 	return vector;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT
  2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
                   ` (5 preceding siblings ...)
  2016-12-19  9:47 ` [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt Paolo Bonzini
@ 2016-12-19 13:05 ` Denis Plotnikov
  2016-12-19 14:30   ` Paolo Bonzini
  6 siblings, 1 reply; 13+ messages in thread
From: Denis Plotnikov @ 2016-12-19 13:05 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm; +Cc: rkrcmar, rkagan



On 19.12.2016 12:47, Paolo Bonzini wrote:
> This is the result of cleaning up the places that set REQ_EVENT unnecessarily.
> The savings on self-IPI kvm-unit-tests are:
>
>     self_ipi_sti_nop                ~300 clock cycles
>     self_ipi_sti_hlt                ~300 clock cycles
>     self_ipi_tpr                    ~400 clock cycles
>     self_ipi_tpr_sti_nop            ~700 clock cycles
>     self_ipi_tpr_sti_hlt            ~700 clock cycles
>
> which is in the 5-10% range.  Please help me comparing this series with
> "[PATCH v2] KVM: x86: avoid redundant REQ_EVENT" on the real workload!
>
> Thanks,
>
> Paolo
This new patch-set does avoid unnecessary interrupt re-injections - checked.

The test (MS Windows, sending messages between multiple windows using 
windows-specific interface),
which showed performance growth with "[PATCH v2] KVM: x86: avoid 
redundant REQ_EVENT" showed pretty much the same performance level with 
this new patch-set.
The test score difference (+2.4% to [PATCH v2]) is within the tolerance 
range(5%).

The test score on mainstream v4.9 kernel CPU Intel i5-2400, guest 
Windows Server 2012 2VCPU, 2GB:

without patch: 31510 (+/- 4%)
    with patch: 36270 (+/- 5%)

difference = (36270-31510)/31510 * 100 = +15% -- looks good!

Thanks!

>
> Paolo Bonzini (6):
>   KVM: x86: add VCPU stat for KVM_REQ_EVENT processing
>   KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI
>   KVM: vmx: speed up TPR below threshold vmexits
>   KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update
>   KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update
>   KVM: lapic: do not scan IRR when delivering an interrupt
>
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/lapic.c            | 58 +++++++++++++++++++++++++----------------
>  arch/x86/kvm/lapic.h            |  1 +
>  arch/x86/kvm/vmx.c              |  2 +-
>  arch/x86/kvm/x86.c              |  2 ++
>  5 files changed, 41 insertions(+), 23 deletions(-)
>

-- 
Best,
Denis

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT
  2016-12-19 13:05 ` [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Denis Plotnikov
@ 2016-12-19 14:30   ` Paolo Bonzini
  2016-12-20 10:01     ` Roman Kagan
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-19 14:30 UTC (permalink / raw)
  To: Denis Plotnikov, linux-kernel, kvm; +Cc: rkrcmar, rkagan



On 19/12/2016 14:05, Denis Plotnikov wrote:
>>
> This new patch-set does avoid unnecessary interrupt re-injections -
> checked.
> 
> The test (MS Windows, sending messages between multiple windows using
> windows-specific interface),
> which showed performance growth with "[PATCH v2] KVM: x86: avoid
> redundant REQ_EVENT" showed pretty much the same performance level with
> this new patch-set.
> The test score difference (+2.4% to [PATCH v2]) is within the tolerance
> range(5%).
> 
> The test score on mainstream v4.9 kernel CPU Intel i5-2400, guest
> Windows Server 2012 2VCPU, 2GB:
> 
> without patch: 31510 (+/- 4%)
>    with patch: 36270 (+/- 5%)
> 
> difference = (36270-31510)/31510 * 100 = +15% -- looks good!

Awesome!  I hope it also qualifies as less ugly. :)

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing
       [not found]   ` <20161220092101.GE2081@rkaganb.sw.ru>
@ 2016-12-20  9:32     ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-20  9:32 UTC (permalink / raw)
  To: Roman Kagan, linux-kernel, kvm, rkrcmar, dplotnikov



On 20/12/2016 10:21, Roman Kagan wrote:
> On Mon, Dec 19, 2016 at 10:47:13AM +0100, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  arch/x86/include/asm/kvm_host.h | 1 +
>>  arch/x86/kvm/x86.c              | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 994d8ed9fc6c..08cfd45a9452 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -850,6 +850,7 @@ struct kvm_vcpu_stat {
>>  	u64 hypercalls;
>>  	u64 irq_injections;
>>  	u64 nmi_injections;
>> +	u64 req_event;
>>  };
>>  
>>  struct x86_instruction_info;
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index e95d94edbdc3..e9b512090865 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -180,6 +180,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>  	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
>>  	{ "irq_injections", VCPU_STAT(irq_injections) },
>>  	{ "nmi_injections", VCPU_STAT(nmi_injections) },
>> +	{ "req_event", VCPU_STAT(req_event) },
>>  	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
>>  	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
>>  	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
>> @@ -6691,6 +6692,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>  	}
>>  
>>  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
>> +		++vcpu->stat.req_event;
>>  		kvm_apic_accept_events(vcpu);
>>  		if (vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED) {
>>  			r = 1;
> 
> Just curious what kind of information you expect to be able to extract
> from this counter?  I mean, I'm not opposed to introducing it, I just
> want to figure out how to use it properly.  Compare against
> irq_injections?

Yes, exactly.  Since vmexit cycle counts are always a bit noisy, I could
compare irq_injections against req_event and get an idea of the expected
improvement.

Also for example the stat shows that the sti_hlt case has a higher # of
req_event than sti_nop.  I have a patch to fix that, but it's left for
later because I don't know of a workload that triggers it.

I don't really need this patch of course.

> Also I guess it may be interesting to count branching to
> cancel_injection label in vcpu_enter_guest.

Interesting one too.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT
  2016-12-19 14:30   ` Paolo Bonzini
@ 2016-12-20 10:01     ` Roman Kagan
  0 siblings, 0 replies; 13+ messages in thread
From: Roman Kagan @ 2016-12-20 10:01 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Denis Plotnikov, linux-kernel, kvm, rkrcmar

On Mon, Dec 19, 2016 at 03:30:17PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/12/2016 14:05, Denis Plotnikov wrote:
> >>
> > This new patch-set does avoid unnecessary interrupt re-injections -
> > checked.
> > 
> > The test (MS Windows, sending messages between multiple windows using
> > windows-specific interface),
> > which showed performance growth with "[PATCH v2] KVM: x86: avoid
> > redundant REQ_EVENT" showed pretty much the same performance level with
> > this new patch-set.
> > The test score difference (+2.4% to [PATCH v2]) is within the tolerance
> > range(5%).
> > 
> > The test score on mainstream v4.9 kernel CPU Intel i5-2400, guest
> > Windows Server 2012 2VCPU, 2GB:
> > 
> > without patch: 31510 (+/- 4%)
> >    with patch: 36270 (+/- 5%)
> > 
> > difference = (36270-31510)/31510 * 100 = +15% -- looks good!
> 
> Awesome!  I hope it also qualifies as less ugly. :)

Absolutely.  FWIW

Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>

Roman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt
  2016-12-19  9:47 ` [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt Paolo Bonzini
@ 2016-12-20 17:11   ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-20 17:11 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov



On 19/12/2016 10:47, Paolo Bonzini wrote:
> +	if (!test_bit(vector, vcpu_to_synic(vcpu)->auto_eoi_bitmap))
> +		apic_set_isr(vector, apic);
>  
> +	apic_clear_irr(vector, apic);
> +	__apic_update_ppr(apic, &ppr);

Hmm, EOI does apic_update_ppr, so for auto-EOI interrupts I think it's
safer to do apic_update_ppr instead.  You could have to interrupts
injected at the same time, and the lower-priority interrupt would be
lost if the higher-priority interrupt does automatic EOI.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI
  2016-12-19  9:47 ` [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI Paolo Bonzini
@ 2016-12-23 12:08   ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-12-23 12:08 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar, rkagan, dplotnikov



On 19/12/2016 10:47, Paolo Bonzini wrote:
> On EOI, there is no need to set KVM_REQ_EVENT unconditionally.  The PPR
> update is already setting it if resetting the ISR bit causes PPR to
> decrease.  Even a level-triggered IOAPIC interrupt will set KVM_REQ_EVENT
> on a reinjection (ioapic_service -> kvm_irq_delivery_to_apic and from
> there to __apic_accept_irq).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/lapic.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e35cbd44c505..70c7428b7a57 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1066,7 +1066,6 @@ static int apic_set_eoi(struct kvm_lapic *apic)
>  		kvm_hv_synic_send_eoi(apic->vcpu, vector);
>  
>  	kvm_ioapic_send_eoi(apic, vector);
> -	kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
>  	return vector;
>  }
>  
> @@ -1081,7 +1080,6 @@ 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);
>  }
>  EXPORT_SYMBOL_GPL(kvm_apic_set_eoi_accelerated);
>  
> 

This patch breaks Windows XP and 2003 (reverting it while keeping the
others results in 3 successful installs, compared to a failure rate of
12/13 with the patch included).

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-12-23 12:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19  9:47 [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Paolo Bonzini
2016-12-19  9:47 ` [PATCH 1/6] KVM: x86: add VCPU stat for KVM_REQ_EVENT processing Paolo Bonzini
     [not found]   ` <20161220092101.GE2081@rkaganb.sw.ru>
2016-12-20  9:32     ` Paolo Bonzini
2016-12-19  9:47 ` [PATCH 2/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on EOI Paolo Bonzini
2016-12-23 12:08   ` Paolo Bonzini
2016-12-19  9:47 ` [PATCH 3/6] KVM: vmx: speed up TPR below threshold vmexits Paolo Bonzini
2016-12-19  9:47 ` [PATCH 4/6] KVM: lapic: avoid unnecessary KVM_REQ_EVENT on IRR scan Paolo Bonzini
2016-12-19  9:47 ` [PATCH 5/6] KVM: lapic: do not set KVM_REQ_EVENT unnecessarily on PPR update Paolo Bonzini
2016-12-19  9:47 ` [PATCH 6/6] KVM: lapic: do not scan IRR when delivering an interrupt Paolo Bonzini
2016-12-20 17:11   ` Paolo Bonzini
2016-12-19 13:05 ` [RFC PATCH v3 0/6] KVM: x86: avoid redundant REQ_EVENT Denis Plotnikov
2016-12-19 14:30   ` Paolo Bonzini
2016-12-20 10:01     ` Roman Kagan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).