[for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[Greg Kurz]

Guest hangs have been observed recently on POWER9 hosts, specifically LC92x
"Boston" systems, when the guests are being rebooted multiple times. The
issue isn't POWER9 specific though. It is caused by a very long standing bug
when using the uncommon accel=kvm,kernel-irqchip=off machine configuration
which happens to be enforced on LC92x because of a host FW limitation. This
affects both the XICS and XIVE emulated interrupt controllers.

The actual fix is in patch 1. Patch 2 is a followup cleanup. The other
patches are unrelated cleanups I came up with while investigating.

Since this bug always existed and we're already in rc4, I think it is better
to fix it in 5.0 and possibly backport it to stable and downstream if needed.

--
Greg

---

Greg Kurz (4):
      ppc: Deassert the external interrupt pin in KVM on reset
      xics: Don't deassert outputs
      ppc: Don't use CPUPPCState::irq_input_state with modern Book3s CPU models
      ppc: Ignore the CPU_INTERRUPT_EXITTB interrupt with KVM


 hw/intc/xics.c                  |    3 ---
 hw/ppc/ppc.c                    |   24 ++++++++++--------------
 include/hw/ppc/ppc.h            |    2 ++
 target/ppc/cpu.h                |    4 +++-
 target/ppc/helper_regs.h        |    5 +++++
 target/ppc/translate_init.inc.c |    1 +
 6 files changed, 21 insertions(+), 18 deletions(-)

[for-5.0 PATCH 1/4] ppc: Deassert the external interrupt pin in KVM on reset

[Greg Kurz]

When a CPU is reset, QEMU makes sure no interrupt is pending by clearing
CPUPPCstate::pending_interrupts in ppc_cpu_reset(). In the case of a
complete machine emulation, eg. a sPAPR machine, an external interrupt
request could still be pending in KVM though, eg. an IPI. It will be
eventually presented to the guest, which is supposed to acknowledge it at
the interrupt controller. If the interrupt controller is emulated in QEMU,
either XICS or XIVE, ppc_set_irq() won't deassert the external interrupt
pin in KVM since it isn't pending anymore for QEMU. When the vCPU re-enters
the guest, the interrupt request is still pending and the vCPU will try
again to acknowledge it. This causes an infinite loop and eventually hangs
the guest.

The code has been broken since the beginning. The issue wasn't hit before
because accel=kvm,kernel-irqchip=off is an awkward setup that never got
used until recently with the LC92x IBM systems (aka, Boston).

Add a ppc_irq_reset() function to do the necessary cleanup, ie. deassert
the IRQ pins of the CPU in QEMU and most importantly the external interrupt
pin for this vCPU in KVM.

Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/ppc.c                    |    8 ++++++++
 include/hw/ppc/ppc.h            |    2 ++
 target/ppc/translate_init.inc.c |    1 +
 3 files changed, 11 insertions(+)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 8dd982fc1e40..fab73f1b1fc9 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1515,3 +1515,11 @@ PowerPCCPU *ppc_get_vcpu_by_pir(int pir)
 
     return NULL;
 }
+
+void ppc_irq_reset(PowerPCCPU *cpu)
+{
+    CPUPPCState *env = &cpu->env;
+
+    env->irq_input_state = 0;
+    kvmppc_set_interrupt(cpu, PPC_INTERRUPT_EXT, 0);
+}
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index 585be6ab98c5..89e1dd065af7 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -77,6 +77,7 @@ static inline void ppc970_irq_init(PowerPCCPU *cpu) {}
 static inline void ppcPOWER7_irq_init(PowerPCCPU *cpu) {}
 static inline void ppcPOWER9_irq_init(PowerPCCPU *cpu) {}
 static inline void ppce500_irq_init(PowerPCCPU *cpu) {}
+static inline void ppc_irq_reset(PowerPCCPU *cpu) {}
 #else
 void ppc40x_irq_init(PowerPCCPU *cpu);
 void ppce500_irq_init(PowerPCCPU *cpu);
@@ -84,6 +85,7 @@ void ppc6xx_irq_init(PowerPCCPU *cpu);
 void ppc970_irq_init(PowerPCCPU *cpu);
 void ppcPOWER7_irq_init(PowerPCCPU *cpu);
 void ppcPOWER9_irq_init(PowerPCCPU *cpu);
+void ppc_irq_reset(PowerPCCPU *cpu);
 #endif
 
 /* PPC machines for OpenBIOS */
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index ba726dec4d00..64a838095c7a 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10461,6 +10461,7 @@ static void ppc_cpu_reset(CPUState *s)
     env->pending_interrupts = 0;
     s->exception_index = POWERPC_EXCP_NONE;
     env->error_code = 0;
