[Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gonglei (Arei)]

Hi,

For recent Linux guests, the usb tablet will be suspended when it is idle.
When Qemu detect the usb tablet event, will call uhci_wakeup(), which
according the value of port->ctrl to decide to call uhci_resume() or not. 
UHCI_PORT_RD bit is set to 1 for UHCI is resuming, and qemu will not call
uhci_resume(). The other way round, qemu will call uhci_resume() and then
post a interrupt to guest os, meanwhile set UHCI_PORT_RD bit to 1.

In the guests, after handle the interrupt, will call uhci_finish_suspend(), which
clear UHCI_PORT_RD bit, Then qemu will know that the UHCI resume
process has finished.

In the situation of save/restore guest os, when the guest os is saving 
memory iteratively, meanwhile the usb tablet work normally (by vnc). When qemu
has set the UHCI_PORT_RD bit to 1, and post a interrupt to guest os, but
guest os has hibernated this moment, so the interrupt cannot be handled
in time. So, the UHCI_PORT_RD bit will not be cleared by uhci_finish_suspend().

When we restore the guest os, and the usb tablet event happen again, but 
because of UHCI_PORT_RD bit being set to 1, qemu consider the guest os is
handling the interrupt of resuming UHCI, so will not post another interrupt.
For guest os, will not handle UHCI event because it not receive an interrupt,
which cause usb tablet freeze.  

So, we should provide a mechanism for avoiding losing interrupt of UHCI.
IMHO the LEGACY SUPPORT REGISTER will give us some help, and I have
finished a patch and tested it work well.

Anything idea? Thanks!


Best regards,
-Gonglei

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gerd Hoffmann]

  Hi,

> In the situation of save/restore guest os, when the guest os is saving 
> memory iteratively, meanwhile the usb tablet work normally (by vnc). When qemu
> has set the UHCI_PORT_RD bit to 1, and post a interrupt to guest os, but
> guest os has hibernated this moment, so the interrupt cannot be handled
> in time. So, the UHCI_PORT_RD bit will not be cleared by uhci_finish_suspend().

> When we restore the guest os, and the usb tablet event happen again, but 
> because of UHCI_PORT_RD bit being set to 1, qemu consider the guest os is
> handling the interrupt of resuming UHCI, so will not post another interrupt.

Another interrupt should not be needed.  If we don't deliver the
interrupt before suspend, we must deliver it after resume.  It must not
get lost, loosing interrupts can cause all sorts of problems.

There was a kernel bug recently causing lost interrupts on migration,
maybe this is the same (rh bug 1036478)?  Paolo?  Is the fix upstream?
Which kernel has it?

cheers,
  Gerd

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gonglei (Arei)]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Tuesday, May 13, 2014 9:27 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; Huangweidong (C); Michael S. Tsirkin
> Subject: Re: usb: usb tablet freeze when save/restore guest os
> 
>   Hi,
> 
> > In the situation of save/restore guest os, when the guest os is saving
> > memory iteratively, meanwhile the usb tablet work normally (by vnc). When
> qemu
> > has set the UHCI_PORT_RD bit to 1, and post a interrupt to guest os, but
> > guest os has hibernated this moment, so the interrupt cannot be handled
> > in time. So, the UHCI_PORT_RD bit will not be cleared by
> uhci_finish_suspend().
> 
> > When we restore the guest os, and the usb tablet event happen again, but
> > because of UHCI_PORT_RD bit being set to 1, qemu consider the guest os is
> > handling the interrupt of resuming UHCI, so will not post another interrupt.
> 
> Another interrupt should not be needed.  If we don't deliver the
> interrupt before suspend, we must deliver it after resume. 
>
The fact is that an interrupt has been delivered during suspend, and guest os don't 
response it.

>It must not get lost, loosing interrupts can cause all sorts of problems.
> 
Yes. Strongly agreed.

> There was a kernel bug recently causing lost interrupts on migration,
> maybe this is the same (rh bug 1036478)?  Paolo?  Is the fix upstream?
> Which kernel has it?
> 
Oh? Sounds good.

Cc'ing Paolo for attention. Thanks.

Best regards,
-Gonglei

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Paolo Bonzini]

Il 14/05/2014 03:47, Gonglei (Arei) ha scritto:
> > There was a kernel bug recently causing lost interrupts on migration,
> > maybe this is the same (rh bug 1036478)?  Paolo?  Is the fix upstream?
> > Which kernel has it?

It is in 3.15, but it only affects edge-triggered interrupts, so not UHCI.

Paolo

> Oh? Sounds good.
>
> Cc'ing Paolo for attention. Thanks.

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gonglei (Arei)]

Hi,

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Wednesday, May 14, 2014 6:26 PM
> To: Gonglei (Arei); Gerd Hoffmann
> Cc: qemu-devel@nongnu.org; Huangweidong (C); Michael S. Tsirkin
> Subject: Re: usb: usb tablet freeze when save/restore guest os
> 
> Il 14/05/2014 03:47, Gonglei (Arei) ha scritto:
> > > There was a kernel bug recently causing lost interrupts on migration,
> > > maybe this is the same (rh bug 1036478)?  Paolo?  Is the fix upstream?
> > > Which kernel has it?
> 
> It is in 3.15, but it only affects edge-triggered interrupts, so not UHCI.
> 
> Paolo
> 
Okay. 

Well then, may I post a formal patch for this issue, Gerd? Thanks.


Best regards,
-Gonglei

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gerd Hoffmann]

  Hi,

> Well then, may I post a formal patch for this issue, Gerd? Thanks.

I'd like to know what the root cause for the lost interrupt is.

Not implementing PIRQ enable could be it, especially as the guest os
seems to use it (otherwise your patch would have no effect).

