USB: EHCI: resolve some unlikely races

This patch (as1589) resolves some unlikely races involving system shutdown or controller death in ehci-hcd: Shutdown races with both root-hub resume and controller resume. Controller death races with root-hub suspend. A new bitflag is added to indicate that the controller has been shut down (whether for system shutdown or because it died). Tests are added in the suspend and resume pathways to avoid reactivating the controller after any sort of shutdown. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

USB: EHCI: resolve some unlikely races
This patch (as1589) resolves some unlikely races involving system shutdown or controller death in ehci-hcd: Shutdown races with both root-hub resume and controller resume. Controller death races with root-hub suspend. A new bitflag is added to indicate that the controller has been shut down (whether for system shutdown or because it died). Tests are added in the suspend and resume pathways to avoid reactivating the controller after any sort of shutdown. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Alan Stern · Greg Kroah-Hartman
1 parent c4f3476436
Showing 3 changed files with 43 additions and 5 deletions Side-by-side Diff
drivers/usb/host/ehci-hcd.c
drivers/usb/host/ehci-hub.c
drivers/usb/host/ehci.h
@@ -343,6 +343,7 @@
 	struct ehci_hcd	*ehci = hcd_to_ehci(hcd);
  
 	spin_lock_irq(&ehci->lock);
+	ehci->shutdown = true;
 	ehci->rh_state = EHCI_RH_STOPPING;
 	ehci->enabled_hrtimer_events = 0;
 	spin_unlock_irq(&ehci->lock);
@@ -823,6 +824,7 @@
 		usb_hc_died(hcd);
  
 		/* Don't let the controller do anything more */
+		ehci->shutdown = true;
 		ehci->rh_state = EHCI_RH_STOPPING;
 		ehci->command &= ~(CMD_RUN | CMD_ASE | CMD_PSE);
 		ehci_writel(ehci, ehci->command, &ehci->regs->command);
@@ -1129,6 +1131,9 @@
 	/* Mark hardware accessible again as we are back to full power by now */
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
  
+	if (ehci->shutdown)
+		return 0;		/* Controller is dead */
+
 	/*
 	 * If CF is still set and we aren't resuming from hibernation
 	 * then we maintained suspend power.
  
@@ -1139,10 +1144,17 @@
 		int	mask = INTR_MASK;
  
 		ehci_prepare_ports_for_controller_resume(ehci);
+
+		spin_lock_irq(&ehci->lock);
+		if (ehci->shutdown)
+			goto skip;
+
 		if (!hcd->self.root_hub->do_remote_wakeup)
 			mask &= ~STS_PCD;
 		ehci_writel(ehci, mask, &ehci->regs->intr_enable);
 		ehci_readl(ehci, &ehci->regs->intr_enable);
+ skip:
+		spin_unlock_irq(&ehci->lock);
 		return 0;
 	}
  
  
  
@@ -1154,14 +1166,20 @@
 	(void) ehci_halt(ehci);
 	(void) ehci_reset(ehci);
  
+	spin_lock_irq(&ehci->lock);
+	if (ehci->shutdown)
+		goto skip;
+
 	ehci_writel(ehci, ehci->command, &ehci->regs->command);
 	ehci_writel(ehci, FLAG_CF, &ehci->regs->configured_flag);
 	ehci_readl(ehci, &ehci->regs->command);	/* unblock posted writes */
  
+	ehci->rh_state = EHCI_RH_SUSPENDED;
+	spin_unlock_irq(&ehci->lock);
+
 	/* here we "know" root ports should always stay powered */
 	ehci_port_power(ehci, 1);
  
-	ehci->rh_state = EHCI_RH_SUSPENDED;
 	return 1;
 }
  
@@ -221,6 +221,8 @@
 	ehci_quiesce(ehci);
  
 	spin_lock_irq (&ehci->lock);
+	if (ehci->rh_state < EHCI_RH_RUNNING)
+		goto done;
  
 	/* Once the controller is stopped, port resumes that are already
 	 * in progress won't complete.  Hence if remote wakeup is enabled
@@ -306,6 +308,10 @@
 	ehci_halt (ehci);
  
 	spin_lock_irq(&ehci->lock);
+	if (ehci->enabled_hrtimer_events & BIT(EHCI_HRTIMER_POLL_DEAD))
+		ehci_handle_controller_death(ehci);
+	if (ehci->rh_state != EHCI_RH_RUNNING)
+		goto done;
 	ehci->rh_state = EHCI_RH_SUSPENDED;
  
 	end_unlink_async(ehci);
@@ -320,6 +326,7 @@
 	ehci_writel(ehci, mask, &ehci->regs->intr_enable);
 	ehci_readl(ehci, &ehci->regs->intr_enable);
  
+ done:
 	ehci->next_statechange = jiffies + msecs_to_jiffies(10);
 	ehci->enabled_hrtimer_events = 0;
 	ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
@@ -342,10 +349,8 @@
 	if (time_before (jiffies, ehci->next_statechange))
 		msleep(5);
 	spin_lock_irq (&ehci->lock);
-	if (!HCD_HW_ACCESSIBLE(hcd)) {
-		spin_unlock_irq(&ehci->lock);
-		return -ESHUTDOWN;
-	}
+	if (!HCD_HW_ACCESSIBLE(hcd) || ehci->shutdown)
+		goto shutdown;
  
 	if (unlikely(ehci->debug)) {
 		if (!dbgp_reset_prep())
@@ -384,6 +389,8 @@
 	spin_unlock_irq(&ehci->lock);
 	msleep(8);
 	spin_lock_irq(&ehci->lock);
+	if (ehci->shutdown)
+		goto shutdown;
  
 	/* clear phy low-power mode before resume */
 	if (ehci->bus_suspended && ehci->has_hostpc) {
@@ -401,6 +408,8 @@
 		spin_unlock_irq(&ehci->lock);
 		msleep(5);
 		spin_lock_irq(&ehci->lock);
+		if (ehci->shutdown)
+			goto shutdown;
 	}
  
 	/* manually resume the ports we suspended during bus_suspend() */
@@ -421,6 +430,8 @@
 		spin_unlock_irq(&ehci->lock);
 		msleep(20);
 		spin_lock_irq(&ehci->lock);
+		if (ehci->shutdown)
+			goto shutdown;
 	}
  
 	i = HCS_N_PORTS (ehci->hcs_params);
  
  
@@ -439,10 +450,18 @@
 	ehci_handover_companion_ports(ehci);
  
 	/* Now we can safely re-enable irqs */
+	spin_lock_irq(&ehci->lock);
+	if (ehci->shutdown)
+		goto shutdown;
 	ehci_writel(ehci, INTR_MASK, &ehci->regs->intr_enable);
 	(void) ehci_readl(ehci, &ehci->regs->intr_enable);
+	spin_unlock_irq(&ehci->lock);
  
 	return 0;
+
+ shutdown:
+	spin_unlock_irq(&ehci->lock);
+	return -ESHUTDOWN;
 }
  
 #else
@@ -118,6 +118,7 @@
 	bool			need_rescan:1;
 	bool			intr_unlinking:1;
 	bool			async_unlinking:1;
+	bool			shutdown:1;
 	struct ehci_qh		*qh_scan_next;
  
 	/* async schedule support */
...	...	@@ -343,6 +343,7 @@
343	343	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
344	344
345	345	spin_lock_irq(&ehci->lock);
	346	+ ehci->shutdown = true;
346	347	ehci->rh_state = EHCI_RH_STOPPING;
347	348	ehci->enabled_hrtimer_events = 0;
348	349	spin_unlock_irq(&ehci->lock);
...	...	@@ -823,6 +824,7 @@
823	824	usb_hc_died(hcd);
824	825
825	826	/* Don't let the controller do anything more */
	827	+ ehci->shutdown = true;
826	828	ehci->rh_state = EHCI_RH_STOPPING;
827	829	ehci->command &= ~(CMD_RUN \| CMD_ASE \| CMD_PSE);
828	830	ehci_writel(ehci, ehci->command, &ehci->regs->command);
...	...	@@ -1129,6 +1131,9 @@
1129	1131	/* Mark hardware accessible again as we are back to full power by now */
1130	1132	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
1131	1133
	1134	+ if (ehci->shutdown)
	1135	+ return 0; /* Controller is dead */
	1136	+
1132	1137	/*
1133	1138	* If CF is still set and we aren't resuming from hibernation
1134	1139	* then we maintained suspend power.
1135	1140
...	...	@@ -1139,10 +1144,17 @@
1139	1144	int mask = INTR_MASK;
1140	1145
1141	1146	ehci_prepare_ports_for_controller_resume(ehci);
	1147	+
	1148	+ spin_lock_irq(&ehci->lock);
	1149	+ if (ehci->shutdown)
	1150	+ goto skip;
	1151	+
1142	1152	if (!hcd->self.root_hub->do_remote_wakeup)
1143	1153	mask &= ~STS_PCD;
1144	1154	ehci_writel(ehci, mask, &ehci->regs->intr_enable);
1145	1155	ehci_readl(ehci, &ehci->regs->intr_enable);
	1156	+ skip:
	1157	+ spin_unlock_irq(&ehci->lock);
1146	1158	return 0;
1147	1159	}
1148	1160
1149	1161
1150	1162
...	...	@@ -1154,14 +1166,20 @@
1154	1166	(void) ehci_halt(ehci);
1155	1167	(void) ehci_reset(ehci);
1156	1168
	1169	+ spin_lock_irq(&ehci->lock);
	1170	+ if (ehci->shutdown)
	1171	+ goto skip;
	1172	+
1157	1173	ehci_writel(ehci, ehci->command, &ehci->regs->command);
1158	1174	ehci_writel(ehci, FLAG_CF, &ehci->regs->configured_flag);
1159	1175	ehci_readl(ehci, &ehci->regs->command); /* unblock posted writes */
1160	1176
	1177	+ ehci->rh_state = EHCI_RH_SUSPENDED;
	1178	+ spin_unlock_irq(&ehci->lock);
	1179	+
