linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] thermal: int340x: Protect trip temperature from dynamic update
@ 2023-01-19 19:29 Srinivas Pandruvada
  2023-01-20 17:18 ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Srinivas Pandruvada @ 2023-01-19 19:29 UTC (permalink / raw)
  To: rafael, rui.zhang, daniel.lezcano
  Cc: linux-pm, linux-kernel, Srinivas Pandruvada, stable

Trip temperatures are read using ACPI methods and stored in the memory
during zone initializtion and when the firmware sends a notification for
change. This trip temperature is returned when the thermal core calls via
callback get_trip_temp().

But it is possible that while updating the memory copy of the trips when
the firmware sends a notification for change, thermal core is reading the
trip temperature via the callback get_trip_temp(). This may return invalid
trip temperature.

To address this add a mutex to protect the invalid temperature reads in
the callback get_trip_temp() and int340x_thermal_read_trips().

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: stable@vger.kernel.org # 5.0+
---
 .../intel/int340x_thermal/int340x_thermal_zone.c | 16 +++++++++++++++-
 .../intel/int340x_thermal/int340x_thermal_zone.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 62c0aa5d0783..fd9080640e03 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -49,6 +49,8 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
 	if (d->override_ops && d->override_ops->get_trip_temp)
 		return d->override_ops->get_trip_temp(zone, trip, temp);
 
+	mutex_lock(&d->trip_mutex);
+
 	if (trip < d->aux_trip_nr)
 		*temp = d->aux_trips[trip];
 	else if (trip == d->crt_trip_id)
@@ -65,10 +67,14 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
 				break;
 			}
 		}
-		if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
+		if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT) {
+			mutex_unlock(&d->trip_mutex);
 			return -EINVAL;
+		}
 	}
 
+	mutex_unlock(&d->trip_mutex);
+
 	return 0;
 }
 
@@ -180,6 +186,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
 	int trip_cnt = int34x_zone->aux_trip_nr;
 	int i;
 
+	mutex_lock(&int34x_zone->trip_mutex);
+
 	int34x_zone->crt_trip_id = -1;
 	if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_CRT",
 					     &int34x_zone->crt_temp))
@@ -207,6 +215,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
 		int34x_zone->act_trips[i].valid = true;
 	}
 
+	mutex_unlock(&int34x_zone->trip_mutex);
+
 	return trip_cnt;
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
@@ -230,6 +240,8 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 	if (!int34x_thermal_zone)
 		return ERR_PTR(-ENOMEM);
 
+	mutex_init(&int34x_thermal_zone->trip_mutex);
+
 	int34x_thermal_zone->adev = adev;
 	int34x_thermal_zone->override_ops = override_ops;
 
@@ -281,6 +293,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
 	kfree(int34x_thermal_zone->aux_trips);
 err_trip_alloc:
+	mutex_destroy(&int34x_thermal_zone->trip_mutex);
 	kfree(int34x_thermal_zone);
 	return ERR_PTR(ret);
 }
@@ -292,6 +305,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
 	thermal_zone_device_unregister(int34x_thermal_zone->zone);
 	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
 	kfree(int34x_thermal_zone->aux_trips);
+	mutex_destroy(&int34x_thermal_zone->trip_mutex);
 	kfree(int34x_thermal_zone);
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
index 3b4971df1b33..8f9872afd0d3 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -32,6 +32,7 @@ struct int34x_thermal_zone {
 	struct thermal_zone_device_ops *override_ops;
 	void *priv_data;
 	struct acpi_lpat_conversion_table *lpat_table;
+	struct mutex trip_mutex;
 };
 
 struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *,
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] thermal: int340x: Protect trip temperature from dynamic update
  2023-01-19 19:29 [PATCH] thermal: int340x: Protect trip temperature from dynamic update Srinivas Pandruvada
@ 2023-01-20 17:18 ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2023-01-20 17:18 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: rafael, rui.zhang, daniel.lezcano, linux-pm, linux-kernel, stable

