On Thu, Jul 26, 2012 at 4:43 PM, Nicholas A. Bellinger
<n...@linux-iscsi.org>wrote:
> [...]
> +static void vhost_scsi_handle_vq(struct vhost_scsi *vs)
> +{
> + struct vhost_virtqueue *vq = &vs->vqs[2];
> + struct virtio_scsi_cmd_req v_req;
> + struct tcm_vhost_tpg *tv_tpg;
> + struct tcm_vhost_cmd *tv_cmd;
> + u32 exp_data_len, data_first, data_num, data_direction;
> + unsigned out, in, i;
> + int head, ret;
> +
> + /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */
> + tv_tpg = vs->vs_tpg;
> + if (unlikely(!tv_tpg)) {
> + pr_err("%s endpoint not set\n", __func__);
> + return;
> + }
> +
> + mutex_lock(&vq->mutex);
> + vhost_disable_notify(&vs->dev, vq);
> +
> + for (;;) {
> + head = vhost_get_vq_desc(&vs->dev, vq, vq->iov,
> + ARRAY_SIZE(vq->iov), &out, &in,
> + NULL, NULL);
> + pr_debug("vhost_get_vq_desc: head: %d, out: %u in: %u\n",
> + head, out, in);
> + /* On error, stop handling until the next kick. */
> + if (unlikely(head < 0))
> + break;
> + /* Nothing new? Wait for eventfd to tell us they
> refilled. */
> + if (head == vq->num) {
> + if (unlikely(vhost_enable_notify(&vs->dev, vq))) {
> + vhost_disable_notify(&vs->dev, vq);
> + continue;
> + }
> + break;
> + }
> +
> +/* FIXME: BIDI operation */
> + if (out == 1 && in == 1) {
>
It seems to me like this is not the way that virtio devices are supposed to
behave - if a guest splits a virtio_scsi_cmd_req or _resp across a page
boundary, then this code won't work.
Quoting the 'Message Framing' part of the virtio spec:
"In particular, no implementation should use the descriptor boundaries to
determine the size of any header in a request. "
+ data_direction = DMA_NONE;
> + data_first = 0;
> + data_num = 0;
> + } else if (out == 1 && in > 1) {
> + data_direction = DMA_FROM_DEVICE;
> + data_first = out + 1;
> + data_num = in - 1;
> + } else if (out > 1 && in == 1) {
> + data_direction = DMA_TO_DEVICE;
> + data_first = 1;
> + data_num = out - 1;
> + } else {
> + vq_err(vq, "Invalid buffer layout out: %u in:
> %u\n",
> + out, in);
> + break;
> + }
> +
> + /*
> + * Check for a sane resp buffer so we can report errors to
> + * the guest.
> + */
> + if (unlikely(vq->iov[out].iov_len !=
> + sizeof(struct
> virtio_scsi_cmd_resp))) {
> + vq_err(vq, "Expecting virtio_scsi_cmd_resp, got
> %zu"
> + " bytes\n", vq->iov[out].iov_len);
> + break;
> + }
> +
> + if (unlikely(vq->iov[0].iov_len != sizeof(v_req))) {
> + vq_err(vq, "Expecting virtio_scsi_cmd_req, got %zu"
> + " bytes\n", vq->iov[0].iov_len);
> + break;
> + }
> + pr_debug("Calling __copy_from_user: vq->iov[0].iov_base:
> %p,"
> + " len: %zu\n", vq->iov[0].iov_base, sizeof(v_req));
> + ret = __copy_from_user(&v_req, vq->iov[0].iov_base,
> + sizeof(v_req));
> + if (unlikely(ret)) {
> + vq_err(vq, "Faulted on virtio_scsi_cmd_req\n");
> + break;
> + }
> +
> + exp_data_len = 0;
> + for (i = 0; i < data_num; i++)
> + exp_data_len += vq->iov[data_first + i].iov_len;
> +
> + tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req,
> + exp_data_len, data_direction);
> + if (IS_ERR(tv_cmd)) {
> + vq_err(vq, "vhost_scsi_allocate_cmd failed %ld\n",
> + PTR_ERR(tv_cmd));
> + break;
> + }
> + pr_debug("Allocated tv_cmd: %p exp_data_len: %d,
> data_direction"
> + ": %d\n", tv_cmd, exp_data_len, data_direction);
> +
> + tv_cmd->tvc_vhost = vs;
> +
> + if (unlikely(vq->iov[out].iov_len !=
> + sizeof(struct virtio_scsi_cmd_resp))) {
> + vq_err(vq, "Expecting virtio_scsi_cmd_resp, got
> %zu"
> + " bytes, out: %d, in: %d\n",
> + vq->iov[out].iov_len, out, in);
> + break;
> + }
> +
> + tv_cmd->tvc_resp = vq->iov[out].iov_base;
> +
> + /*
> + * Copy in the recieved CDB descriptor into tv_cmd->tvc_cdb
> + * that will be used by tcm_vhost_new_cmd_map() and down
> into
> + * target_setup_cmd_from_cdb()
> + */
> + memcpy(tv_cmd->tvc_cdb, v_req.cdb, TCM_VHOST_MAX_CDB_SIZE);
> + /*
> + * Check that the recieved CDB size does not exceeded our
> + * hardcoded max for tcm_vhost
> + */
> + /* TODO what if cdb was too small for varlen cdb header? */
> + if (unlikely(scsi_command_size(tv_cmd->tvc_cdb) >
> + TCM_VHOST_MAX_CDB_SIZE)) {
> + vq_err(vq, "Received SCSI CDB with command_size:
> %d that"
> + " exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
> + scsi_command_size(tv_cmd->tvc_cdb),
> + TCM_VHOST_MAX_CDB_SIZE);
> + break; /* TODO */
> + }
> + tv_cmd->tvc_lun = ((v_req.lun[2] << 8) | v_req.lun[3]) &
> 0x3FFF;
> +
> + pr_debug("vhost_scsi got command opcode: %#02x, lun: %d\n",
> + tv_cmd->tvc_cdb[0], tv_cmd->tvc_lun);
> +
> + if (data_direction != DMA_NONE) {
> + ret = vhost_scsi_map_iov_to_sgl(tv_cmd,
> + &vq->iov[data_first], data_num,
> + data_direction == DMA_TO_DEVICE);
> + if (unlikely(ret)) {
> + vq_err(vq, "Failed to map iov to sgl\n");
> + break; /* TODO */
> + }
> + }
> +
> + /*
> + * Save the descriptor from vhost_get_vq_desc() to be used
> to
> + * complete the virtio-scsi request in TCM callback
> context via
> + * tcm_vhost_queue_data_in() and tcm_vhost_queue_status()
> + */
> + tv_cmd->tvc_vq_desc = head;
> + /*
> + * Dispatch tv_cmd descriptor for cmwq execution in process
> + * context provided by tcm_vhost_workqueue. This also
> ensures
> + * tv_cmd is executed on the same kworker CPU as this vhost
> + * thread to gain positive L2 cache locality effects..
> + */
> + INIT_WORK(&tv_cmd->work, tcm_vhost_submission_work);
> + queue_work(tcm_vhost_workqueue, &tv_cmd->work);
> + }
> +
> + mutex_unlock(&vq->mutex);
> +}
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Typing one-handed, please don't mistake brevity for rudeness.
