Re: [PATCH 18/36] usb: gadget: f_tcm: Don't set static stream_id

[Dmitry Bogdanov <d.bogdanov@yadro.com>]

On Wed, Jul 06, 2022 at 04:36:15PM -0700, Thinh Nguyen wrote:
> Host can assign stream ID value greater than number of streams
> allocated. The tcm function needs to keep track of which stream is
> available to assign the stream ID. This patch doesn't track that, but at
> least it makes sure that there's no Oops if the host send tag with a
> value greater than the number of supported streams.
> 
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  drivers/usb/gadget/function/f_tcm.c | 32 +++++------------------------
>  1 file changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 270ec631481d..7721216dc9bc 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -532,6 +532,7 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd)
>  	}
>  
>  	stream->req_in->is_last = 1;
> +	stream->req_in->stream_id = cmd->tag;
>  	stream->req_in->complete = uasp_status_data_cmpl;
>  	stream->req_in->length = se_cmd->data_length;
>  	stream->req_in->context = cmd;
> @@ -556,6 +557,7 @@ static void uasp_prepare_status(struct usbg_cmd *cmd)
>  	iu->len = cpu_to_be16(se_cmd->scsi_sense_length);
>  	iu->status = se_cmd->scsi_status;
>  	stream->req_status->is_last = 1;
> +	stream->req_status->stream_id = cmd->tag;
>  	stream->req_status->context = cmd;
>  	stream->req_status->length = se_cmd->scsi_sense_length + 16;
>  	stream->req_status->buf = iu;
> @@ -786,19 +788,6 @@ static int uasp_alloc_cmd(struct f_uas *fu)
>  	return -ENOMEM;
>  }
>  
> -static void uasp_setup_stream_res(struct f_uas *fu, int max_streams)
> -{
> -	int i;
> -
> -	for (i = 0; i < max_streams; i++) {
> -		struct uas_stream *s = &fu->stream[i];
> -
> -		s->req_in->stream_id = i + 1;
> -		s->req_out->stream_id = i + 1;
> -		s->req_status->stream_id = i + 1;
> -	}
> -}
> -
>  static int uasp_prepare_reqs(struct f_uas *fu)
>  {
>  	int ret;
> @@ -819,7 +808,6 @@ static int uasp_prepare_reqs(struct f_uas *fu)
>  	ret = uasp_alloc_cmd(fu);
>  	if (ret)
>  		goto err_free_stream;
> -	uasp_setup_stream_res(fu, max_streams);
>  
>  	ret = usb_ep_queue(fu->ep_cmd, fu->cmd.req, GFP_ATOMIC);
>  	if (ret)
> @@ -995,6 +983,7 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req)
>  	}
>  
>  	req->is_last = 1;
> +	req->stream_id = cmd->tag;
>  	req->complete = usbg_data_write_cmpl;
>  	req->length = se_cmd->data_length;
>  	req->context = cmd;
> @@ -1125,16 +1114,8 @@ static int usbg_submit_command(struct f_uas *fu,
>  	}
>  	memcpy(cmd->cmd_buf, cmd_iu->cdb, cmd_len);
>  
> -	if (fu->flags & USBG_USE_STREAMS) {
> -		if (cmd->tag > UASP_SS_EP_COMP_NUM_STREAMS)
> -			goto err;
> -		if (!cmd->tag)
> -			cmd->stream = &fu->stream[0];
> -		else
> -			cmd->stream = &fu->stream[cmd->tag - 1];
> -	} else {
> -		cmd->stream = &fu->stream[0];
> -	}
> +	cmd->stream = &fu->stream[cmd->tag %
> +		UASP_SS_EP_COMP_NUM_STREAMS];
Use USBG_NUM_CMDS instead of UASP_SS_EP_COMP_NUM_STREAMS like in other
places.
>  
>  	switch (cmd_iu->prio_attr & 0x7) {
>  	case UAS_HEAD_TAG:
> @@ -1161,9 +1142,6 @@ static int usbg_submit_command(struct f_uas *fu,
>  	queue_work(tpg->workqueue, &cmd->work);
>  
>  	return 0;
> -err:
> -	usbg_release_cmd(&cmd->se_cmd);
> -	return -EINVAL;
>  }
>  
>  static void bot_cmd_work(struct work_struct *work)