usb: dwc3: gadget: Check for prepared TRBs

Commit Message

Thinh Nguyen May 15, 2020, 11:40 p.m. UTC

There are cases where the endpoint needs to be restarted. For example,
it may need to restart for NoStream rejection to reinitiate stream. If
so, check and make sure we don't prepare beyond the current transfer
when we restart the endpoint.

DWC_usb32 internal burst transfer feature will look into TRBs beyond a
transfer. Other controllers will stop on the last TRB, but not
DWC_usb32. This may cause the controller to incorrectly process TRBs of
a different transfer. Make sure to explicitly prevent preparing TRBs of
a different transfer.

This should only affect DWC_usb32 releases prior to v1.00a since it
doesn't use SET_ENDPOINT_PRIME to reinitiate stream. However, it's
better to be cautious in case users don't want to use SET_ENDPOINT_PRIME
command. Also, it's possible other controller IPs may share the same
features as DWC_usb32 in new releases.

Signed-off-by: Thinh Nguyen <[e-mail]>
---
 drivers/usb/dwc3/gadget.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

kernel test robot May 16, 2020, 1:50 a.m. UTC | #1

Hi Thinh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on usb/usb-testing peter.chen-usb/ci-for-usb-next v5.7-rc5 next-20200515]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-dwc3-gadget-Check-for-prepared-TRBs/20200516-074413
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[e-mail]>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_trbs':
>> drivers/usb/dwc3/gadget.c:1203:42: error: 'struct usb_request' has no member named 'is_last'
if (dep->stream_capable && req->request.is_last)
^

vim +1203 drivers/usb/dwc3/gadget.c

  1166	
  1167	/*
  1168	 * dwc3_prepare_trbs - setup TRBs from requests
  1169	 * @dep: endpoint for which requests are being prepared
  1170	 *
  1171	 * The function goes through the requests list and sets up TRBs for the
  1172	 * transfers. The function returns once there are no more TRBs available or
  1173	 * it runs out of requests.
  1174	 */
  1175	static void dwc3_prepare_trbs(struct dwc3_ep *dep)
  1176	{
  1177		struct dwc3_request	*req, *n;
  1178	
  1179		BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM);
  1180	
  1181		/*
  1182		 * We can get in a situation where there's a request in the started list
  1183		 * but there weren't enough TRBs to fully kick it in the first time
  1184		 * around, so it has been waiting for more TRBs to be freed up.
  1185		 *
  1186		 * In that case, we should check if we have a request with pending_sgs
  1187		 * in the started list and prepare TRBs for that request first,
  1188		 * otherwise we will prepare TRBs completely out of order and that will
  1189		 * break things.
  1190		 */
  1191		list_for_each_entry(req, &dep->started_list, list) {
  1192			if (req->num_pending_sgs > 0)
  1193				dwc3_prepare_one_trb_sg(dep, req);
  1194	
  1195			if (!dwc3_calc_trbs_left(dep))
  1196				return;
  1197	
  1198			/*
  1199			 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
  1200			 * burst capability may try to read and use TRBs beyond the
  1201			 * active transfer instead of stopping.
  1202			 */
> 1203			if (dep->stream_capable && req->request.is_last)
  1204				return;
  1205		}
  1206	
  1207		list_for_each_entry_safe(req, n, &dep->pending_list, list) {
  1208			struct dwc3	*dwc = dep->dwc;
  1209			int		ret;
  1210	
  1211			ret = usb_gadget_map_request_by_dev(dwc->sysdev, &req->request,
  1212							    dep->direction);
  1213			if (ret)
  1214				return;
  1215	
  1216			req->sg			= req->request.sg;
  1217			req->start_sg		= req->sg;
  1218			req->num_queued_sgs	= 0;
  1219			req->num_pending_sgs	= req->request.num_mapped_sgs;
  1220	
  1221			if (req->num_pending_sgs > 0)
  1222				dwc3_prepare_one_trb_sg(dep, req);
  1223			else
  1224				dwc3_prepare_one_trb_linear(dep, req);
  1225	
  1226			if (!dwc3_calc_trbs_left(dep))
  1227				return;
  1228		}
  1229	}
  1230	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Thinh Nguyen May 16, 2020, 2:04 a.m. UTC | #2

kbuild test robot wrote:
> Hi Thinh,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on balbi-usb/next]
> [also build test ERROR on usb/usb-testing peter.chen-usb/ci-for-usb-next v5.7-rc5 next-20200515]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system.

Just dropping a note:
This patch is base on Felipe's testing/next branch.

BR,
Thinh

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index fea4fde1b5e3..45b7e6dca781 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1232,6 +1232,14 @@  static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 
 		if (!dwc3_calc_trbs_left(dep))
 			return;
+
+		/*
+		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
+		 * burst capability may try to read and use TRBs beyond the
+		 * active transfer instead of stopping.
+		 */
+		if (dep->stream_capable && req->request.is_last)
+			return;
 	}
 
 	list_for_each_entry_safe(req, n, &dep->pending_list, list) {