[Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command

[Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command

[Cong Meng]

Correct the command names of opcode 0x85 and 0xa1, and calculate
their xfer size from CDB.

ChangeLog:
v2: For opcode 0xa1 on TYPE_ROM device, do not calc the xfer size

Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com>
---
 hw/scsi-bus.c  |   19 ++++++++++++++++---
 hw/scsi-defs.h |    4 ++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..8af1e86 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -867,6 +867,18 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
             cmd->xfer = buf[9] | (buf[8] << 8);
         }
         break;
+    case ATA_PASSTHROUGH_12:
+        if (dev->type != TYPE_ROM) {
+            if ((buf[2] & 0x3) == 2) {
+                cmd->xfer = buf[4] * dev->blocksize;
+            }
+        }
+        break;
+    case ATA_PASSTHROUGH_16:
+        if ((buf[2] & 0x3) == 2) {
+            cmd->xfer = ((buf[5] << 8) | buf[6]) * dev->blocksize;
+        }
+        break;
     }
     return 0;
 }
@@ -996,7 +1008,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
     case SEND_DVD_STRUCTURE:
     case PERSISTENT_RESERVE_OUT:
     case MAINTENANCE_OUT:
-    case ATA_PASSTHROUGH:
+    case ATA_PASSTHROUGH_12:
+    case ATA_PASSTHROUGH_16:
         cmd->mode = SCSI_XFER_TO_DEV;
         break;
     default:
@@ -1335,7 +1348,7 @@ static const char *scsi_command_name(uint8_t cmd)
         [ PERSISTENT_RESERVE_OUT   ] = "PERSISTENT_RESERVE_OUT",
         [ WRITE_FILEMARKS_16       ] = "WRITE_FILEMARKS_16",
         [ EXTENDED_COPY            ] = "EXTENDED_COPY",
-        [ ATA_PASSTHROUGH          ] = "ATA_PASSTHROUGH",
+        [ ATA_PASSTHROUGH_16       ] = "ATA_PASSTHROUGH_16",
         [ ACCESS_CONTROL_IN        ] = "ACCESS_CONTROL_IN",
         [ ACCESS_CONTROL_OUT       ] = "ACCESS_CONTROL_OUT",
         [ READ_16                  ] = "READ_16",
@@ -1352,7 +1365,7 @@ static const char *scsi_command_name(uint8_t cmd)
         [ SERVICE_ACTION_IN_16     ] = "SERVICE_ACTION_IN_16",
         [ WRITE_LONG_16            ] = "WRITE_LONG_16",
         [ REPORT_LUNS              ] = "REPORT_LUNS",
-        [ BLANK                    ] = "BLANK",
+        [ ATA_PASSTHROUGH_12       ] = "ATA_PASSTHROUGH_12",
         [ MOVE_MEDIUM              ] = "MOVE_MEDIUM",
         [ EXCHANGE_MEDIUM          ] = "EXCHANGE MEDIUM",
         [ LOAD_UNLOAD              ] = "LOAD_UNLOAD",
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8a73f74..d7a4019 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -100,7 +100,7 @@
 #define READ_REVERSE_16       0x81
 #define ALLOW_OVERWRITE       0x82
 #define EXTENDED_COPY         0x83
-#define ATA_PASSTHROUGH       0x85
+#define ATA_PASSTHROUGH_16    0x85
 #define ACCESS_CONTROL_IN     0x86
 #define ACCESS_CONTROL_OUT    0x87
 #define READ_16               0x88
@@ -117,7 +117,7 @@
 #define SERVICE_ACTION_IN_16  0x9e
 #define WRITE_LONG_16         0x9f
 #define REPORT_LUNS           0xa0
-#define BLANK                 0xa1
+#define ATA_PASSTHROUGH_12    0xa1
 #define MAINTENANCE_IN        0xa3
 #define MAINTENANCE_OUT       0xa4
 #define MOVE_MEDIUM           0xa5
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command

[Paolo Bonzini]

Il 01/08/2012 11:05, Cong Meng ha scritto:
> +    case ATA_PASSTHROUGH_12:
> +        if (dev->type != TYPE_ROM) {
> +            if ((buf[2] & 0x3) == 2) {
> +                cmd->xfer = buf[4] * dev->blocksize;
> +            }
> +        }
> +        break;
> +    case ATA_PASSTHROUGH_16:
> +        if ((buf[2] & 0x3) == 2) {
> +            cmd->xfer = ((buf[5] << 8) | buf[6]) * dev->blocksize;
> +        }
> +        break;

