[PATCHv2 0/2] KVM: micro-optimization and interrupt disabling

[PATCHv2 0/2] KVM: micro-optimization and interrupt disabling

[Christian Borntraeger]

This rework allows to avoid some cycles by not disabling interrupts
twice.

Christian Borntraeger (2):
  KVM: provide irq_unsafe kvm_guest_{enter|exit}
  KVM: arm/mips/x86/power use __kvm_guest_{enter|exit}

 arch/arm/kvm/arm.c         |  4 ++--
 arch/mips/kvm/mips.c       |  4 ++--
 arch/powerpc/kvm/powerpc.c |  2 +-
 arch/s390/kvm/kvm-s390.c   | 10 ++++++----
 arch/x86/kvm/x86.c         |  2 +-
 include/linux/kvm_host.h   | 27 ++++++++++++++++++---------
 6 files changed, 30 insertions(+), 19 deletions(-)

-- 
2.3.0

[PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Christian Borntraeger]

Several kvm architectures disable interrupts before kvm_guest_enter.
kvm_guest_enter then uses local_irq_save/restore to disable interrupts
again or for the first time. Lets provide underscore versions of
kvm_guest_{enter|exit} that assume being called locked.
kvm_guest_enter now disables interrupts for the full function and
thus we can remove the check for preemptible.

This patch then adopts s390/kvm to use local_irq_disable/enable calls
which are slighty cheaper that local_irq_save/restore and call these
new functions.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 10 ++++++----
 include/linux/kvm_host.h | 27 ++++++++++++++++++---------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 46f37df..c25d4e5 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2010,12 +2010,14 @@ 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();
-		kvm_guest_enter();
-		preempt_enable();
+		local_irq_disable();
+		__kvm_guest_enter();
+		local_irq_enable();
 		exit_reason = sie64a(vcpu->arch.sie_block,
 				     vcpu->run->s.regs.gprs);
-		kvm_guest_exit();
+		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/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d12b210..2d3b3ce 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -749,16 +749,10 @@ static inline void kvm_iommu_unmap_pages(struct kvm *kvm,
 }
 #endif
 
-static inline void kvm_guest_enter(void)
+/* must be called with irqs disabled */
+static inline void __kvm_guest_enter(void)
 {
-	unsigned long flags;
-
-	BUG_ON(preemptible());
-
-	local_irq_save(flags);
 	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
 	 * is very similar to exiting to userspace from rcu point of view. In
@@ -769,12 +763,27 @@ static inline void kvm_guest_enter(void)
 	rcu_virt_note_context_switch(smp_processor_id());
 }
 
+/* must be called with irqs disabled */
+static inline void __kvm_guest_exit(void)
+{
+	guest_exit();
+}
+
+static inline void kvm_guest_enter(void)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	__kvm_guest_enter();
+	local_irq_restore(flags);
+}
+
 static inline void kvm_guest_exit(void)
 {
 	unsigned long flags;
 
 	local_irq_save(flags);
-	guest_exit();
+	__kvm_guest_exit();
 	local_irq_restore(flags);
 }
 
-- 
2.3.0

[PATCH 2/2] KVM: arm/mips/x86/power use __kvm_guest_{enter|exit}

[Christian Borntraeger]

Use __kvm_guest_{enter|exit} instead of kvm_guest_{enter|exit}
where interrupts are disabled.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/arm/kvm/arm.c         | 4 ++--
 arch/mips/kvm/mips.c       | 4 ++--
 arch/powerpc/kvm/powerpc.c | 2 +-
 arch/x86/kvm/x86.c         | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 5560f74..050c274 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -533,13 +533,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		 * Enter the guest
 		 */
 		trace_kvm_entry(*vcpu_pc(vcpu));
-		kvm_guest_enter();
+		__kvm_guest_enter();
 		vcpu->mode = IN_GUEST_MODE;
 
 		ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
 
 		vcpu->mode = OUTSIDE_GUEST_MODE;
-		kvm_guest_exit();
+		__kvm_guest_exit();
 		trace_kvm_exit(kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu));
 		/*
 		 * We may have taken a host interrupt in HYP mode (ie
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index c9eccf5..539c12c 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -388,7 +388,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	kvm_mips_deliver_interrupts(vcpu,
 				    kvm_read_c0_guest_cause(vcpu->arch.cop0));
 
-	kvm_guest_enter();
+	__kvm_guest_enter();
 
 	/* Disable hardware page table walking while in guest */
 	htw_stop();
@@ -398,7 +398,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	/* Re-enable HTW before enabling interrupts */
 	htw_start();
 
-	kvm_guest_exit();
+	__kvm_guest_exit();
 	local_irq_enable();
 
 	if (vcpu->sigset_active)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 27c0fac..e5c0be5 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -115,7 +115,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 			continue;
 		}
 
