[power_sampler] Add sampling of CPU temperature via SMC.

Bug: 1254332
Change-Id: I182e694672048cf8d4fa78f40baebd47dd8ec2ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3312177
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Francois Pierre Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/main@{#947662}

chrome/browser/metrics/power/power_metrics_provider_mac.mm

@@ -110,15 +110,15 @@ class PowerMetricsProvider::Impl {
       return;
 
     RecordSMCHistogram("Power.Mac.Total.", suffix,
-                       smc_reader_->ReadTotalPowerW());
+                       smc_reader_->ReadKey(SMCKeyIdentifier::TotalPower));
     RecordSMCHistogram("Power.Mac.CPU.", suffix,
-                       smc_reader_->ReadCPUPackageCPUPowerW());
+                       smc_reader_->ReadKey(SMCKeyIdentifier::CPUPower));
     RecordSMCHistogram("Power.Mac.GPUi.", suffix,
-                       smc_reader_->ReadCPUPackageGPUPowerW());
+                       smc_reader_->ReadKey(SMCKeyIdentifier::iGPUPower));
     RecordSMCHistogram("Power.Mac.GPU0.", suffix,
-                       smc_reader_->ReadGPU0PowerW());
+                       smc_reader_->ReadKey(SMCKeyIdentifier::GPU0Power));
     RecordSMCHistogram("Power.Mac.GPU1.", suffix,
-                       smc_reader_->ReadGPU1PowerW());
+                       smc_reader_->ReadKey(SMCKeyIdentifier::GPU1Power));
   }
 
   void RecordIsOnBattery() {

components/power_metrics/smc_internal_types_mac.h

@@ -9,12 +9,17 @@
 #include <stdint.h>
 
 // List of known SMC key identifiers.
+//
+// This is a good reference: https://logi.wiki/index.php/SMC_Sensor_Codes
+// Additional keys can be discovered with
+// https://github.com/theopolis/smc-fuzzer
 enum class SMCKeyIdentifier : uint32_t {
-  TotalPower = 'PSTR',  // Power: System Total Rail (watts)
-  CPUPower = 'PCPC',    // Power: CPU Package CPU (watts)
-  iGPUPower = 'PCPG',   // Power: CPU Package GPU (watts)
-  GPU0Power = 'PG0R',   // Power: GPU 0 Rail (watts)
-  GPU1Power = 'PG1R',   // Power: GPU 1 Rail (watts)
+  TotalPower = 'PSTR',      // Power: System Total Rail (watts)
+  CPUPower = 'PCPC',        // Power: CPU Package CPU (watts)
+  iGPUPower = 'PCPG',       // Power: CPU Package GPU (watts)
+  GPU0Power = 'PG0R',       // Power: GPU 0 Rail (watts)
+  GPU1Power = 'PG1R',       // Power: GPU 1 Rail (watts)
+  CPUTemperature = 'TC0F',  // Temperature: CPU Die PECI (Celsius)
 };
 
 // Types from PowerManagement/pmconfigd/PrivateLib.c

components/power_metrics/smc_mac.h

@@ -13,6 +13,7 @@
 
 #include <memory>
 
+#include "base/containers/flat_map.h"
 #include "base/mac/scoped_ioobject.h"
 #include "components/power_metrics/smc_internal_types_mac.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -26,13 +27,9 @@ class SMCReader {
 
   virtual ~SMCReader();
 
-  // Returns the power consumption of various hardware components in watts.
+  // Returns the value of a key, or nullopt if not available.
   // Virtual for testing.
-  virtual absl::optional<double> ReadTotalPowerW();
-  virtual absl::optional<double> ReadCPUPackageCPUPowerW();
-  virtual absl::optional<double> ReadCPUPackageGPUPowerW();
-  virtual absl::optional<double> ReadGPU0PowerW();
-  virtual absl::optional<double> ReadGPU1PowerW();
+  virtual absl::optional<double> ReadKey(SMCKeyIdentifier identifier);
 
  protected:
   explicit SMCReader(base::mac::ScopedIOObject<io_object_t> connect);
@@ -42,6 +39,8 @@ class SMCReader {
    public:
     SMCKey(base::mac::ScopedIOObject<io_object_t> connect,
            SMCKeyIdentifier key_identifier);
+    SMCKey(SMCKey&&);
+    SMCKey& operator=(SMCKey&&);
     ~SMCKey();
 
     bool Exists() const;
@@ -51,15 +50,12 @@ class SMCReader {
     bool CallSMCFunction(uint8_t function, SMCParamStruct* out);
 
     base::mac::ScopedIOObject<io_object_t> connect_;
-    const SMCKeyIdentifier key_identifier_;
+    SMCKeyIdentifier key_identifier_;
     SMCKeyInfoData key_info_;
   };
 
-  SMCKey total_power_key_;
-  SMCKey cpu_package_cpu_power_key_;
-  SMCKey cpu_package_gpu_power_key_;
-  SMCKey gpu0_power_key_;
-  SMCKey gpu1_power_key_;
+  base::mac::ScopedIOObject<io_object_t> connect_;
+  base::flat_map<SMCKeyIdentifier, SMCKey> keys_;
 };
 
 }  // namespace power_metrics

