Skip to content

usb: gadget: f_tcm: Enhance UASP driver

Thinh Nguyen linux-usb oe-kbuild oe-kbuild-all stable 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v3 Latest28 patches 7 repliesIn review

Patches in v3 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/28usb: gadget: f_tcm: Don't free command immediatelydrivers/usb/gadget/function/f_tcm.c+0 -20 replies
2/28usb: gadget: f_tcm: Translate error to sensedrivers/usb/gadget/function/f_tcm.c+2 -20 replies
3/28usb: gadget: f_tcm: Decrement command ref count on cleanupdrivers/usb/gadget/function/f_tcm.c+1 -00 replies
4/28usb: gadget: f_tcm: Fix Get/SetInterface return valuedrivers/usb/gadget/function/f_tcm.c+10 -20 replies
5/28usb: gadget: f_tcm: ep_autoconfig with fullspeed endpointdrivers/usb/gadget/function/f_tcm.c+14 -180 replies
6/28usb: gadget: f_tcm: Don't prepare BOT write request twicedrivers/usb/gadget/function/f_tcm.c+0 -170 replies
7/28usb: gadget: f_tcm: Increase stream countdrivers/usb/gadget/function/tcm.h+1 -10 replies
8/28usb: gadget: f_tcm: Increase bMaxBurstdrivers/usb/gadget/function/f_tcm.c+4 -20 replies
9/28usb: gadget: f_tcm: Limit number of sessionsdrivers/usb/gadget/function/tcm.h+1 -10 replies
10/28usb: gadget: f_tcm: Get stream by sbitmap numberdrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+6 -210 replies
11/28usb: gadget: f_tcm: Don't set static stream_iddrivers/usb/gadget/function/f_tcm.c+3 -140 replies
12/28usb: gadget: f_tcm: Allocate matching number of commands to streamsdrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+61 -560 replies
13/28usb: gadget: f_tcm: Handle multiple commands in paralleldrivers/usb/gadget/function/f_tcm.c+8 -120 replies
14/28usb: gadget: f_tcm: Use extra number of commandsdrivers/usb/gadget/function/tcm.h+1 -10 replies
15/28usb: gadget: f_tcm: Return ATA cmd directiondrivers/usb/gadget/function/f_tcm.c+2 -00 replies
16/28usb: gadget: f_tcm: Execute command on write completiondrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+34 -90 replies
17/28usb: gadget: f_tcm: Minor cleanup redundant codedrivers/usb/gadget/function/f_tcm.c+0 -20 replies
18/28usb: gadget: f_tcm: Handle abort commanddrivers/usb/gadget/function/f_tcm.c+18 -00 replies
19/28usb: gadget: f_tcm: Cleanup requests on ep disabledrivers/usb/gadget/function/f_tcm.c+13 -20 replies
20/28usb: gadget: f_tcm: Stop proceeding further on -ESHUTDOWNdrivers/usb/gadget/function/f_tcm.c+7 -30 replies
21/28usb: gadget: f_tcm: Save CPU ID per commanddrivers/usb/gadget/function/f_tcm.c+1 -00 replies
22/28usb: gadget: f_tcm: Send sense on cancelled transferdrivers/usb/gadget/function/f_tcm.c+2 -10 replies
23/28usb: gadget: f_tcm: Handle TASK_MANAGEMENT commandsdrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+206 -190 replies
24/28usb: gadget: f_tcm: Check overlapped commanddrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+127 -13 replies
25/28usb: gadget: f_tcm: Stall on invalid CBWdrivers/usb/gadget/function/f_tcm.c drivers/usb/gadget/function/tcm.h+16 -10 replies
26/28usb: gadget: f_tcm: Requeue command request on errordrivers/usb/gadget/function/f_tcm.c+6 -10 replies
27/28usb: gadget: f_tcm: Track BOT command krefdrivers/usb/gadget/function/f_tcm.c+2 -10 replies
28/28usb: gadget: f_tcm: Refactor goto check_conditiondrivers/usb/gadget/function/f_tcm.c+12 -140 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.

Read the v3 thread

Cover letter [PATCH v3 00/28] usb: gadget: f_tcm: Enhance UASP driver

Apologies for the delay; after two years and multiple requests to resume this series, I squeezed some time to push an update. This series applies on top of Greg's usb-testing branch.

If possible, please help test this series and get this merged as my resources are nil for this work.

Example Bringup Steps
=====================
To test UASP, here's an example perl script snippet to bring it up.

Note: the script was cut down and quickly rewritten, so sorry if I make mistakes.

    my $MY_UAS_VID = xxxx;
    my $MY_UAS_PID = yyyy;
    my $SERIAL = "1234";
    my $VENDOR = "VENDOR";
    my $MY_VER = "VER";

    my $vendor_id = "my_vid";
    my $product_id = "my_pid";
    my $revision = "my_rev";

    # Must update:
    my $backing_storage = "/tmp/some_file";
    my $backing_storage_size = 1024*1024*16;
    my $use_ramdisk = 0;

    my $g = "/sys/kernel/config/usb_gadget/g1";

    system("modprobe libcomposite");
    system("modprobe usb_f_tcm");
    system("mkdir -p $g");
    system("mkdir -p $g/configs/c.1");
    system("mkdir -p $g/functions/tcm.0");
    system("mkdir -p $g/strings/0x409");
    system("mkdir -p $g/configs/c.1/strings/0x409");

    my $tp = "/sys/kernel/config/target/usb_gadget/naa.0/tpgt_1";

    my $tf;
    my $ctrl;

    if ($use_ramdisk) {
        $tf = "/sys/kernel/config/target/core/rd_mcp_0/ramdisk";
        $ctrl = 'rd_pages=524288';
    } else {
        $tf = "/sys/kernel/config/target/core/fileio_0/fileio";
        $ctrl = 'fd_dev_name=$backing_storage,fd_dev_size=$backing_storage_size,fd_async_io=1';
    }

    system("mkdir -p /etc/target");

    system("mkdir -p $tp");
    system("mkdir -p $tf");
    system("mkdir -p $tp/lun/lun_0");

    system("echo naa.0         > $tp/nexus");
    system("echo $ctrl         > $tf/control");
    system("echo 1             > $tf/attrib/emulate_ua_intlck_ctrl");
    system("echo 123           > $tf/wwn/vpd_unit_serial");
    system("echo $vendor_id    > $tf/wwn/vendor_id");
    system("echo $product_id   > $tf/wwn/product_id");
    system("echo $revision     > $tf/wwn/revision");
    system("echo 1             > $tf/enable");

    system("ln -s $tf $tp/lun/lun_0/virtual_scsi_port");
    system("echo 1             > $tp/enable");

    system("echo $MY_UAS_PID   > $g/idProduct");

    system("ln -s $g/functions/tcm.0 $g/configs/c.1");

    system("echo $MY_UAS_VID   > $g/idVendor");
    system("echo $SERIAL       > $g/strings/0x409/serialnumber");
    system("echo $VENDOR       > $g/strings/0x409/manufacturer");
    system("echo \"$MY_VER\"   > $g/strings/0x409/product");
    system("echo \"Conf 1\"    > $g/configs/c.1/strings/0x409/configuration");
    system("echo super-speed-plus > $g/max_speed");

    # Make sure the UDC is available
    system("echo $my_udc       > $g/UDC");

Target Subsystem Fixes
======================
I have eliminated unnecessary changes related to the Target subsystem and reworked f_tcm to minimize the modifications required in the Target subsystem. There are unimplemented Task Management Requests in the Target subsystem, but the basic flow should still work.

Regardless, you should still need to apply at least these 2 fixes:

1) Fix Data Corruption
----------------------

Properly increment the "len" base on the command requested length instead of the SG entry length.

If you're using File backend, then you need to fix target_core_file. If you're using other backend such as Ramdisk, then you need a similar fix there.

---
 drivers/target/target_core_file.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/target/target_core_file.c +12 -2
index 2d78ef74633c..d9fc048c1734 100644
@@ -283,7 +283,12 @@ fd_execute_rw_aio(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	for_each_sg(sgl, sg, sgl_nents, i) {
 		bvec_set_page(&aio_cmd->bvecs[i], sg_page(sg), sg->length,
 			      sg->offset);
-		len += sg->length;
+		if (len + sg->length >= cmd->data_length) {
+			len = cmd->data_length;
+			break;
+		} else {
+			len += sg->length;
+		}
 	}
 
 	iov_iter_bvec(&iter, is_write, aio_cmd->bvecs, sgl_nents, len);
@@ -328,7 +333,12 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd,
 
 	for_each_sg(sgl, sg, sgl_nents, i) {
 		bvec_set_page(&bvec[i], sg_page(sg), sg->length, sg->offset);
-		len += sg->length;
+		if (len + sg->length >= data_length) {
+			len = data_length;
+			break;
+		} else {
+			len += sg->length;
+		}
 	}
 
 	iov_iter_bvec(&iter, is_write, bvec, sgl_nents, len);
Signature · 13 lines
2) Fix Sense Data Length
------------------------

