diff mbox

[v4,2/9] ppc/xics: Fix migration failure with kernel-irqchip=off

Message ID 1474266577-11704-3-git-send-email-nikunj@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nikunj A. Dadhania Sept. 19, 2016, 6:29 a.m. UTC
With a single cpu VM running with kernel-irqchip=off and a flood ping
running in the guest. Migration fails once in few times.

Found that whenever there is an interrupt (in this case lsi int 3 from
e1000), we raise an interrupt using qemu_irq_pulse() and also see that
the kvm ioctl is complete.

67351@1468011062.810020:xics_set_irq_lsi set_irq_lsi: srcno 3 [irq 0x1003]
67351@1468011062.810031:xics_icp_irq cpu 0 trying to deliver irq 0x1003 priority 0x5
67351@1468011062.810038:xics_icp_raise raising IRQ new XIRR=0xff001003
new pending priority=0x5

After migration on the target side, interrupts(prio 0x5) are rejected as
there is a interrupt pending (pending_priority 0x5). Moreover, we never
get an icp_accept from the guest, so it hangs and crashes.

Basically, resend the irq pulse(lsi) to the guest.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/intc/xics.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Comments

David Gibson Sept. 21, 2016, 7:21 a.m. UTC | #1
On Mon, Sep 19, 2016 at 11:59:30AM +0530, Nikunj A Dadhania wrote:
> With a single cpu VM running with kernel-irqchip=off and a flood ping
> running in the guest. Migration fails once in few times.
> 
> Found that whenever there is an interrupt (in this case lsi int 3 from
> e1000), we raise an interrupt using qemu_irq_pulse() and also see that
> the kvm ioctl is complete.

Uh.. for an lsi qemu_irq_pulse() should not ever be used - that should
only be used for edge or message interrupts.

> 67351@1468011062.810020:xics_set_irq_lsi set_irq_lsi: srcno 3 [irq 0x1003]
> 67351@1468011062.810031:xics_icp_irq cpu 0 trying to deliver irq 0x1003 priority 0x5
> 67351@1468011062.810038:xics_icp_raise raising IRQ new XIRR=0xff001003
> new pending priority=0x5
> 
> After migration on the target side, interrupts(prio 0x5) are rejected as
> there is a interrupt pending (pending_priority 0x5). Moreover, we never
> get an icp_accept from the guest, so it hangs and crashes.

Sorry, I'm still having a lot of trouble following this description of
what's happening.

> Basically, resend the irq pulse(lsi) to the guest.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hw/intc/xics.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 69162f0..f765b08 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -209,7 +209,7 @@ static const TypeInfo xics_common_info = {
>  #define CPPR(ss)   (((ss)->xirr) >> 24)
>  
>  static void ics_reject(ICSState *ics, int nr);
> -static void ics_resend(ICSState *ics);
> +static void ics_resend(ICSState *ics, int server);
>  static void ics_eoi(ICSState *ics, int nr);
>  
>  static void icp_check_ipi(XICSState *xics, int server)
> @@ -238,7 +238,7 @@ static void icp_resend(XICSState *xics, int server)
>      if (ss->mfrr < CPPR(ss)) {
>          icp_check_ipi(xics, server);
>      }
> -    ics_resend(xics->ics);
> +    ics_resend(xics->ics, server);
>  }
>  
>  void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
> @@ -512,13 +512,24 @@ static void ics_reject(ICSState *ics, int nr)
>      }
>  }
>  
> -static void ics_resend(ICSState *ics)
> +static void ics_resend(ICSState *ics, int server)
>  {
>      int i;
> +    ICPState *ss = ics->xics->ss + server;
> +    ICSIRQState *irq;
>  
>      for (i = 0; i < ics->nr_irqs; i++) {
>          /* FIXME: filter by server#? */
> -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> +        irq = &ics->irqs[i];
> +        if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) {
> +            continue;
> +        }
> +
> +        if (irq->flags & XICS_FLAGS_IRQ_LSI) {
> +            if (irq->status & XICS_STATUS_SENT) {
> +                qemu_irq_raise(ss->output);
> +                continue;

Directly reraising the CPU irq line from an ics function rather than
an icp function seems very dubious.  It really seems like instead we
need to be recalculating the output line state from the ICP state,
after we've done all the ICS resends.

> +            }
>              resend_lsi(ics, i);
>          } else {
>              resend_msi(ics, i);
diff mbox

Patch

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 69162f0..f765b08 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -209,7 +209,7 @@  static const TypeInfo xics_common_info = {
 #define CPPR(ss)   (((ss)->xirr) >> 24)
 
 static void ics_reject(ICSState *ics, int nr);
-static void ics_resend(ICSState *ics);
+static void ics_resend(ICSState *ics, int server);
 static void ics_eoi(ICSState *ics, int nr);
 
 static void icp_check_ipi(XICSState *xics, int server)
@@ -238,7 +238,7 @@  static void icp_resend(XICSState *xics, int server)
     if (ss->mfrr < CPPR(ss)) {
         icp_check_ipi(xics, server);
     }
-    ics_resend(xics->ics);
+    ics_resend(xics->ics, server);
 }
 
 void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
@@ -512,13 +512,24 @@  static void ics_reject(ICSState *ics, int nr)
     }
 }
 
-static void ics_resend(ICSState *ics)
+static void ics_resend(ICSState *ics, int server)
 {
     int i;
+    ICPState *ss = ics->xics->ss + server;
+    ICSIRQState *irq;
 
     for (i = 0; i < ics->nr_irqs; i++) {
         /* FIXME: filter by server#? */
-        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
+        irq = &ics->irqs[i];
+        if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) {
+            continue;
+        }
+
+        if (irq->flags & XICS_FLAGS_IRQ_LSI) {
+            if (irq->status & XICS_STATUS_SENT) {
+                qemu_irq_raise(ss->output);
+                continue;
+            }
             resend_lsi(ics, i);
         } else {
             resend_msi(ics, i);