components/power_metrics/smc_mac.mm

@@ -35,24 +35,14 @@ std::unique_ptr<SMCReader> SMCReader::Create() {
 
 SMCReader::~SMCReader() = default;
 
-absl::optional<double> SMCReader::ReadTotalPowerW() {
-  return total_power_key_.Read();
-}
+absl::optional<double> SMCReader::ReadKey(SMCKeyIdentifier identifier) {
+  auto it = keys_.find(identifier);
+  if (it == keys_.end()) {
+    auto result = keys_.emplace(identifier, SMCKey(connect_, identifier));
+    it = result.first;
+  }
 
-absl::optional<double> SMCReader::ReadCPUPackageCPUPowerW() {
-  return cpu_package_cpu_power_key_.Read();
-}
-
-absl::optional<double> SMCReader::ReadCPUPackageGPUPowerW() {
-  return cpu_package_gpu_power_key_.Read();
-}
-
-absl::optional<double> SMCReader::ReadGPU0PowerW() {
-  return gpu0_power_key_.Read();
-}
-
-absl::optional<double> SMCReader::ReadGPU1PowerW() {
-  return gpu1_power_key_.Read();
+  return it->second.Read();
 }
 
 SMCReader::SMCKey::SMCKey(base::mac::ScopedIOObject<io_object_t> connect,
@@ -64,6 +54,9 @@ SMCReader::SMCKey::SMCKey(base::mac::ScopedIOObject<io_object_t> connect,
     key_info_ = out.keyInfo;
 }
 
+SMCReader::SMCKey::SMCKey(SMCKey&&) = default;
+SMCReader::SMCKey& SMCReader::SMCKey::operator=(SMCKey&&) = default;
+
 SMCReader::SMCKey::~SMCKey() = default;
 
 bool SMCReader::SMCKey::Exists() const {
@@ -125,10 +118,6 @@ bool SMCReader::SMCKey::CallSMCFunction(uint8_t function, SMCParamStruct* out) {
 }
 
 SMCReader::SMCReader(base::mac::ScopedIOObject<io_object_t> connect)
-    : total_power_key_(connect, SMCKeyIdentifier::TotalPower),
-      cpu_package_cpu_power_key_(connect, SMCKeyIdentifier::CPUPower),
-      cpu_package_gpu_power_key_(connect, SMCKeyIdentifier::iGPUPower),
-      gpu0_power_key_(connect, SMCKeyIdentifier::GPU0Power),
-      gpu1_power_key_(connect, SMCKeyIdentifier::GPU1Power) {}
+    : connect_(std::move(connect)) {}
 
 }  // namespace power_metrics

tools/mac/power/power_sampler/smc_sampler.mm

@@ -41,20 +41,26 @@ Sampler::DatumNameUnits SMCSampler::GetDatumNameUnits() {
                      {"cpu_package_cpu_power", "w"},
                      {"cpu_package_gpu_power", "w"},
                      {"gpu0_power", "w"},
-                     {"gpu1_power", "w"}};
+                     {"gpu1_power", "w"},
+                     {"cpu_temperature", "C"}};
   return ret;
 }
 
 Sampler::Sample SMCSampler::GetSample(base::TimeTicks sample_time) {
   Sample sample;
 
-  MaybeAddToSample(&sample, "total_power", smc_reader_->ReadTotalPowerW());
+  MaybeAddToSample(&sample, "total_power",
+                   smc_reader_->ReadKey(SMCKeyIdentifier::TotalPower));
   MaybeAddToSample(&sample, "cpu_package_cpu_power",
-                   smc_reader_->ReadCPUPackageCPUPowerW());
+                   smc_reader_->ReadKey(SMCKeyIdentifier::CPUPower));
   MaybeAddToSample(&sample, "cpu_package_gpu_power",
-                   smc_reader_->ReadCPUPackageGPUPowerW());
-  MaybeAddToSample(&sample, "gpu0_power", smc_reader_->ReadGPU0PowerW());
-  MaybeAddToSample(&sample, "gpu1_power", smc_reader_->ReadGPU1PowerW());
+                   smc_reader_->ReadKey(SMCKeyIdentifier::iGPUPower));
+  MaybeAddToSample(&sample, "gpu0_power",
+                   smc_reader_->ReadKey(SMCKeyIdentifier::GPU0Power));
+  MaybeAddToSample(&sample, "gpu1_power",
+                   smc_reader_->ReadKey(SMCKeyIdentifier::GPU1Power));
+  MaybeAddToSample(&sample, "cpu_temperature",
+                   smc_reader_->ReadKey(SMCKeyIdentifier::CPUTemperature));
 
   return sample;
 }

tools/mac/power/power_sampler/smc_sampler_unittest.mm

@@ -5,6 +5,7 @@
 #include "tools/mac/power/power_sampler/smc_sampler.h"
 #include <memory>
 
+#include "base/containers/flat_map.h"
 #include "base/memory/ptr_util.h"
 #include "components/power_metrics/smc_mac.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -23,39 +24,17 @@ class TestSMCReader : public power_metrics::SMCReader {
   TestSMCReader()
       : power_metrics::SMCReader(base::mac::ScopedIOObject<io_object_t>()) {}
 
-  void set_total_power(absl::optional<double> total_power) {
-    total_power_ = total_power;
-  }
-  void set_cpu_package_cpu_power(absl::optional<double> cpu_package_cpu_power) {
-    cpu_package_cpu_power_ = cpu_package_cpu_power;
-  }
-  void set_cpu_package_gpu_power(absl::optional<double> cpu_package_gpu_power) {
-    cpu_package_gpu_power_ = cpu_package_gpu_power;
-  }
-  void set_gpu0_power(absl::optional<double> gpu0_power) {
-    gpu0_power_ = gpu0_power;
-  }
-  void set_gpu1_power(absl::optional<double> gpu1_power) {
-    gpu1_power_ = gpu1_power;
+  void set_key(SMCKeyIdentifier key, absl::optional<double> value) {
+    keys_[key] = value;
   }
 
   // power_metrics::SMCReader:
-  absl::optional<double> ReadTotalPowerW() override { return total_power_; }
-  absl::optional<double> ReadCPUPackageCPUPowerW() override {
-    return cpu_package_cpu_power_;
+  absl::optional<double> ReadKey(SMCKeyIdentifier identifier) override {
+    return keys_[identifier];
   }
-  absl::optional<double> ReadCPUPackageGPUPowerW() override {
-    return cpu_package_gpu_power_;
-  }
-  absl::optional<double> ReadGPU0PowerW() override { return gpu0_power_; }
-  absl::optional<double> ReadGPU1PowerW() override { return gpu1_power_; }
 
  private:
-  absl::optional<double> total_power_;
-  absl::optional<double> cpu_package_cpu_power_;
-  absl::optional<double> cpu_package_gpu_power_;
-  absl::optional<double> gpu0_power_;
-  absl::optional<double> gpu1_power_;
+  base::flat_map<SMCKeyIdentifier, absl::optional<double>> keys_;
 };
 
 }  // namespace