Hmm, I think you're only handling this partially.

Four bits of buf[2] count; bits 0..1 are T_LENGTH, bit 2 is BYTE_BLOCK,
bit 4 is T_TYPE:

If buf[2] is xxxxxx00, cmd->xfer = 0

else

   if buf[2] is xxxxx0xx, xfer_unit = 1
   else if buf[2] is xxx0x1xx, xfer_unit = 512
   else xfer_unit = dev->blocksize (this is when buf[2] is xxx1x1xx)

   if buf[2] is xxxxxx01, set cmd->xfer to the FEATURES field
   if buf[2] is xxxxxx10, set cmd->xfer to the SECTOR_COUNT

   for ATA_PASSTHROUGH_16, if buf[1] bit 0 is 0, then cmd->xfer &= 255;

   cmd->xfer *= xfer_unit;

Also we cannot support buf[2] is xxxxxx11.  Please add a check to
hw/scsi-generic.c, so that the request is failed in this case.

This is better encapsulated in a separate function, of course.

On top of this, the direction is not necessarily TO_DEV (as in the
current code for scsi_cmd_xfer_mode).  It is TO_DEV if buf[2] bit 3
(T_DIR) is zero; it is FROM_DEV if buf[2] bit 3 is one.

Do you have a copy of the SAT (SCSI/ATA translation) standard?  This is
all in paragraph 12.2.2.2 in my copy.

Paolo

Re: [Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command

[Cong Meng]

On Wed 01 Aug 2012 05:42:08 PM CST, Paolo Bonzini wrote:
> Il 01/08/2012 11:05, Cong Meng ha scritto:
>> +    case ATA_PASSTHROUGH_12:
>> +        if (dev->type != TYPE_ROM) {
>> +            if ((buf[2] & 0x3) == 2) {
>> +                cmd->xfer = buf[4] * dev->blocksize;
>> +            }
>> +        }
>> +        break;
>> +    case ATA_PASSTHROUGH_16:
>> +        if ((buf[2] & 0x3) == 2) {
>> +            cmd->xfer = ((buf[5] << 8) | buf[6]) * dev->blocksize;
>> +        }
>> +        break;
>
> Hmm, I think you're only handling this partially.

Ah, I will complete it.
Cong.

>
> Four bits of buf[2] count; bits 0..1 are T_LENGTH, bit 2 is BYTE_BLOCK,
> bit 4 is T_TYPE:
>
> If buf[2] is xxxxxx00, cmd->xfer = 0
>
> else
>
>     if buf[2] is xxxxx0xx, xfer_unit = 1
>     else if buf[2] is xxx0x1xx, xfer_unit = 512
>     else xfer_unit = dev->blocksize (this is when buf[2] is xxx1x1xx)
>
>     if buf[2] is xxxxxx01, set cmd->xfer to the FEATURES field
>     if buf[2] is xxxxxx10, set cmd->xfer to the SECTOR_COUNT
>
>     for ATA_PASSTHROUGH_16, if buf[1] bit 0 is 0, then cmd->xfer &= 255;
>
>     cmd->xfer *= xfer_unit;
>
> Also we cannot support buf[2] is xxxxxx11.  Please add a check to
> hw/scsi-generic.c, so that the request is failed in this case.
>
> This is better encapsulated in a separate function, of course.
>
> On top of this, the direction is not necessarily TO_DEV (as in the
> current code for scsi_cmd_xfer_mode).  It is TO_DEV if buf[2] bit 3
> (T_DIR) is zero; it is FROM_DEV if buf[2] bit 3 is one.
>
> Do you have a copy of the SAT (SCSI/ATA translation) standard?  This is
> all in paragraph 12.2.2.2 in my copy.
>
> Paolo
>

Re: [Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command

[Peter Maydell]

On 1 August 2012 11:08, Cong Meng <mc@linux.vnet.ibm.com> wrote:
> On Wed 01 Aug 2012 05:42:08 PM CST, Paolo Bonzini wrote:
>>
>> Hmm, I think you're only handling this partially.
>
>
> Ah, I will complete it.

Since you're redoing the patch anyway, you could correct the
typo in the commit message summary line: it should be "support".

thanks
-- PMM