[Qemu-devel] [PATCH] ide: fix ATAPI read drive structure command

[Alex Williamson <alex.williamson@hp.com>]

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 <alex.williamson@hp.com>
--

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: