[Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Yonit Halperin]

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
---
 hw/qxl.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..5dfda11 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
     qxl_set_irq(d);
 }
 
-/* called from spice server thread context only */
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
 {
     uint32_t old_pending;
@@ -1463,7 +1462,15 @@ static void qxl_vm_change_state_handler(void *opaque, int running, int reason)
     PCIQXLDevice *qxl = opaque;
     qemu_spice_vm_change_state_handler(&qxl->ssd, running, reason);
 
-    if (!running && qxl->mode == QXL_MODE_NATIVE) {
+    if (running) {
+        if (qxl->ram->int_pending) {
+            /*
+             * if qxl_send_events was called from spice server context before
+             * migration ended, qxl_set_irq for these events might not have been called
+             */
+            qxl_set_irq(qxl);
+        }
+    } else if (qxl->mode == QXL_MODE_NATIVE) {
         /* dirty all vram (which holds surfaces) and devram (primary surface)
          * to make sure they are saved */
         /* FIXME #1: should go out during "live" stage */
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Gerd Hoffmann]

On 08/31/11 10:20, Yonit Halperin wrote:
> if qxl_send_events was called from spice server context, and then
> migration had completed before a call to pipe_read, the target
> guest qxl driver didn't get the interrupt. In addition,
> qxl_send_events ignored further interrupts of the same kind, since
> ram->int_pending was set. As a result, the guest driver was stacked
> or very slow (when the waiting for the interrupt was with timeout).

> -    if (!running&&  qxl->mode == QXL_MODE_NATIVE) {
> +    if (running) {
> +        if (qxl->ram->int_pending) {
> +            /*
> +             * if qxl_send_events was called from spice server context before
> +             * migration ended, qxl_set_irq for these events might not have been called
> +             */
> +            qxl_set_irq(qxl);
> +        }

You can call qxl_set_irq unconditionally,
it checks for int_pending anyway.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Yonit Halperin]

On 08/31/2011 01:23 PM, Gerd Hoffmann wrote:
> On 08/31/11 10:20, Yonit Halperin wrote:
>> if qxl_send_events was called from spice server context, and then
>> migration had completed before a call to pipe_read, the target
>> guest qxl driver didn't get the interrupt. In addition,
>> qxl_send_events ignored further interrupts of the same kind, since
>> ram->int_pending was set. As a result, the guest driver was stacked
>> or very slow (when the waiting for the interrupt was with timeout).
>
>> - if (!running&& qxl->mode == QXL_MODE_NATIVE) {
>> + if (running) {
>> + if (qxl->ram->int_pending) {
>> + /*
>> + * if qxl_send_events was called from spice server context before
>> + * migration ended, qxl_set_irq for these events might not have been
>> called
>> + */
>> + qxl_set_irq(qxl);
>> + }
>
> You can call qxl_set_irq unconditionally,
> it checks for int_pending anyway.
>
Hi,
qxl_set_irq doesn't test int_pending, but it will call qemu_set_irq 
with level=0 if !int_pending.
> cheers,
> Gerd
>

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Gerd Hoffmann]

   Hi,

>> You can call qxl_set_irq unconditionally,
>> it checks for int_pending anyway.

> Hi,
> qxl_set_irq doesn't test int_pending, but it will call qemu_set_irq with
> level=0 if !int_pending.

Yes.  Also checks int_mask.  That is fine, isn't it?

BTW: qxl_update_irq would be a better name, this is what it actually 
does: update irq line state from device state.

cheers,
   Gerd