| Message ID | 20140501114019.1a63728e@endymion.delvare (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 05/01/2014 02:40 AM, Jean Delvare wrote: > The Intel 64 and IA-32 Architectures Software Developer's Manual says > that TjMax is stored in bits 23:16 of MSR_TEMPERATURE TARGET (0x1a2). > That's 8 bits, not 7, so it must be masked with 0xFF rather than 0x7F. > > The manual has no mention of which values should be considered valid, > which kind of implies that they all are. Arbitrarily discarding values > outside a specific range is wrong. The upper range check had to be > fixed recently (commit 144b44b1) and the lower range check is just as > wrong. See bug #75071: > > https://bugzilla.kernel.org/show_bug.cgi?id=75071 > > There are many Xeon processor series with TjMax of 70, 71 or 80 > degrees Celsius, way below the arbitrary 85 degrees Celsius limit. > There may be other (past or future) models with even lower limits. > > So drop this arbitrary check. The only value that would be clearly > invalid is 0. Everything else should be accepted. > > After these changes, turbostat is aligned with what the coretemp > driver does. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Len Brown <len.brown@intel.com> > Cc: Josh Triplett <josh@joshtriplett.org> Acked-by: Guenter Roeck <linux@roeck-us.net> -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 01, 2014 at 11:40:19AM +0200, Jean Delvare wrote: > The Intel 64 and IA-32 Architectures Software Developer's Manual says > that TjMax is stored in bits 23:16 of MSR_TEMPERATURE TARGET (0x1a2). > That's 8 bits, not 7, so it must be masked with 0xFF rather than 0x7F. > > The manual has no mention of which values should be considered valid, > which kind of implies that they all are. Arbitrarily discarding values > outside a specific range is wrong. The upper range check had to be > fixed recently (commit 144b44b1) and the lower range check is just as > wrong. See bug #75071: > > https://bugzilla.kernel.org/show_bug.cgi?id=75071 > > There are many Xeon processor series with TjMax of 70, 71 or 80 > degrees Celsius, way below the arbitrary 85 degrees Celsius limit. > There may be other (past or future) models with even lower limits. > > So drop this arbitrary check. The only value that would be clearly > invalid is 0. Everything else should be accepted. > > After these changes, turbostat is aligned with what the coretemp > driver does. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Len Brown <len.brown@intel.com> > Cc: Josh Triplett <josh@joshtriplett.org> Reviewed-by: Josh Triplett <josh@joshtriplett.org> > tools/power/x86/turbostat/turbostat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- linux-3.15-rc3.orig/tools/power/x86/turbostat/turbostat.c 2014-04-14 09:42:33.140152144 +0200 > +++ linux-3.15-rc3/tools/power/x86/turbostat/turbostat.c 2014-05-01 11:22:54.635123682 +0200 > @@ -1971,13 +1971,13 @@ int set_temperature_target(struct thread > if (get_msr(0, MSR_IA32_TEMPERATURE_TARGET, &msr)) > goto guess; > > - target_c_local = (msr >> 16) & 0x7F; > + target_c_local = (msr >> 16) & 0xFF; > > if (verbose) > fprintf(stderr, "cpu%d: MSR_IA32_TEMPERATURE_TARGET: 0x%08llx (%d C)\n", > cpu, msr, target_c_local); > > - if (target_c_local < 85 || target_c_local > 127) > + if (!target_c_local) > goto guess; > > tcc_activation_temp = target_c_local; > > > -- > Jean Delvare > SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-3.15-rc3.orig/tools/power/x86/turbostat/turbostat.c 2014-04-14 09:42:33.140152144 +0200 +++ linux-3.15-rc3/tools/power/x86/turbostat/turbostat.c 2014-05-01 11:22:54.635123682 +0200 @@ -1971,13 +1971,13 @@ int set_temperature_target(struct thread if (get_msr(0, MSR_IA32_TEMPERATURE_TARGET, &msr)) goto guess; - target_c_local = (msr >> 16) & 0x7F; + target_c_local = (msr >> 16) & 0xFF; if (verbose) fprintf(stderr, "cpu%d: MSR_IA32_TEMPERATURE_TARGET: 0x%08llx (%d C)\n", cpu, msr, target_c_local); - if (target_c_local < 85 || target_c_local > 127) + if (!target_c_local) goto guess; tcc_activation_temp = target_c_local;
The Intel 64 and IA-32 Architectures Software Developer's Manual says that TjMax is stored in bits 23:16 of MSR_TEMPERATURE TARGET (0x1a2). That's 8 bits, not 7, so it must be masked with 0xFF rather than 0x7F. The manual has no mention of which values should be considered valid, which kind of implies that they all are. Arbitrarily discarding values outside a specific range is wrong. The upper range check had to be fixed recently (commit 144b44b1) and the lower range check is just as wrong. See bug #75071: https://bugzilla.kernel.org/show_bug.cgi?id=75071 There are many Xeon processor series with TjMax of 70, 71 or 80 degrees Celsius, way below the arbitrary 85 degrees Celsius limit. There may be other (past or future) models with even lower limits. So drop this arbitrary check. The only value that would be clearly invalid is 0. Everything else should be accepted. After these changes, turbostat is aligned with what the coretemp driver does. Signed-off-by: Jean Delvare <jdelvare@suse.de> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Len Brown <len.brown@intel.com> Cc: Josh Triplett <josh@joshtriplett.org> --- tools/power/x86/turbostat/turbostat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)