Upgrade thermald to 2.5.1 for Jammy (22.04)

finish the sru template as we discussed

Thank you for the re-upload.

I think this remains blocked on answers to my questions in https://lists.ubuntu.com/archives/ubuntu-release/2022-October/005495.html ?

@Robie,
* Thermald is enabled since KBL, my test cases has covered.
The earlier machine before KBL, i couldn't find them on the cert LAB.
since the older machine than KBL, i thought it may be not supported fully by Intel.
if the 2.5.1 cause a regression on older machine, we could file a bug on launchpad.

* For the TCC,
I have replied fo Jammsy,
~~~
TCC adjustment has been offloaded to kernel driver intel_tcc_cooling,
it's registered as a thermal cooling device.
2eb87d75f980) thermal/drivers/intel: Introduce tcc cooling driver.
This was merged to mainline since 5.13. Focal is using hwe-5.15.
Ref. https://www.phoronix.com/news/Linux-5.13-Intel-Cooling-Driver
~~~

Timo has a replied for Focal,
~~~
commit fdf4f2fb8e8990c131b2b1a5a9c03681bb16e87a
Author: Srinivas Pandruvada <email address hidden>
Date: Mon Jul 22 18:03:02 2019 -0700

     drivers: thermal: processor_thermal_device: Export sysfs interface
for TCC offset

so a backport to focal (which is planned) should be safe in that regard.
~~~

Is there any question I missed? thanks

@Timo, i have finished.thanks
~~~
[ Where problems could occur ]
~~~

@Robie,
1. is it possible that users are using thermald on hardware not covered by
upstream tests?
[Koba] As per my test cases, the older machine than kbl would be not covered.
but thermald is enabled since 2016, i thought Intel may not support the older fully.
If there's a regression, we could ask user to report on launchpad and help to fix.

2. By "all the unit tests must pass in all the supported
Intel CPUs", who defines "supported"?
[Koba], there's a supported CPU list in the thermald source,
~~~
@src/thd_engine.cpp,
supported_ids_t id_table[] = {
...
>------->-------{ 6, 0x97 }, // Alderlake
>------->-------{ 6, 0x9a }, // Alderlake
>------->-------{ 6, 0xb7 }, // Raptorlake
>------->-------{ 6, 0xba }, // Raptorlake
>------->-------{ 6, 0xbf }, // Raptorlake
...
}
~~~
thermald is maintained by Intel and definitely Intel define "supported".

3. Is it possible that Ubuntu users have hardware not covered by that definition of "supported"?
[Koba], I think it's impossible if there's one Intel platform missed in the supported list. HWE would find it at the developing stage because thermald would complain it first then HWE would check with Intel.

4. Is there any risk to users of non-Intel hardware?
[Koba] There's only one chance that you add the '--ignore-cpuid-check'.
by the default, thermald would not work on non-Intel hardware.

5. How complete is upstream's test coverage?
[Koba] it cover all used modules and loaded policy tables.
a. used modules, rapl_control, intel_pstate, intel_powerclamp, cpufreq, processor.
b. load policy table from xml file or acpi tables.
c. Evaluate the temperature and check if the rules act correctly after activate/escalate/deescalate the cooling devices.

6. What assurance is there that there will be no feature
regressions?
i could only explain there may be corner cases for PL1 min/max feature.

---
for this commit, https://github.com/intel/thermal_daemon/commit/7e490fc79d784b3faf8314af98ec14981ba7fb75

1) Is this safe in relation to Ubuntu kernel versions?
[Koba] I would say it's safe on Jammy/Focal
~~~
Jammy,
~~~~~~
TCC adjustment has been offloaded to kernel driver intel_tcc_cooling,
it's registered as a thermal cooling device.
2eb87d75f980) thermal/drivers/intel: Introduce tcc cooling driver.
This was merged to mainline since 5.13. Focal is using hwe-5.15.
Ref. https://www.phoronix.com/news/Linux-5.13-Intel-Cooling-Driver
~~~~~~

#Timo has a replied for Focal,
~~~~~~
commit fdf4f2fb8e8990c131b2b1a5a9c03681bb16e87a
Author: Srinivas Pandruvada <email address hidden>
Date: Mon Jul 22 18:03:02 2019 -0700

     drivers: thermal: processor_thermal_device: Export sysfs interface
for TCC offset

