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

[Michael S. Tsirkin]

On Wed, Aug 31, 2011 at 03:37:33PM +0300, 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.

This is a general issue with interrupt migration, and PCI core has code
to handle this, migrating interrupts.  So rather than work around this
in qxl I'd like us to first understand whether there really exists such
a problem, since if yes it would affect other devices.

Could you help with that please?

> In addition,
> qxl_send_events ignored further interrupts of the same kind, since
> ram->int_pending was set.

Maybe this is the only issue?
A way to check would be to call
    uint32_t pending = le32_to_cpu(d->ram->int_pending);
    uint32_t mask    = le32_to_cpu(d->ram->int_mask);
    int level = !!(pending & mask);
    qxl_ring_set_dirty(d);

instead of qxl_set_irq, and see if that is enough.

Note: I don't object to reusing qxl_set_irq in
production, just let us make sure we don't hide bugs.

> As a result, the guest driver was stacked
> or very slow (when the waiting for the interrupt was with timeout).

You need to sign off :)

> ---
>  hw/qxl.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index b34bccf..c7edc60 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,13 @@ 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_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
>