Patchwork [v3] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC

login
register
mail settings
Submitter Alon Levy
Date July 28, 2012, 10:23 a.m.
Message ID <1343471002-14640-1-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/173861/
State New
Headers show

Comments

Alon Levy - July 28, 2012, 10:23 a.m.
bumps spice-protocol to 0.12.0 for new IO.
revision bumped to 4 for new IO support, enabled for spice-server >= 0.11.1
on migration reissue spice_qxl_monitors_config_async.

RHBZ: 770842

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 configure          |  2 +-
 hw/qxl.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
 hw/qxl.h           |  6 ++++++
 trace-events       |  1 +
 ui/spice-display.h |  1 +
 5 files changed, 56 insertions(+), 3 deletions(-)
Gerd Hoffmann - Aug. 6, 2012, 7:31 a.m.
> diff --git a/configure b/configure
> index cef0a71..5fcd315 100755
> --- a/configure
> +++ b/configure
> @@ -2630,7 +2630,7 @@ EOF
>    spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
>    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
>    if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
> -     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
> +     $pkg_config --atleast-version=0.12.0 spice-protocol > /dev/null 2>&1 && \

I'd prefer to not require cutting-edge spice bits to build ...

>      case QXL_IO_FLUSH_SURFACES_ASYNC:
> +#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
> +    case QXL_IO_MONITORS_CONFIG_ASYNC:
> +#endif

... and given you #ifdef the new stuff anyway it should not be needed.

cheers,
  Gerd
Alon Levy - Aug. 9, 2012, 1:41 p.m.
On Mon, Aug 06, 2012 at 09:31:19AM +0200, Gerd Hoffmann wrote:
> > diff --git a/configure b/configure
> > index cef0a71..5fcd315 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2630,7 +2630,7 @@ EOF
> >    spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
> >    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
> >    if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
> > -     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
> > +     $pkg_config --atleast-version=0.12.0 spice-protocol > /dev/null 2>&1 && \
> 
> I'd prefer to not require cutting-edge spice bits to build ...

I only increased the protocol requirement, not the server.

This is actually not related to the SERVER_VERSION below, it's needed
for the new io code QXL_IO_MONITORS_CONFIG_ASYNC. I would like to
propose we add spice-protocol as a submodule to fix this dependency,
since it's a small module containing headers, so even if it isn't used
it shouldn't place a large burden on qemu developers.

Meanwhile I'll add a define based on the pkg-config spice-protocol
version, which I can test from hw/qxl.c, since the commit adding
QXL_IO_MONITORS_CONFIG_ASYNC didn't introduce any new define I can
check.

> 
> >      case QXL_IO_FLUSH_SURFACES_ASYNC:
> > +#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
> > +    case QXL_IO_MONITORS_CONFIG_ASYNC:
> > +#endif
> 
> ... and given you #ifdef the new stuff anyway it should not be needed.
> 
> cheers,
>   Gerd
>
Gerd Hoffmann - Aug. 9, 2012, 1:50 p.m.
On 08/09/12 15:41, Alon Levy wrote:
> On Mon, Aug 06, 2012 at 09:31:19AM +0200, Gerd Hoffmann wrote:
>>> diff --git a/configure b/configure
>>> index cef0a71..5fcd315 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2630,7 +2630,7 @@ EOF
>>>    spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
>>>    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
>>>    if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
>>> -     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
>>> +     $pkg_config --atleast-version=0.12.0 spice-protocol > /dev/null 2>&1 && \
>>
>> I'd prefer to not require cutting-edge spice bits to build ...
> 
> I only increased the protocol requirement, not the server.

Distros tend to update those in sync, so this doesn't help much for the
average user, this change would likely cause spice detection fail on
almost every released distro ...

> Meanwhile I'll add a define based on the pkg-config spice-protocol
> version, which I can test from hw/qxl.c, since the commit adding
> QXL_IO_MONITORS_CONFIG_ASYNC didn't introduce any new define I can
> check.

Just checking "SPICE_SERVER_VERSION >= 0x000b01" doesn't work?  I'd
expect spice-server 0.11.1+ having a dependency on a recent enougth
spice-protocol so you can expect QXL_IO_MONITORS_CONFIG_ASYNC being
present then, no?

thanks,
  Gerd
