Re: [Qemu-devel] Unknown command 0xffffff in SVGA command FIFO

[balrog]

From: Andrzej Zaborowski <address@hidden>

Hi Janne,
I came up with this version, it kind of reverses the logic of your
patch but reuses the _items function (renamed _length), please
see if it looks ok and possibly even works.

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 12bff48..464f8bc 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -519,11 +519,15 @@ static inline void vmsvga_cursor_define(struct 
vmsvga_state_s *s,
 
 #define CMD(f) le32_to_cpu(s->cmd->f)
 
-static inline int vmsvga_fifo_empty(struct vmsvga_state_s *s)
+static inline int vmsvga_fifo_length(struct vmsvga_state_s *s)
 {
+    int num;
     if (!s->config || !s->enable)
-        return 1;
-    return (s->cmd->next_cmd == s->cmd->stop);
+        return 0;
+    num = CMD(next_cmd) - CMD(stop);
+    if (num < 0)
+        num += CMD(max) - CMD(min);
+    return num >> 2;
 }
 
 static inline uint32_t vmsvga_fifo_read_raw(struct vmsvga_state_s *s)
@@ -543,13 +547,23 @@ static inline uint32_t vmsvga_fifo_read(struct 
vmsvga_state_s *s)
 static void vmsvga_fifo_run(struct vmsvga_state_s *s)
 {
     uint32_t cmd, colour;
-    int args = 0;
+    int args, len;
     int x, y, dx, dy, width, height;
     struct vmsvga_cursor_definition_s cursor;
-    while (!vmsvga_fifo_empty(s))
+    uint32_t cmd_start;
+
+    len = vmsvga_fifo_length(s);
+    while (len > 0) {
+        /* May need to go back to the start of the command if incomplete */
+        cmd_start = s->cmd->stop;
+
         switch (cmd = vmsvga_fifo_read(s)) {
         case SVGA_CMD_UPDATE:
         case SVGA_CMD_UPDATE_VERBOSE:
+            len -= 5;
+            if (len < 0)
+                goto rewind;
+
             x = vmsvga_fifo_read(s);
             y = vmsvga_fifo_read(s);
             width = vmsvga_fifo_read(s);
@@ -558,6 +572,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             break;
 
         case SVGA_CMD_RECT_FILL:
+            len -= 6;
+            if (len < 0)
+                goto rewind;
+
             colour = vmsvga_fifo_read(s);
             x = vmsvga_fifo_read(s);
             y = vmsvga_fifo_read(s);
@@ -571,6 +589,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
 #endif
 
         case SVGA_CMD_RECT_COPY:
+            len -= 7;
+            if (len < 0)
+                goto rewind;
+
             x = vmsvga_fifo_read(s);
             y = vmsvga_fifo_read(s);
             dx = vmsvga_fifo_read(s);
@@ -585,6 +607,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
 #endif
 
         case SVGA_CMD_DEFINE_CURSOR:
+            len -= 8;
+            if (len < 0)
+                goto rewind;
+
             cursor.id = vmsvga_fifo_read(s);
             cursor.hot_x = vmsvga_fifo_read(s);
             cursor.hot_y = vmsvga_fifo_read(s);
@@ -593,11 +619,14 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             vmsvga_fifo_read(s);
             cursor.bpp = vmsvga_fifo_read(s);
 
+            args = SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y, cursor.bpp);
            if (SVGA_BITMAP_SIZE(x, y) > sizeof cursor.mask ||
-               SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image) {
-                   args = SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y, 
cursor.bpp);
+               SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image)
                    goto badcmd;
-           }
+
+            len -= args;
+            if (len < 0)
+                goto rewind;
 
             for (args = 0; args < SVGA_BITMAP_SIZE(x, y); args ++)
                 cursor.mask[args] = vmsvga_fifo_read_raw(s);
@@ -616,6 +645,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
          * for so we can avoid FIFO desync if driver uses them illegally.
          */
         case SVGA_CMD_DEFINE_ALPHA_CURSOR:
+            len -= 6;
+            if (len < 0)
+                goto rewind;
+
             vmsvga_fifo_read(s);
             vmsvga_fifo_read(s);
             vmsvga_fifo_read(s);
@@ -630,6 +663,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             args = 7;
             goto badcmd;
         case SVGA_CMD_DRAW_GLYPH_CLIPPED:
+            len -= 4;
+            if (len < 0)
+                goto rewind;
+
             vmsvga_fifo_read(s);
             vmsvga_fifo_read(s);
             args = 7 + (vmsvga_fifo_read(s) >> 2);
@@ -650,13 +687,22 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             break; /* Nop */
 
         default:
+            args = 0;
         badcmd:
+            len -= args;
+            if (len < 0)
+                goto rewind;
             while (args --)
                 vmsvga_fifo_read(s);
             printf("%s: Unknown command 0x%02x in SVGA command FIFO\n",
                             __FUNCTION__, cmd);
             break;
+
+        rewind:
+            s->cmd->stop = cmd_start;
+            break;
         }
+    }
 
     s->syncing = 0;
 }