Re: [PATCHv2] target/user: Disallow full passthrough (pass_level=0)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 13, 2015 at 09:43:23AM -0700, Andy Grover wrote:
> How far is this:
>
> https://git.kernel.org/cgit/linux/kernel/git/grover/linux.git/commit/?h=tcmu-iomode-option7
>
> from what you're describing? This is enumerating all actual commands we 
> want to send to userspace, is it not?

It goes in a different direction actually.  To demonstrate what I mean
I came up with the patch below.  It compiles, breaks the full passthrough
case and certainly doesn't work, so it's only intended as an explanation.

So instead of passing up anything resembling SCSI comes we passes
what comes out of the bottom of sbc_parse_cdb: reads, writes, flushes,
ignoring the discard and write_same cases for now to save me work.

This uses sbc_parse_cdb the way the other non-pscsi backends do: to
sequence down the complex SCSI protocol to a few simple building
blocks that all the backends can implement trivially.  This gets
you support for BIDI commands, COMPARE AND WRITE and all that
magic for free.


diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index d59df02..f7091fd 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -375,7 +375,8 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, size_t cmd_size, size_t d
 	return true;
 }
 
-static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
+static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd,
+		struct scatterlist *sgl, u32 sgl_nents, enum tcmu_operation op)
 {
 	struct tcmu_dev *udev = tcmu_cmd->tcmu_dev;
 	struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
@@ -385,7 +386,6 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 	struct iovec *iov;
 	int iov_cnt;
 	uint32_t cmd_head;
-	uint64_t cdb_off;
 	bool copy_to_data_area;
 
 	if (test_bit(TCMU_DEV_BIT_BROKEN, &udev->flags))
@@ -399,11 +399,9 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 	 * b/c size == offsetof one-past-element.
 	*/
 	base_command_size = max(offsetof(struct tcmu_cmd_entry,
-					 req.iov[se_cmd->t_bidi_data_nents +
-						 se_cmd->t_data_nents + 2]),
+					 req.iov[sgl_nents + 2]),
 				sizeof(struct tcmu_cmd_entry));
-	command_size = base_command_size
-		+ round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
+	command_size = base_command_size;
 
 	WARN_ON(command_size & (TCMU_OP_ALIGN_SIZE-1));
 
@@ -471,21 +469,15 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 	iov_cnt = 0;
 	copy_to_data_area = (se_cmd->data_direction == DMA_TO_DEVICE
 		|| se_cmd->se_cmd_flags & SCF_BIDI);
-	alloc_and_scatter_data_area(udev, se_cmd->t_data_sg,
-		se_cmd->t_data_nents, &iov, &iov_cnt, copy_to_data_area);
+	alloc_and_scatter_data_area(udev, sgl,
+		sgl_nents, &iov, &iov_cnt, copy_to_data_area);
 	entry->req.iov_cnt = iov_cnt;
 	entry->req.iov_dif_cnt = 0;
 
-	/* Handle BIDI commands */
-	iov_cnt = 0;
-	alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
-		se_cmd->t_bidi_data_nents, &iov, &iov_cnt, false);
-	entry->req.iov_bidi_cnt = iov_cnt;
-
 	/* All offsets relative to mb_addr, not start of entry! */
-	cdb_off = CMDR_OFF + cmd_head + base_command_size;
-	memcpy((void *) mb + cdb_off, se_cmd->t_task_cdb, scsi_command_size(se_cmd->t_task_cdb));
-	entry->req.cdb_off = cdb_off;
+	entry->req.opcode = op;
+	entry->req.sector = se_cmd->t_task_lba;
+	entry->req.len = se_cmd->data_length;
 	tcmu_flush_dcache_range(entry, sizeof(*entry));
 
 	UPDATE_HEAD(mb->cmd_head, command_size, udev->cmdr_size);
@@ -502,7 +494,8 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 	return 0;
 }
 
