Re: [win32-qxl]Do not allow duplicate IDs in video mode info buffer.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/17/2015 02:40 PM, Christophe Fergeau wrote:
On Wed, Sep 16, 2015 at 03:27:49PM -0400, Sandy Stutsman wrote:

Modes are returned in a buffer from qemu with IDs in increasing but not
necessarily consecutive order. (Some modes may be omitted if they don't fit
into the framebuffer.) Two custom modes are appended to the local buffer to
support arbitrary resolutions. The first custom id must be set to +1 of the
last id in the mode buffer, not to the last index +1 of the buffer.

E.G. these are the last entries in the mode buffer returned from QEMU:
     Index      Mode id (unfortunately named ModeIndex in the struct)
     ...
     124         128
     125         130
     126         132
     127         134 (last mode returned from QEMU)

Custom Modes:
     128         135 (Not 128!)
     129         136

If the first custom mode id is set to 128 (the number of modes returned
from QEMU), it will duplicate the id for the mode at index 124. That could
cause the FindMode function which searches the mode list by mode id
to return the wrong mode information.

The custom mode id's are now set to be greater than the last mode id
returned from QEMU.
---
  xddm/miniport/qxl.c | 15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/xddm/miniport/qxl.c b/xddm/miniport/qxl.c
index e0fb87f..22537e2 100644
--- a/xddm/miniport/qxl.c
+++ b/xddm/miniport/qxl.c
@@ -596,6 +596,7 @@ VP_STATUS InitModes(QXLExtension *dev)
      ULONG n_modes;
      ULONG i;
      VP_STATUS error;
+    ULONG custom_mode_id = 0;

      PAGED_CODE();
      DEBUG_PRINT((dev, 0, "%s\n", __FUNCTION__));
@@ -632,6 +633,11 @@ VP_STATUS InitModes(QXLExtension *dev)
              return error;
          }
      }
+    /*  Mode ids are increasing in the buffer but may not be consecutive,
+     *  so set the first custom id to be 1 larger than the last id in the buffer.
+     *  (ModeIndex isn't really an index, it is an ID).
+     */
+    custom_mode_id = modes->modes[modes->n_modes-1].id + 1;

      /* 2 dummy modes for custom display resolution */
      /* This is necessary to bypass Windows mode index check, that
@@ -640,7 +646,7 @@ VP_STATUS InitModes(QXLExtension *dev)

      for (i = dev->custom_mode; i <= dev->custom_mode + 1; ++i) {
          memcpy(&modes_info[i], &modes_info[0], sizeof(VIDEO_MODE_INFORMATION));
-        modes_info[i].ModeIndex = i;
+        modes_info[i].ModeIndex = custom_mode_id++;
      }

      dev->n_modes = n_modes;
@@ -1002,6 +1008,7 @@ static VP_STATUS SetCustomDisplay(QXLExtension *dev_ext, QXLEscapeSetCustomDispl
      uint32_t xres = custom_display->xres;
      uint32_t yres = custom_display->yres;
      uint32_t bpp = custom_display->bpp;
+    PVIDEO_MODE_INFORMATION pMode;

      /* alternate custom mode index */
      if (dev_ext->custom_mode == (dev_ext->n_modes - 1))
@@ -1014,11 +1021,11 @@ static VP_STATUS SetCustomDisplay(QXLExtension *dev_ext, QXLEscapeSetCustomDispl
                      __FUNCTION__, xres, yres, bpp, dev_ext->rom->surface0_area_size));
          return ERROR_NOT_ENOUGH_MEMORY;
      }
-
-    ret = FillVidModeInfo(&dev_ext->modes[dev_ext->custom_mode],
+    pMode = &dev_ext->modes[dev_ext->custom_mode];
+    ret = FillVidModeInfo(pMode,
                            custom_display->xres, custom_display->yres,
                            custom_display->bpp,
-                          dev_ext->custom_mode);
+                          pMode->ModeIndex);

This looks good to me, ACK.
I'd add a commit on top of that getting rid of
the last argument to FillVidModeInfo as all that function does with this
argument is set pMode->ModeIndex to be equal to its value.

This patch is good, but I wonder if the problem is not with qemu.
If we change in qemu/hw/display/qxl.c:
- modes->modes[n].id          = cpu_to_le32(i);
+ modes->modes[n].id          = cpu_to_le32(n);

Does that solves the problem ?

Thanks,
    Uri.

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]