From patchwork Wed Jul 21 15:02:10 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janne Huttunen X-Patchwork-Id: 59448 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id D2E8DB6EF1 for ; Thu, 22 Jul 2010 01:02:57 +1000 (EST) Received: from localhost ([127.0.0.1]:57167 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Obaov-000815-PV for incoming@patchwork.ozlabs.org; Wed, 21 Jul 2010 11:02:53 -0400 Received: from [140.186.70.92] (port=55345 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ObaoK-00080M-GG for qemu-devel@nongnu.org; Wed, 21 Jul 2010 11:02:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1ObaoI-00062l-Jp for qemu-devel@nongnu.org; Wed, 21 Jul 2010 11:02:16 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:44402) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1ObaoI-00062R-C0 for qemu-devel@nongnu.org; Wed, 21 Jul 2010 11:02:14 -0400 Received: by fxm5 with SMTP id 5so3931125fxm.4 for ; Wed, 21 Jul 2010 08:02:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=Q+975t3Br4PxGeB+GgVWsm8CfIAcA/odO621pSDbUoQ=; b=QKn00nqjY0Xi0/CMamrFri0B7GvNnL578U+rh8diyPFLRMdEH3hFiNkvQVMwsVMLe2 PDep0J7/MfXcAAYphbpk6qWxQeWaMGiO3RsTbgc4O3lASNcf5iIGMiBjxgOmcNzqy4YJ pnRxU6A0xBe3gHbYrizSqvAnpjk4XgIhWHcMo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=jBrF1/EyRgIgncIPIM5I2QROg2CazlzOza1eL9ZSXd0abTlRMnM6jDn4kkFLyJhlET zv5Sc3wvbJj/nGk1ZcBto+fgSPo8wBLhVZFcV7q7ZXORB0lvHwNvlVV5sPaF40X51J9N 0Fngk1HV1r2pIfdvwRpNKgz+fTv5Yvyrx68Ms= Received: by 10.223.111.206 with SMTP id t14mr419190fap.32.1279724533061; Wed, 21 Jul 2010 08:02:13 -0700 (PDT) Received: from [91.156.42.174] (a91-156-42-174.elisa-laajakaista.fi [91.156.42.174]) by mx.google.com with ESMTPS id e22sm1339941faa.2.2010.07.21.08.02.11 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 21 Jul 2010 08:02:12 -0700 (PDT) Message-ID: <4C470BF2.30506@gmail.com> Date: Wed, 21 Jul 2010 18:02:10 +0300 From: Janne Huttunen User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100620 Icedove/3.0.5 MIME-Version: 1.0 To: andrzej zaborowski Subject: Re: [Qemu-devel] Unknown command 0xffffff in SVGA command FIFO References: <4C46D733.1000208@gmail.com> <4C46E48D.9080303@gmail.com> In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org > I see no way to tell whether the guest is currently in the middle of > writing a command. So it seems the only way to check is to peek the > first word in the fifo (which *is* written entirely before a NEXT_CMD > update) and look up the expected command length, and then check > whether enough data is in the FIFO. Do you want to implement this? It's not quite that simple. There are a bunch of command where amount of arguments depends on other arguments. 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 :-/ . 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) + break; + 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) + break; + 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) + break; + 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) + break; + 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) + break; + + 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) + break; + x = vmsvga_fifo_peek(s, 4); + y = vmsvga_fifo_peek(s, 5); + if (vmsvga_fifo_items(s) < x * y + 6) + break; + 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) + break; + vmsvga_fifo_read(s); args = 6; goto badcmd; case SVGA_CMD_RECT_ROP_COPY: + if (vmsvga_fifo_items(s) < 8) + break; + vmsvga_fifo_read(s); args = 7; goto badcmd; case SVGA_CMD_DRAW_GLYPH_CLIPPED: + if (vmsvga_fifo_items(s) < 4) + break; + args = 7 + (vmsvga_fifo_peek(s, 3) >> 2); + if (vmsvga_fifo_items(s) < args + 4) + break; + 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) + break; + vmsvga_fifo_read(s); args = 12; goto badcmd; diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 12bff48..7730ae0 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) {