[PATCH] qemu: fix ATAPI read drive structure command

[PATCH] qemu: fix ATAPI read drive structure command

[Alex Williamson]

I believe this fixes bug 1967368.  Windows Vista hangs if you try to run
diskpart.exe with a DVD image loaded.  This does not occur on Xen
because their version of Qemu doesn't try to emulate the
GPCMD_READ_DVD_STRUCTURE command.  If I comment out the version in KVM,
diskpart.exe works.  Digging through the versions of the MMC-6 spec I
can find online, I believe the problem is that we're ignoring the
allocation length field.  This specifies the maximum number of bytes
that may be returned by the drive.  Instead we're returning the maximum
possible table size each time.  I also found that we seem to be using
the wrong field for the format request.  Byte 2 is MSB of the address
field, we want byte 7.  I also added a few missing comments for the
fields as we fill them in.  I've only tested this with Vista since it's
the only thing that I know using this command.  Thanks,

	Alex


qemu: 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.  This fixes bug 1967368
(diskpart.exe in Vista hangs with DVD image loaded).

Signed-off-by: Alex Williamson <alex.williamson@hp.com>

--

diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c
index 69363a9..0246fb6 100644
--- a/qemu/hw/ide.c
+++ b/qemu/hw/ide.c
@@ -1653,7 +1653,8 @@ 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)
@@ -1672,20 +1673,26 @@ 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);
+
+                    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:

Re: [PATCH] qemu: fix ATAPI read drive structure command

[Avi Kivity]

Alex Williamson wrote:
> I believe this fixes bug 1967368.  Windows Vista hangs if you try to run
> diskpart.exe with a DVD image loaded.  This does not occur on Xen
> because their version of Qemu doesn't try to emulate the
> GPCMD_READ_DVD_STRUCTURE command.  If I comment out the version in KVM,
> diskpart.exe works.  Digging through the versions of the MMC-6 spec I
> can find online, I believe the problem is that we're ignoring the
> allocation length field.  This specifies the maximum number of bytes
> that may be returned by the drive.  Instead we're returning the maximum
> possible table size each time.  I also found that we seem to be using
> the wrong field for the format request.  Byte 2 is MSB of the address
> field, we want byte 7.  I also added a few missing comments for the
> fields as we fill them in.  I've only tested this with Vista since it's
> the only thing that I know using this command.  Thanks,
>   

This seems to fix a bug in upstream qemu, so the preferred path for this 
is to post the patch on qemu-devel, and kvm will get it on the next merge.


-- 
error compiling committee.c: too many arguments to function