On Wed, Jul 06, 2022 at 04:38:01PM -0700, Thinh Nguyen wrote:
> Handle target_core_fabric_ops TASK MANAGEMENT functions and their
> response. If a TASK MANAGEMENT command is received, the driver will
> interpret the function TMF_*, translate to TMR_*, and fire off a command
> work executing target_submit_tmr(). On completion, it will handle the
> TASK MANAGEMENT response through uasp_send_tm_response().
>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
> ---
> NOTE: I appologize for this big patch. I feel that this feature needs to be
> viewed in its entirety to see the whole picture and easier review.
>
>
> drivers/usb/gadget/function/f_tcm.c | 260 +++++++++++++++++++++++++---
> drivers/usb/gadget/function/tcm.h | 7 +-
> 2 files changed, 241 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index fa09999adda7..a68436f97f91 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -12,6 +12,7 @@
> #include <linux/string.h>
> #include <linux/configfs.h>
> #include <linux/ctype.h>
> +#include <linux/delay.h>
> #include <linux/usb/ch9.h>
> #include <linux/usb/composite.h>
> #include <linux/usb/gadget.h>
> @@ -462,6 +463,53 @@ static int usbg_bot_setup(struct usb_function *f,
>
> /* Start uas.c code */
>
> +static int tcm_to_uasp_response(enum tcm_tmrsp_table code)
> +{
> + switch (code) {
> + case TMR_FUNCTION_FAILED:
> + return RC_TMF_FAILED;
> + case TMR_FUNCTION_COMPLETE:
> + return RC_TMF_COMPLETE;
> + case TMR_FUNCTION_REJECTED:
> + return RC_TMF_NOT_SUPPORTED;
> + case TMR_LUN_DOES_NOT_EXIST:
> + return RC_INCORRECT_LUN;
> + case TMR_OVERLAPPED_TAG_ATTEMPTED:
> + return RC_OVERLAPPED_TAG;
> + case TMR_TASK_DOES_NOT_EXIST:
> + return RC_INVALID_INFO_UNIT;
> + case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED:
> + default:
> + return RC_TMF_NOT_SUPPORTED;
> + }
> +}
> +
> +static unsigned char uasp_to_tcm_func(int code)
> +{
> + switch (code) {
> + case TMF_ABORT_TASK:
> + return TMR_ABORT_TASK;
> + case TMF_ABORT_TASK_SET:
> + return TMR_ABORT_TASK_SET;
> + case TMF_CLEAR_TASK_SET:
> + return TMR_CLEAR_TASK_SET;
> + case TMF_LOGICAL_UNIT_RESET:
> + return TMR_LUN_RESET;
> + case TMF_I_T_NEXUS_RESET:
> + return TMR_I_T_NEXUS_RESET;
> + case TMF_CLEAR_ACA:
> + return TMR_CLEAR_ACA;
> + case TMF_QUERY_TASK:
> + return TMR_QUERY_TASK;
> + case TMF_QUERY_TASK_SET:
> + return TMR_QUERY_TASK_SET;
> + case TMF_QUERY_ASYNC_EVENT:
> + return TMR_QUERY_ASYNC_EVENT;
> + default:
> + return TMR_UNKNOWN;
> + }
> +}
> +
> static void uasp_cleanup_one_stream(struct f_uas *fu, struct uas_stream *stream)
> {
> /* We have either all three allocated or none */
> @@ -506,6 +554,11 @@ static void uasp_cleanup_old_alt(struct f_uas *fu)
> uasp_free_cmdreq(fu);
> }
>
> +static struct uas_stream *uasp_get_stream_by_tag(struct f_uas *fu, u16 tag)
> +{
> + return &fu->stream[tag % USBG_NUM_CMDS];
> +}
> +
> static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req);
>
> static int uasp_prepare_r_request(struct usbg_cmd *cmd)
> @@ -513,7 +566,7 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd)
> struct se_cmd *se_cmd = &cmd->se_cmd;
> struct f_uas *fu = cmd->fu;
> struct usb_gadget *gadget = fuas_to_gadget(fu);
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
>
> if (!gadget->sg_supported) {
> cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC);
> @@ -546,7 +599,7 @@ static void uasp_prepare_status(struct usbg_cmd *cmd)
> {
> struct se_cmd *se_cmd = &cmd->se_cmd;
> struct sense_iu *iu = &cmd->sense_iu;
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(cmd->fu, cmd->tag);
>
> cmd->state = UASP_QUEUE_COMMAND;
> iu->iu_id = IU_ID_STATUS;
> @@ -565,11 +618,36 @@ static void uasp_prepare_status(struct usbg_cmd *cmd)
> stream->req_status->complete = uasp_status_data_cmpl;
> }
>
> +static void uasp_prepare_response(struct usbg_cmd *cmd)
> +{
> + struct se_cmd *se_cmd = &cmd->se_cmd;
> + struct response_iu *rsp_iu = &cmd->response_iu;
> + struct uas_stream *stream = uasp_get_stream_by_tag(cmd->fu, cmd->tag);
> +
> + cmd->state = UASP_QUEUE_COMMAND;
> + rsp_iu->iu_id = IU_ID_RESPONSE;
> + rsp_iu->tag = cpu_to_be16(cmd->tag);
> +
> + if (cmd->tmr_rsp != TMR_RESPONSE_UNKNOWN)
> + rsp_iu->response_code =
> + tcm_to_uasp_response(cmd->tmr_rsp);
> + else
> + rsp_iu->response_code =
> + tcm_to_uasp_response(se_cmd->se_tmr_req->response);
> +
> + stream->req_status->is_last = 1;
> + stream->req_status->stream_id = cmd->tag;
> + stream->req_status->context = cmd;
> + stream->req_status->length = sizeof(struct response_iu);
> + stream->req_status->buf = rsp_iu;
> + stream->req_status->complete = uasp_status_data_cmpl;
> +}
> +
> static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> {
> struct usbg_cmd *cmd = req->context;
> - struct uas_stream *stream = cmd->stream;
> struct f_uas *fu = cmd->fu;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> struct se_session *se_sess = cmd->se_cmd.se_sess;
> int ret;
>
> @@ -604,6 +682,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> break;
>
> case UASP_QUEUE_COMMAND:
> + stream->cmd = NULL;
>
> target_free_tag(se_sess, &cmd->se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> @@ -617,6 +696,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> return;
>
> cleanup:
> + stream->cmd = NULL;
> target_free_tag(se_sess, &cmd->se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> }
> @@ -624,7 +704,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> static int uasp_send_status_response(struct usbg_cmd *cmd)
> {
> struct f_uas *fu = cmd->fu;
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> struct sense_iu *iu = &cmd->sense_iu;
>
> iu->tag = cpu_to_be16(cmd->tag);
> @@ -633,10 +713,22 @@ static int uasp_send_status_response(struct usbg_cmd *cmd)
> return usb_ep_queue(fu->ep_status, stream->req_status, GFP_ATOMIC);
> }
>
> +static int uasp_send_tm_response(struct usbg_cmd *cmd)
> +{
> + struct f_uas *fu = cmd->fu;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> + struct response_iu *iu = &cmd->response_iu;
> +
> + iu->tag = cpu_to_be16(cmd->tag);
> + cmd->fu = fu;
> + uasp_prepare_response(cmd);
> + return usb_ep_queue(fu->ep_status, stream->req_status, GFP_ATOMIC);
> +}
> +
> static int uasp_send_read_response(struct usbg_cmd *cmd)
> {
> struct f_uas *fu = cmd->fu;
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> struct sense_iu *iu = &cmd->sense_iu;
> int ret;
>
> @@ -682,7 +774,7 @@ static int uasp_send_read_response(struct usbg_cmd *cmd)
> static int uasp_send_write_request(struct usbg_cmd *cmd)
> {
> struct f_uas *fu = cmd->fu;
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> struct sense_iu *iu = &cmd->sense_iu;
> int ret;
>
> @@ -943,8 +1035,10 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)
> {
> struct usbg_cmd *cmd = req->context;
> struct se_cmd *se_cmd = &cmd->se_cmd;
> + struct uas_stream *stream = uasp_get_stream_by_tag(cmd->fu, cmd->tag);
>
> if (req->status == -ESHUTDOWN) {
> + stream->cmd = NULL;
> target_free_tag(se_cmd->se_sess, se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> return;
> @@ -962,6 +1056,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)
> se_cmd->data_length);
> }
>
> + cmd->state = UASP_QUEUE_COMMAND;
> target_execute_cmd(se_cmd);
> return;
>
> @@ -1042,9 +1137,66 @@ static int usbg_send_read_response(struct se_cmd *se_cmd)
> return uasp_send_read_response(cmd);
> }
>
> -static void usbg_cmd_work(struct work_struct *work)
> +static void usbg_submit_tmr(struct usbg_cmd *cmd)
> +{
> + struct se_cmd *se_cmd;
> + struct tcm_usbg_nexus *tv_nexus;
> + struct uas_stream *stream;
> + int flags = TARGET_SCF_ACK_KREF;
> +
> + se_cmd = &cmd->se_cmd;
> + tv_nexus = cmd->fu->tpg->tpg_nexus;
> + stream = uasp_get_stream_by_tag(cmd->fu, cmd->tag);
> +
> + /* Failure detected by f_tcm */
> + if (cmd->tmr_rsp != TMR_RESPONSE_UNKNOWN) {
> + if (cmd->tmr_rsp == TMR_OVERLAPPED_TAG_ATTEMPTED) {
> + /*
> + * There's no guarantee of a matching completion order
> + * between different endpoints. i.e. The device may
> + * receive a new (CDB) command request completion of the
> + * command endpoint before it gets notified of the
> + * previous command status completion from a status
> + * endpoint. The driver still needs to detect
> + * misbehaving host and respond with an overlap command
> + * tag. To prevent false overlapped tag failure, give
> + * the active and matching stream id a short time (1ms)
> + * to complete before respond with overlapped command
> + * failure.
> + */
> + msleep(1);
> +
> + /* If the stream is completed, retry the command */
> + if (!stream->cmd) {
> + usbg_submit_command(cmd->fu, cmd->req);
> + return;
> + }
> +
> + /* Overlap command tag detected. Abort command. */
> + cmd->state = UASP_QUEUE_COMMAND;
> + stream->cmd->se_cmd.transport_state |= CMD_T_ABORTED;
> + target_get_sess_cmd(&stream->cmd->se_cmd, true);
> +
> + /* This will trigger command abort handler */
> + target_execute_cmd(&stream->cmd->se_cmd);
> + transport_generic_free_cmd(&stream->cmd->se_cmd, 1);
> + }
> +
> +
> + target_submit_tmr_fail_response(se_cmd, cmd->tmr_rsp,
> + tv_nexus->tvn_se_sess, cmd->unpacked_lun,
> + GFP_ATOMIC, cmd->tag, flags);
I think there is no reason to reject TMR via Core, you may use
your uasp_send_tm_response(cmd) directly like other fabric drivers does.
That will need some coding to distinguish a completion of the response
initiated from Core and from fabric driver.
> + return;
> + }
> +
> + target_submit_tmr(se_cmd, tv_nexus->tvn_se_sess,
> + cmd->response_iu.add_response_info,
> + cmd->unpacked_lun, NULL, cmd->tmr_func,
> + GFP_ATOMIC, cmd->tag, flags);
> +}
> +
> +static void usbg_submit_cmd(struct usbg_cmd *cmd)
> {
> - struct usbg_cmd *cmd = container_of(work, struct usbg_cmd, work);
> struct se_cmd *se_cmd;
> struct tcm_usbg_nexus *tv_nexus;
> struct usbg_tpg *tpg;
> @@ -1073,6 +1225,16 @@ static void usbg_cmd_work(struct work_struct *work)
> TCM_UNSUPPORTED_SCSI_OPCODE, 0);
> }
>
> +static void usbg_cmd_work(struct work_struct *work)
> +{
> + struct usbg_cmd *cmd = container_of(work, struct usbg_cmd, work);
> +
> + if (cmd->tmr_func || cmd->tmr_rsp != TMR_RESPONSE_UNKNOWN)
> + usbg_submit_tmr(cmd);
That looks very strange - in response of received SCSI command you will
send a TMR response???
I am about cmd->tmr_rsp != TMR_RESPONSE_UNKNOWN case.
> + else
> + usbg_submit_cmd(cmd);
> +}
> +
> static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
> struct tcm_usbg_nexus *tv_nexus, u32 scsi_tag)
> {
> @@ -1099,37 +1261,84 @@ static void usbg_release_cmd(struct se_cmd *);
>
> static int usbg_submit_command(struct f_uas *fu, struct usb_request *req)
> {
> - struct command_iu *cmd_iu = req->buf;
> + struct iu *iu = req->buf;
> struct usbg_cmd *cmd;
> struct usbg_tpg *tpg = fu->tpg;
> struct tcm_usbg_nexus *tv_nexus;
> + struct uas_stream *stream;
> + struct command_iu *cmd_iu;
> u32 cmd_len;
> u16 scsi_tag;
>
> - if (cmd_iu->iu_id != IU_ID_COMMAND) {
> - pr_err("Unsupported type %d\n", cmd_iu->iu_id);
> - return -EINVAL;
> - }
> -
> tv_nexus = tpg->tpg_nexus;
> if (!tv_nexus) {
> pr_err("Missing nexus, ignoring command\n");
> return -EINVAL;
> }
>
> - cmd_len = (cmd_iu->len & ~0x3) + 16;
> - if (cmd_len > USBG_MAX_CMD)
> - return -EINVAL;
> -
> - scsi_tag = be16_to_cpup(&cmd_iu->tag);
> + scsi_tag = be16_to_cpup(&iu->tag);
> cmd = usbg_get_cmd(fu, tv_nexus, scsi_tag);
> if (IS_ERR(cmd)) {
> pr_err("usbg_get_cmd failed\n");
> return -ENOMEM;
> }
> - memcpy(cmd->cmd_buf, cmd_iu->cdb, cmd_len);
>
> - cmd->stream = &fu->stream[cmd->tag % USBG_NUM_CMDS];
> + cmd->req = req;
> + cmd->fu = fu;
> + cmd->tag = scsi_tag;
> + cmd->se_cmd.tag = scsi_tag;
> + cmd->tmr_func = 0;
> + cmd->tmr_rsp = TMR_RESPONSE_UNKNOWN;
TMR_* constant are fabric agnostic enum. Better use RC_TMF_* values in
variables of this driver.
> +
> + cmd_iu = (struct command_iu *)iu;
> +
> + /* Command and Task Management IUs share the same LUN offset */
> + cmd->unpacked_lun = scsilun_to_int(&cmd_iu->lun);
> +
> + if (iu->iu_id != IU_ID_COMMAND && iu->iu_id != IU_ID_TASK_MGMT) {
> + cmd->tmr_rsp = TMR_TASK_DOES_NOT_EXIST;
> + goto skip;
> + }
> +
> + /*
> + * For simplicity, we use mod operation to quickly find an in-progress
> + * matching command tag and respond with overlapped command. The
> + * assumption is that the UASP class driver will limit to using tag id
> + * from 1 to USBG_NUM_CMDS. This is based on observation from the
> + * Windows and Linux UASP storage class driver behavior. If an unusual
> + * UASP class driver uses a tag greater than USBG_NUM_CMDS, then this
> + * method may no longer work due to possible stream id collision. In
> + * that case, we need to use a proper algorithm to fetch the stream (or
> + * simply walk through all active streams to check for overlap).
> + */
> + stream = uasp_get_stream_by_tag(fu, scsi_tag);
> + if (stream->cmd) {
> + WARN_ONCE(stream->cmd->tag != scsi_tag,
WARN is used to indicate a non fatal bug in the code. May be you want to
use pr_warn/pr_err here?
> + "Command tag %d collided with Stream id %d\n",
> + scsi_tag, stream->cmd->tag);
> +
> + cmd->tmr_rsp = TMR_OVERLAPPED_TAG_ATTEMPTED;
> + goto skip;
> + }
> +
> + stream->cmd = cmd;
> +
> + if (iu->iu_id == IU_ID_TASK_MGMT) {
> + struct task_mgmt_iu *tm_iu;
> +
> + tm_iu = (struct task_mgmt_iu *)iu;
> + cmd->tmr_func = uasp_to_tcm_func(tm_iu->function);
> + goto skip;
> + }
> +
> + cmd_len = (cmd_iu->len & ~0x3) + 16;
> + if (cmd_len > USBG_MAX_CMD) {
> + pr_err("invalid len %d\n", cmd_len);
> + target_free_tag(tv_nexus->tvn_se_sess, &cmd->se_cmd);
> + stream->cmd = NULL;
> + return -EINVAL;
> + }
> + memcpy(cmd->cmd_buf, cmd_iu->cdb, cmd_len);
>
> switch (cmd_iu->prio_attr & 0x7) {
> case UAS_HEAD_TAG:
> @@ -1150,9 +1359,7 @@ static int usbg_submit_command(struct f_uas *fu, struct usb_request *req)
> break;
> }
>
> - cmd->unpacked_lun = scsilun_to_int(&cmd_iu->lun);
> - cmd->req = req;
> -
> +skip:
> INIT_WORK(&cmd->work, usbg_cmd_work);
> queue_work(tpg->workqueue, &cmd->work);
>
> @@ -1298,13 +1505,16 @@ static int usbg_get_cmd_state(struct se_cmd *se_cmd)
>
> static void usbg_queue_tm_rsp(struct se_cmd *se_cmd)
> {
> + struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd);
> +
> + uasp_send_tm_response(cmd);
> }
>
> static void usbg_aborted_task(struct se_cmd *se_cmd)
> {
> struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd);
> struct f_uas *fu = cmd->fu;
> - struct uas_stream *stream = cmd->stream;
> + struct uas_stream *stream = uasp_get_stream_by_tag(fu, cmd->tag);
> int ret = 0;
>
> if (stream->req_out->status == -EINPROGRESS)
> diff --git a/drivers/usb/gadget/function/tcm.h b/drivers/usb/gadget/function/tcm.h
> index 5157af1b166b..f1cd2399fd69 100644
> --- a/drivers/usb/gadget/function/tcm.h
> +++ b/drivers/usb/gadget/function/tcm.h
> @@ -82,8 +82,11 @@ struct usbg_cmd {
> u16 tag;
> u16 prio_attr;
> struct sense_iu sense_iu;
> + struct response_iu response_iu;
> enum uas_state state;
> - struct uas_stream *stream;
> + int tmr_func;
> + int tmr_rsp;
> +#define TMR_RESPONSE_UNKNOWN 0xff
>
> /* BOT only */
> __le32 bot_tag;
> @@ -96,6 +99,8 @@ struct uas_stream {
> struct usb_request *req_in;
> struct usb_request *req_out;
> struct usb_request *req_status;
> +
> + struct usbg_cmd *cmd;
> };
>
> struct usbg_cdb {
