Re: [Qemu-devel] [PATCH v2] scsi-bus: fix to allow some special SCSI commands

[TAMUKI Shoichi]

Hello,

From: Paolo Bonzini <address@hidden>
Subject: Re: [PATCH v2] scsi-bus: fix to allow some special SCSI commands
Date: Tue, 15 Jul 2014 19:05:25 +0200

> > Currently, some special SCSI commands sent from the initiator in a
> > guest do not reach the target device.  To avoid this, extended (0x7e,)
> > variable length (0x7f,) and vendor specific (0xc0..0xff) opcodes are
> > now treated as valid CDBs.
> > 
> > Originally, the most significant 3 bits of a SCSI opcode specified the
> > length of the CDB.  However, when variable-length CDBs were created,
> > this correspondence was changed, and the entire opcode must be
> > examined to determine the CDB length.  The CDBs with the opcodes above
> > are done that way for now.
> > 
> > Signed-off-by: TAMUKI Shoichi <address@hidden>
> > ---
> > v2: add a new argument to scsi_req_new(), and modify all invocations
> > in hw/{scsi,usb}, since this function is not called only for virtio-
> > scsi.
> 
> I think that for scsi-generic it is harmless to pass extra bytes at the
> end of the CDB, and QEMU right now does not support more than 16 bytes
> for the CDB (see SCSI_CMD_BUF_SIZE in include/hw/scsi/scsi.h).
> 
> Assuming 16-byte commands are enough, does this patch work for you?
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 4341754..51e4f37 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -1194,6 +1194,9 @@
>       case 2:
>           cmd->len = 10;
>           break;
> +    case 3:
> +        cmd->len = SCSI_CMD_BUF_SIZE;
> +        break;
>       case 4:
>           cmd->len = 16;
>           break;

Yes, it is ok for me since I currently care neither 0x7e extended
opcode nor 0x7f variable length opcode.

> You will probably also need to pass the transfer length and direction
> down from the device model to scsi-generic.c.  Effectively you will be
> ignoring cmd->xfer and cmd->mode if the host device can provide them if
> the first byte in the cdb identifies a vendor-specific command.  You can
> add a callback to SCSIBusInfo, and call it from scsi_req_parse; for
> virtio-scsi the callback could look something like this:
> 
> int virtio_scsi_parse_req(SCSICommand *cmd, void *hba_private)
> {
>      VirtIOSCSIReq *req = hba_private;
> 
>      cmd->xfer = req->qsgl.size;
>      if (cmd->xfer == 0) {
>          cmd->mode = SCSI_XFER_NONE;
>      } else if (iov_size(req->elem._sg, req->elem.in_num)
>                 > req->resp_size)) {
>          cmd->mode = SCSI_XFER_FROM_DEV;
>      } else {
>          cmd->mode = SCSI_XFER_TO_DEV;
>      }
> }

Thank you for pointing that out.

> I'll try to prepare a complete patch tomorrow, but I would like to
> understand your actual requirements for the CDB length.

I am very glad to hear that.  My actual requirement is to pass vendor-
specific commands and their CDB length is less than 16 bytes.

Thanks for your cooperation.

Regards,
TAMUKI Shoichi