Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

thermal: intel: Protect clearing of thermal status bits

The clearing of the package thermal status is done by Read-Modify-Write
operation. This may result in clearing of some new status bits which are
being or about to be processed.

For example, while clearing of HFI status, after read of thermal status
register, a new thermal status bit is set by the hardware. But during
write back, the newly generated status bit will be set to 0 or cleared.
So, it is not safe to do read-modify-write.

Since thermal status Read-Write bits can be set to only 0 not 1, it is
safe to set all other bits to 1 which are not getting cleared.

Create a common interface for clearing package thermal status bits. Use
this interface to replace existing code to clear thermal package status
bits.

It is safe to call from different CPUs without protection as there is no
read-modify-write. Also wrmsrl results in just single instruction. For
example while CPU 0 and CPU 3 are clearing bit 1 and 3 respectively. If
CPU 3 wins the race, it will write 0x4000aa2, then CPU 1 will write
0x4000aa8. The bits which are not part of clear are set to 1. The default
mask for bits, which can be written here is 0x4000aaa.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

authored by

Srinivas Pandruvada and committed by
Rafael J. Wysocki
930d06bf 6fe1e64b

+22 -24
+2 -6
drivers/thermal/intel/intel_hfi.c
··· 42 42 43 43 #include "../thermal_core.h" 44 44 #include "intel_hfi.h" 45 - 46 - #define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | \ 47 - BIT(9) | BIT(11) | BIT(26)) 45 + #include "thermal_interrupt.h" 48 46 49 47 /* Hardware Feedback Interface MSR configuration bits */ 50 48 #define HW_FEEDBACK_PTR_VALID_BIT BIT(0) ··· 302 304 * Let hardware know that we are done reading the HFI table and it is 303 305 * free to update it again. 304 306 */ 305 - pkg_therm_status_msr_val &= THERM_STATUS_CLEAR_PKG_MASK & 306 - ~PACKAGE_THERM_STATUS_HFI_UPDATED; 307 - wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS, pkg_therm_status_msr_val); 307 + thermal_clear_package_intr_status(PACKAGE_LEVEL, PACKAGE_THERM_STATUS_HFI_UPDATED); 308 308 309 309 queue_delayed_work(hfi_updates_wq, &hfi_instance->update_work, 310 310 HFI_UPDATE_INTERVAL);
+12 -11
drivers/thermal/intel/therm_throt.c
··· 190 190 }; 191 191 #endif /* CONFIG_SYSFS */ 192 192 193 - #define CORE_LEVEL 0 194 - #define PACKAGE_LEVEL 1 195 - 196 193 #define THERM_THROT_POLL_INTERVAL HZ 197 194 #define THERM_STATUS_PROCHOT_LOG BIT(1) 198 195 199 196 #define THERM_STATUS_CLEAR_CORE_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(13) | BIT(15)) 200 197 #define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(26)) 201 198 202 - static void clear_therm_status_log(int level) 199 + /* 200 + * Clear the bits in package thermal status register for bit = 1 201 + * in bitmask 202 + */ 203 + void thermal_clear_package_intr_status(int level, u64 bit_mask) 203 204 { 205 + u64 msr_val; 204 206 int msr; 205 - u64 mask, msr_val; 206 207 207 208 if (level == CORE_LEVEL) { 208 209 msr = MSR_IA32_THERM_STATUS; 209 - mask = THERM_STATUS_CLEAR_CORE_MASK; 210 + msr_val = THERM_STATUS_CLEAR_CORE_MASK; 210 211 } else { 211 212 msr = MSR_IA32_PACKAGE_THERM_STATUS; 212 - mask = THERM_STATUS_CLEAR_PKG_MASK; 213 + msr_val = THERM_STATUS_CLEAR_PKG_MASK; 213 214 } 214 215 215 - rdmsrl(msr, msr_val); 216 - msr_val &= mask; 217 - wrmsrl(msr, msr_val & ~THERM_STATUS_PROCHOT_LOG); 216 + msr_val &= ~bit_mask; 217 + wrmsrl(msr, msr_val); 218 218 } 219 + EXPORT_SYMBOL_GPL(thermal_clear_package_intr_status); 219 220 220 221 static void get_therm_status(int level, bool *proc_hot, u8 *temp) 221 222 { ··· 296 295 state->average = avg; 297 296 298 297 re_arm: 299 - clear_therm_status_log(state->level); 298 + thermal_clear_package_intr_status(state->level, THERM_STATUS_PROCHOT_LOG); 300 299 schedule_delayed_work_on(this_cpu, &state->therm_work, THERM_THROT_POLL_INTERVAL); 301 300 } 302 301
+6
drivers/thermal/intel/thermal_interrupt.h
··· 2 2 #ifndef _INTEL_THERMAL_INTERRUPT_H 3 3 #define _INTEL_THERMAL_INTERRUPT_H 4 4 5 + #define CORE_LEVEL 0 6 + #define PACKAGE_LEVEL 1 7 + 5 8 /* Interrupt Handler for package thermal thresholds */ 6 9 extern int (*platform_thermal_package_notify)(__u64 msr_val); 7 10 ··· 17 14 18 15 /* Handle HWP interrupt */ 19 16 extern void notify_hwp_interrupt(void); 17 + 18 + /* Common function to clear Package thermal status register */ 19 + extern void thermal_clear_package_intr_status(int level, u64 bit_mask); 20 20 21 21 #endif /* _INTEL_THERMAL_INTERRUPT_H */
+2 -7
drivers/thermal/intel/x86_pkg_temp_thermal.c
··· 265 265 struct thermal_zone_device *tzone = NULL; 266 266 int cpu = smp_processor_id(); 267 267 struct zone_device *zonedev; 268 - u64 msr_val, wr_val; 269 268 270 269 mutex_lock(&thermal_zone_mutex); 271 270 raw_spin_lock_irq(&pkg_temp_lock); ··· 278 279 } 279 280 zonedev->work_scheduled = false; 280 281 281 - rdmsrl(MSR_IA32_PACKAGE_THERM_STATUS, msr_val); 282 - wr_val = msr_val & ~(THERM_LOG_THRESHOLD0 | THERM_LOG_THRESHOLD1); 283 - if (wr_val != msr_val) { 284 - wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS, wr_val); 285 - tzone = zonedev->tzone; 286 - } 282 + thermal_clear_package_intr_status(PACKAGE_LEVEL, THERM_LOG_THRESHOLD0 | THERM_LOG_THRESHOLD1); 283 + tzone = zonedev->tzone; 287 284 288 285 enable_pkg_thres_interrupt(); 289 286 raw_spin_unlock_irq(&pkg_temp_lock);