From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K0rgh-0005DU-3j for qemu-devel@nongnu.org; Tue, 27 May 2008 01:25:31 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K0rgf-0005CW-Rv for qemu-devel@nongnu.org; Tue, 27 May 2008 01:25:30 -0400 Received: from [199.232.76.173] (port=47337 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K0rgf-0005CT-JV for qemu-devel@nongnu.org; Tue, 27 May 2008 01:25:29 -0400 Received: from g4t0017.houston.hp.com ([15.201.24.20]:3152) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1K0rgf-0004B2-JS for qemu-devel@nongnu.org; Tue, 27 May 2008 01:25:29 -0400 From: Alex Williamson In-Reply-To: 5b31733c0709081221l5915a81bs9954de3cfb4f2452@mail.gmail.com Content-Type: text/plain Date: Mon, 26 May 2008 23:25:17 -0600 Message-Id: <1211865917.8201.56.camel@bling> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [PATCH] ide: fix ATAPI read drive structure command Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel I've found that the introduction of support for the ATAPI GPCMD_READ_DVD_STRUCTURE command causes a problem for Windows Vista guests under QEMU. To test the bug, simply boot a Vista VM under QEMU with a DVD image loaded using the -cdrom option. If you now run diskpart.exe, the command will hang indefinitely. The main issue is that the read disk structure command contains a field indicating the maximum length to be returned. We ignore this field and always return the maximum possible table size. I also found that we're getting the format byte from the wrong field in the request (byte 2 is the MSB of the address field, we want byte 7). The patch below fixes these issues for me and adds a few extra comments. Perhaps Filip can confirm whether it still works for the original usage on Darwin/x86. Thanks, Alex Fix ATAPI read drive structure command Make use of the allocation length field in the command and only return the number of bytes requested. Fix location of format byte in command. Add comments for more fields as we fill them in. Signed-off-by: Alex Williamson -- diff -r 1f1541286539 trunk/hw/ide.c --- a/trunk/hw/ide.c Sun May 25 15:01:05 2008 -0600 +++ b/trunk/hw/ide.c Mon May 26 23:15:06 2008 -0600 @@ -1652,13 +1652,15 @@ static void ide_atapi_cmd(IDEState *s) { int media = packet[1]; int layer = packet[6]; - int format = packet[2]; + int format = packet[7]; + int length = ube16_to_cpu(packet + 8); uint64_t total_sectors; if (media != 0 || layer != 0) { ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, ASC_INV_FIELD_IN_CMD_PACKET); + break; } switch (format) { @@ -1671,20 +1673,28 @@ static void ide_atapi_cmd(IDEState *s) break; } - memset(buf, 0, 2052); + if (length == 0) + length = 2048 + 4; + if (length < 20) { + ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, + ASC_INV_FIELD_IN_CMD_PACKET); + break; + } + + memset(buf, 0, length); buf[4] = 1; // DVD-ROM, part version 1 buf[5] = 0xf; // 120mm disc, maximum rate unspecified buf[6] = 0; // one layer, embossed data - buf[7] = 0; + buf[7] = 0; // default densities - cpu_to_ube32(buf + 8, 0); - cpu_to_ube32(buf + 12, total_sectors - 1); - cpu_to_ube32(buf + 16, total_sectors - 1); + cpu_to_ube32(buf + 8, 0); // start sector + cpu_to_ube32(buf + 12, total_sectors - 1); // end sector + cpu_to_ube32(buf + 16, total_sectors - 1); // l0 end sector - cpu_to_be16wu((uint16_t *)buf, 2048 + 4); + cpu_to_be16wu((uint16_t *)buf, length); - ide_atapi_cmd_reply(s, 2048 + 3, 2048 + 4); + ide_atapi_cmd_reply(s, length, length); break; default: