| Message ID | 1343811943-3972-1-git-send-email-mc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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 >
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
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
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(-)