linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] ACPICA: Avoid evaluating methods too early during system resume
@ 2021-09-28 19:17 Rafael J. Wysocki
  2021-09-29 16:03 ` kernel test robot
  0 siblings, 1 reply; 2+ messages in thread
From: Rafael J. Wysocki @ 2021-09-28 19:17 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Bob Moore, Reik Keutterling

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

During wakeup from system-wide sleep states, acpi_get_sleep_type_data()
is called and it tries to get memory from the slab allocator in order
to evaluate a control method, but if KFENCE is enabled in the kernel,
the memory allocation attempt causes an IRQ work to be queued and a
self-IPI to be sent to the CPU running the code which requires the
memory controller to be ready, so if that happens too early in the
wakeup path, it doesn't work.

Prevent that from taking place by calling acpi_get_sleep_type_data()
for S0 upfront, when preparing to enter a given sleep state, and
saving the data obtained by it for later use during system wakeup.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214271
Reported-by: Reik Keutterling <spielkind@gmail.com>
Tested-by: Reik Keutterling <spielkind@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/acglobal.h  |    2 ++
 drivers/acpi/acpica/hwesleep.c  |    8 ++------
 drivers/acpi/acpica/hwsleep.c   |    9 +++------
 drivers/acpi/acpica/hwxfsleep.c |    7 +++++++
 4 files changed, 14 insertions(+), 12 deletions(-)

Index: linux-pm/drivers/acpi/acpica/acglobal.h
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/acglobal.h
+++ linux-pm/drivers/acpi/acpica/acglobal.h
@@ -226,6 +226,8 @@ extern struct acpi_bit_register_info
     acpi_gbl_bit_register_info[ACPI_NUM_BITREG];
 ACPI_GLOBAL(u8, acpi_gbl_sleep_type_a);
 ACPI_GLOBAL(u8, acpi_gbl_sleep_type_b);
+ACPI_GLOBAL(u8, acpi_gbl_sleep_type_a_s0);
+ACPI_GLOBAL(u8, acpi_gbl_sleep_type_b_s0);
 
 /*****************************************************************************
  *
Index: linux-pm/drivers/acpi/acpica/hwxfsleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwxfsleep.c
+++ linux-pm/drivers/acpi/acpica/hwxfsleep.c
@@ -217,6 +217,13 @@ acpi_status acpi_enter_sleep_state_prep(
 		return_ACPI_STATUS(status);
 	}
 
+	status = acpi_get_sleep_type_data(ACPI_STATE_S0,
+					  &acpi_gbl_sleep_type_a_s0,
+					  &acpi_gbl_sleep_type_b_s0);
+	if (ACPI_FAILURE(status)) {
+		acpi_gbl_sleep_type_a_s0 = ACPI_SLEEP_TYPE_INVALID;
+	}
+
 	/* Execute the _PTS method (Prepare To Sleep) */
 
 	arg_list.count = 1;
Index: linux-pm/drivers/acpi/acpica/hwsleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwsleep.c
+++ linux-pm/drivers/acpi/acpica/hwsleep.c
@@ -192,10 +192,7 @@ acpi_status acpi_hw_legacy_wake_prep(u8
 	 * This is unclear from the ACPI Spec, but it is required
 	 * by some machines.
 	 */
-	status = acpi_get_sleep_type_data(ACPI_STATE_S0,
-					  &acpi_gbl_sleep_type_a,
-					  &acpi_gbl_sleep_type_b);
-	if (ACPI_SUCCESS(status)) {
+	if (acpi_gbl_sleep_type_a_s0 != ACPI_SLEEP_TYPE_INVALID) {
 		sleep_type_reg_info =
 		    acpi_hw_get_bit_register_info(ACPI_BITREG_SLEEP_TYPE);
 		sleep_enable_reg_info =
@@ -216,9 +213,9 @@ acpi_status acpi_hw_legacy_wake_prep(u8
 
 			/* Insert the SLP_TYP bits */
 
-			pm1a_control |= (acpi_gbl_sleep_type_a <<
+			pm1a_control |= (acpi_gbl_sleep_type_a_s0 <<
 					 sleep_type_reg_info->bit_position);
-			pm1b_control |= (acpi_gbl_sleep_type_b <<
+			pm1b_control |= (acpi_gbl_sleep_type_b_s0 <<
 					 sleep_type_reg_info->bit_position);
 
 			/* Write the control registers and ignore any errors */
Index: linux-pm/drivers/acpi/acpica/hwesleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwesleep.c
+++ linux-pm/drivers/acpi/acpica/hwesleep.c
@@ -147,17 +147,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 acpi_status acpi_hw_extended_wake_prep(u8 sleep_state)
 {
-	acpi_status status;
 	u8 sleep_type_value;
 
 	ACPI_FUNCTION_TRACE(hw_extended_wake_prep);
 
-	status = acpi_get_sleep_type_data(ACPI_STATE_S0,
-					  &acpi_gbl_sleep_type_a,
-					  &acpi_gbl_sleep_type_b);
-	if (ACPI_SUCCESS(status)) {
+	if (acpi_gbl_sleep_type_a_s0 != ACPI_SLEEP_TYPE_INVALID) {
 		sleep_type_value =
-		    ((acpi_gbl_sleep_type_a << ACPI_X_SLEEP_TYPE_POSITION) &
+		    ((acpi_gbl_sleep_type_a_s0 << ACPI_X_SLEEP_TYPE_POSITION) &
 		     ACPI_X_SLEEP_TYPE_MASK);
 
 		(void)acpi_write((u64)(sleep_type_value | ACPI_X_SLEEP_ENABLE),




^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: [PATCH v1] ACPICA: Avoid evaluating methods too early during system resume
  2021-09-28 19:17 [PATCH v1] ACPICA: Avoid evaluating methods too early during system resume Rafael J. Wysocki
@ 2021-09-29 16:03 ` kernel test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2021-09-29 16:03 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: llvm, kbuild-all, Bob Moore, Reik Keutterling