so a backport to focal (which is planned) should be safe in that regard.
~~~~~~
~~~
2) Did this actually get checked before upload?
[Koba] i checked the related kernel commits if it's landed on Focal/Jammy.

3) What in your proposed QA process would catch this kind of change to ensure that
the specific requirements for each such deprecation is met in Ubun...

Ok. So, sorry about all the back and forth. Partially this is because I'm more familiar with thermald than others on the SRU team, and so don't necessarily make things explicit that should be.

At a high level, what the SRU team (in general, so that *I* don't have to be the single point of failure) is looking for is:

*) What is the scope of potential regressions - what hardware *could* this effect, what possible effects could it have
  - The answer to “what hardware” is: the list of CPUIDs in tdh_engine.cpp:id_table. This is SandyBridge onwards
  - The answer to “what effects” is: temperature throttling problems - either reduced performance of CPU (and GPU?) due to unnecessary throttling, or instability due to not controlling temperatures <FEEL FREE TO EXPAND HERE>

*) What is the scope of *upstream* support - what systems do *they* test on, and expect to continue to work.
  - Relatedly: what testing does upstream do
  - What do we do if upstream doesn't test on hardware that we support (ie: *we* care about all the hardware)

*) What is the process we are going to use to verify that upstream doesn't drop support for systems?
  - Upstream doesn't seem to make it very easy to identify this
  - eg: the current SRU includes dropping the MSR poking support. What process do we/will we have to catch such cases?

*) What is the process for testing that an upload does not regress
  - The [Test Case] above is good for systems with KBL or newer processors
  - thermald also supports systems from SandyBridge onwards - how are we testing these? These are still supported by Ubuntu; we need a testing system more than “maybe users will report regressions”, particularly since it's not necessarily going to be clear to users that “my system got slower” is related to the thermald update.

Most of those questions are covered above, I think, but some could do with your input.

Particularly the SandyBridge+ question is an important one to answer.

@Chris,
1) as per Intel, increased cpu power may throttle iGPU, the current thermald can't balance this issue.
Ref. https://bugs.launchpad.net/ubuntu/+source/thermald/+bug/1944389/comments/23

2.1) i can't answer what testing does upstream do. i asked this but didn't get answer.
@Srinivas, could you help on this?
2.2) cant find the machine older than KBL in canonical cert lab. I am still looking for those and request to Dell.
in case, if can't find the older machine, how could we do?

3.1) just check which kernel support this. if there're some patches missed in generic/oem kernels, I would sru.

4.1) for sandybridge, Im looking for the machines older than kbl.
in case , can't find them, how could I do? i only could get the way that user could report it on launchpad.