-		kvm_guest_enter();
+		__kvm_guest_enter();
 		return 1;
 	}
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32bf19e..eb8aa96 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6292,7 +6292,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	if (req_immediate_exit)
 		smp_send_reschedule(vcpu->cpu);
 
-	kvm_guest_enter();
+	__kvm_guest_enter();
 
 	if (unlikely(vcpu->arch.switch_db_regs)) {
 		set_debugreg(0, 7);
-- 
2.3.0

Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Paolo Bonzini]

On 30/04/2015 13:43, Christian Borntraeger wrote:
> +/* must be called with irqs disabled */
> +static inline void __kvm_guest_enter(void)
>  {
> -	unsigned long flags;
> -
> -	BUG_ON(preemptible());

Please keep the BUG_ON() in kvm_guest_enter.  Otherwise looks good, thanks!

Paolo

> -	local_irq_save(flags);
>  	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
>  	 * is very similar to exiting to userspace from rcu point of view. In
> @@ -769,12 +763,27 @@ static inline void kvm_guest_enter(void)
>  	rcu_virt_note_context_switch(smp_processor_id());
>  }
>  
> +/* must be called with irqs disabled */
> +static inline void __kvm_guest_exit(void)
> +{
> +	guest_exit();
> +}
> +
> +static inline void kvm_guest_enter(void)
> +{
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	__kvm_guest_enter();
> +	local_irq_restore(flags);
> +}
> +

Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Christian Borntraeger]

Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
> 
> 
> On 30/04/2015 13:43, Christian Borntraeger wrote:
>> +/* must be called with irqs disabled */
>> +static inline void __kvm_guest_enter(void)
>>  {
>> -	unsigned long flags;
>> -
>> -	BUG_ON(preemptible());
> 
> Please keep the BUG_ON() in kvm_guest_enter.  Otherwise looks good, thanks!

would be 
	BUG_ON(!irqs_disabled())
then?

Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Christian Borntraeger]

Am 30.04.2015 um 14:01 schrieb Christian Borntraeger:
> Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
>>
>>
>> On 30/04/2015 13:43, Christian Borntraeger wrote:
>>> +/* must be called with irqs disabled */
>>> +static inline void __kvm_guest_enter(void)
>>>  {
>>> -	unsigned long flags;
>>> -
>>> -	BUG_ON(preemptible());
>>
>> Please keep the BUG_ON() in kvm_guest_enter.  Otherwise looks good, thanks!

Ah, you mean have the BUG_ON in the non underscore version? Yes, makes sense.

Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Christian Borntraeger]

Am 30.04.2015 um 14:02 schrieb Christian Borntraeger:
> Am 30.04.2015 um 14:01 schrieb Christian Borntraeger:
>> Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
>>>
>>>
>>> On 30/04/2015 13:43, Christian Borntraeger wrote:
>>>> +/* must be called with irqs disabled */
>>>> +static inline void __kvm_guest_enter(void)
>>>>  {
>>>> -	unsigned long flags;
>>>> -
>>>> -	BUG_ON(preemptible());
>>>
>>> Please keep the BUG_ON() in kvm_guest_enter.  Otherwise looks good, thanks!
> 
> Ah, you mean have the BUG_ON in the non underscore version? Yes, makes sense.

Hmmm, too quick.
the BUG_ON was there to make sure that rcu_virt_note_context_switch is safe.
The reworked code pulls the rcu_virt_note_context_switch within the irq_save
section so we no longer need this BUG_ON, no?

Christian

Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}

[Paolo Bonzini]

On 30/04/2015 14:07, Christian Borntraeger wrote:
>>>>> >>>> +static inline void __kvm_guest_enter(void)
>>>>> >>>>  {
>>>>> >>>> -	unsigned long flags;
>>>>> >>>> -
>>>>> >>>> -	BUG_ON(preemptible());
>>>> >>>
>>>> >>> Please keep the BUG_ON() in kvm_guest_enter.  Otherwise looks good, thanks!
>> > 
>> > Ah, you mean have the BUG_ON in the non underscore version? Yes, makes sense.
> Hmmm, too quick.
> the BUG_ON was there to make sure that rcu_virt_note_context_switch is safe.
> The reworked code pulls the rcu_virt_note_context_switch within the irq_save
> section so we no longer need this BUG_ON, no?

Right.  I can apply the patches then!

Paolo

Re: [PATCH 2/2] KVM: arm/mips/x86/power use __kvm_guest_{enter|exit}

[Christoffer Dall]

On Thu, Apr 30, 2015 at 01:43:31PM +0200, Christian Borntraeger wrote:
> Use __kvm_guest_{enter|exit} instead of kvm_guest_{enter|exit}
> where interrupts are disabled.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

For the ARM part:

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>