Alon Levy - Aug. 9, 2012, 2:18 p.m.
On Thu, Aug 09, 2012 at 03:50:12PM +0200, Gerd Hoffmann wrote:
> On 08/09/12 15:41, Alon Levy wrote:
> > On Mon, Aug 06, 2012 at 09:31:19AM +0200, Gerd Hoffmann wrote:
> >>> diff --git a/configure b/configure
> >>> index cef0a71..5fcd315 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -2630,7 +2630,7 @@ EOF
> >>>    spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
> >>>    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
> >>>    if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
> >>> -     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
> >>> +     $pkg_config --atleast-version=0.12.0 spice-protocol > /dev/null 2>&1 && \
> >>
> >> I'd prefer to not require cutting-edge spice bits to build ...
> > 
> > I only increased the protocol requirement, not the server.
> 
> Distros tend to update those in sync, so this doesn't help much for the
> average user, this change would likely cause spice detection fail on
> almost every released distro ...

You're right. This would be solved with a submodule.

> 
> > Meanwhile I'll add a define based on the pkg-config spice-protocol
> > version, which I can test from hw/qxl.c, since the commit adding
> > QXL_IO_MONITORS_CONFIG_ASYNC didn't introduce any new define I can
> > check.
> 
> Just checking "SPICE_SERVER_VERSION >= 0x000b01" doesn't work?  I'd
> expect spice-server 0.11.1+ having a dependency on a recent enougth
> spice-protocol so you can expect QXL_IO_MONITORS_CONFIG_ASYNC being
> present then, no?

Since spice-server now uses submodules, it doesn't check for an external
spice-protocol, so no.

> 
> thanks,
>   Gerd
>

Patch

diff --git a/configure b/configure
index cef0a71..5fcd315 100755
--- a/configure
+++ b/configure
@@ -2630,7 +2630,7 @@  EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
   if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
-     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
+     $pkg_config --atleast-version=0.12.0 spice-protocol > /dev/null 2>&1 && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
     libs_softmmu="$libs_softmmu $spice_libs"
diff --git a/hw/qxl.c b/hw/qxl.c
index 3a883ce..2131f20 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -249,6 +249,19 @@  static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
     }
 }
 
+#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+    trace_qxl_spice_monitors_config(qxl->id);
+    qxl->guest_monitors_config = qxl->ram->monitors_config;
+    spice_qxl_monitors_config_async(&qxl->ssd.qxl,
+            qxl->ram->monitors_config,
+            MEMSLOT_GROUP_GUEST,
+            (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                      QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
 {
     trace_qxl_spice_reset_image_cache(qxl->id);
@@ -538,6 +551,7 @@  static const char *io_port_to_string(uint32_t io_port)
                                         = "QXL_IO_DESTROY_ALL_SURFACES_ASYNC",
         [QXL_IO_FLUSH_SURFACES_ASYNC]   = "QXL_IO_FLUSH_SURFACES_ASYNC",
         [QXL_IO_FLUSH_RELEASE]          = "QXL_IO_FLUSH_RELEASE",
+        [QXL_IO_MONITORS_CONFIG_ASYNC]  = "QXL_IO_MONITORS_CONFIG_ASYNC",
     };
     return io_port_to_string[io_port];
 }
@@ -819,6 +833,7 @@  static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     case QXL_IO_DESTROY_PRIMARY_ASYNC:
     case QXL_IO_UPDATE_AREA_ASYNC:
     case QXL_IO_FLUSH_SURFACES_ASYNC:
+    case QXL_IO_MONITORS_CONFIG_ASYNC:
         break;
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
@@ -894,6 +909,8 @@  static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
     case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:
         qxl_render_update_area_done(qxl, cookie);
         break;
+    case QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG:
+        break;
     default:
         fprintf(stderr, "qxl: %s: unexpected cookie type %d\n",
                 __func__, cookie->type);
@@ -1333,7 +1350,7 @@  static void ioport_write(void *opaque, target_phys_addr_t addr,
             io_port, io_port_to_string(io_port));
         /* be nice to buggy guest drivers */
         if (io_port >= QXL_IO_UPDATE_AREA_ASYNC &&
-            io_port <= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+            io_port <= QXL_IO_MONITORS_CONFIG_ASYNC) {
             qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
         }
         return;
@@ -1361,6 +1378,9 @@  static void ioport_write(void *opaque, target_phys_addr_t addr,
         io_port = QXL_IO_DESTROY_ALL_SURFACES;
         goto async_common;
     case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
+    case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
 async_common:
         async = QXL_ASYNC;
         qemu_mutex_lock(&d->async_lock);
@@ -1490,6 +1510,11 @@  async_common:
         d->mode = QXL_MODE_UNDEFINED;
         qxl_spice_destroy_surfaces(d, async);
         break;
+#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
+    case QXL_IO_MONITORS_CONFIG_ASYNC:
+        qxl_spice_monitors_config_async(d);
+        break;
+#endif
     default:
         qxl_set_guest_bug(d, "%s: unexpected ioport=0x%x\n", __func__, io_port);
     }
@@ -1785,9 +1810,15 @@  static int qxl_init_common(PCIQXLDevice *qxl)
         io_size = 16;
         break;
     case 3: /* qxl-3 */
+        pci_device_rev = QXL_REVISION_STABLE_V10;
+        io_size = 32; /* PCI region size must be pow2 */
+        break;
+#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
+    case 4: /* qxl-4 */
         pci_device_rev = QXL_DEFAULT_REVISION;
         io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
         break;
+#endif
     default:
         error_report("Invalid revision %d for qxl device (max %d)",
                      qxl->revision, QXL_DEFAULT_REVISION);
@@ -1986,7 +2017,20 @@  static int qxl_post_load(void *opaque, int version)
         }
         qxl_spice_loadvm_commands(d, cmds, out);
         g_free(cmds);
