The initial version of TCMU (in 3.18) does not properly handle
bidirectional SCSI commands -- those with both an in and out buffer. In
looking to fix this it also became clear that TCMU's support for adding
new types of entries (opcodes) to the command ring was broken. We need
to fix this now, so that future issues can be handled properly by adding
new opcodes.
We make the most of this ABI break by enabling bidi cmd handling within
TCMP_OP_CMD opcode. Add an iov_bidi_cnt field to tcmu_cmd_entry.req.
This enables TCMU to describe bidi commands, but further kernel work is
needed for full bidi support.
Enlarge tcmu_cmd_entry_hdr by 32 bits by pulling in cmd_id and __pad1. Turn
__pad1 into two 8 bit flags fields, for kernel-set and userspace-set flags,
"kflags" and "uflags" respectively.
Update version fields so userspace can tell the interface is changed.
Update tcmu-design.txt with details of how new stuff works:
- Specify an additional requirement for userspace to set CMD_SKIPPED
(bit 0) in hdr.uflags for PAD and unknown/unhandled opcodes.
- Define how Data-In and Data-Out fields are described in req.iov[]
Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
---
Documentation/target/tcmu-design.txt | 38 +++++++++++++++++++++++------------
drivers/target/target_core_user.c | 16 +++++++++++----
include/uapi/linux/target_core_user.h | 13 +++++++-----
3 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/Documentation/target/tcmu-design.txt b/Documentation/target/tcmu-design.txt
index 5518465..0b3ecc3 100644
--- a/Documentation/target/tcmu-design.txt
+++ b/Documentation/target/tcmu-design.txt
@@ -140,25 +140,37 @@ processed by userspace.
TCMU commands start with a common header containing "len_op", a 32-bit
value that stores the length, as well as the opcode in the lowest
-unused bits. Currently only two opcodes are defined, TCMU_OP_PAD and
-TCMU_OP_CMD. When userspace encounters a command with PAD opcode, it
-should skip ahead by the bytes in "length". (The kernel inserts PAD
-entries to ensure each CMD entry fits contigously into the circular
-buffer.)
-
-When userspace handles a CMD, it finds the SCSI CDB (Command Data
-Block) via tcmu_cmd_entry.req.cdb_off. This is an offset from the
-start of the overall shared memory region, not the entry. The data
-in/out buffers are accessible via tht req.iov[] array. Note that
-each iov.iov_base is also an offset from the start of the region.
-
-TCMU currently does not support BIDI operations.
+unused bits. It also contains cmd_id and flags fields for setting by
+the kernel (kflags) and userspace (uflags).
+
+Currently only two opcodes are defined, TCMU_OP_CMD and TCMU_OP_PAD.
+
+When the opcode is CMD, the entry in the command ring is a struct
+tcmu_cmd_entry. Userspace finds the SCSI CDB (Command Data Block) via
+tcmu_cmd_entry.req.cdb_off. This is an offset from the start of the
+overall shared memory region, not the entry. The data in/out buffers
+are accessible via tht req.iov[] array. iov_cnt contains the number of
+entries in iov[] needed to describe either the Data-In or Data-Out
+buffers. For bidirectional commands, iov_cnt specifies how many iovec
+entries cover the Data-Out area, and iov_bidi_count specifies how many
+iovec entries immediately after that in iov[] cover the Data-In
+area. Just like other fields, iov.iov_base is an offset from the start
+of the region.
When completing a command, userspace sets rsp.scsi_status, and
rsp.sense_buffer if necessary. Userspace then increments
mailbox.cmd_tail by entry.hdr.length (mod cmdr_size) and signals the
kernel via the UIO method, a 4-byte write to the file descriptor.
+When the opcode is PAD, userspace should set CMD_SKIPPED bit (bit 0)
+in hdr.uflags, and then similarly update cmd_tail as above. (The
+kernel inserts PAD entries to ensure each CMD entry fits contigously
+into the circular buffer.)
+
+More opcodes may be added in the future. If userspace encounters an
+opcode it does not handle, it is treated like a PAD: set CMD_SKIPPED,
+and update cmd_tail to skip it.
+
The Data Area:
This is shared-memory space after the command ring. The organization
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 1fbf304..0a9bc4c 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -346,6 +346,9 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
tcmu_flush_dcache_range(entry, sizeof(*entry));
tcmu_hdr_set_op(&entry->hdr, TCMU_OP_PAD);
tcmu_hdr_set_len(&entry->hdr, pad_size);
+ entry->hdr.cmd_id = 0; /* not used for PAD */
+ entry->hdr.kflags = 0;
+ entry->hdr.uflags = 0;
UPDATE_HEAD(mb->cmd_head, pad_size, udev->cmdr_size);
@@ -357,7 +360,9 @@ static int tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
tcmu_flush_dcache_range(entry, sizeof(*entry));
tcmu_hdr_set_op(&entry->hdr, TCMU_OP_CMD);
tcmu_hdr_set_len(&entry->hdr, command_size);
- entry->cmd_id = tcmu_cmd->cmd_id;
+ entry->hdr.cmd_id = tcmu_cmd->cmd_id;
+ entry->hdr.kflags = 0;
+ entry->hdr.uflags = 0;
/*
* Fix up iovecs, and handle if allocation in data ring wrapped.
@@ -543,13 +548,16 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
tcmu_flush_dcache_range(entry, sizeof(*entry));
if (tcmu_hdr_get_op(&entry->hdr) == TCMU_OP_PAD) {
+ if (!(entry->hdr.uflags & TCMU_OP_UFLAG_CMD_SKIPPED))
+ pr_warn("PAD completed without SKIPPED bit set\n");
+
UPDATE_HEAD(udev->cmdr_last_cleaned, tcmu_hdr_get_len(&entry->hdr), udev->cmdr_size);
continue;
}
WARN_ON(tcmu_hdr_get_op(&entry->hdr) != TCMU_OP_CMD);
spin_lock(&udev->commands_lock);
- cmd = idr_find(&udev->commands, entry->cmd_id);
+ cmd = idr_find(&udev->commands, entry->hdr.cmd_id);
if (cmd)
idr_remove(&udev->commands, cmd->cmd_id);
spin_unlock(&udev->commands_lock);
@@ -840,14 +848,14 @@ static int tcmu_configure_device(struct se_device *dev)
udev->data_size = TCMU_RING_SIZE - CMDR_SIZE;
mb = udev->mb_addr;
- mb->version = 1;
+ mb->version = 2;
mb->cmdr_off = CMDR_OFF;
mb->cmdr_size = udev->cmdr_size;
WARN_ON(!PAGE_ALIGNED(udev->data_off));
WARN_ON(udev->data_size % PAGE_SIZE);
- info->version = "1";
+ info->version = "2";
info->mem[0].name = "tcm-user command & data buffer";
info->mem[0].addr = (phys_addr_t) udev->mb_addr;
diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h
index b483d19..ccb7ff1 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -64,7 +64,12 @@ enum tcmu_opcode {
* Only a few opcodes, and length is 8-byte aligned, so use low bits for opcode.
*/
struct tcmu_cmd_entry_hdr {
- __u32 len_op;
+ __u32 len_op;
+ __u16 cmd_id;
+ __u8 kflags;
+#define TCMU_OP_UFLAG_CMD_SKIPPED 0x1
+ __u8 uflags;
+
} __packed;
#define TCMU_OP_MASK 0x7
@@ -97,13 +102,11 @@ static inline void tcmu_hdr_set_len(struct tcmu_cmd_entry_hdr *hdr, __u32 len)
struct tcmu_cmd_entry {
struct tcmu_cmd_entry_hdr hdr;
- uint16_t cmd_id;
- uint16_t __pad1;
-
union {
struct {
uint64_t cdb_off;
- uint64_t iov_cnt;
+ uint32_t iov_cnt;
+ uint32_t iov_bidi_cnt;
struct iovec iov[0];
} req;
struct {
--
2.1.0
--
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
