Re: kern/58372: Temperature reading broken on Intel Core 2 Duo E6300 (patch provided)

[Sotiris Lamprinidis <sotiris%lamprinidis.com@localhost>]

The following reply was made to PR kern/58372; it has been noted by GNATS.

From: Sotiris Lamprinidis <sotiris%lamprinidis.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/58372: Temperature reading broken on Intel Core 2 Duo E6300
 (patch provided)
Date: Fri, 28 Jun 2024 17:41:17 +0200

 I see a fix was pulled - my bad.
 
 Also thanks for the input. Indeed tjmax=3D94 would make the reading 72 =
 (-22) under load
 which seems right.
 
 There is also this (model < 0x17) check which looks weird since we're =
 already inside
 the ((model =3D=3D 0x0f) || (model =3D=3D 0x0e)) block. Since we're =
 here, maybe
 something like this would be reasonable: =20
 
 Index: sys/arch/x86/x86/coretemp.c
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/sys/arch/x86/x86/coretemp.c,v
 retrieving revision 1.38.4.2
 diff -u -u -r1.38.4.2 coretemp.c
 --- sys/arch/x86/x86/coretemp.c 22 Jun 2024 09:57:21 -0000      1.38.4.2
 +++ sys/arch/x86/x86/coretemp.c 28 Jun 2024 15:34:23 -0000
 @@ -275,6 +275,8 @@
         sc->sc_tjmax =3D TJMAX_DEFAULT;
 
         if ((model =3D=3D 0x0f && stepping >=3D 2) || (model =3D=3D =
 0x0e)) {
 +               /* Default for 65mm Core 2s */
 +               sc->sc_tjmax =3D 94;
                 /*
                  * Check MSR_IA32_PLATFORM_ID(0x17) bit 28. It's not =
 documented
                  * in the datasheet, but the following page describes =
 the
 @@ -284,9 +286,15 @@
                  *   Was: =
 http://softwarecommunity.intel.com/Wiki/Mobility/
                  *     720.htm
                  */
 -               if (rdmsr_safe(MSR_IA32_PLATFORM_ID, &msr) !=3D 0)
 -                       goto notee;
 -               if ((model < 0x17) && ((msr & __BIT(28)) =3D=3D 0))
 +               if (rdmsr_safe(MSR_IA32_PLATFORM_ID, &msr) =3D=3D =
 EFAULT) {
 +                       aprint_normal("\n");
 +                       aprint_error_dev(sc->sc_dev,
 +                           "Failed to read MSR_IA32_PLATFORM_ID. "
 +                           "Using default (%d)\n", sc->sc_tjmax);
 +                       return 1;
 +               }
 +
 +               if ((msr & __BIT(28)) =3D=3D 0)
                         goto notee;
 
                 if (rdmsr_safe(MSR_IA32_EXT_CONFIG, &msr) =3D=3D EFAULT) =
 {
                                                                          =
                                                                          =
                                                                          =
                                                                          =
                                                                          =
                                                                          =
                                                                          =
                                                                   =20
 Thanks,
 S.
 
 > A couple of things. The zero value assignment itself was a regression
 > that has since been addressed differently and pulled up to the 9 and =
 10
 > branches. It still doesn't address that we can't correctly read the
 > value in some cases, it just moves the default back to 100 as it was
 > previously.
 >=20
 > 74 seems oddly low. I realize that's what's in the table you =
 referenced
 > but I would expect it to be a bit higher. That table lists T case max,
 > which is not the same as T junction max, as I read Intel's =
 definitions.
 > If you look at page 72 in the datasheet, you'll see under "THERMTRIP#"
 > it says "the processor will automatically shut down when the silicon
 > has reached a temperature approximately 20 =C2=B0C above the maximum =
 TC.
 > Assertion of THERMTRIP# (Thermal Trip) indicates the processor =
 junction
 > temperature has reached a level beyond where permanent silicon damage
 > may occur." So my interpretation of that is TjMax is somewhere between
 > 74 and 94, but not clearly defined. (I could be quite wrong, this is
 > not an area I've delved into much.)
 >=20
 > Regards,
 >=20
 > Dave
 >=20
 > PS my reference for T junction vs. T case is:
 > =
 https://www.intel.com/content/www/us/en/support/articles/000005597/process=
 ors.html
 >=20