@Raof, i have modified the patches,
only picked the necessary and drop the part of refactoring.
* drop these two commits,
~~~
* Dropped, Remove /dev/cpu/*/msr access for TCC
* part of refactoring and keep the required,
  Separate Adaptive engine and GDDV
~~~

Would you please review it first. thanks

The attachment "thermald_Jammy_2.4.9-1ubuntu0.3.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

Hi there! Sorry for the delay in getting to this. Some review:
*) It'd be easier to review if debian/changelog mentioned each of the patches and what they do. For example, rather than having the changelog entry:
  * Support ITMTv2 for Raptor Lake (LP: #2007579)
you could have the changelog entries:
  * debian/patches/0003-Parse-ITMT-Table.patch:
  * debian/patches/0006-Install-ITMT-target.patch:
  * debian/patches/0018-Process-ITMT-v2.patch:
    - Support ITMTv2 for Raptor Lake (LP: #2007579)

Since you know why you selected the patches you've added, this means I don't need to try and reverse-engineer what bug each of the patches is trying to address.

*) 0015-Ensure-there-is-one-trips-element-per-zone.patch: Why is this patch here? It looks to me like it's patching code that we don't build?

Overall this looks like it's mostly there; it's not entirely clear to me what *all* the patches are for, but it looks like they're (mostly) sensible patches.

Chris reviewed your proposed debdiff almost 2 months ago, are you planning to address the comments? I am re-checking this because it is in our general sponsorship queue. If you do not need sponsorship for this upload, please, unsubscribe ~ubuntu-sponsors.

@Lukas, im working on this and will upload the next, thanks

@Raof,
as per request, modified the changelog
please help to review.

Thank you for your patience.

I believe Chris should be the one doing the review here, but I'd like to mention that it would be great if you could add DEP-3 headers to each patch:

https://dep-team.pages.debian.net/deps/dep3/

This helps us better document and understand where each patch came from.

Moreover, your debdiff is unnecessarily adding a ".pc" directory, which is generated by quilt. Please remove it.

Your d/changelog entry has a typo: s/doesnt/doesn't/

You have two patches numbered 0015-... in d/changelog, but only one actual 0015-... patch.

Finally, I don't see an answer to Chris' question regarding 0013-Ensure-there-is-one-trips-element-per-zone.patch.

Thanks!

One more thing:

- There's a trailing whitespace at line 22 on d/changelog.

In addition to the things identified above:

* I've added a question on https://bugs.launchpad.net/ubuntu/+source/thermald/+bug/2018275 about what that's actually fixing.

* debian/patches/0004-Fixed-enumeration-of-cpu-thermal-sensors.patch: this looks reasonable, but I'd like to make sure that were testing on at least one system that should hit this fix (if I understand it correctly, it should apply to anything having > 4 cores, which should be easy to arrange)

Otherwise this looks reasonable, thanks!

Waiting for another debdiff to address the points raised in previous comments.

@Raof
* Replied 1st, please check
Ref. https://bugs.launchpad.net/ubuntu/+source/thermald/+bug/2018275/comments/2
* Replied debian/patches/0004-Fixed-enumeration-of-cpu-thermal-sensors.patch, i can't find the platform that have multiple endpoints in /sys/device/platform/coretemp.x(x => 0~).
*Replied 0013-Ensure-there-is-one-trips-element-per-zone.patch, my fault and will remove it.
*Replied #14, will remove the duplicated 0015 from changelog.

FWIW the packaging changes look good to me. But I'd like raof (or someone with more knowledge about this) to check koba's reply before uploading the package.

Relaying communication from off-Launchpad: Since we have no way to test it, 0004-Fixed-enumeration-of-cpu-thermal-sensors.patch should be dropped, and apart from that this is now acceptable.

+202307051456,
@Raof, @Lucas
v3: because the patch, 0004-Fixed-enumeration-of-cpu-thermal-sensors.patch, can't verify on the proper machine, pull it out from patches.

Thanks. I have uploaded thermald 2.4.9-1ubuntu0.3 jammy with a small change and unsubscribed ~ubuntu-sponsors. I added "Cherry-pick following fixes from thermald 2.5.1 and 2.5.2 (LP: #1995606)" to d/changelog since this bug was not referenced.

Hello koba, or anyone else affected,

Accepted thermald into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/thermald/2.4.9-1ubuntu0.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Verified against thermald-2.4.9-1ubuntu0.3,
===
LP#1995606
Verified,
~~~
RPL,
$ sudo apt policy thermald
thermald:
  Installed: 2.4.9-1ubuntu0.3
  Candidate: 2.4.9-1ubuntu0.3
  Version table:
 *** 2.4.9-1ubuntu0.3 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2.4.9-1ubuntu0.2 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
     2.4.9-1 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy/main amd64 Packages
$ cat /proc/cpuinfo | head
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 186
model name : 13th Gen Intel(R) CoreT i7-13700H
stepping : 2
microcode : 0x4114
cpu MHz : 400.151
cache size : 24576 KB
physical id : 0
~~~
ADL
u@u:~$ sudo apt policy thermald
thermald:
  Installed: 2.4.9-1ubuntu0.3
  Candidate: 2.4.9-1ubuntu0.3
  Version table:
 *** 2.4.9-1ubuntu0.3 500
        500 http://archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2.4.9-1ubuntu0.2 500
        500 http://archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
     2.4.9-1 500
        500 http://archive.ubuntu.com/ubuntu jammy/main amd64 Packages
u@u:~$ cat /proc/cpuinfo | head
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 154
model name : 12th Gen Intel(R) Core(TM) i5-1250P
stepping : 3
microcode : 0x429
cpu MHz : 1178.666
cache size : 12288 KB
physical id : 0

~~~
CML,
$ sudo apt policy thermald
thermald:
  Installed: 2.4.9-1ubuntu0.3
  Candidate: 2.4.9-1ubuntu0.3
  Version table:
 *** 2.4.9-1ubuntu0.3 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2.4.9-1ubuntu0.2 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
     2.4.9-1 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy/main amd64 Packages
$ cat /proc/cpuinfo |head
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 165
model name : Intel(R) Core(TM) i9-10880H CPU @ 2.30GHz
stepping : 2
microcode : 0xf4
cpu MHz : 2300.000
cache size : 16384 KB
physical id : 0
~~~
CFL
$ sudo apt policy thermald
thermald:
  Installed: 2.4.9-1ubuntu0.3
  Candidate: 2.4.9-1ubuntu0.3
  Version table:
 *** 2.4.9-1ubuntu0.3 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2.4.9-1ubuntu0.2 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
     2.4.9-1 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy/main amd64 Packages
u@u-G3-3779:~$ cat /proc/cpuinfo |head
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 158
model name : Intel(R) Core(TM) i5-8300H CPU @ 2.30GHz
stepping : 10
microcode : 0xf0
cpu MHz : 1000.040
cache size : 8192 KB
physical id : 0
~~~
KBL
$ sudo apt policy thermald
thermald:
  Installed: 2.4.9-1ubuntu0.3
  Candidate: 2.4.9-1ubuntu0.3
  Version table:
 *** 2.4.9-1ubuntu0.3 500
        500 http://tw.archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2.4.9-1ubuntu0.2 500
        500 http://tw.archive.ubuntu...

I see that all the individual bugs have been verified, but has the test plan documented on *this* bug been carried out? That is:

[Test case]
For these CPU series, RPL/ADL/TGL/CML/CFL/KBL, the following tests will be run on machines in the CI lab:

1. Run stress-ng, and observe the temperature/frequency/power with s-tui
  - Temperatures should stay just below trip values
  - Power/performance profiles should stay roughly the same between old thermald and new thermald (unless specifically expected eg: to fix premature/insufficient throttling)
2. check if thermald could read rules from /dev/acpi_thermal_rel and generate the xml file on /etc/thermald/ correctly.
  - this depends on if acpi_thermal_rel exist.
  - if the machine suppots acpi_thermal_rel, the "thermal-conf.xml.auto"
 could be landed in etc/thermald/.
  - if not, the user-defined xml could be created, then jump to (3).
  - run thermald with --loglevel=debug, and compare the log with xml.auto file. check if the configuration could be parsed correctly.
3. check if theramd-conf.xml and thermal-cpu-cdev-order.xml can be loaded correctly.
  - run thermald with --loglevel=debug, and compare the log with xml files.
  - if parsed correctly, the configurations from XML files would appear in the log.

4. Run unit tests, the scripts are under test folder, using emul_temp to simulate the High temperatue and check thermald would throttle CPU through the related cooling device.
  - rapl.sh
  - intel_pstate.sh
  - powerclamp.sh
  - processor.sh
5. check if the power/frequency would be throttled once the temperature reach the trip-points of thermal zone.
6. check if system would be throttled even the temperature is under the trip-points.

@Chris, Sure, this's my regular processes but didn't collect the logs.
I will update the logs about this test cases.

The verification of the Stable Release Update for thermald has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package thermald - 2.4.9-1ubuntu0.3

---------------
thermald (2.4.9-1ubuntu0.3) jammy; urgency=medium

  * Cherry-pick following fixes from thermald 2.5.1 and 2.5.2 (LP: #1995606)
  * debian/patches/0013-Add-AlderLake-N.patch
    - Add support for Adler Lake N (LP: #2012260)
  * debian/patches/0007-Add-INT3400-base-path-for-Raptor-Lake.patch
    - Fix RPL: Add INT3400 base path(LP: #1989044)
  * debian/patches/0014-Process-ITMT-v2.patch
    - Support ITMTv2 for Raptor Lake (LP: #2007579)
  * debian/patches/0008-Install-passive-default.patch
    - Fix throttled GPU (LP: #1981087)
  * debian/patches/0012-Always-match-motion-0.patch
    - Fix in-motion function doesn't work (LP: #2018275)
  * debian/patches/0003-Parse-ITMT-Table.patch
  * debian/patches/0004-Add-capability-for-min-max-per-trip.patch
  * debian/patches/0005-Install-ITMT_target.patch
  * debian/patches/0006-Use-per-trip-min-max.patch
  * debian/patches/0009-Parse-idsp-and-trips.patch
  * debian/patches/0010-use-PL1-max-min-from-PPCC-when-policies-match.patch
  * debian/patches/0011-Parse-GDDV-before-thd_engine-init.patch
    - Fix i9-12900k shutdown when run Prime95 and stress-ng (LP: #2018236)

 -- Koba Ko <email address hidden> Wed, 05 Jul 2023 13:37:32 +0200