All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Tokarev <mjt@tls.msk.ru>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: qemu-trivial@nongnu.org, qemu-stable <qemu-stable@nongnu.org>,
	Alon Levy <alevy@redhat.com>, QEMU-devel <qemu-devel@nongnu.org>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-trivial] [PATCH] qxl: Fix QXLRam initialisation.
Date: Tue, 18 Jun 2013 07:01:53 +0400	[thread overview]
Message-ID: <51BFCDA1.8040507@msgid.tls.msk.ru> (raw)
In-Reply-To: <1371487106-17194-1-git-send-email-anthony.perard@citrix.com>

17.06.2013 20:38, Anthony PERARD wrote:
> The qxl driver expect NULL for QXLRam.memory_configs, but this is never
> initialized.
> 
> If memory is set to 0xc2c2.., it leads to a spice-critical error when
> trying to start qxl.

This bug has been introduced by:

 commit 020af1c45fec664d5d4cf3b8e5117f8bc1d691f2
 Author: Alon Levy <alevy@redhat.com>
 Date:   Wed Aug 22 11:16:25 2012 +0300

    qxl: add QXL_IO_MONITORS_CONFIG_ASYNC

which went in past 1.2.0, so it might be a good candidate for -stable
as well.

The issue looks real indeed, and is a good catch.

But maybe instead of doing it this way, we should just memset the
whole thing to 0, to avoid similar bugs in the future?  Are there
any arches out there which have cpu_to_le32(0) != 0 ?

Thanks,

/mjt

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hw/display/qxl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c475cb1..937a402 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -387,6 +387,7 @@ static void init_qxl_ram(PCIQXLDevice *d)
>      d->ram->int_pending = cpu_to_le32(0);
>      d->ram->int_mask    = cpu_to_le32(0);
>      d->ram->update_surface = 0;
> +    d->ram->monitors_config = 0;
>      SPICE_RING_INIT(&d->ram->cmd_ring);
>      SPICE_RING_INIT(&d->ram->cursor_ring);
>      SPICE_RING_INIT(&d->ram->release_ring);
> 



WARNING: multiple messages have this Message-ID (diff)
From: Michael Tokarev <mjt@tls.msk.ru>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: qemu-trivial@nongnu.org, qemu-stable <qemu-stable@nongnu.org>,
	Alon Levy <alevy@redhat.com>, QEMU-devel <qemu-devel@nongnu.org>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] qxl: Fix QXLRam initialisation.
Date: Tue, 18 Jun 2013 07:01:53 +0400	[thread overview]
Message-ID: <51BFCDA1.8040507@msgid.tls.msk.ru> (raw)
In-Reply-To: <1371487106-17194-1-git-send-email-anthony.perard@citrix.com>

17.06.2013 20:38, Anthony PERARD wrote:
> The qxl driver expect NULL for QXLRam.memory_configs, but this is never
> initialized.
> 
> If memory is set to 0xc2c2.., it leads to a spice-critical error when
> trying to start qxl.

This bug has been introduced by:

 commit 020af1c45fec664d5d4cf3b8e5117f8bc1d691f2
 Author: Alon Levy <alevy@redhat.com>
 Date:   Wed Aug 22 11:16:25 2012 +0300

    qxl: add QXL_IO_MONITORS_CONFIG_ASYNC

which went in past 1.2.0, so it might be a good candidate for -stable
as well.

The issue looks real indeed, and is a good catch.

But maybe instead of doing it this way, we should just memset the
whole thing to 0, to avoid similar bugs in the future?  Are there
any arches out there which have cpu_to_le32(0) != 0 ?

Thanks,

/mjt

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hw/display/qxl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c475cb1..937a402 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -387,6 +387,7 @@ static void init_qxl_ram(PCIQXLDevice *d)
>      d->ram->int_pending = cpu_to_le32(0);
>      d->ram->int_mask    = cpu_to_le32(0);
>      d->ram->update_surface = 0;
> +    d->ram->monitors_config = 0;
>      SPICE_RING_INIT(&d->ram->cmd_ring);
>      SPICE_RING_INIT(&d->ram->cursor_ring);
>      SPICE_RING_INIT(&d->ram->release_ring);
> 

  reply	other threads:[~2013-06-18  3:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-17 16:38 [Qemu-trivial] [PATCH] qxl: Fix QXLRam initialisation Anthony PERARD
2013-06-17 16:38 ` [Qemu-devel] " Anthony PERARD
2013-06-18  3:01 ` Michael Tokarev [this message]
2013-06-18  3:01   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2013-06-18  5:37   ` Gerd Hoffmann
2013-06-18  5:37     ` [Qemu-devel] " Gerd Hoffmann
2013-06-18 12:42     ` Anthony PERARD
2013-06-18 12:42       ` [Qemu-devel] " Anthony PERARD
2013-06-19  7:45 ` Michael Tokarev
2013-06-19  7:45   ` [Qemu-devel] " Michael Tokarev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51BFCDA1.8040507@msgid.tls.msk.ru \
    --to=mjt@tls.msk.ru \
    --cc=alevy@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.