diff mbox series

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

Message ID d9863f8d7065cd9d5f6923ce002a86f6ee6509a9.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive)
State Superseded
Headers show
Series usb: gadget: f_tcm: Enhance UASP driver | expand

Commit Message

Thinh Nguyen July 6, 2022, 11:36 p.m. UTC
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(-)

Comments

Dmitry Bogdanov July 8, 2022, 7 a.m. UTC | #1
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)
Thinh Nguyen July 9, 2022, 12:12 a.m. UTC | #2
On 7/8/2022, Dmitry Bogdanov wrote:
> 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.

Sure, I'll do that.

Thanks,
Thinh

>>   
>>   	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)
diff mbox series

Patch

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];
 
 	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)