* [PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling @ 2015-04-28 10:32 Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit Christian Borntraeger 0 siblings, 2 replies; 6+ messages in thread From: Christian Borntraeger @ 2015-04-28 10:32 UTC (permalink / raw) To: Paolo Bonzini Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf, Christian Borntraeger I was able to get rid of some nanoseconds for a guest exit loop on s390. I did my best to not break other architectures but review and comments on the general approach is welcome. Downside is that the existing irq_save things will just work no matter what the callers have done, the new code must do the right thing in the callers. Is that approach acceptible? Does anybody else see some measurable difference for guest exits? Christian Borntraeger (2): KVM: Push down irq_save to architectures before kvm_guest_enter KVM: push down irq_save from kvm_guest_exit arch/powerpc/kvm/book3s_hv.c | 4 ++++ arch/powerpc/kvm/book3s_pr.c | 2 ++ arch/powerpc/kvm/booke.c | 4 ++-- arch/s390/kvm/kvm-s390.c | 6 ++++-- arch/x86/kvm/x86.c | 2 ++ include/linux/kvm_host.h | 18 ++++++++---------- 6 files changed, 22 insertions(+), 14 deletions(-) -- 2.3.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter 2015-04-28 10:32 [PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling Christian Borntraeger @ 2015-04-28 10:32 ` Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit Christian Borntraeger 1 sibling, 0 replies; 6+ messages in thread From: Christian Borntraeger @ 2015-04-28 10:32 UTC (permalink / raw) To: Paolo Bonzini Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf, Christian Borntraeger local_irq_disable can be cheaper than local_irq_save, especially when done only once instead of twice. We can push down the local_irq_save (and replace it with local_irq_disable) to save some cycles. x86, mips and arm already disable the interrupts before calling kvm_guest_enter. Here we save one local_irq_save/restore pair. power and s390 are reworked to disable the interrupts before calling kvm_guest_enter. s390 saves a preempt_disable/enable pair but also saves some cycles as local_irq_disable/enable can be cheaper than local_irq_save/restore on some machines. power should be almost a no-op change (interrupts are disabled slighty longer). Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- arch/powerpc/kvm/book3s_hv.c | 2 ++ arch/s390/kvm/kvm-s390.c | 4 ++-- include/linux/kvm_host.h | 14 ++++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index de74756..a5f392d 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1779,7 +1779,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) spin_unlock(&vc->lock); + local_irq_disable(); kvm_guest_enter(); + local_irq_enable(); srcu_idx = srcu_read_lock(&vc->kvm->srcu); diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 46f37df..9f4c954 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2010,9 +2010,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) * As PF_VCPU will be used in fault handler, between * guest_enter and guest_exit should be no uaccess. */ - preempt_disable(); + local_irq_disable(); kvm_guest_enter(); - preempt_enable(); + local_irq_enable(); exit_reason = sie64a(vcpu->arch.sie_block, vcpu->run->s.regs.gprs); kvm_guest_exit(); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index d12b210..a34bf6ed 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -751,13 +751,15 @@ static inline void kvm_iommu_unmap_pages(struct kvm *kvm, static inline void kvm_guest_enter(void) { - unsigned long flags; - - BUG_ON(preemptible()); - - local_irq_save(flags); + /* + * guest_enter needs disabled irqs and rcu_virt_note_context_switch + * wants disabled preemption. Ensure that the caller has disabled + * irqs for kvm_guest_enter. Please note: Some architectures (e.g. + * s390) will reenable irqs before entering the guest, but this is + * ok. We just need a stable CPU for the accounting itself. + */ + WARN_ON(!irqs_disabled()); guest_enter(); - local_irq_restore(flags); /* KVM does not hold any references to rcu protected data when it * switches CPU into a guest mode. In fact switching to a guest mode -- 2.3.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit 2015-04-28 10:32 [PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter Christian Borntraeger @ 2015-04-28 10:32 ` Christian Borntraeger 2015-04-28 11:37 ` Paolo Bonzini 1 sibling, 1 reply; 6+ messages in thread From: Christian Borntraeger @ 2015-04-28 10:32 UTC (permalink / raw) To: Paolo Bonzini Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf, Christian Borntraeger Some architectures already have irq disabled when calling kvm_guest_exit. Push down the disabling into the architectures to avoid double disabling. This also allows to replace irq_save with irq_disable which might be cheaper. arm and mips already have interrupts disabled. s390/power/x86 need adoptions. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- arch/powerpc/kvm/book3s_hv.c | 2 ++ arch/powerpc/kvm/book3s_pr.c | 2 ++ arch/powerpc/kvm/booke.c | 4 ++-- arch/s390/kvm/kvm-s390.c | 2 ++ arch/x86/kvm/x86.c | 2 ++ include/linux/kvm_host.h | 4 ---- 6 files changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index a5f392d..63b35b1 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1811,7 +1811,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) /* make sure updates to secondary vcpu structs are visible now */ smp_mb(); + local_irq_disable(); kvm_guest_exit(); + local_irq_enable(); preempt_enable(); cond_resched(); diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index f573839..eafcb8c 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, /* We get here with MSR.EE=1 */ + local_irq_disable(); trace_kvm_exit(exit_nr, vcpu); + local_irq_enable(); kvm_guest_exit(); switch (exit_nr) { diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 6c1316a..f1d6af3 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1004,11 +1004,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; } - local_irq_enable(); - trace_kvm_exit(exit_nr, vcpu); kvm_guest_exit(); + local_irq_enable(); + run->exit_reason = KVM_EXIT_UNKNOWN; run->ready_for_interrupt_injection = 1; diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 9f4c954..0aa2534 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2015,7 +2015,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) local_irq_enable(); exit_reason = sie64a(vcpu->arch.sie_block, vcpu->run->s.regs.gprs); + local_irq_disable(); kvm_guest_exit(); + local_irq_enable(); vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); rc = vcpu_post_run(vcpu, exit_reason); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 32bf19e..a5fbd45 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6351,7 +6351,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) */ barrier(); + local_irq_disable(); kvm_guest_exit(); + local_irq_enable(); preempt_enable(); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index a34bf6ed..fe699fb 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void) static inline void kvm_guest_exit(void) { - unsigned long flags; - - local_irq_save(flags); guest_exit(); - local_irq_restore(flags); } /* -- 2.3.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit 2015-04-28 10:32 ` [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit Christian Borntraeger @ 2015-04-28 11:37 ` Paolo Bonzini 2015-04-28 14:10 ` Christian Borntraeger 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2015-04-28 11:37 UTC (permalink / raw) To: Christian Borntraeger Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf On 28/04/2015 12:32, Christian Borntraeger wrote: > Some architectures already have irq disabled when calling > kvm_guest_exit. Push down the disabling into the architectures > to avoid double disabling. This also allows to replace > irq_save with irq_disable which might be cheaper. > arm and mips already have interrupts disabled. s390/power/x86 > need adoptions. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/powerpc/kvm/book3s_hv.c | 2 ++ > arch/powerpc/kvm/book3s_pr.c | 2 ++ > arch/powerpc/kvm/booke.c | 4 ++-- > arch/s390/kvm/kvm-s390.c | 2 ++ > arch/x86/kvm/x86.c | 2 ++ > include/linux/kvm_host.h | 4 ---- > 6 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index a5f392d..63b35b1 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1811,7 +1811,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) > > /* make sure updates to secondary vcpu structs are visible now */ > smp_mb(); > + local_irq_disable(); > kvm_guest_exit(); > + local_irq_enable(); > > preempt_enable(); > cond_resched(); > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index f573839..eafcb8c 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c > @@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, > > /* We get here with MSR.EE=1 */ > > + local_irq_disable(); > trace_kvm_exit(exit_nr, vcpu); > + local_irq_enable(); > kvm_guest_exit(); This looks wrong. > > switch (exit_nr) { > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 6c1316a..f1d6af3 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -1004,11 +1004,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > break; > } > > - local_irq_enable(); > - > trace_kvm_exit(exit_nr, vcpu); > kvm_guest_exit(); > > + local_irq_enable(); > + > run->exit_reason = KVM_EXIT_UNKNOWN; > run->ready_for_interrupt_injection = 1; > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 9f4c954..0aa2534 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2015,7 +2015,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > local_irq_enable(); > exit_reason = sie64a(vcpu->arch.sie_block, > vcpu->run->s.regs.gprs); > + local_irq_disable(); > kvm_guest_exit(); > + local_irq_enable(); > vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > > rc = vcpu_post_run(vcpu, exit_reason); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 32bf19e..a5fbd45 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6351,7 +6351,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > */ > barrier(); > > + local_irq_disable(); > kvm_guest_exit(); > + local_irq_enable(); > > preempt_enable(); > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index a34bf6ed..fe699fb 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void) > > static inline void kvm_guest_exit(void) > { > - unsigned long flags; > - > - local_irq_save(flags); Why no WARN_ON here? I think the patches are sensible, especially since you can add warnings. This kind of code definitely knows if it has interrupts disabled or enabled Alternatively, the irq-disabled versions could be called __kvm_guest_{enter,exit}. Then you can use those directly when it makes sense. Paolo > guest_exit(); > - local_irq_restore(flags); > } > > /* > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit 2015-04-28 11:37 ` Paolo Bonzini @ 2015-04-28 14:10 ` Christian Borntraeger 2015-04-28 14:12 ` Paolo Bonzini 0 siblings, 1 reply; 6+ messages in thread From: Christian Borntraeger @ 2015-04-28 14:10 UTC (permalink / raw) To: Paolo Bonzini Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf Am 28.04.2015 um 13:37 schrieb Paolo Bonzini: >> --- a/arch/powerpc/kvm/book3s_pr.c >> +++ b/arch/powerpc/kvm/book3s_pr.c >> @@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, >> >> /* We get here with MSR.EE=1 */ >> >> + local_irq_disable(); >> trace_kvm_exit(exit_nr, vcpu); >> + local_irq_enable(); >> kvm_guest_exit(); > > This looks wrong. > Yes it is. >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void) >> >> static inline void kvm_guest_exit(void) >> { >> - unsigned long flags; >> - >> - local_irq_save(flags); > > Why no WARN_ON here? Yes,WARN_ON makes sense. Hmm, on the other hand the irqs_disabled call might be somewhat expensive again (would boil down on s390 to call stnsm (store and and system mask) once for query and once for setting. so... > > I think the patches are sensible, especially since you can add warnings. > This kind of code definitely knows if it has interrupts disabled or enabled > > Alternatively, the irq-disabled versions could be called > __kvm_guest_{enter,exit}. Then you can use those directly when it makes > sense. ..having a special __kvm_guest_{enter,exit} without the WARN_ON might be even the cheapest way. In that way I could leave everything besides s390 alone and arch maintainers can do a followup patch if appropriate. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit 2015-04-28 14:10 ` Christian Borntraeger @ 2015-04-28 14:12 ` Paolo Bonzini 0 siblings, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2015-04-28 14:12 UTC (permalink / raw) To: Christian Borntraeger Cc: KVM, kvm-ppc, kvmarm, linux-mips, Cornelia Huck, Alexander Graf On 28/04/2015 16:10, Christian Borntraeger wrote: > > Alternatively, the irq-disabled versions could be called > > __kvm_guest_{enter,exit}. Then you can use those directly when it makes > > sense. > > ..having a special __kvm_guest_{enter,exit} without the WARN_ON might be even > the cheapest way. In that way I could leave everything besides s390 alone and > arch maintainers can do a followup patch if appropriate. That's certainly fine with me. Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-28 14:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-04-28 10:32 [PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter Christian Borntraeger 2015-04-28 10:32 ` [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit Christian Borntraeger 2015-04-28 11:37 ` Paolo Bonzini 2015-04-28 14:10 ` Christian Borntraeger 2015-04-28 14:12 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox