mirror of
				https://github.com/torvalds/linux.git
				synced 2025-10-31 16:48:26 +02:00 
			
		
		
		
	ACPI: PM: s2idle: Avoid possible race related to the EC GPE
It is theoretically possible for the ACPI EC GPE to be set after the
s2idle_ops->wake() called from s2idle_loop() has returned and before
the subsequent pm_wakeup_pending() check is carried out.  If that
happens, the resulting wakeup event will cause the system to resume
even though it may be a spurious one.
To avoid that race, first make the ->wake() callback in struct
platform_s2idle_ops return a bool value indicating whether or not
to let the system resume and rearrange s2idle_loop() to use that
value instad of the direct pm_wakeup_pending() call if ->wake() is
present.
Next, rework acpi_s2idle_wake() to process EC events and check
pm_wakeup_pending() before re-arming the SCI for system wakeup
to prevent it from triggering prematurely and add comments to
that function to explain the rationale for the new code flow.
Fixes: 56b9918490 ("PM: sleep: Simplify suspend-to-idle control flow")
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
			
			
This commit is contained in:
		
							parent
							
								
									f0ac20c3f6
								
							
						
					
					
						commit
						e3728b50cd
					
				
					 3 changed files with 37 additions and 18 deletions
				
			
		|  | @ -990,21 +990,28 @@ static void acpi_s2idle_sync(void) | |||
| 	acpi_os_wait_events_complete(); /* synchronize Notify handling */ | ||||
| } | ||||
| 
 | ||||
| static void acpi_s2idle_wake(void) | ||||
| static bool acpi_s2idle_wake(void) | ||||
| { | ||||
| 	/*
 | ||||
| 	 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has | ||||
| 	 * not triggered while suspended, so bail out. | ||||
| 	 */ | ||||
| 	if (!acpi_sci_irq_valid() || | ||||
| 	    irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) | ||||
| 		return; | ||||
| 	if (!acpi_sci_irq_valid()) | ||||
| 		return pm_wakeup_pending(); | ||||
| 
 | ||||
| 	while (pm_wakeup_pending()) { | ||||
| 		/*
 | ||||
| 		 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the | ||||
| 		 * SCI has not triggered while suspended, so bail out (the | ||||
| 		 * wakeup is pending anyway and the SCI is not the source of | ||||
| 		 * it). | ||||
| 		 */ | ||||
| 		if (irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) | ||||
| 			return true; | ||||
| 
 | ||||
| 		/*
 | ||||
| 		 * If there are no EC events to process, the wakeup is regarded | ||||
| 		 * as a genuine one. | ||||
| 		 */ | ||||
| 		if (!acpi_ec_dispatch_gpe()) | ||||
| 			return true; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * If there are EC events to process, the wakeup may be a spurious one | ||||
| 	 * coming from the EC. | ||||
| 	 */ | ||||
| 	if (acpi_ec_dispatch_gpe()) { | ||||
| 		/*
 | ||||
| 		 * Cancel the wakeup and process all pending events in case | ||||
| 		 * there are any wakeup ones in there. | ||||
|  | @ -1017,8 +1024,19 @@ static void acpi_s2idle_wake(void) | |||
| 
 | ||||
| 		acpi_s2idle_sync(); | ||||
| 
 | ||||
| 		/*
 | ||||
| 		 * The SCI is in the "suspended" state now and it cannot produce | ||||
| 		 * new wakeup events till the rearming below, so if any of them | ||||
| 		 * are pending here, they must be resulting from the processing | ||||
| 		 * of EC events above or coming from somewhere else. | ||||
| 		 */ | ||||
| 		if (pm_wakeup_pending()) | ||||
| 			return true; | ||||
| 
 | ||||
| 		rearm_wake_irq(acpi_sci_irq); | ||||
| 	} | ||||
| 
 | ||||
| 	return false; | ||||
| } | ||||
| 
 | ||||
| static void acpi_s2idle_restore_early(void) | ||||
|  |  | |||
|  | @ -191,7 +191,7 @@ struct platform_s2idle_ops { | |||
| 	int (*begin)(void); | ||||
| 	int (*prepare)(void); | ||||
| 	int (*prepare_late)(void); | ||||
| 	void (*wake)(void); | ||||
| 	bool (*wake)(void); | ||||
| 	void (*restore_early)(void); | ||||
| 	void (*restore)(void); | ||||
| 	void (*end)(void); | ||||
|  |  | |||
|  | @ -131,11 +131,12 @@ static void s2idle_loop(void) | |||
| 	 * to avoid them upfront. | ||||
| 	 */ | ||||
| 	for (;;) { | ||||
| 		if (s2idle_ops && s2idle_ops->wake) | ||||
| 			s2idle_ops->wake(); | ||||
| 
 | ||||
| 		if (pm_wakeup_pending()) | ||||
| 		if (s2idle_ops && s2idle_ops->wake) { | ||||
| 			if (s2idle_ops->wake()) | ||||
| 				break; | ||||
| 		} else if (pm_wakeup_pending()) { | ||||
| 			break; | ||||
| 		} | ||||
| 
 | ||||
| 		pm_wakeup_clear(false); | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Rafael J. Wysocki
						Rafael J. Wysocki