commit 4dedc07ffbbc66002e0fd2b97d5516fe6aca5eea Author: Andrzej Zaborowski Date: Fri Sep 10 02:23:31 2010 +0200 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. Index: qemu-0.13.0/hw/vmware_vga.c =================================================================== --- qemu-0.13.0.orig/hw/vmware_vga.c 2011-01-15 18:06:06.000000000 +0800 +++ qemu-0.13.0/hw/vmware_vga.c 2011-01-15 18:17:04.000000000 +0800 @@ -521,27 +521,37 @@ static uint32_t last_cmd; -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) { - int offset = CMD(stop); + uint32_t cmd = s->fifo[CMD(stop) >> 2]; + /* If parameter is not available in FIFO, return 0 rather than random + * value. Also update the stop as missing parameter will be inserted + * soonly, else it will be treated as new command next time. + * With rewinding in vmsvga_fifo_run, this unlikely happen. + */ if (unlikely(s->cmd->next_cmd == s->cmd->stop)) { fprintf(stderr, "%s: FIFO empty during CMD %i\n", __FUNCTION__, last_cmd); - return 0x00000000; + cmd = 0; } - s->cmd->stop = cpu_to_le32(offset + 4); - if (offset + 4 >= CMD(max)) + s->cmd->stop = cpu_to_le32(CMD(stop) + 4); + if (CMD(stop) >= CMD(max)) s->cmd->stop = s->cmd->min; - return s->fifo[offset >> 2]; + + return cmd; } static inline uint32_t vmsvga_fifo_read(struct vmsvga_state_s *s) @@ -552,13 +562,23 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) { uint32_t 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 (last_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); @@ -567,6 +587,10 @@ 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); @@ -576,10 +600,15 @@ 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); @@ -590,10 +619,15 @@ 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); @@ -602,11 +636,14 @@ 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); @@ -625,6 +662,10 @@ * 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); @@ -639,6 +680,10 @@ 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); @@ -659,14 +704,22 @@ 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__, last_cmd); break; + rewind: + s->cmd->stop = cmd_start; + break; } + } s->syncing = 0; }