diff mbox series

[v2] ACPI: utils: Make acpi_handle_list dynamically allocated

Message ID 20230927201725.2339488-1-vi@endrift.com
State New
Headers show
Series [v2] ACPI: utils: Make acpi_handle_list dynamically allocated | expand

Commit Message

Vicki Pfau Sept. 27, 2023, 8:17 p.m. UTC
This fixes a long-standing "TBD" comment in the ACPI headers regarding making
the acpi_handle_list struct's size dynamic. The number 10, which along with the
comment dates back to 2.4.23, seems like it may have been arbitrarily chosen,
and isn't sufficient in all cases. This patch finally makes the size dynamic
and updates its users to handle the modified API.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/acpi/acpi_lpss.c                      |  5 +++-
 drivers/acpi/scan.c                           |  1 +
 drivers/acpi/thermal.c                        | 23 ++++++++++++++-----
 drivers/acpi/utils.c                          |  5 ++--
 .../platform/surface/surface_acpi_notify.c    |  5 +++-
 include/acpi/acpi_bus.h                       |  4 +---
 6 files changed, 30 insertions(+), 13 deletions(-)

Comments

kernel test robot Nov. 6, 2023, 4:26 a.m. UTC | #1
Hi Vicki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.6]
[cannot apply to rafael-pm/linux-next next-20231106]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vicki-Pfau/ACPI-utils-Make-acpi_handle_list-dynamically-allocated/20230928-043202
base:   linus/master
patch link:    https://lore.kernel.org/r/20230927201725.2339488-1-vi%40endrift.com
patch subject: [PATCH v2] ACPI: utils: Make acpi_handle_list dynamically allocated
config: x86_64-randconfig-123-20230930 (https://download.01.org/0day-ci/archive/20231106/202311061141.kEqwdu4M-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231106/202311061141.kEqwdu4M-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311061141.kEqwdu4M-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/acpi/thermal.c:310:54: sparse: sparse: incompatible types for operation (!=):
>> drivers/acpi/thermal.c:310:54: sparse:    unsigned int *
>> drivers/acpi/thermal.c:310:54: sparse:    unsigned int [addressable] [usertype] count
   drivers/acpi/thermal.c:377:64: sparse: sparse: incompatible types for operation (!=):
   drivers/acpi/thermal.c:377:64: sparse:    unsigned int *
   drivers/acpi/thermal.c:377:64: sparse:    unsigned int [addressable] [usertype] count

vim +310 drivers/acpi/thermal.c

   190	
   191	static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
   192	{
   193		acpi_status status;
   194		unsigned long long tmp;
   195		struct acpi_handle_list devices;
   196		bool valid = false;
   197		int i;
   198	
   199		/* Critical Shutdown */
   200		if (flag & ACPI_TRIPS_CRITICAL) {
   201			status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
   202			tz->trips.critical.temperature = tmp;
   203			/*
   204			 * Treat freezing temperatures as invalid as well; some
   205			 * BIOSes return really low values and cause reboots at startup.
   206			 * Below zero (Celsius) values clearly aren't right for sure..
   207			 * ... so lets discard those as invalid.
   208			 */
   209			if (ACPI_FAILURE(status)) {
   210				tz->trips.critical.valid = false;
   211				acpi_handle_debug(tz->device->handle,
   212						  "No critical threshold\n");
   213			} else if (tmp <= 2732) {
   214				pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
   215				tz->trips.critical.valid = false;
   216			} else {
   217				tz->trips.critical.valid = true;
   218				acpi_handle_debug(tz->device->handle,
   219						  "Found critical threshold [%lu]\n",
   220						  tz->trips.critical.temperature);
   221			}
   222			if (tz->trips.critical.valid) {
   223				if (crt == -1) {
   224					tz->trips.critical.valid = false;
   225				} else if (crt > 0) {
   226					unsigned long crt_k = celsius_to_deci_kelvin(crt);
   227	
   228					/*
   229					 * Allow override critical threshold
   230					 */
   231					if (crt_k > tz->trips.critical.temperature)
   232						pr_info("Critical threshold %d C\n", crt);
   233	
   234					tz->trips.critical.temperature = crt_k;
   235				}
   236			}
   237		}
   238	
   239		/* Critical Sleep (optional) */
   240		if (flag & ACPI_TRIPS_HOT) {
   241			status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
   242			if (ACPI_FAILURE(status)) {
   243				tz->trips.hot.valid = false;
   244				acpi_handle_debug(tz->device->handle,
   245						  "No hot threshold\n");
   246			} else {
   247				tz->trips.hot.temperature = tmp;
   248				tz->trips.hot.valid = true;
   249				acpi_handle_debug(tz->device->handle,
   250						  "Found hot threshold [%lu]\n",
   251						  tz->trips.hot.temperature);
   252			}
   253		}
   254	
   255		/* Passive (optional) */
   256		if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.trip.valid) ||
   257		    flag == ACPI_TRIPS_INIT) {
   258			valid = tz->trips.passive.trip.valid;
   259			if (psv == -1) {
   260				status = AE_SUPPORT;
   261			} else if (psv > 0) {
   262				tmp = celsius_to_deci_kelvin(psv);
   263				status = AE_OK;
   264			} else {
   265				status = acpi_evaluate_integer(tz->device->handle,
   266							       "_PSV", NULL, &tmp);
   267			}
   268	
   269			if (ACPI_FAILURE(status)) {
   270				tz->trips.passive.trip.valid = false;
   271			} else {
   272				tz->trips.passive.trip.temperature = tmp;
   273				tz->trips.passive.trip.valid = true;
   274				if (flag == ACPI_TRIPS_INIT) {
   275					status = acpi_evaluate_integer(tz->device->handle,
   276								       "_TC1", NULL, &tmp);
   277					if (ACPI_FAILURE(status))
   278						tz->trips.passive.trip.valid = false;
   279					else
   280						tz->trips.passive.tc1 = tmp;
   281	
   282					status = acpi_evaluate_integer(tz->device->handle,
   283								       "_TC2", NULL, &tmp);
   284					if (ACPI_FAILURE(status))
   285						tz->trips.passive.trip.valid = false;
   286					else
   287						tz->trips.passive.tc2 = tmp;
   288	
   289					status = acpi_evaluate_integer(tz->device->handle,
   290								       "_TSP", NULL, &tmp);
   291					if (ACPI_FAILURE(status))
   292						tz->trips.passive.trip.valid = false;
   293					else
   294						tz->trips.passive.tsp = tmp;
   295				}
   296			}
   297		}
   298		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.trip.valid) {
   299			memset(&devices, 0, sizeof(struct acpi_handle_list));
   300			status = acpi_evaluate_reference(tz->device->handle, "_PSL",
   301							 NULL, &devices);
   302			if (ACPI_FAILURE(status)) {
   303				acpi_handle_info(tz->device->handle,
   304						 "Invalid passive threshold\n");
   305				tz->trips.passive.trip.valid = false;
   306			} else {
   307				tz->trips.passive.trip.valid = true;
   308			}
   309	
 > 310			if (&tz->trips.passive.devices.count != devices.count ||
   311				   memcmp(tz->trips.passive.devices.handles,
   312				   devices.handles, sizeof(acpi_handle) * devices.count)) {
   313				kfree(tz->trips.passive.devices.handles);
   314				memcpy(&tz->trips.passive.devices, &devices,
   315				       sizeof(struct acpi_handle_list));
   316				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
   317			}
   318		}
   319		if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) {
   320			if (valid != tz->trips.passive.trip.valid)
   321				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
   322		}
   323	
   324		/* Active (optional) */
   325		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
   326			char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
   327			valid = tz->trips.active[i].trip.valid;
   328	
   329			if (act == -1)
   330				break; /* disable all active trip points */
   331	
   332			if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
   333			    tz->trips.active[i].trip.valid)) {
   334				status = acpi_evaluate_integer(tz->device->handle,
   335							       name, NULL, &tmp);
   336				if (ACPI_FAILURE(status)) {
   337					tz->trips.active[i].trip.valid = false;
   338					if (i == 0)
   339						break;
   340	
   341					if (act <= 0)
   342						break;
   343	
   344					if (i == 1)
   345						tz->trips.active[0].trip.temperature =
   346								celsius_to_deci_kelvin(act);
   347					else
   348						/*
   349						 * Don't allow override higher than
   350						 * the next higher trip point
   351						 */
   352						tz->trips.active[i-1].trip.temperature =
   353							min_t(unsigned long,
   354							      tz->trips.active[i-2].trip.temperature,
   355							      celsius_to_deci_kelvin(act));
   356	
   357					break;
   358				} else {
   359					tz->trips.active[i].trip.temperature = tmp;
   360					tz->trips.active[i].trip.valid = true;
   361				}
   362			}
   363	
   364			name[2] = 'L';
   365			if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) {
   366				memset(&devices, 0, sizeof(struct acpi_handle_list));
   367				status = acpi_evaluate_reference(tz->device->handle,
   368								 name, NULL, &devices);
   369				if (ACPI_FAILURE(status)) {
   370					acpi_handle_info(tz->device->handle,
   371							 "Invalid active%d threshold\n", i);
   372					tz->trips.active[i].trip.valid = false;
   373				} else {
   374					tz->trips.active[i].trip.valid = true;
   375				}
   376	
   377				if (&tz->trips.active[i].devices.count != devices.count ||
   378					   memcmp(tz->trips.active[i].devices.handles,
   379					   devices.handles, sizeof(acpi_handle) * devices.count)) {
   380					kfree(tz->trips.active[i].devices.handles);
   381					memcpy(&tz->trips.active[i].devices, &devices,
   382					       sizeof(struct acpi_handle_list));
   383					ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
   384				}
   385			}
   386			if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
   387				if (valid != tz->trips.active[i].trip.valid)
   388					ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
   389	
   390			if (!tz->trips.active[i].trip.valid)
   391				break;
   392		}
   393	
   394		if (flag & ACPI_TRIPS_DEVICES) {
   395			memset(&devices, 0, sizeof(devices));
   396			status = acpi_evaluate_reference(tz->device->handle, "_TZD",
   397							 NULL, &devices);
   398			if (ACPI_SUCCESS(status) && (tz->devices.count != devices.count ||
   399			    memcmp(tz->devices.handles, devices.handles,
   400			    sizeof(acpi_handle) * devices.count))) {
   401				kfree(tz->devices.handles);
   402				tz->devices = devices;
   403				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
   404			}
   405		}
   406	}
   407
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 539e700de4d2..4b3aa84faf70 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -591,10 +591,13 @@  static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle)
 	}
 
 	for (i = 0; i < dep_devices.count; i++) {
-		if (dep_devices.handles[i] == handle)
+		if (dep_devices.handles[i] == handle) {
+			kfree(dep_devices.handles);
 			return true;
+		}
 	}
 