@@ -81,15 +60,17 @@ TEST_F(SMCSamplerTest, NameAndGetDatumNameUnits) {
                                    std::make_pair("cpu_package_cpu_power", "w"),
                                    std::make_pair("cpu_package_gpu_power", "w"),
                                    std::make_pair("gpu0_power", "w"),
-                                   std::make_pair("gpu1_power", "w")));
+                                   std::make_pair("gpu1_power", "w"),
+                                   std::make_pair("cpu_temperature", "C")));
 }
 
 TEST_F(SMCSamplerTest, GetSample_AllFieldsAvailable) {
-  reader_->set_total_power(1);
-  reader_->set_cpu_package_cpu_power(2);
-  reader_->set_cpu_package_gpu_power(3);
-  reader_->set_gpu0_power(4);
-  reader_->set_gpu1_power(5);
+  reader_->set_key(SMCKeyIdentifier::TotalPower, 1);
+  reader_->set_key(SMCKeyIdentifier::CPUPower, 2);
+  reader_->set_key(SMCKeyIdentifier::iGPUPower, 3);
+  reader_->set_key(SMCKeyIdentifier::GPU0Power, 4);
+  reader_->set_key(SMCKeyIdentifier::GPU1Power, 5);
+  reader_->set_key(SMCKeyIdentifier::CPUTemperature, 6);
 
   Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
   EXPECT_THAT(sample,
@@ -97,69 +78,88 @@ TEST_F(SMCSamplerTest, GetSample_AllFieldsAvailable) {
                                    std::make_pair("cpu_package_cpu_power", 2),
                                    std::make_pair("cpu_package_gpu_power", 3),
                                    std::make_pair("gpu0_power", 4),
-                                   std::make_pair("gpu1_power", 5)));
+                                   std::make_pair("gpu1_power", 5),
+                                   std::make_pair("cpu_temperature", 6)));
 }
 
 TEST_F(SMCSamplerTest, GetSample_IndividualFieldNotAvailable) {
-  reader_->set_total_power(1);
-  reader_->set_cpu_package_cpu_power(2);
-  reader_->set_cpu_package_gpu_power(3);
-  reader_->set_gpu0_power(4);
-  reader_->set_gpu1_power(5);
+  reader_->set_key(SMCKeyIdentifier::TotalPower, 1);
+  reader_->set_key(SMCKeyIdentifier::CPUPower, 2);
+  reader_->set_key(SMCKeyIdentifier::iGPUPower, 3);
+  reader_->set_key(SMCKeyIdentifier::GPU0Power, 4);
+  reader_->set_key(SMCKeyIdentifier::GPU1Power, 5);
+  reader_->set_key(SMCKeyIdentifier::CPUTemperature, 6);
 
   {
-    reader_->set_total_power(absl::nullopt);
+    reader_->set_key(SMCKeyIdentifier::TotalPower, absl::nullopt);
     Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
     EXPECT_THAT(sample,
                 UnorderedElementsAre(std::make_pair("cpu_package_cpu_power", 2),
                                      std::make_pair("cpu_package_gpu_power", 3),
                                      std::make_pair("gpu0_power", 4),
-                                     std::make_pair("gpu1_power", 5)));
-    reader_->set_total_power(1);
+                                     std::make_pair("gpu1_power", 5),
+                                     std::make_pair("cpu_temperature", 6)));
+    reader_->set_key(SMCKeyIdentifier::TotalPower, 1);
   }
 
   {
-    reader_->set_cpu_package_cpu_power(absl::nullopt);
+    reader_->set_key(SMCKeyIdentifier::CPUPower, absl::nullopt);
     Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
     EXPECT_THAT(sample,
                 UnorderedElementsAre(std::make_pair("total_power", 1),
                                      std::make_pair("cpu_package_gpu_power", 3),
                                      std::make_pair("gpu0_power", 4),
-                                     std::make_pair("gpu1_power", 5)));
-    reader_->set_cpu_package_cpu_power(2);
+                                     std::make_pair("gpu1_power", 5),
+                                     std::make_pair("cpu_temperature", 6)));
+    reader_->set_key(SMCKeyIdentifier::CPUPower, 2);
   }
 
   {
-    reader_->set_cpu_package_gpu_power(absl::nullopt);
+    reader_->set_key(SMCKeyIdentifier::iGPUPower, absl::nullopt);
     Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
     EXPECT_THAT(sample,
                 UnorderedElementsAre(std::make_pair("total_power", 1),
                                      std::make_pair("cpu_package_cpu_power", 2),
                                      std::make_pair("gpu0_power", 4),
-                                     std::make_pair("gpu1_power", 5)));
-    reader_->set_cpu_package_gpu_power(3);
+                                     std::make_pair("gpu1_power", 5),
+                                     std::make_pair("cpu_temperature", 6)));
+    reader_->set_key(SMCKeyIdentifier::iGPUPower, 3);
   }
 
   {
-    reader_->set_gpu0_power(absl::nullopt);
+    reader_->set_key(SMCKeyIdentifier::GPU0Power, absl::nullopt);
     Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
     EXPECT_THAT(sample,
                 UnorderedElementsAre(std::make_pair("total_power", 1),
                                      std::make_pair("cpu_package_cpu_power", 2),
                                      std::make_pair("cpu_package_gpu_power", 3),
-                                     std::make_pair("gpu1_power", 5)));
-    reader_->set_gpu0_power(4);
+                                     std::make_pair("gpu1_power", 5),
+                                     std::make_pair("cpu_temperature", 6)));
+    reader_->set_key(SMCKeyIdentifier::GPU0Power, 4);
   }
 
   {
-    reader_->set_gpu1_power(absl::nullopt);
+    reader_->set_key(SMCKeyIdentifier::GPU1Power, absl::nullopt);
     Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
     EXPECT_THAT(sample,
                 UnorderedElementsAre(std::make_pair("total_power", 1),
                                      std::make_pair("cpu_package_cpu_power", 2),
                                      std::make_pair("cpu_package_gpu_power", 3),
-                                     std::make_pair("gpu0_power", 4)));
-    reader_->set_gpu1_power(5);
+                                     std::make_pair("gpu0_power", 4),
+                                     std::make_pair("cpu_temperature", 6)));
+    reader_->set_key(SMCKeyIdentifier::GPU1Power, 5);
+  }
+
+  {
+    reader_->set_key(SMCKeyIdentifier::CPUTemperature, absl::nullopt);
+    Sampler::Sample sample = sampler_->GetSample(base::TimeTicks());
+    EXPECT_THAT(sample,
+                UnorderedElementsAre(std::make_pair("total_power", 1),
+                                     std::make_pair("cpu_package_cpu_power", 2),
+                                     std::make_pair("cpu_package_gpu_power", 3),
+                                     std::make_pair("gpu0_power", 4),
+                                     std::make_pair("gpu1_power", 5)));
+    reader_->set_key(SMCKeyIdentifier::CPUTemperature, 6);
   }
 }