-static int tcmu_queue_cmd(struct se_cmd *se_cmd)
+static sense_reason_t tcmu_queue_cmd(struct se_cmd *se_cmd,
+		struct scatterlist *sgl, u32 sgl_nents, enum tcmu_operation op)
 {
 	struct se_device *se_dev = se_cmd->se_dev;
 	struct tcmu_dev *udev = TCMU_DEV(se_dev);
@@ -511,9 +504,9 @@ static int tcmu_queue_cmd(struct se_cmd *se_cmd)
 
 	tcmu_cmd = tcmu_alloc_cmd(se_cmd);
 	if (!tcmu_cmd)
-		return -ENOMEM;
+		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
-	ret = tcmu_queue_cmd_ring(tcmu_cmd);
+	ret = tcmu_queue_cmd_ring(tcmu_cmd, sgl, sgl_nents, op);
 	if (ret < 0) {
 		pr_err("TCMU: Could not queue command\n");
 		spin_lock_irq(&udev->commands_lock);
@@ -521,9 +514,10 @@ static int tcmu_queue_cmd(struct se_cmd *se_cmd)
 		spin_unlock_irq(&udev->commands_lock);
 
 		kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
+		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
-	return ret;
+	return TCM_NO_SENSE;
 }
 
 static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *entry)
@@ -1084,33 +1078,20 @@ static sense_reason_t
 tcmu_execute_rw(struct se_cmd *se_cmd, struct scatterlist *sgl, u32 sgl_nents,
 		enum dma_data_direction data_direction)
 {
-	int ret;
-
-	ret = tcmu_queue_cmd(se_cmd);
-
-	if (ret != 0)
-		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-	else
-		return TCM_NO_SENSE;
+	return tcmu_queue_cmd(se_cmd, sgl, sgl_nents,
+			data_direction == DMA_TO_DEVICE ?
+				TCMU_OP_WRITE : TCMU_OP_READ);
 }
 
 static sense_reason_t
-tcmu_pass_op(struct se_cmd *se_cmd)
+tcmu_execute_sync_cache(struct se_cmd *se_cmd)
 {
-	int ret = tcmu_queue_cmd(se_cmd);
-
-	if (ret != 0)
-		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-	else
-		return TCM_NO_SENSE;
+	return tcmu_queue_cmd(se_cmd, NULL, 0, TCMU_OP_FLUSH);
 }
 
 static struct sbc_ops tcmu_sbc_ops = {
-	.execute_rw = tcmu_execute_rw,
-	.execute_sync_cache	= tcmu_pass_op,
-	.execute_write_same	= tcmu_pass_op,
-	.execute_write_same_unmap = tcmu_pass_op,
-	.execute_unmap		= tcmu_pass_op,
+	.execute_rw		= tcmu_execute_rw,
+	.execute_sync_cache	= tcmu_execute_sync_cache,
 };
 
 static sense_reason_t
@@ -1121,6 +1102,7 @@ tcmu_parse_cdb(struct se_cmd *cmd)
 	sense_reason_t ret;
 
 	switch (udev->pass_level) {
+#if 0
 	case TCMU_PASS_ALL:
 		/* We're just like pscsi, then */
 		/*
@@ -1147,6 +1129,7 @@ tcmu_parse_cdb(struct se_cmd *cmd)
 		}
 		ret = TCM_NO_SENSE;
 		break;
+#endif
 	case TCMU_PASS_IO:
 		ret = sbc_parse_cdb(cmd, &tcmu_sbc_ops);
 		break;
diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h
index b67f99d..26a0fe2 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -103,6 +103,12 @@ static inline void tcmu_hdr_set_len(__u32 *len_op, __u32 len)
 /* Currently the same as SCSI_SENSE_BUFFERSIZE */
 #define TCMU_SENSE_BUFFERSIZE 96
 
+enum tcmu_operation {
+	TCMU_OP_READ,
+	TCMU_OP_WRITE,
+	TCMU_OP_FLUSH,
+};
+
 struct tcmu_cmd_entry {
 	struct tcmu_cmd_entry_hdr hdr;
 
@@ -111,9 +117,9 @@ struct tcmu_cmd_entry {
 			uint32_t iov_cnt;
 			uint32_t iov_bidi_cnt;
 			uint32_t iov_dif_cnt;
-			uint64_t cdb_off;
-			uint64_t __pad1;
-			uint64_t __pad2;
+			uint64_t opcode;
+			uint64_t sector;
+			uint64_t len;
 			struct iovec iov[0];
 		} req;
 		struct {
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux