Patchwork [24/37] KVM: PPC: booke: rework rescheduling checks

login
register
mail settings
Submitter Alexander Graf
Date Feb. 24, 2012, 2:26 p.m.
Message ID <1330093591-19523-25-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/142913/
State Not Applicable
Headers show

Comments

Alexander Graf - Feb. 24, 2012, 2:26 p.m.
Instead of checking whether we should reschedule only when we exited
due to an interrupt, let's always check before entering the guest back
again. This gets the target more in line with the other archs.

Also while at it, generalize the whole thing so that eventually we could
have a single kvmppc_prepare_to_enter function for all ppc targets that
does signal and reschedule checking for us.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm_ppc.h |    2 +-
 arch/powerpc/kvm/book3s.c          |    4 ++-
 arch/powerpc/kvm/booke.c           |   70 ++++++++++++++++++++++++-----------
 3 files changed, 52 insertions(+), 24 deletions(-)
Bharat Bhushan - Feb. 27, 2012, 4:34 p.m.
> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of
> Alexander Graf
> Sent: Friday, February 24, 2012 7:56 PM
> To: kvm-ppc@vger.kernel.org
> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> Subject: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
> 
> Instead of checking whether we should reschedule only when we exited due to an
> interrupt, let's always check before entering the guest back again. This gets
> the target more in line with the other archs.
> 
> Also while at it, generalize the whole thing so that eventually we could have a
> single kvmppc_prepare_to_enter function for all ppc targets that does signal and
> reschedule checking for us.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/include/asm/kvm_ppc.h |    2 +-
>  arch/powerpc/kvm/book3s.c          |    4 ++-
>  arch/powerpc/kvm/booke.c           |   70 ++++++++++++++++++++++++-----------
>  3 files changed, 52 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index e709975..7f0a3da 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -95,7 +95,7 @@ extern int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
> extern void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu);  extern void
> kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
> 
> -extern void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
> +extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
>  extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);  extern void
> kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);  extern void
> kvmppc_core_queue_dec(struct kvm_vcpu *vcpu); diff --git
> a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index 7d54f4e..c8ead7b
> 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -258,7 +258,7 @@ static bool clear_irqprio(struct kvm_vcpu *vcpu, unsigned
> int priority)
>  	return true;
>  }
> 
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long *pending = &vcpu->arch.pending_exceptions;
>  	unsigned long old_pending = vcpu->arch.pending_exceptions; @@ -283,6
> +283,8 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> 
>  	/* Tell the guest about our interrupt status */
>  	kvmppc_update_int_pending(vcpu, *pending, old_pending);
> +
> +	return 0;
>  }
> 
>  pfn_t kvmppc_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn) diff --git
> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 9979be1..3fcec2c
> 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -439,8 +439,9 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu
> *vcpu)  }
> 
>  /* Check pending exceptions and deliver one, if possible. */ -void
> kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
> +	int r = 0;
>  	WARN_ON_ONCE(!irqs_disabled());
> 
>  	kvmppc_core_check_exceptions(vcpu);
> @@ -451,8 +452,44 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  		local_irq_disable();
> 
>  		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
> -		kvmppc_core_check_exceptions(vcpu);
> +		r = 1;
>  	};
> +
> +	return r;
> +}
> +
> +/*
> + * Common checks before entering the guest world.  Call with interrupts
> + * disabled.
> + *
> + * returns !0 if a signal is pending and check_signal is true  */
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> +check_signal) {
> +	int r = 0;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +	while (true) {
> +		if (need_resched()) {
> +			local_irq_enable();
> +			cond_resched();
> +			local_irq_disable();
> +			continue;
> +		}
> +
> +		if (kvmppc_core_prepare_to_enter(vcpu)) {

kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should not this be called only on lightweight_exit?

Thanks
-Bharat

> +			/* interrupts got enabled in between, so we
> +			   are back at square 1 */
> +			continue;
> +		}
> +
> +		if (check_signal && signal_pending(current))
> +			r = 1;
> +
> +		break;
> +	}
> +
> +	return r;
>  }
> 
>  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) @@ -470,10
> +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
>  	}
> 
>  	local_irq_disable();
> -
> -	kvmppc_core_prepare_to_enter(vcpu);
> -
> -	if (signal_pending(current)) {
> +	if (kvmppc_prepare_to_enter(vcpu, true)) {
>  		kvm_run->exit_reason = KVM_EXIT_INTR;
>  		ret = -EINTR;
>  		goto out;
> @@ -598,25 +632,21 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
> 
>  	switch (exit_nr) {
>  	case BOOKE_INTERRUPT_MACHINE_CHECK:
> -		kvm_resched(vcpu);
>  		r = RESUME_GUEST;
>  		break;
> 
>  	case BOOKE_INTERRUPT_EXTERNAL:
>  		kvmppc_account_exit(vcpu, EXT_INTR_EXITS);
> -		kvm_resched(vcpu);
>  		r = RESUME_GUEST;
>  		break;
> 
>  	case BOOKE_INTERRUPT_DECREMENTER:
>  		kvmppc_account_exit(vcpu, DEC_EXITS);
> -		kvm_resched(vcpu);
>  		r = RESUME_GUEST;
>  		break;
> 
>  	case BOOKE_INTERRUPT_DOORBELL:
>  		kvmppc_account_exit(vcpu, DBELL_EXITS);
> -		kvm_resched(vcpu);
>  		r = RESUME_GUEST;
>  		break;
> 
> @@ -865,19 +895,15 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>  		BUG();
>  	}
> 
> +	/*
> +	 * To avoid clobbering exit_reason, only check for signals if we
> +	 * aren't already exiting to userspace for some other reason.
> +	 */
>  	local_irq_disable();
> -
> -	kvmppc_core_prepare_to_enter(vcpu);
> -
> -	if (!(r & RESUME_HOST)) {
> -		/* To avoid clobbering exit_reason, only check for signals if
> -		 * we aren't already exiting to userspace for some other
> -		 * reason. */
> -		if (signal_pending(current)) {
> -			run->exit_reason = KVM_EXIT_INTR;
> -			r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> -			kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> -		}
> +	if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
> +		run->exit_reason = KVM_EXIT_INTR;
> +		r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> +		kvmppc_account_exit(vcpu, SIGNAL_EXITS);
>  	}
> 
>  	return r;
> --
> 1.6.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a
> message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
Alexander Graf - Feb. 27, 2012, 5:33 p.m.
On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
>
>> +}
>> +
>> +/*
>> + * Common checks before entering the guest world.  Call with interrupts
>> + * disabled.
>> + *
>> + * returns !0 if a signal is pending and check_signal is true  */
>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
>> +check_signal) {
>> +	int r = 0;
>> +
>> +	WARN_ON_ONCE(!irqs_disabled());
>> +	while (true) {
>> +		if (need_resched()) {
>> +			local_irq_enable();
>> +			cond_resched();
>> +			local_irq_disable();
>> +			continue;
>> +		}
>> +
>> +		if (kvmppc_core_prepare_to_enter(vcpu)) {
> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should not this be called only on lightweight_exit?

Yeah, we don't need to call it when exiting anyways. That's a functional 
change though, which this patch is trying not to introduce. So we should 
rather do that as a patch on top.

Alex
Scott Wood - Feb. 27, 2012, 7:28 p.m.
On 02/24/2012 08:26 AM, Alexander Graf wrote:
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long *pending = &vcpu->arch.pending_exceptions;
>  	unsigned long old_pending = vcpu->arch.pending_exceptions;
> @@ -283,6 +283,8 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  
>  	/* Tell the guest about our interrupt status */
>  	kvmppc_update_int_pending(vcpu, *pending, old_pending);
> +
> +	return 0;
>  }
>  
>  pfn_t kvmppc_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn)
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 9979be1..3fcec2c 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -439,8 +439,9 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Check pending exceptions and deliver one, if possible. */
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
> +	int r = 0;
>  	WARN_ON_ONCE(!irqs_disabled());
>  
>  	kvmppc_core_check_exceptions(vcpu);
> @@ -451,8 +452,44 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  		local_irq_disable();
>  
>  		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
> -		kvmppc_core_check_exceptions(vcpu);
> +		r = 1;
>  	};
> +
> +	return r;
> +}
> +
> +/*
> + * Common checks before entering the guest world.  Call with interrupts
> + * disabled.
> + *
> + * returns !0 if a signal is pending and check_signal is true
> + */
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool check_signal)
> +{
> +	int r = 0;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +	while (true) {
> +		if (need_resched()) {
> +			local_irq_enable();
> +			cond_resched();
> +			local_irq_disable();
> +			continue;
> +		}
> +
> +		if (kvmppc_core_prepare_to_enter(vcpu)) {
> +			/* interrupts got enabled in between, so we
> +			   are back at square 1 */
> +			continue;
> +		}
> +
> +
> +		if (check_signal && signal_pending(current))
> +			r = 1;

If there is a signal pending and MSR[WE] is set, we'll loop forever
without reaching this check.

-Scott

Patch

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index e709975..7f0a3da 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -95,7 +95,7 @@  extern int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
 extern void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
 
-extern void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
+extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
 extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
 extern void kvmppc_core_queue_dec(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 7d54f4e..c8ead7b 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -258,7 +258,7 @@  static bool clear_irqprio(struct kvm_vcpu *vcpu, unsigned int priority)
 	return true;
 }
 
-void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
+int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 {
 	unsigned long *pending = &vcpu->arch.pending_exceptions;
 	unsigned long old_pending = vcpu->arch.pending_exceptions;
@@ -283,6 +283,8 @@  void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 
 	/* Tell the guest about our interrupt status */
 	kvmppc_update_int_pending(vcpu, *pending, old_pending);
+
+	return 0;
 }
 
 pfn_t kvmppc_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 9979be1..3fcec2c 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -439,8 +439,9 @@  static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
 }
 
 /* Check pending exceptions and deliver one, if possible. */
-void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
+int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 {
+	int r = 0;
 	WARN_ON_ONCE(!irqs_disabled());
 
 	kvmppc_core_check_exceptions(vcpu);
@@ -451,8 +452,44 @@  void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 		local_irq_disable();
 
 		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
-		kvmppc_core_check_exceptions(vcpu);
+		r = 1;
 	};
+
+	return r;
+}
+
+/*
+ * Common checks before entering the guest world.  Call with interrupts
+ * disabled.
+ *
+ * returns !0 if a signal is pending and check_signal is true
+ */
+static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool check_signal)
+{
+	int r = 0;
+
+	WARN_ON_ONCE(!irqs_disabled());
+	while (true) {
+		if (need_resched()) {
+			local_irq_enable();
+			cond_resched();
+			local_irq_disable();
+			continue;
+		}
+
+		if (kvmppc_core_prepare_to_enter(vcpu)) {
+			/* interrupts got enabled in between, so we
+			   are back at square 1 */
+			continue;
+		}
+
+		if (check_signal && signal_pending(current))
+			r = 1;
+
+		break;
+	}
+
+	return r;
 }
 
 int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