-
+        if (d->guest_monitors_config) {
+            /*
+             * don't use QXL_COOKIE_TYPE_IO:
+             *  - we are not running yet (post_load), we will assert
+             *    in send_events
+             *  - this is not a guest io, but a reply, so async_io isn't set.
+             */
+            spice_qxl_monitors_config_async(&d->ssd.qxl,
+                    d->guest_monitors_config,
+                    MEMSLOT_GROUP_GUEST,
+                    (uintptr_t)qxl_cookie_new(
+                        QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
+                        0));
+        }
         break;
     case QXL_MODE_COMPAT:
         /* note: no need to call qxl_create_memslots, qxl_set_mode
@@ -2053,6 +2097,7 @@  static VMStateDescription qxl_vmstate = {
         VMSTATE_ARRAY(guest_surfaces.cmds, PCIQXLDevice, NUM_SURFACES, 0,
                       vmstate_info_uint64, uint64_t),
         VMSTATE_UINT64(guest_cursor, PCIQXLDevice),
+        VMSTATE_UINT64(guest_monitors_config, PCIQXLDevice),
         VMSTATE_END_OF_LIST()
     },
 };
diff --git a/hw/qxl.h b/hw/qxl.h
index 172baf6..d6649bc 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -71,6 +71,8 @@  typedef struct PCIQXLDevice {
     } guest_surfaces;
     QXLPHYSICAL        guest_cursor;
 
+    QXLPHYSICAL        guest_monitors_config;
+
     QemuMutex          track_lock;
 
     /* thread signaling */
@@ -128,7 +130,11 @@  typedef struct PCIQXLDevice {
         }                                                               \
     } while (0)
 
+#if SPICE_SERVER_VERSION >= 0x000b01 /* 0.11.1 */
+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12
+#else
 #define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
+#endif
 
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
diff --git a/trace-events b/trace-events
index 2a5f074..3db04ad 100644
--- a/trace-events
+++ b/trace-events
@@ -954,6 +954,7 @@  qxl_spice_destroy_surfaces(int qid, int async) "%d async=%d"
 qxl_spice_destroy_surface_wait_complete(int qid, uint32_t id) "%d sid=%d"
 qxl_spice_destroy_surface_wait(int qid, uint32_t id, int async) "%d sid=%d async=%d"
 qxl_spice_flush_surfaces_async(int qid, uint32_t surface_count, uint32_t num_free_res) "%d s#=%d, res#=%d"
+qxl_spice_monitors_config(int id) "%d"
 qxl_spice_loadvm_commands(int qid, void *ext, uint32_t count) "%d ext=%p count=%d"
 qxl_spice_oom(int qid) "%d"
 qxl_spice_reset_cursor(int qid) "%d"
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 12e50b6..7fa095f 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -51,6 +51,7 @@  typedef enum qxl_async_io {
 enum {
     QXL_COOKIE_TYPE_IO,
     QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,
+    QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
 };
 
 typedef struct QXLCookie {