The check for the PIRQ enable bit should be in uhci_update_irq though,
and you should check the single bit only, not the whole legacy support
register.

cheers,
  Gerd

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gonglei (Arei)]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Thursday, May 15, 2014 2:50 PM
> To: Gonglei (Arei)
> Cc: Paolo Bonzini; qemu-devel@nongnu.org; Huangweidong (C); Michael S.
> Tsirkin
> Subject: Re: usb: usb tablet freeze when save/restore guest os
> 
>   Hi,
> 
> > Well then, may I post a formal patch for this issue, Gerd? Thanks.
> 
> I'd like to know what the root cause for the lost interrupt is.
> 
Hi, Gerd. I must clarify the scene of this issue.

1)The problem occurred on Xen platform.	
the process of hibernate vm on Xen:
 suspend vm (pause all vcpus)
	call xc_save to save memory
		stop qemu (vm_stop)
			save qemu 
				destroy vm

2)The process of hibernate vm on KVM:
 vm_stop
	do_vm_stop
		pause_all_vcpus
		runstate_set(state); //change the runstate to paused

3)the usb tablet backtrace:
vnc_client_read
	protocol_client_msg
		pointer_event
			qemu_input_event_sync
in qemu_input_event_sync(), there is a check vm runstate, as below:

if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) {
    return;
}
On KVM platform, the usb tablet event will be return at there.

As for Xen, It's too later. After suspend vm, the qemu process can response the event
of usb tablet event. Because guest os's vcpus are paused, guest os cannot response 
interrupt injected by qemu. Then the interrupt will be lost.

> Not implementing PIRQ enable could be it, especially as the guest os
> seems to use it (otherwise your patch would have no effect).
> 
> The check for the PIRQ enable bit should be in uhci_update_irq though,
> and you should check the single bit only, not the whole legacy support
> register.
> 
Agreed. Thanks.

> cheers,
>   Gerd
> 
> 

Best regards,
-Gonglei

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gerd Hoffmann]

  Hi,

> As for Xen, It's too later. After suspend vm, the qemu process can response the event
> of usb tablet event. Because guest os's vcpus are paused, guest os cannot response 
> interrupt injected by qemu. Then the interrupt will be lost.

When the guest can't handle the interrupt (due to being stopped) it
should continue to be tagged pending in the ioapic.  And this should be
migrated over with the ioapic state, so the guest should receive and
handle the interrupt after being restarted to the target machine.

IIRC ioapic migration works differently in kvm and xen.  Maybe you've
hit a xen live migration bug.

cheers,
  Gerd

Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

[Gonglei (Arei)]

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/hcd-uhci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 0820244..fe0ad81 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -680,6 +680,12 @@ static void uhci_wakeup(USBPort *port1)
 {
     UHCIState *s = port1->opaque;
     UHCIPort *port = &s->ports[port1->index];
+    uint8_t *pci_conf = s->dev.config;
+
+    /* Do not resume UHCI when UHCI PIRQ was disabled */
+    if (pci_conf[0xc1] == 0x00 && pci_conf[0xc0] == 0x00) {
+        return;
+    }

     if (port->ctrl & UHCI_PORT_SUSPEND && !(port->ctrl & UHCI_PORT_RD)) {
         port->ctrl |= UHCI_PORT_RD;
@@ -1241,6 +1247,10 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
     /* TODO: reset value should be 0. */
     pci_conf[USB_SBRN] = USB_RELEASE_1; // release number

+    /* LEGACY SUPPORT REGISTER - UHCI11D 5.2.1 */
+    pci_conf[0xc0] |= 0x00;
+    pci_conf[0xc1] |= 0x20; /* Enable USB PIRQ */
+
     pci_config_set_interrupt_pin(pci_conf, u->info.irq_pin + 1);

     if (s->masterbus) {
-- 
1.7.12.4

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, May 13, 2014 5:05 PM
> To: qemu-devel@nongnu.org
> Cc: 'Gerd Hoffmann'; Huangweidong (C); 'Michael S. Tsirkin'
> Subject: usb: usb tablet freeze when save/restore guest os
> 
> Hi,
> 
> For recent Linux guests, the usb tablet will be suspended when it is idle.
> When Qemu detect the usb tablet event, will call uhci_wakeup(), which
> according the value of port->ctrl to decide to call uhci_resume() or not.
> UHCI_PORT_RD bit is set to 1 for UHCI is resuming, and qemu will not call
> uhci_resume(). The other way round, qemu will call uhci_resume() and then
> post a interrupt to guest os, meanwhile set UHCI_PORT_RD bit to 1.
> 
> In the guests, after handle the interrupt, will call uhci_finish_suspend(), which
> clear UHCI_PORT_RD bit, Then qemu will know that the UHCI resume
> process has finished.
> 
> In the situation of save/restore guest os, when the guest os is saving
> memory iteratively, meanwhile the usb tablet work normally (by vnc). When
> qemu
> has set the UHCI_PORT_RD bit to 1, and post a interrupt to guest os, but
> guest os has hibernated this moment, so the interrupt cannot be handled
> in time. So, the UHCI_PORT_RD bit will not be cleared by uhci_finish_suspend().
> 
> When we restore the guest os, and the usb tablet event happen again, but
> because of UHCI_PORT_RD bit being set to 1, qemu consider the guest os is
> handling the interrupt of resuming UHCI, so will not post another interrupt.
> For guest os, will not handle UHCI event because it not receive an interrupt,
> which cause usb tablet freeze.
> 
> So, we should provide a mechanism for avoiding losing interrupt of UHCI.
> IMHO the LEGACY SUPPORT REGISTER will give us some help, and I have
> finished a patch and tested it work well.
> 
> Anything idea? Thanks!
> 
> 
> Best regards,
> -Gonglei
>