The transport_get_sense_buffer() and transport_copy_sense_to_cmd() take
sense data length to be the allocated sense buffer length
TRANSPORT_SENSE_BUFFER. However, the sense data length is depending on
the sense data description. Check the sense data to set the proper
cmd->scsi_sense_length.

See SPC4-r37 section 4.5.2.1.
---
 drivers/target/target_core_transport.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
drivers/target/target_core_transport.c +15 -4
index 8d8f4ad4f59e..da75d6873ab5 100644
@@ -804,8 +804,6 @@ static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd)
 	if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION)
 		return NULL;
 
-	cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER;
-
 	pr_debug("HBA_[%u]_PLUG[%s]: Requesting sense for SAM STATUS: 0x%02x\n",
 		dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status);
 	return cmd->sense_buffer;
@@ -824,7 +822,13 @@ void transport_copy_sense_to_cmd(struct se_cmd *cmd, unsigned char *sense)
 	}
 
 	cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE;
+
+	/* Sense data length = min sense data + additional sense data length */
+	cmd->scsi_sense_length = min_t(u16, cmd_sense_buf[7] + 8,
+				       TRANSPORT_SENSE_BUFFER);
+
 	memcpy(cmd_sense_buf, sense, cmd->scsi_sense_length);
+
 	spin_unlock_irqrestore(&cmd->t_state_lock, flags);
 }
 EXPORT_SYMBOL(transport_copy_sense_to_cmd);
@@ -3521,12 +3525,19 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason)
 
 	cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE;
 	cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
-	cmd->scsi_sense_length  = TRANSPORT_SENSE_BUFFER;
+
 	scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq);
 	if (sd->add_sense_info)
 		WARN_ON_ONCE(scsi_set_sense_information(buffer,
-							cmd->scsi_sense_length,
+							TRANSPORT_SENSE_BUFFER,
 							cmd->sense_info) < 0);
+	/*
+	 * CHECK CONDITION returns sense data, and sense data is minimum 8
+	 * bytes long plus additional Sense Data Length.
+	 * See SPC4-r37 section 4.5.2.1.
+	 */
+	cmd->scsi_sense_length = min_t(u16, buffer[7] + 8,
+				       TRANSPORT_SENSE_BUFFER);
 }
 
 int
Signature · 47 lines
Changes in v3:
 - v2: https://lore.kernel.org/linux-usb/[email protected]/
 - Moved patches around so fixes patches go first
 - Use hashtable to map tag to uas stream
 - Move target_execute_cmd() out of interrupt context
 - Various cleanup
 - Additional fixes over the 2 years


Thinh Nguyen (28):
  usb: gadget: f_tcm: Don't free command immediately
  usb: gadget: f_tcm: Translate error to sense
  usb: gadget: f_tcm: Decrement command ref count on cleanup
  usb: gadget: f_tcm: Fix Get/SetInterface return value
  usb: gadget: f_tcm: ep_autoconfig with fullspeed endpoint
  usb: gadget: f_tcm: Don't prepare BOT write request twice
  usb: gadget: f_tcm: Increase stream count
  usb: gadget: f_tcm: Increase bMaxBurst
  usb: gadget: f_tcm: Limit number of sessions
  usb: gadget: f_tcm: Get stream by sbitmap number
  usb: gadget: f_tcm: Don't set static stream_id
  usb: gadget: f_tcm: Allocate matching number of commands to streams
  usb: gadget: f_tcm: Handle multiple commands in parallel
  usb: gadget: f_tcm: Use extra number of commands
  usb: gadget: f_tcm: Return ATA cmd direction
  usb: gadget: f_tcm: Execute command on write completion
  usb: gadget: f_tcm: Minor cleanup redundant code
  usb: gadget: f_tcm: Handle abort command
  usb: gadget: f_tcm: Cleanup requests on ep disable
  usb: gadget: f_tcm: Stop proceeding further on -ESHUTDOWN
  usb: gadget: f_tcm: Save CPU ID per command
  usb: gadget: f_tcm: Send sense on cancelled transfer
  usb: gadget: f_tcm: Handle TASK_MANAGEMENT commands
  usb: gadget: f_tcm: Check overlapped command
  usb: gadget: f_tcm: Stall on invalid CBW
  usb: gadget: f_tcm: Requeue command request on error
  usb: gadget: f_tcm: Track BOT command kref
  usb: gadget: f_tcm: Refactor goto check_condition

 drivers/usb/gadget/function/f_tcm.c | 711 ++++++++++++++++++++--------
 drivers/usb/gadget/function/tcm.h   |  28 +-
 2 files changed, 547 insertions(+), 192 deletions(-)


base-commit: d8d936c51388442f769a81e512b505dcf87c6a51
-- 
2.28.0