+	kfree(dep_devices.handles);
 	return false;
 }
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 691d4b7686ee..2fbe354db0c0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2032,6 +2032,7 @@  static u32 acpi_scan_check_dep(acpi_handle handle, bool check_dep)
 		mutex_unlock(&acpi_dep_list_lock);
 	}
 
+	kfree(dep_devices.handles);
 	return count;
 }
 
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 312730f8272e..48ddc56f96f6 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -307,8 +307,10 @@  static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 			tz->trips.passive.trip.valid = true;
 		}
 
-		if (memcmp(&tz->trips.passive.devices, &devices,
-			   sizeof(struct acpi_handle_list))) {
+		if (&tz->trips.passive.devices.count != devices.count ||
+			   memcmp(tz->trips.passive.devices.handles,
+			   devices.handles, sizeof(acpi_handle) * devices.count)) {
+			kfree(tz->trips.passive.devices.handles);
 			memcpy(&tz->trips.passive.devices, &devices,
 			       sizeof(struct acpi_handle_list));
 			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
@@ -372,8 +374,10 @@  static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 				tz->trips.active[i].trip.valid = true;
 			}
 
-			if (memcmp(&tz->trips.active[i].devices, &devices,
-				   sizeof(struct acpi_handle_list))) {
+			if (&tz->trips.active[i].devices.count != devices.count ||
+				   memcmp(tz->trips.active[i].devices.handles,
+				   devices.handles, sizeof(acpi_handle) * devices.count)) {
+				kfree(tz->trips.active[i].devices.handles);
 				memcpy(&tz->trips.active[i].devices, &devices,
 				       sizeof(struct acpi_handle_list));
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
@@ -391,8 +395,10 @@  static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		memset(&devices, 0, sizeof(devices));
 		status = acpi_evaluate_reference(tz->device->handle, "_TZD",
 						 NULL, &devices);
-		if (ACPI_SUCCESS(status) &&
-		    memcmp(&tz->devices, &devices, sizeof(devices))) {
+		if (ACPI_SUCCESS(status) && (tz->devices.count != devices.count ||
+		    memcmp(tz->devices.handles, devices.handles,
+		    sizeof(acpi_handle) * devices.count))) {
+			kfree(tz->devices.handles);
 			tz->devices = devices;
 			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 		}
@@ -974,6 +980,7 @@  static int acpi_thermal_add(struct acpi_device *device)
 static void acpi_thermal_remove(struct acpi_device *device)
 {
 	struct acpi_thermal *tz;
+	int i;
 
 	if (!device || !acpi_driver_data(device))
 		return;
@@ -986,6 +993,10 @@  static void acpi_thermal_remove(struct acpi_device *device)
 	flush_workqueue(acpi_thermal_pm_queue);
 	acpi_thermal_unregister_thermal_zone(tz);
 
+	kfree(tz->trips.passive.devices.handles);
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
+		kfree(tz->trips.active[i].devices.handles);
+	kfree(tz->devices.handles);
 	kfree(tz);
 }
 
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 2ea14648a661..96f821c41756 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -370,7 +370,8 @@  acpi_evaluate_reference(acpi_handle handle,
 		goto end;
 	}
 
-	if (package->package.count > ACPI_MAX_HANDLES) {
+	list->handles = kcalloc(package->package.count, sizeof(*list->handles), GFP_KERNEL);
+	if (!list->handles) {
 		kfree(package);
 		return AE_NO_MEMORY;
 	}
@@ -402,7 +403,7 @@  acpi_evaluate_reference(acpi_handle handle,
       end:
 	if (ACPI_FAILURE(status)) {
 		list->count = 0;
-		//kfree(list->handles);
+		kfree(list->handles);
 	}
 
 	kfree(buffer.pointer);
diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c
index 897cdd9c3aae..6932d4b35c6c 100644
--- a/drivers/platform/surface/surface_acpi_notify.c
+++ b/drivers/platform/surface/surface_acpi_notify.c
@@ -753,10 +753,13 @@  static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle)
 	}
 
 	for (i = 0; i < dep_devices.count; i++) {
-		if (dep_devices.handles[i] == supplier)
+		if (dep_devices.handles[i] == supplier) {
+			kfree(dep_devices.handles);
 			return true;
+		}
 	}
 
+	kfree(dep_devices.handles);
 	return false;
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 254685085c82..b4bf12343a22 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -12,11 +12,9 @@ 
 #include <linux/device.h>
 #include <linux/property.h>
 
-/* TBD: Make dynamic */
-#define ACPI_MAX_HANDLES	10
 struct acpi_handle_list {
 	u32 count;
-	acpi_handle handles[ACPI_MAX_HANDLES];
+	acpi_handle* handles;
 };
 
 /* acpi_utils.h */