vmware_vga: Add checks to deal with non-atomic fifo writes.

Janne Huttunen noticed that the FIFO end pointer is updated by the
guest after writing each word to the FIFO, at least the X.org driver
which is open does this.  This means that there's no way for the
host to know if the guest is in the middle a write operation.  Qemu
thus needs to read the beginning of the command up to when it's able
to tell how many words are expected for the given command.  It will
abort reading and rewind the FIFO if there aren't enough words yet,
this should be relatively rare but it is suspected to have been the
cause of the occasional FIFO overrun that killed the display.

1 files changed, 56 insertions, 8 deletions

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 12bff480eb..b3c0a823c9 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);
@@ -567,10 +585,15 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             vmsvga_fill_rect(s, colour, x, y, width, height);
             break;
 #else
+            args = 0;
             goto badcmd;
 #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);
@@ -581,10 +604,15 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             vmsvga_copy_rect(s, x, y, dx, dy, width, height);
             break;
 #else
+            args = 0;
             goto badcmd;
 #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 +621,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 +647,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 +665,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 +689,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;
 }