+    ppc_irq_reset(cpu);
 
     /* tininess for underflow is detected before rounding */
     set_float_detect_tininess(float_tininess_before_rounding,

[for-5.0 PATCH 2/4] xics: Don't deassert outputs

[Greg Kurz]

The correct way to do this is to deassert the input pins on the CPU side.
This is the case since a previous change.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xics.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 0b259a09c545..1952009e6d22 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -289,9 +289,6 @@ void icp_reset(ICPState *icp)
     icp->pending_priority = 0xff;
     icp->mfrr = 0xff;
 
-    /* Make all outputs are deasserted */
-    qemu_set_irq(icp->output, 0);
-
     if (kvm_irqchip_in_kernel()) {
         Error *local_err = NULL;
 

[for-5.0 PATCH 3/4] ppc: Don't use CPUPPCState::irq_input_state with modern Book3s CPU models

[Greg Kurz]

The power7_set_irq() and power9_set_irq() functions set this but it is
never used actually. Modern Book3s compatible CPUs are only supported
by the pnv and spapr machines. They have an interrupt controller, XICS
for POWER7/8 and XIVE for POWER9, whose models don't require to track
IRQ input states at the CPU level.

Drop these lines to avoid confusion.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/ppc.c     |   16 ++--------------
 target/ppc/cpu.h |    4 +++-
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index fab73f1b1fc9..45834f98d176 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -275,10 +275,9 @@ void ppc970_irq_init(PowerPCCPU *cpu)
 static void power7_set_irq(void *opaque, int pin, int level)
 {
     PowerPCCPU *cpu = opaque;
-    CPUPPCState *env = &cpu->env;
 
     LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+            &cpu->env, pin, level);
 
     switch (pin) {
     case POWER7_INPUT_INT:
@@ -292,11 +291,6 @@ static void power7_set_irq(void *opaque, int pin, int level)
         LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
         return;
     }
-    if (level) {
-        env->irq_input_state |= 1 << pin;
-    } else {
-        env->irq_input_state &= ~(1 << pin);
-    }
 }
 
 void ppcPOWER7_irq_init(PowerPCCPU *cpu)
@@ -311,10 +305,9 @@ void ppcPOWER7_irq_init(PowerPCCPU *cpu)
 static void power9_set_irq(void *opaque, int pin, int level)
 {
     PowerPCCPU *cpu = opaque;
-    CPUPPCState *env = &cpu->env;
 
     LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+            &cpu->env, pin, level);
 
     switch (pin) {
     case POWER9_INPUT_INT:
@@ -334,11 +327,6 @@ static void power9_set_irq(void *opaque, int pin, int level)
         LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
         return;
     }
-    if (level) {
-        env->irq_input_state |= 1 << pin;
-    } else {
-        env->irq_input_state &= ~(1 << pin);
-    }
 }
 
 void ppcPOWER9_irq_init(PowerPCCPU *cpu)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e3e82327b723..f9528fc29d98 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1090,7 +1090,9 @@ struct CPUPPCState {
 #if !defined(CONFIG_USER_ONLY)
     /*
      * This is the IRQ controller, which is implementation dependent
-     * and only relevant when emulating a complete machine.
+     * and only relevant when emulating a complete machine. Note that
+     * this isn't used by recent Book3s compatible CPUs (POWER7 and
+     * newer).
      */
     uint32_t irq_input_state;
     void **irq_inputs;

[for-5.0 PATCH 4/4] ppc: Ignore the CPU_INTERRUPT_EXITTB interrupt with KVM

[Greg Kurz]

This only makes sense with an emulated CPU. Don't set the bit in
CPUState::interrupt_request when using KVM to avoid confusions.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/helper_regs.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
index 85dfe7687fbb..d78c2af63eac 100644
--- a/target/ppc/helper_regs.h
+++ b/target/ppc/helper_regs.h
@@ -22,6 +22,7 @@
 
 #include "qemu/main-loop.h"
 #include "exec/exec-all.h"
+#include "sysemu/kvm.h"
 
 /* Swap temporary saved registers with GPRs */
 static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
@@ -102,6 +103,10 @@ static inline void hreg_compute_hflags(CPUPPCState *env)
 
 static inline void cpu_interrupt_exittb(CPUState *cs)
 {
+    if (!kvm_enabled()) {
+        return;
+    }
+
     if (!qemu_mutex_iothread_locked()) {
         qemu_mutex_lock_iothread();
         cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);

Re: [for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]

On Wed, Dec 04, 2019 at 08:43:31PM +0100, Greg Kurz wrote:
> Guest hangs have been observed recently on POWER9 hosts, specifically LC92x
> "Boston" systems, when the guests are being rebooted multiple times. The
> issue isn't POWER9 specific though. It is caused by a very long standing bug
> when using the uncommon accel=kvm,kernel-irqchip=off machine configuration
> which happens to be enforced on LC92x because of a host FW limitation. This
> affects both the XICS and XIVE emulated interrupt controllers.
> 
> The actual fix is in patch 1. Patch 2 is a followup cleanup. The other
> patches are unrelated cleanups I came up with while investigating.
> 
> Since this bug always existed and we're already in rc4, I think it is better
> to fix it in 5.0 and possibly backport it to stable and downstream if needed.

Applied to ppc-for-5.0.


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

On Mon, 9 Dec 2019 12:14:28 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Dec 04, 2019 at 08:43:31PM +0100, Greg Kurz wrote:
> > Guest hangs have been observed recently on POWER9 hosts, specifically LC92x
> > "Boston" systems, when the guests are being rebooted multiple times. The
> > issue isn't POWER9 specific though. It is caused by a very long standing bug
> > when using the uncommon accel=kvm,kernel-irqchip=off machine configuration
> > which happens to be enforced on LC92x because of a host FW limitation. This
> > affects both the XICS and XIVE emulated interrupt controllers.
> > 
> > The actual fix is in patch 1. Patch 2 is a followup cleanup. The other
> > patches are unrelated cleanups I came up with while investigating.
> > 
> > Since this bug always existed and we're already in rc4, I think it is better
> > to fix it in 5.0 and possibly backport it to stable and downstream if needed.
> 
> Applied to ppc-for-5.0.
> 
> 

According to Cornelia's comments, it seems I need to respin this against
the s390-next branch to avoid conflicts.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[Cornelia Huck]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

On Mon, 9 Dec 2019 11:59:47 +0100
Greg Kurz <groug@kaod.org> wrote:

> On Mon, 9 Dec 2019 12:14:28 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Dec 04, 2019 at 08:43:31PM +0100, Greg Kurz wrote:  
> > > Guest hangs have been observed recently on POWER9 hosts, specifically LC92x
> > > "Boston" systems, when the guests are being rebooted multiple times. The
> > > issue isn't POWER9 specific though. It is caused by a very long standing bug
> > > when using the uncommon accel=kvm,kernel-irqchip=off machine configuration
> > > which happens to be enforced on LC92x because of a host FW limitation. This
> > > affects both the XICS and XIVE emulated interrupt controllers.
> > > 
> > > The actual fix is in patch 1. Patch 2 is a followup cleanup. The other
> > > patches are unrelated cleanups I came up with while investigating.
> > > 
> > > Since this bug always existed and we're already in rc4, I think it is better
> > > to fix it in 5.0 and possibly backport it to stable and downstream if needed.  
> > 
> > Applied to ppc-for-5.0.
> > 
> >   
> 
> According to Cornelia's comments, it seems I need to respin this against
> the s390-next branch to avoid conflicts.


Aren't these ppc-only patches, though? Confused.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [for-5.0 PATCH 0/4] ppc: Fix interrupt controller emulation

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

On Mon, 9 Dec 2019 12:07:35 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, 9 Dec 2019 11:59:47 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Mon, 9 Dec 2019 12:14:28 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Wed, Dec 04, 2019 at 08:43:31PM +0100, Greg Kurz wrote:  
> > > > Guest hangs have been observed recently on POWER9 hosts, specifically LC92x
> > > > "Boston" systems, when the guests are being rebooted multiple times. The
> > > > issue isn't POWER9 specific though. It is caused by a very long standing bug
> > > > when using the uncommon accel=kvm,kernel-irqchip=off machine configuration
> > > > which happens to be enforced on LC92x because of a host FW limitation. This
> > > > affects both the XICS and XIVE emulated interrupt controllers.
> > > > 
> > > > The actual fix is in patch 1. Patch 2 is a followup cleanup. The other
> > > > patches are unrelated cleanups I came up with while investigating.
> > > > 
> > > > Since this bug always existed and we're already in rc4, I think it is better
> > > > to fix it in 5.0 and possibly backport it to stable and downstream if needed.  
> > > 
> > > Applied to ppc-for-5.0.
> > > 
> > >   
> > 
> > According to Cornelia's comments, it seems I need to respin this against
> > the s390-next branch to avoid conflicts.
> 
> 
> Aren't these ppc-only patches, though? Confused.

Oops... I've mixed up with the CPUReset series, sorry for the confusion :-)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]