On Thu, Jan 19, 2023 at 8:29 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> Trip temperatures are read using ACPI methods and stored in the memory
> during zone initializtion and when the firmware sends a notification for
> change. This trip temperature is returned when the thermal core calls via
> callback get_trip_temp().
>
> But it is possible that while updating the memory copy of the trips when
> the firmware sends a notification for change, thermal core is reading the
> trip temperature via the callback get_trip_temp(). This may return invalid
> trip temperature.
>
> To address this add a mutex to protect the invalid temperature reads in
> the callback get_trip_temp() and int340x_thermal_read_trips().
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: stable@vger.kernel.org # 5.0+

One nit below.

> ---
>  .../intel/int340x_thermal/int340x_thermal_zone.c | 16 +++++++++++++++-
>  .../intel/int340x_thermal/int340x_thermal_zone.h |  1 +
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 62c0aa5d0783..fd9080640e03 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -49,6 +49,8 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,

I would add a new local var:

int ret = 0;

>         if (d->override_ops && d->override_ops->get_trip_temp)
>                 return d->override_ops->get_trip_temp(zone, trip, temp);
>
> +       mutex_lock(&d->trip_mutex);
> +
>         if (trip < d->aux_trip_nr)
>                 *temp = d->aux_trips[trip];
>         else if (trip == d->crt_trip_id)
> @@ -65,10 +67,14 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
>                                 break;
>                         }
>                 }
> -               if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
> +               if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT) {
> +                       mutex_unlock(&d->trip_mutex);
>                         return -EINVAL;

And then do

                        ret = -EINVAL;

instead of the above.

> +               }
>         }
>
> +       mutex_unlock(&d->trip_mutex);
> +
>         return 0;

And

    return ret;

here.

>  }
>
> @@ -180,6 +186,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
>         int trip_cnt = int34x_zone->aux_trip_nr;
>         int i;
>
> +       mutex_lock(&int34x_zone->trip_mutex);
> +
>         int34x_zone->crt_trip_id = -1;
>         if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_CRT",
>                                              &int34x_zone->crt_temp))
> @@ -207,6 +215,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
>                 int34x_zone->act_trips[i].valid = true;
>         }
>
> +       mutex_unlock(&int34x_zone->trip_mutex);
> +
>         return trip_cnt;
>  }
>  EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
> @@ -230,6 +240,8 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>         if (!int34x_thermal_zone)
>                 return ERR_PTR(-ENOMEM);
>
> +       mutex_init(&int34x_thermal_zone->trip_mutex);
> +
>         int34x_thermal_zone->adev = adev;
>         int34x_thermal_zone->override_ops = override_ops;
>
> @@ -281,6 +293,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>         acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
>         kfree(int34x_thermal_zone->aux_trips);
>  err_trip_alloc:
> +       mutex_destroy(&int34x_thermal_zone->trip_mutex);
>         kfree(int34x_thermal_zone);
>         return ERR_PTR(ret);
>  }
> @@ -292,6 +305,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
>         thermal_zone_device_unregister(int34x_thermal_zone->zone);
>         acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
>         kfree(int34x_thermal_zone->aux_trips);
> +       mutex_destroy(&int34x_thermal_zone->trip_mutex);
>         kfree(int34x_thermal_zone);
>  }
>  EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> index 3b4971df1b33..8f9872afd0d3 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -32,6 +32,7 @@ struct int34x_thermal_zone {
>         struct thermal_zone_device_ops *override_ops;
>         void *priv_data;
>         struct acpi_lpat_conversion_table *lpat_table;
> +       struct mutex trip_mutex;
>  };
>
>  struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *,
> --
> 2.38.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-20 17:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19 19:29 [PATCH] thermal: int340x: Protect trip temperature from dynamic update Srinivas Pandruvada
2023-01-20 17:18 ` Rafael J. Wysocki

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).