1161	1180	/* here we "know" root ports should always stay powered */
1162	1181	ehci_port_power(ehci, 1);
1163	1182
1164		- ehci->rh_state = EHCI_RH_SUSPENDED;
1165	1183	return 1;
1166	1184	}
1167	1185
...	...	@@ -221,6 +221,8 @@
221	221	ehci_quiesce(ehci);
222	222
223	223	spin_lock_irq (&ehci->lock);
	224	+ if (ehci->rh_state < EHCI_RH_RUNNING)
	225	+ goto done;
224	226
225	227	/* Once the controller is stopped, port resumes that are already
226	228	* in progress won't complete. Hence if remote wakeup is enabled
...	...	@@ -306,6 +308,10 @@
306	308	ehci_halt (ehci);
307	309
308	310	spin_lock_irq(&ehci->lock);
	311	+ if (ehci->enabled_hrtimer_events & BIT(EHCI_HRTIMER_POLL_DEAD))
	312	+ ehci_handle_controller_death(ehci);
	313	+ if (ehci->rh_state != EHCI_RH_RUNNING)
	314	+ goto done;
309	315	ehci->rh_state = EHCI_RH_SUSPENDED;
310	316
311	317	end_unlink_async(ehci);
...	...	@@ -320,6 +326,7 @@
320	326	ehci_writel(ehci, mask, &ehci->regs->intr_enable);
321	327	ehci_readl(ehci, &ehci->regs->intr_enable);
322	328
	329	+ done:
323	330	ehci->next_statechange = jiffies + msecs_to_jiffies(10);
324	331	ehci->enabled_hrtimer_events = 0;
325	332	ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
...	...	@@ -342,10 +349,8 @@
342	349	if (time_before (jiffies, ehci->next_statechange))
343	350	msleep(5);
344	351	spin_lock_irq (&ehci->lock);
345		- if (!HCD_HW_ACCESSIBLE(hcd)) {
346		- spin_unlock_irq(&ehci->lock);
347		- return -ESHUTDOWN;
348		- }
	352	+ if (!HCD_HW_ACCESSIBLE(hcd) \|\| ehci->shutdown)
	353	+ goto shutdown;
349	354
350	355	if (unlikely(ehci->debug)) {
351	356	if (!dbgp_reset_prep())
...	...	@@ -384,6 +389,8 @@
384	389	spin_unlock_irq(&ehci->lock);
385	390	msleep(8);
386	391	spin_lock_irq(&ehci->lock);
	392	+ if (ehci->shutdown)
	393	+ goto shutdown;
387	394
388	395	/* clear phy low-power mode before resume */
389	396	if (ehci->bus_suspended && ehci->has_hostpc) {
...	...	@@ -401,6 +408,8 @@
401	408	spin_unlock_irq(&ehci->lock);
402	409	msleep(5);
403	410	spin_lock_irq(&ehci->lock);
	411	+ if (ehci->shutdown)
	412	+ goto shutdown;
404	413	}
405	414
406	415	/* manually resume the ports we suspended during bus_suspend() */
...	...	@@ -421,6 +430,8 @@
421	430	spin_unlock_irq(&ehci->lock);
422	431	msleep(20);
423	432	spin_lock_irq(&ehci->lock);
	433	+ if (ehci->shutdown)
	434	+ goto shutdown;
424	435	}
425	436
426	437	i = HCS_N_PORTS (ehci->hcs_params);
427	438
428	439
...	...	@@ -439,10 +450,18 @@
439	450	ehci_handover_companion_ports(ehci);
440	451
441	452	/* Now we can safely re-enable irqs */
	453	+ spin_lock_irq(&ehci->lock);
	454	+ if (ehci->shutdown)
	455	+ goto shutdown;
442	456	ehci_writel(ehci, INTR_MASK, &ehci->regs->intr_enable);
443	457	(void) ehci_readl(ehci, &ehci->regs->intr_enable);
	458	+ spin_unlock_irq(&ehci->lock);
444	459
445	460	return 0;
	461	+
	462	+ shutdown:
	463	+ spin_unlock_irq(&ehci->lock);
	464	+ return -ESHUTDOWN;
446	465	}
447	466
448	467	#else
...	...	@@ -118,6 +118,7 @@
118	118	bool need_rescan:1;
119	119	bool intr_unlinking:1;
120	120	bool async_unlinking:1;
	121	+ bool shutdown:1;
121	122	struct ehci_qh *qh_scan_next;
122	123
123	124	/* async schedule support */