@@ -470,10 +507,7 @@  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 	}
 
 	local_irq_disable();
-
-	kvmppc_core_prepare_to_enter(vcpu);
-
-	if (signal_pending(current)) {
+	if (kvmppc_prepare_to_enter(vcpu, true)) {
 		kvm_run->exit_reason = KVM_EXIT_INTR;
 		ret = -EINTR;
 		goto out;
@@ -598,25 +632,21 @@  int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
 
 	switch (exit_nr) {
 	case BOOKE_INTERRUPT_MACHINE_CHECK:
-		kvm_resched(vcpu);
 		r = RESUME_GUEST;
 		break;
 
 	case BOOKE_INTERRUPT_EXTERNAL:
 		kvmppc_account_exit(vcpu, EXT_INTR_EXITS);
-		kvm_resched(vcpu);
 		r = RESUME_GUEST;
 		break;
 
 	case BOOKE_INTERRUPT_DECREMENTER:
 		kvmppc_account_exit(vcpu, DEC_EXITS);
-		kvm_resched(vcpu);
 		r = RESUME_GUEST;
 		break;
 
 	case BOOKE_INTERRUPT_DOORBELL:
 		kvmppc_account_exit(vcpu, DBELL_EXITS);
-		kvm_resched(vcpu);
 		r = RESUME_GUEST;
 		break;
 
@@ -865,19 +895,15 @@  int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		BUG();
 	}
 
+	/*
+	 * To avoid clobbering exit_reason, only check for signals if we
+	 * aren't already exiting to userspace for some other reason.
+	 */
 	local_irq_disable();
-
-	kvmppc_core_prepare_to_enter(vcpu);
-
-	if (!(r & RESUME_HOST)) {
-		/* To avoid clobbering exit_reason, only check for signals if
-		 * we aren't already exiting to userspace for some other
-		 * reason. */
-		if (signal_pending(current)) {
-			run->exit_reason = KVM_EXIT_INTR;
-			r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
-			kvmppc_account_exit(vcpu, SIGNAL_EXITS);
-		}
+	if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
+		run->exit_reason = KVM_EXIT_INTR;
+		r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
+		kvmppc_account_exit(vcpu, SIGNAL_EXITS);
 	}
 
 	return r;