From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52229 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ObbiQ-0000sO-NT for qemu-devel@nongnu.org; Wed, 21 Jul 2010 12:00:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1ObbiP-000890-0n for qemu-devel@nongnu.org; Wed, 21 Jul 2010 12:00:14 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:55860) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1ObbiO-00088J-PG for qemu-devel@nongnu.org; Wed, 21 Jul 2010 12:00:12 -0400 Received: by fxm5 with SMTP id 5so3990457fxm.4 for ; Wed, 21 Jul 2010 09:00:11 -0700 (PDT) Message-ID: <4C47198A.2080308@gmail.com> Date: Wed, 21 Jul 2010 19:00:10 +0300 From: Janne Huttunen MIME-Version: 1.0 Subject: Re: [Qemu-devel] Unknown command 0xffffff in SVGA command FIFO References: <4C46D733.1000208@gmail.com> <4C46E48D.9080303@gmail.com> <4C470BF2.30506@gmail.com> In-Reply-To: <4C470BF2.30506@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: andrzej zaborowski Cc: qemu-devel@nongnu.org > Here's an experiment for sanity checking the lengths and leaving > the command in the FIFO if it is not complete. It fixes the > problem for me (running it right now), but I agree that it's not > very elegant to look at :-/ . And here's another version with couple of stupid bugs removed (it obviously is not a good idea to busyloop if the command is incomplete). diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 12bff48..839f715 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -526,6 +526,17 @@ static inline int vmsvga_fifo_empty(struct vmsvga_state_s *s) return (s->cmd->next_cmd == s->cmd->stop); } +static inline int vmsvga_fifo_items(struct vmsvga_state_s *s) +{ + int num; + if (!s->config || !s->enable) + 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) { uint32_t cmd = s->fifo[CMD(stop) >> 2]; @@ -540,6 +551,14 @@ static inline uint32_t vmsvga_fifo_read(struct vmsvga_state_s *s) return le32_to_cpu(vmsvga_fifo_read_raw(s)); } +static inline uint32_t vmsvga_fifo_peek(struct vmsvga_state_s *s, uint32_t offs) +{ + offs = (offs << 2) + CMD(stop); + if (offs >= CMD(max)) + offs = offs - CMD(max) + CMD(min); + return le32_to_cpu(s->fifo[offs >> 2]); +} + static void vmsvga_fifo_run(struct vmsvga_state_s *s) { uint32_t cmd, colour; @@ -547,9 +566,12 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) int x, y, dx, dy, width, height; struct vmsvga_cursor_definition_s cursor; while (!vmsvga_fifo_empty(s)) - switch (cmd = vmsvga_fifo_read(s)) { + switch (cmd = vmsvga_fifo_peek(s, 0)) { case SVGA_CMD_UPDATE: case SVGA_CMD_UPDATE_VERBOSE: + if (vmsvga_fifo_items(s) < 5) + goto out; + vmsvga_fifo_read(s); x = vmsvga_fifo_read(s); y = vmsvga_fifo_read(s); width = vmsvga_fifo_read(s); @@ -558,6 +580,9 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) break; case SVGA_CMD_RECT_FILL: + if (vmsvga_fifo_items(s) < 6) + goto out; + vmsvga_fifo_read(s); colour = vmsvga_fifo_read(s); x = vmsvga_fifo_read(s); y = vmsvga_fifo_read(s); @@ -571,6 +596,9 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) #endif case SVGA_CMD_RECT_COPY: + if (vmsvga_fifo_items(s) < 7) + goto out; + vmsvga_fifo_read(s); x = vmsvga_fifo_read(s); y = vmsvga_fifo_read(s); dx = vmsvga_fifo_read(s); @@ -585,13 +613,20 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) #endif case SVGA_CMD_DEFINE_CURSOR: - cursor.id = vmsvga_fifo_read(s); - cursor.hot_x = vmsvga_fifo_read(s); - cursor.hot_y = vmsvga_fifo_read(s); - cursor.width = x = vmsvga_fifo_read(s); - cursor.height = y = vmsvga_fifo_read(s); - vmsvga_fifo_read(s); - cursor.bpp = vmsvga_fifo_read(s); + if (vmsvga_fifo_items(s) < 8) + goto out; + cursor.id = vmsvga_fifo_peek(s, 1); + cursor.hot_x = vmsvga_fifo_peek(s, 2); + cursor.hot_y = vmsvga_fifo_peek(s, 3); + cursor.width = x = vmsvga_fifo_peek(s, 4); + cursor.height = y = vmsvga_fifo_peek(s, 5); + cursor.bpp = vmsvga_fifo_peek(s, 7); + + if (vmsvga_fifo_items(s) < SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y, cursor.bpp) + 8) + goto out; + + for (args = 0; args < 8; args++) + vmsvga_fifo_read(s); if (SVGA_BITMAP_SIZE(x, y) > sizeof cursor.mask || SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image) { @@ -616,25 +651,43 @@ 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: - vmsvga_fifo_read(s); - vmsvga_fifo_read(s); - vmsvga_fifo_read(s); - x = vmsvga_fifo_read(s); - y = vmsvga_fifo_read(s); + if (vmsvga_fifo_items(s) < 6) + goto out; + x = vmsvga_fifo_peek(s, 4); + y = vmsvga_fifo_peek(s, 5); + if (vmsvga_fifo_items(s) < x * y + 6) + goto out; + for (args = 0; args < 6; args++) + vmsvga_fifo_read(s); args = x * y; goto badcmd; case SVGA_CMD_RECT_ROP_FILL: + if (vmsvga_fifo_items(s) < 7) + goto out; + vmsvga_fifo_read(s); args = 6; goto badcmd; case SVGA_CMD_RECT_ROP_COPY: + if (vmsvga_fifo_items(s) < 8) + goto out; + vmsvga_fifo_read(s); args = 7; goto badcmd; case SVGA_CMD_DRAW_GLYPH_CLIPPED: + if (vmsvga_fifo_items(s) < 4) + goto out; + args = 7 + (vmsvga_fifo_peek(s, 3) >> 2); + if (vmsvga_fifo_items(s) < args + 4) + goto out; + vmsvga_fifo_read(s); + vmsvga_fifo_read(s); vmsvga_fifo_read(s); vmsvga_fifo_read(s); - args = 7 + (vmsvga_fifo_read(s) >> 2); goto badcmd; case SVGA_CMD_SURFACE_ALPHA_BLEND: + if (vmsvga_fifo_items(s) < 13) + goto out; + vmsvga_fifo_read(s); args = 12; goto badcmd; @@ -647,6 +700,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) case SVGA_CMD_FRONT_ROP_FILL: case SVGA_CMD_FENCE: case SVGA_CMD_INVALID_CMD: + vmsvga_fifo_read(s); break; /* Nop */ default: @@ -658,6 +712,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) break; } +out: s->syncing = 0; }