[-- Attachment #1: Type: text/plain, Size: 5120 bytes --]

Hi "Rafael,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on v5.15-rc3 next-20210921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Rafael-J-Wysocki/ACPICA-Avoid-evaluating-methods-too-early-during-system-resume/20210929-094420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-r011-20210929 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5fe36d7853aa294e5f0085e4b036a28fe97f25ea
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Rafael-J-Wysocki/ACPICA-Avoid-evaluating-methods-too-early-during-system-resume/20210929-094420
        git checkout 5fe36d7853aa294e5f0085e4b036a28fe97f25ea
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/acpi/acpica/hwsleep.c:195:6: warning: variable 'status' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (acpi_gbl_sleep_type_a_s0 != ACPI_SLEEP_TYPE_INVALID) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/acpi/acpica/hwsleep.c:228:21: note: uninitialized use occurs here
           return_ACPI_STATUS(status);
                              ^~~~~~
   include/acpi/acoutput.h:457:48: note: expanded from macro 'return_ACPI_STATUS'
   #define return_ACPI_STATUS(s)           return(s)
                                                  ^
   drivers/acpi/acpica/hwsleep.c:195:2: note: remove the 'if' if its condition is always true
           if (acpi_gbl_sleep_type_a_s0 != ACPI_SLEEP_TYPE_INVALID) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/acpi/acpica/hwsleep.c:182:20: note: initialize the variable 'status' to silence this warning
           acpi_status status;
                             ^
                              = 0
   1 warning generated.


vim +195 drivers/acpi/acpica/hwsleep.c

   165	
   166	/*******************************************************************************
   167	 *
   168	 * FUNCTION:    acpi_hw_legacy_wake_prep
   169	 *
   170	 * PARAMETERS:  sleep_state         - Which sleep state we just exited
   171	 *
   172	 * RETURN:      Status
   173	 *
   174	 * DESCRIPTION: Perform the first state of OS-independent ACPI cleanup after a
   175	 *              sleep.
   176	 *              Called with interrupts ENABLED.
   177	 *
   178	 ******************************************************************************/
   179	
   180	acpi_status acpi_hw_legacy_wake_prep(u8 sleep_state)
   181	{
   182		acpi_status status;
   183		struct acpi_bit_register_info *sleep_type_reg_info;
   184		struct acpi_bit_register_info *sleep_enable_reg_info;
   185		u32 pm1a_control;
   186		u32 pm1b_control;
   187	
   188		ACPI_FUNCTION_TRACE(hw_legacy_wake_prep);
   189	
   190		/*
   191		 * Set SLP_TYPE and SLP_EN to state S0.
   192		 * This is unclear from the ACPI Spec, but it is required
   193		 * by some machines.
   194		 */
 > 195		if (acpi_gbl_sleep_type_a_s0 != ACPI_SLEEP_TYPE_INVALID) {
   196			sleep_type_reg_info =
   197			    acpi_hw_get_bit_register_info(ACPI_BITREG_SLEEP_TYPE);
   198			sleep_enable_reg_info =
   199			    acpi_hw_get_bit_register_info(ACPI_BITREG_SLEEP_ENABLE);
   200	
   201			/* Get current value of PM1A control */
   202	
   203			status = acpi_hw_register_read(ACPI_REGISTER_PM1_CONTROL,
   204						       &pm1a_control);
   205			if (ACPI_SUCCESS(status)) {
   206	
   207				/* Clear the SLP_EN and SLP_TYP fields */
   208	
   209				pm1a_control &= ~(sleep_type_reg_info->access_bit_mask |
   210						  sleep_enable_reg_info->
   211						  access_bit_mask);
   212				pm1b_control = pm1a_control;
   213	
   214				/* Insert the SLP_TYP bits */
   215	
   216				pm1a_control |= (acpi_gbl_sleep_type_a_s0 <<
   217						 sleep_type_reg_info->bit_position);
   218				pm1b_control |= (acpi_gbl_sleep_type_b_s0 <<
   219						 sleep_type_reg_info->bit_position);
   220	
   221				/* Write the control registers and ignore any errors */
   222	
   223				(void)acpi_hw_write_pm1_control(pm1a_control,
   224								pm1b_control);
   225			}
   226		}
   227	
   228		return_ACPI_STATUS(status);
   229	}
   230	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34063 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-29 16:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 19:17 [PATCH v1] ACPICA: Avoid evaluating methods too early during system resume Rafael J. Wysocki
2021-09-29 16:03 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).