| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0gjWhY8AnipzMEgLOp1hsKT6c0PKKKtE+OYSsp0OMxT6w@mail.gmail.com>
Date: Wed, 20 Sep 2023 15:06:18 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Linux ACPI <linux-acpi@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Zhang Rui <rui.zhang@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH v1 7/9] ACPI: thermal: Untangle initialization and updates
of active trips
On Tue, Sep 12, 2023 at 8:47 PM Rafael J. Wysocki <rjw@...ysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Separate the code needed to update active trips (in a response to a
> notification from the platform firmware) as well as to initialize them
> from the code that is only necessary for their initialization and
> cleanly divide it into functions that each carry out a specific action.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/acpi/thermal.c | 197 ++++++++++++++++++++++++-------------------------
> 1 file changed, 100 insertions(+), 97 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -184,94 +184,6 @@ static int acpi_thermal_temp(struct acpi
> tz->kelvin_offset);
> }
>
> -static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
> -{
> - acpi_status status;
> - unsigned long long tmp;
> - struct acpi_handle_list devices;
> - bool valid = false;
> - int i;
> -
> - /* Active (optional) */
> - for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> - char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
> - valid = tz->trips.active[i].trip.valid;
> -
> - if (act == -1)
> - break; /* disable all active trip points */
> -
> - if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
> - tz->trips.active[i].trip.valid)) {
> - status = acpi_evaluate_integer(tz->device->handle,
> - name, NULL, &tmp);
> - if (ACPI_FAILURE(status)) {
> - tz->trips.active[i].trip.valid = false;
> - if (i == 0)
> - break;
> -
> - if (act <= 0)
> - break;
> -
> - if (i == 1)
> - tz->trips.active[0].trip.temperature =
> - celsius_to_deci_kelvin(act);
> - else
> - /*
> - * Don't allow override higher than
> - * the next higher trip point
> - */
> - tz->trips.active[i-1].trip.temperature =
> - min_t(unsigned long,
> - tz->trips.active[i-2].trip.temperature,
> - celsius_to_deci_kelvin(act));
> -
> - break;
> - } else {
> - tz->trips.active[i].trip.temperature = tmp;
> - tz->trips.active[i].trip.valid = true;
> - }
> - }
> -
> - name[2] = 'L';
> - if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) {
> - memset(&devices, 0, sizeof(struct acpi_handle_list));
> - status = acpi_evaluate_reference(tz->device->handle,
> - name, NULL, &devices);
> - if (ACPI_FAILURE(status)) {
> - acpi_handle_info(tz->device->handle,
> - "Invalid active%d threshold\n", i);
> - tz->trips.active[i].trip.valid = false;
> - } else {
> - tz->trips.active[i].trip.valid = true;
> - }
> -
> - if (memcmp(&tz->trips.active[i].devices, &devices,
> - sizeof(struct acpi_handle_list))) {
> - memcpy(&tz->trips.active[i].devices, &devices,
> - sizeof(struct acpi_handle_list));
> - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
> - }
> - }
> - if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
> - if (valid != tz->trips.active[i].trip.valid)
> - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
> -
> - if (!tz->trips.active[i].trip.valid)
> - break;
> - }
> -
> - if (flag & ACPI_TRIPS_DEVICES) {
> - 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))) {
> - tz->devices = devices;
> - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
> - }
> - }
> -}
> -
> static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip,
> int temp)
> {
> @@ -338,6 +250,78 @@ static void acpi_thermal_update_passive_
> ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
> }
>
> +static long get_active_temp(struct acpi_thermal *tz, int index)
> +{
> + char method[] = { '_', 'A', 'C', '0' + index, '\0' };
> + unsigned long long tmp;
> + acpi_status status;
> +
> + status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp);
> + if (ACPI_FAILURE(status))
> + return THERMAL_TEMP_INVALID;
> +
> + /*
> + * If an override has been provided, apply it so there are no active
> + * trips with thresholds greater than the override.
> + */
> + if (act > 0) {
> + unsigned long long override = celsius_to_deci_kelvin(act);
> +
> + if (tmp > override)
> + tmp = override;
> + }
> + return tmp;
> +}
> +
> +static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index)
> +{
> + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
> +
> + if (!acpi_trip->valid)
> + return;
> +
> + update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index));
> + if (!acpi_trip->valid)
> + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
> +}
> +
> +static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare)
> +{
> + char method[] = { '_', 'A', 'L', '0' + index, '\0' };
> + struct acpi_handle_list devices;
> + acpi_status status;
> +
> + memset(&devices, 0, sizeof(devices));
> +
> + status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices);
> + if (ACPI_FAILURE(status)) {
> + acpi_handle_info(tz->device->handle,
> + "Missing device list for active threshold %d\n",
> + index);
> + return false;
> + }
> +
> + if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices)))
> + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device");
> +
> + memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices));
> + return true;
> +}
> +
> +static void acpi_thermal_update_active_devices(struct acpi_thermal *tz, int index)
> +{
> + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
> +
> + if (!acpi_trip->valid)
> + return;
> +
> + if (update_active_devices(tz, index, true))
> + return;
> +
> + update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
> + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
> +}
> +
> static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
> {
> struct acpi_thermal_trip *acpi_trip = trip->priv;
> @@ -358,18 +342,18 @@ static void acpi_thermal_adjust_thermal_
> unsigned long data)
> {
> struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
> - int flag;
> + int i;
>
> if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
> acpi_thermal_update_passive_trip(tz);
> - flag = ACPI_TRIPS_THRESHOLDS;
> + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
> + acpi_thermal_update_active_trip(tz, i);
> } else {
> acpi_thermal_update_passive_devices(tz);
> - flag = ACPI_TRIPS_DEVICES;
> + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
> + acpi_thermal_update_active_devices(tz, i);
> }
>
> - __acpi_thermal_trips_update(tz, flag);
> -
> for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz);
> }
>
> @@ -498,6 +482,28 @@ fail:
> return false;
> }
>
> +static bool acpi_thermal_init_active_trip(struct acpi_thermal *tz, int index)
> +{
> + long temp;
> +
> + if (act == -1)
> + goto fail;
> +
> + temp = get_active_temp(tz, index);
> + if (temp == THERMAL_TEMP_INVALID)
> + goto fail;
> +
> + if (!update_active_devices(tz, false, index))
This should be
if (!update_active_devices(tz, index, false))
and I've already fixed it in the tree.
> + goto fail;
> +
> + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp);
> + return true;
> +
> +fail:
> + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID);
> + return false;
> +}
> +
> static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> {
> unsigned int count = 0;
> @@ -506,11 +512,8 @@ static int acpi_thermal_get_trip_points(
> if (acpi_thermal_init_passive_trip(tz))
> count++;
>
> - /* Active trip points (optional). */
> - __acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
> -
> for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> - if (tz->trips.active[i].trip.valid)
> + if (acpi_thermal_init_active_trip(tz, i))
> count++;
> else
> break;
>
>
>
Powered by blists - more mailing lists