Re: [Qemu-devel] virtio-scsi spec, first public draft

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

On Thu, May 05, 2011 at 12:28:31AM +0200, Paolo Bonzini wrote:
> Virtio SCSI Controller Device Spec
> ==================================
> 
> The virtio controller device groups together one or more simple virtual
> devices (ie. disk), and allows communicating to these devices using the
> SCSI protocol.  A controller device represents a SCSI host with many
> targets attached.
> 
> The virtio controller services two kinds of requests:
> 
> - command requests for a logical unit;
> 
> - task management functions related to a logical unit, target or
> command.
> 
> The controller is also able to send out notifications about added
> and removed devices.
> 
> v4:
>     First public version
> 
> Configuration
> -------------
> 
> Subsystem Device ID
>     TBD
> 
> Virtqueues
>     0..n-1:one requestq per target
>     n:control transmitq
>     n+1:control receiveq

1 requestq per target makes it harder to support large numbers or
dynamic targets.  You mention detaching targets so is there a way to add
a target?

The following would be simpler:
    0:requestq
    1:control transmitq
    2:control receiveq

Requests must include a target port identifier/name so that they can be
delivered to the correct target.  Adding or removing targets is easy
with a single requestq since the virtqueues don't change.

> Feature bits
>     VIRTIO_SCSI_F_INOUT - Whether a single request can include both
>         read-only and write-only data buffers.

Why make this an optional feature?

> Device configuration layout
>     struct virtio_scsi_config {
>         u32 num_targets;
>     }
> 
>     num_targets is the number of targets, and the id of the
>     virtqueue used for the control receiveq.
> 
> Device initialization
> ---------------------
> 
> The initialization routine should first of all discover the controller's
> control virtqueues.
> 
> The driver should then place at least a buffer in the control receiveq.
> Buffers returned by the device on the control receiveq may be referred
> to as "events" in the rest of the document.
> 
> The driver can immediately issue requests (for example, INQUIRY or
> REPORT LUNS) or task management functions (for example, I_T RESET).
> 
> Device operation: request queue
> -------------------------------
> 
> The driver queues requests to the virtqueue, and they are used by the device
> (not necessarily in order).
> 
> Requests have the following format:
> 
>     struct virtio_scsi_req
>     {
>         u32 type;
>         ...
>         u8 response;
>     }
> 
>     #define VIRTIO_SCSI_T_BARRIER         0x80000000
> 
>     The type identifies the remaining fields.  The value
>     VIRTIO_SCSI_T_BARRIER can be ORed in the type as well.  This bit
>     indicates that this request acts as a barrier and that all preceding
>     requests must be complete before this one, and all following requests
>     must not be started until this is complete.  Note that a barrier
>     does not flush caches in the underlying backend device in host,
>     and thus does not serve as data consistency guarantee.  The driver
>     must send a SYNCHRONIZE CACHE command to flush the host cache.

Why are these barrier semantics needed?

>     Valid response values are defined separately for each command.
> 
> - Task management function
> 
>     #define VIRTIO_SCSI_T_TMF                      0
> 
>     #define VIRTIO_SCSI_T_TMF_ABORT_TASK_SET       1
>     #define VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET      4
>     #define VIRTIO_SCSI_T_TMF_LOGICAL_UNIT_RESET   5
>     #define VIRTIO_SCSI_T_TMF_QUERY_TASK_SET       7
> 
>     #define VIRTIO_SCSI_T_TMF_LOGICAL_UNIT_DETACH  (1 << 24)
> 
>     struct virtio_scsi_req_tmf
>     {
>         u32 subtype;
>         u8 lun[8];
>         u8 additional[];
>         u8 response;
>     }
> 
>     /* command-specific response values */
>     #define VIRTIO_SCSI_S_FUNCTION_COMPLETE        0
>     #define VIRTIO_SCSI_S_NO_TARGET                1
>     #define VIRTIO_SCSI_S_TARGET_FAILURE           2
>     #define VIRTIO_SCSI_S_FUNCTION_SUCCEEDED       3
>     #define VIRTIO_SCSI_S_FUNCTION_REJECTED        4
>     #define VIRTIO_SCSI_S_INCORRECT_LUN            5
> 
>     The type is VIRTIO_SCSI_T_LUN_INFO, possibly with the
>     VIRTIO_SCSI_T_BARRIER bit ORed in.

Did you mean "type is VIRTIO_SCSI_T_TMF"?

> 
>     The subtype and lun field are filled in by the driver, the additional
>     and response field is filled in by the device.  Unknown LUNs are
>     ignored; also, the lun field is ignored for the I_T NEXUS RESET
>     command.

In/out buffers must be separate in virtio so I think it makes sense to
split apart a struct virtio_scsi_tmf_req and struct
virtio_scsi_tmf_resp.

>     Task management functions accepting an I_T_L_Q nexus (ABORT TASK,
>     QUERY TASK) are only accessible through the control transmitq.
>     Task management functions not in the above list are not accessible
>     in this version of the specification.  Future versions may allow
>     access to them through additional features.
> 
>     VIRTIO_SCSI_T_TMF_LOGICAL_UNIT_DETACH asks the device to make the
>     logical unit (and the target as well if this is the last logical
>     unit) disappear.  It takes an I_T_L nexus.  This non-standard TMF
>     should be used in response to a host request to shutdown a target
>     or LUN, after having placed the LUN in a clean state.

Do we need an initiator-driven detach?  If the initiator doesn't care
about a device anymore it simply doesn't communicate with it or allocate
resources for it.  I think the real detach should be performed on the
target side (e.g. QEMU monitor command removes the target from the SCSI
bus).  So I guess I'm asking what is the real use-case for this
function?

>     The outcome of the task management function is written by the device
>     in the response field.  A value of VIRTIO_SCSI_S_NO_TARGET means
>     that (even though the virtqueue exists) there is no target with this
>     number.  Other return values map 1-to-1 with those defined in SAM.
> 
> - SCSI command
> 
>     #define VIRTIO_SCSI_T_CMD	                    1
> 
>     struct virtio_scsi_req_cmd {
>         u32 type;
>         u32 ioprio;
>         u8 lun[8];
>         u64 id;
>         u32 num_dataout, num_datain;
>         char cdb[];
>         char data[][num_dataout+num_datain];
>         u8 sense[];
>         u32 sense_len;
>         u32 residual;
>         u8 status;
>         u8 response;
>     };

We don't need explicit buffer size fields since virtqueue elements
include sizes.  For example:

size_t sense_len = elem->in_sg[sense_idx].iov_len;
memcpy(elem->in_sg[sense_idx].iov_buf, sense_buf,
       MIN(sense_len, sizeof(sense_buf)));

Stefan