kvm/ppc: interrupt disabling fixes

Commit Message

Tiejun Chen May 7, 2013, 10 a.m. UTC

On 05/07/2013 11:32 AM, Scott Wood wrote:
> booke64 was not maintaing consistent lazy ee state when exiting the

One typo ;-)

s/maintaing/maintaining


> guest, leading to warnings and worse.
>
> booke32 was less affected due to the absence of lazy ee, but it was
> still feeding bad information into trace_hardirqs_off/on -- we don't
> want guest execution to be seen as an "IRQs off" interval.  book3s_pr
> also has this problem.
>
> book3s_pr and booke both used kvmppc_lazy_ee_enable() without
> hard-disabling EE first, which could lead to races when irq_happened is
> cleared, or if an interrupt happens after kvmppc_lazy_ee_enable(), and
> possibly other issues.
>
> Now, on book3s_pr and booke, always hard-disable interrupts before
> kvmppc_prepare_to_enter(), but leave them soft-enabled.  On book3s,
> this should results in the right lazy EE state when the asm code
> hard-enables on an exit.  On booke, we call hard_irq_disable() rather
> than hard-enable immediately.

Looks we always need to call hard_irq_disable() before 
kvmppc_prepare_to_enter(), so I think we can add hard_irq_disable() directly 
into kvmppc_prepare_to_enater() since this can avoid forgetting to call 
hard_irq_disable() when call kvmppc_prepare_to_enater() somewhere in the future.

Here I assume Ben's fix to hard_irq_disable() is before this :) So what about this?

Patch

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index d7339df..e4e2120 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -397,6 +397,13 @@  static inline void kvmppc_mmu_flush_icache(pfn_t pfn)
  static inline void kvmppc_lazy_ee_enable(void)
  {
  #ifdef CONFIG_PPC64
+	/*
+	 * To avoid races, the caller must have gone directly from having
+	 * interrupts fully-enabled to hard-disabled.
+	 */
+	WARN_ON(local_paca->irq_happened != PACA_IRQ_HARD_DIS);
+	trace_hardirqs_on();
+
  	/* Only need to enable IRQs by hard enabling them after this */
  	local_paca->irq_happened = 0;
  	local_paca->soft_enabled = 1;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index d09baf1..1ea65cd 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -884,7 +884,6 @@  program_interrupt:
  		 * and if we really did time things so badly, then we just exit
  		 * again due to a host external interrupt.
  		 */
-		local_irq_disable();
  		s = kvmppc_prepare_to_enter(vcpu);
  		if (s <= 0) {
  			local_irq_enable();
@@ -1121,7 +1120,6 @@  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct 
kvm_vcpu *vcpu)
  	 * really did time things so badly, then we just exit again due to
  	 * a host external interrupt.
  	 */
-	local_irq_disable();
  	ret = kvmppc_prepare_to_enter(vcpu);
  	if (ret <= 0) {
  		local_irq_enable();
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index dc1f590..d412749 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -666,7 +666,6 @@  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu 
*vcpu)
  		return -EINVAL;
  	}

-	local_irq_disable();
  	s = kvmppc_prepare_to_enter(vcpu);
  	if (s <= 0) {
  		local_irq_enable();
@@ -832,6 +831,10 @@  int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu 
*vcpu,
  {
  	int r = RESUME_HOST;
  	int s;
+#ifdef CONFIG_PPC64
+	WARN_ON(local_paca->irq_happened != 0);
+#endif
+	hard_irq_disable();

  	/* update before a new last_exit_type is rewritten */
  	kvmppc_update_timing_stats(vcpu);
@@ -1143,7 +1146,6 @@  int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
  	 * aren't already exiting to userspace for some other reason.
  	 */
  	if (!(r & RESUME_HOST)) {
-		local_irq_disable();
  		s = kvmppc_prepare_to_enter(vcpu);
  		if (s <= 0) {
  			local_irq_enable();
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 31084c6..147ac0e 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -64,7 +64,8 @@  int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
  {
  	int r = 1;

-	WARN_ON_ONCE(!irqs_disabled());
+	hard_irq_disable();
+
  	while (true) {
  		if (need_resched()) {
  			local_irq_enable();