crosperf: migrate device setup functions for telemetry_Crosperf

This is a patch working together with crrev.com/c/1949606 to migrate
device setup functions from suite_runner to autotest telmetry_Crosperf.

In this patch, we modified the logic of different suite types along with
skylab mode for suite_runner.

In result_cache, we handle the wait time log file to accumulate cool
down time per machine.

We also move intel_pstate kernel updating code into SetupDevice() since
it doesn't increase overhead by checking every iteration.

TEST=Passed all unittests; tested with different benchmark modes (Note
that currently skylab server tests cannot be tested because of
regression: crbug.com/984103).
BUG=chromium:1020655

Cq-Depend: chromium:1949606
Change-Id: If8e5099d096d2568d4f54584a8fcfd3c0f99c8f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1949626
Reviewed-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Denis Nikitin <denik@chromium.org>
Tested-by: Zhizhou Yang <zhizhouy@google.com>
Auto-Submit: Zhizhou Yang <zhizhouy@google.com>
Commit-Queue: Denis Nikitin <denik@chromium.org>

9 files changed, 244 insertions, 236 deletions

diff --git a/cros_utils/device_setup_utils.py b/cros_utils/device_setup_utils.py
index 93e681bd..ea705263 100755
--- a/cros_utils/device_setup_utils.py
+++ b/cros_utils/device_setup_utils.py
@@ -18,15 +18,13 @@ __author__ = 'zhizhouy@google.com (Zhizhou Yang)'
 import re
 import time
 
+from contextlib import contextmanager
+
 from cros_utils import command_executer
 
 
 class DutWrapper(object):
-  """Wrap DUT parameters inside.
-
-  Eventially CommandExecuter will reqiure only one
-  argument - command.
-  """
+  """Wrap DUT parameters inside."""
 
   def __init__(self,
                chromeos_root,
@@ -43,6 +41,7 @@ class DutWrapper(object):
     self.dut_config = dut_config
 
   def RunCommandOnDut(self, command, ignore_status=False):
+    """Helper function to run command on DUT."""
     ret, msg, err_msg = self.ce.CrosRunCommandWOutput(
         command, machine=self.remote, chromeos_root=self.chromeos_root)
 
@@ -60,15 +59,17 @@ class DutWrapper(object):
     return ret, msg, err_msg
 
   def DisableASLR(self):
+    """Disable ASLR on DUT."""
     disable_aslr = ('set -e; '
                     'if [[ -e /proc/sys/kernel/randomize_va_space ]]; then '
                     '  echo 0 > /proc/sys/kernel/randomize_va_space; '
                     'fi')
     if self.log_level == 'average':
       self.logger.LogOutput('Disable ASLR.')
-    self.RunCommandOnDut(disable_aslr, ignore_status=False)
+    self.RunCommandOnDut(disable_aslr)
 
   def SetCpuGovernor(self, governor, ignore_status=False):
+    """Setup CPU Governor on DUT."""
     set_gov_cmd = (
         'for f in `ls -d /sys/devices/system/cpu/cpu*/cpufreq 2>/dev/null`; do '
         # Skip writing scaling_governor if cpu is offline.
@@ -85,6 +86,7 @@ class DutWrapper(object):
     return ret
 
   def DisableTurbo(self):
+    """Disable Turbo on DUT."""
     dis_turbo_cmd = (
         'if [[ -e /sys/devices/system/cpu/intel_pstate/no_turbo ]]; then '
         '  if grep -q 0 /sys/devices/system/cpu/intel_pstate/no_turbo;  then '
@@ -284,6 +286,7 @@ class DutWrapper(object):
                            (avail_ngt_target, max_freq_path, min_freq_path))
 
   def WaitCooldown(self):
+    """Wait for DUT to cool down to certain temperature."""
     waittime = 0
     timeout_in_sec = int(self.dut_config['cooldown_time']) * 60
     # Temperature from sensors come in uCelsius units.
@@ -327,11 +330,13 @@ class DutWrapper(object):
       self.RunCommandOnDut(sed_command + FILE)
 
   def StopUI(self):
+    """Stop UI on DUT."""
     # Added "ignore_status" for the case when crosperf stops ui service which
     # was already stopped. Command is going to fail with 1.
     self.RunCommandOnDut('stop ui', ignore_status=True)
 
   def StartUI(self):
+    """Start UI on DUT."""
     # Similar to StopUI, `start ui` fails if the service is already started.
     self.RunCommandOnDut('start ui', ignore_status=True)
 
@@ -456,3 +461,78 @@ class DutWrapper(object):
           'Final verification failed with status %d' % ret_code)
 
     self.logger.LogOutput('Kernel cmdline updated successfully.')
+
+  @contextmanager
+  def PauseUI(self):
+    """Stop UI before and Start UI after the context block.
+
+    Context manager will make sure UI is always resumed at the end.
+    """
+    self.StopUI()
+    try:
+      yield
+
+    finally:
+      self.StartUI()
+
+  def SetupDevice(self):
+    """Setup device to get it ready for testing.
+
+    @Returns Wait time of cool down for this benchmark run.
+    """
+    self.logger.LogOutput('Update kernel cmdline if necessary and reboot')
+    intel_pstate = self.dut_config['intel_pstate']
+    if intel_pstate and self.KerncmdUpdateNeeded(intel_pstate):
+      self.UpdateKerncmdIntelPstate(intel_pstate)
+
+    wait_time = 0
+    # Pause UI while configuring the DUT.
+    # This will accelerate setup (waiting for cooldown has x10 drop)
+    # and help to reset a Chrome state left after the previous test.
+    with self.PauseUI():
+      # Unless the user turns on ASLR in the flag, we first disable ASLR
+      # before running the benchmarks
+      if not self.dut_config['enable_aslr']:
+        self.DisableASLR()
+
+      # CPU usage setup comes first where we enable/disable cores.
+      self.SetupCpuUsage()
+      cpu_online_status = self.GetCpuOnline()
+      # List of online cores of type int (core number).
+      online_cores = [
+          core for core, status in cpu_online_status.items() if status
+      ]
+      if self.dut_config['cooldown_time']:
+        # Setup power conservative mode for effective cool down.
+        # Set ignore status since powersave may no be available
+        # on all platforms and we are going to handle it.
+        ret = self.SetCpuGovernor('powersave', ignore_status=True)
+        if ret:
+          # "powersave" is not available, use "ondemand".
+          # Still not a fatal error if it fails.
+          ret = self.SetCpuGovernor('ondemand', ignore_status=True)
+        # TODO(denik): Run comparison test for 'powersave' and 'ondemand'
+        # on scarlet and kevin64.
+        # We might have to consider reducing freq manually to the min
+        # if it helps to reduce waiting time.
+        wait_time = self.WaitCooldown()
+
+      # Setup CPU governor for the benchmark run.
+      # It overwrites the previous governor settings.
+      governor = self.dut_config['governor']
+      # FIXME(denik): Pass online cores to governor setup.
+      self.SetCpuGovernor(governor)
+
+      # Disable Turbo and Setup CPU freq should ALWAYS proceed governor setup
+      # since governor may change:
+      # - frequency;
+      # - turbo/boost.
+      self.DisableTurbo()
+      self.SetupCpuFreq(online_cores)
+
+      self.DecreaseWaitTime()
+      # FIXME(denik): Currently we are not recovering the previous cpufreq
+      # settings since we do reboot/setup every time anyway.
+      # But it may change in the future and then we have to recover the
+      # settings.
+    return wait_time
diff --git a/cros_utils/device_setup_utils_unittest.py b/cros_utils/device_setup_utils_unittest.py
index b2f76f7d..24fe8b3f 100755
--- a/cros_utils/device_setup_utils_unittest.py
+++ b/cros_utils/device_setup_utils_unittest.py
@@ -162,8 +162,7 @@ class DutWrapperTest(unittest.TestCase):
                    'if [[ -e /proc/sys/kernel/randomize_va_space ]]; then '
                    '  echo 0 > /proc/sys/kernel/randomize_va_space; '
                    'fi')
-    self.dw.RunCommandOnDut.assert_called_once_with(
-        set_cpu_cmd, ignore_status=False)
+    self.dw.RunCommandOnDut.assert_called_once_with(set_cpu_cmd)
 
   def test_set_cpu_governor(self):
     self.dw.RunCommandOnDut = mock.Mock(return_value=(0, '', ''))
@@ -615,6 +614,90 @@ class DutWrapperTest(unittest.TestCase):
     self.dw.RunCommandOnDut.assert_called_once_with(
         'start ui', ignore_status=True)
 
+  def test_setup_device(self):
+
+    def FakeRunner(command, ignore_status=False):
+      # pylint fix for unused variable.
+      del command, ignore_status
+      return 0, '', ''
+
+    def SetupMockFunctions():
+      self.dw.RunCommandOnDut = mock.Mock(return_value=FakeRunner)
+      self.dw.KerncmdUpdateNeeded = mock.Mock(return_value=True)
+      self.dw.UpdateKerncmdIntelPstate = mock.Mock(return_value=0)
+      self.dw.DisableASLR = mock.Mock(return_value=0)
+      self.dw.SetupCpuUsage = mock.Mock(return_value=0)
+      self.dw.SetupCpuFreq = mock.Mock(return_value=0)
+      self.dw.GetCpuOnline = mock.Mock(return_value={0: 1, 1: 1, 2: 0})
+      self.dw.SetCpuGovernor = mock.Mock(return_value=0)
+      self.dw.DisableTurbo = mock.Mock(return_value=0)
+      self.dw.StopUI = mock.Mock(return_value=0)
+      self.dw.StartUI = mock.Mock(return_value=0)
+      self.dw.WaitCooldown = mock.Mock(return_value=0)
+      self.dw.DecreaseWaitTime = mock.Mock(return_value=0)
+
+    self.dw.dut_config['enable_aslr'] = False
+    self.dw.dut_config['cooldown_time'] = 0
+    self.dw.dut_config['governor'] = 'fake_governor'
+    self.dw.dut_config['cpu_freq_pct'] = 65
+    self.dw.dut_config['intel_pstate'] = 'no_hwp'
+
+    SetupMockFunctions()
+    self.dw.SetupDevice()
+
+    self.dw.KerncmdUpdateNeeded.assert_called_once()
+    self.dw.UpdateKerncmdIntelPstate.assert_called_once()
+    self.dw.DisableASLR.assert_called_once()
+    self.dw.SetupCpuUsage.assert_called_once_with()
+    self.dw.SetupCpuFreq.assert_called_once_with([0, 1])
+    self.dw.GetCpuOnline.assert_called_once_with()
+    self.dw.SetCpuGovernor.assert_called_once_with('fake_governor')
+    self.dw.DisableTurbo.assert_called_once_with()
+    self.dw.DecreaseWaitTime.assert_called_once_with()
+    self.dw.StopUI.assert_called_once_with()
+    self.dw.StartUI.assert_called_once_with()
+    self.dw.WaitCooldown.assert_not_called()
+
+    # Test SetupDevice with cooldown
+    self.dw.dut_config['cooldown_time'] = 10
+
+    SetupMockFunctions()
+    self.dw.GetCpuOnline = mock.Mock(return_value={0: 0, 1: 1})
+
+    self.dw.SetupDevice()
+
+    self.dw.WaitCooldown.assert_called_once_with()
+    self.dw.DisableASLR.assert_called_once()
+    self.dw.DisableTurbo.assert_called_once_with()
+    self.dw.SetupCpuUsage.assert_called_once_with()
+    self.dw.SetupCpuFreq.assert_called_once_with([1])
+    self.dw.SetCpuGovernor.assert_called()
+    self.dw.GetCpuOnline.assert_called_once_with()
+    self.dw.StopUI.assert_called_once_with()
+    self.dw.StartUI.assert_called_once_with()
+    self.assertGreater(self.dw.SetCpuGovernor.call_count, 1)
+    self.assertEqual(self.dw.SetCpuGovernor.call_args,
+                     mock.call('fake_governor'))
+
+    # Test SetupDevice with cooldown
+    SetupMockFunctions()
+    self.dw.SetupCpuUsage = mock.Mock(side_effect=RuntimeError())
+
+    with self.assertRaises(RuntimeError):
+      self.dw.SetupDevice()
+
+    # This call injected an exception.
+    self.dw.SetupCpuUsage.assert_called_once_with()
+    # Calls following the expeption are skipped.
+    self.dw.WaitCooldown.assert_not_called()
+    self.dw.DisableTurbo.assert_not_called()
+    self.dw.SetupCpuFreq.assert_not_called()
+    self.dw.SetCpuGovernor.assert_not_called()
+    self.dw.GetCpuOnline.assert_not_called()
+    # Check that Stop/Start UI are always called.
+    self.dw.StopUI.assert_called_once_with()
+    self.dw.StartUI.assert_called_once_with()
+
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/crosperf/crosperf.py b/crosperf/crosperf.py
index fddd18b9..75494371 100755
--- a/crosperf/crosperf.py
+++ b/crosperf/crosperf.py
@@ -90,11 +90,6 @@ def RunCrosperf(argv):
       dest='log_dir',
       default='',
       help='The log_dir, default is under <crosperf_logs>/logs')
-  parser.add_argument(
-      '--no_hwp',
-      default=False,
-      action='store_true',
-      help='Disable intel_pstate on Intel CPU with HWP support.')
 
   SetupParserOptions(parser)
   options, args = parser.parse_known_args(argv)
@@ -121,8 +116,6 @@ def RunCrosperf(argv):
     experiment_file.GetGlobalSettings().SetField('name', experiment_name)
   experiment = ExperimentFactory().GetExperiment(experiment_file,
                                                  working_directory, log_dir)
-  if options.no_hwp:
-    experiment.intel_pstate = 'no_hwp'
 
   json_report = experiment_file.GetGlobalSettings().GetField('json_report')
 
diff --git a/crosperf/experiment.py b/crosperf/experiment.py
index d6d9ee4c..45a028ad 100644
--- a/crosperf/experiment.py
+++ b/crosperf/experiment.py
@@ -57,7 +57,6 @@ class Experiment(object):
     self.ignore_min_max = ignore_min_max
     self.skylab = skylab
     self.l = logger.GetLogger(log_dir)
-    self.intel_pstate = dut_config['intel_pstate']
 
     if not self.benchmarks:
       raise RuntimeError('No benchmarks specified')
diff --git a/crosperf/results_cache.py b/crosperf/results_cache.py
index 977e3e22..bd80e8a7 100644
--- a/crosperf/results_cache.py
+++ b/crosperf/results_cache.py
@@ -58,6 +58,7 @@ class Result(object):
     self.turbostat_log_file = ''
     self.cpustats_log_file = ''
     self.top_log_file = ''
+    self.wait_time_log_file = ''
     self.chrome_version = ''
     self.err = None
     self.chroot_results_dir = ''
@@ -110,7 +111,7 @@ class Result(object):
   def CopyResultsTo(self, dest_dir):
     self.CopyFilesTo(dest_dir, self.perf_data_files)
     self.CopyFilesTo(dest_dir, self.perf_report_files)
-    if len(self.perf_data_files) or len(self.perf_report_files):
+    if self.perf_data_files or self.perf_report_files:
       self._logger.LogOutput('Perf results files stored in %s.' % dest_dir)
 
   def GetNewKeyvals(self, keyvals_dict):
@@ -316,6 +317,10 @@ class Result(object):
     """Get cpustats log path string."""
     return self.FindFilesInResultsDir('-name top.log').split('\n')[0]
 
+  def GetWaitTimeFile(self):
+    """Get wait time log path string."""
+    return self.FindFilesInResultsDir('-name wait_time.log').split('\n')[0]
+
   def _CheckDebugPath(self, option, path):
     relative_path = path[1:]
     out_chroot_path = os.path.join(self.chromeos_root, 'chroot', relative_path)
@@ -415,6 +420,7 @@ class Result(object):
     self.turbostat_log_file = self.GetTurbostatFile()
     self.cpustats_log_file = self.GetCpustatsFile()
     self.top_log_file = self.GetTopFile()
+    self.wait_time_log_file = self.GetWaitTimeFile()
     # TODO(asharif): Do something similar with perf stat.
 
     # Grab keyvals from the directory.
@@ -818,6 +824,16 @@ class Result(object):
       cpustats = self.ProcessCpustatsResults()
     if self.top_log_file:
       self.top_cmds = self.ProcessTopResults()
+    if self.wait_time_log_file:
+      with open(self.wait_time_log_file) as f:
+        wait_time = f.readline().strip()
+        try:
+          wait_time = float(wait_time)
+        except ValueError:
+          raise ValueError('Wait time in log file is not a number.')
+      # This is for accumulating wait time for telemtry_Crosperf runs only,
+      # for test_that runs, please refer to suite_runner.
+      self.machine.AddCooldownWaitTime(wait_time)
 
     for param_key, param in cpustats.items():
       for param_type, param_values in param.items():
@@ -980,10 +996,6 @@ class Result(object):
 class TelemetryResult(Result):
   """Class to hold the results of a single Telemetry run."""
 
-  def __init__(self, logger, label, log_level, machine, cmd_exec=None):
-    super(TelemetryResult, self).__init__(logger, label, log_level, machine,
-                                          cmd_exec)
-
   def PopulateFromRun(self, out, err, retval, test, suite, cwp_dso):
     self.out = out
     self.err = err
@@ -1237,6 +1249,9 @@ class ResultsCache(object):
 class MockResultsCache(ResultsCache):
   """Class for mock testing, corresponding to ResultsCache class."""
 
+  # FIXME: pylint complains about this mock init method, we should probably
+  # replace all Mock classes in Crosperf with simple Mock.mock().
+  # pylint: disable=arguments-differ
   def Init(self, *args):
     pass
 
diff --git a/crosperf/results_cache_unittest.py b/crosperf/results_cache_unittest.py
index 7ce04221..c1c27265 100755
--- a/crosperf/results_cache_unittest.py
+++ b/crosperf/results_cache_unittest.py
@@ -434,6 +434,7 @@ class ResultTest(unittest.TestCase):
     self.callGetTurbostatFile = False
     self.callGetCpustatsFile = False
     self.callGetTopFile = False
+    self.callGetWaitTimeFile = False
     self.args = None
     self.callGatherPerfResults = False
     self.mock_logger = mock.Mock(spec=logger.Logger)
@@ -1126,6 +1127,10 @@ class ResultTest(unittest.TestCase):
       self.callGetTopFile = True
       return []
 
+    def FakeGetWaitTimeFile():
+      self.callGetWaitTimeFile = True
+      return []
+
     def FakeProcessResults(show_results=False):
       if show_results:
         pass
@@ -1143,6 +1148,7 @@ class ResultTest(unittest.TestCase):
     self.callGetTurbostatFile = False
     self.callGetCpustatsFile = False
     self.callGetTopFile = False
+    self.callGetWaitTimeFile = False
     self.callProcessResults = False
 
     self.result.GetResultsDir = FakeGetResultsDir
@@ -1152,6 +1158,7 @@ class ResultTest(unittest.TestCase):
     self.result.GetTurbostatFile = FakeGetTurbostatFile
     self.result.GetCpustatsFile = FakeGetCpustatsFile
     self.result.GetTopFile = FakeGetTopFile
+    self.result.GetWaitTimeFile = FakeGetWaitTimeFile
     self.result.ProcessResults = FakeProcessResults
 
     self.result.PopulateFromRun(OUTPUT, '', 0, 'test', 'telemetry_Crosperf',
@@ -1163,6 +1170,7 @@ class ResultTest(unittest.TestCase):
     self.assertTrue(self.callGetTurbostatFile)
     self.assertTrue(self.callGetCpustatsFile)
     self.assertTrue(self.callGetTopFile)
+    self.assertTrue(self.callGetWaitTimeFile)
     self.assertTrue(self.callProcessResults)
 
   def FakeGetKeyvals(self, show_all=False):
diff --git a/crosperf/schedv2.py b/crosperf/schedv2.py
index d15f18c0..9a4f9660 100644
--- a/crosperf/schedv2.py
+++ b/crosperf/schedv2.py
@@ -20,7 +20,6 @@ import test_flag
 from machine_image_manager import MachineImageManager
 from cros_utils import command_executer
 from cros_utils import logger
-from cros_utils.device_setup_utils import DutWrapper
 
 
 class DutWorker(Thread):
@@ -55,7 +54,6 @@ class DutWorker(Thread):
         Note - 'br' below means 'benchmark_run'.
     """
 
-    intel_pstate = self._sched.get_experiment().intel_pstate
     # Firstly, handle benchmarkruns that have cache hit.
     br = self._sched.get_cached_benchmark_run()
     while br:
@@ -89,15 +87,6 @@ class DutWorker(Thread):
                                     'working thread {}.'.format(self))
             break
         else:
-          self._logger.LogOutput('Update kernel cmdline if necessary '
-                                 'and reboot')
-          run_on_dut = DutWrapper(
-              self._sched.get_labels(0).chromeos_root,
-              self._dut.name,
-              logger=self._logger)
-          if run_on_dut.KerncmdUpdateNeeded(intel_pstate):
-            run_on_dut.UpdateKerncmdIntelPstate(intel_pstate)
-
           # Execute the br.
           self._execute_benchmark_run(br)
     finally:
diff --git a/crosperf/suite_runner.py b/crosperf/suite_runner.py
index 418358e0..eebd7682 100644
--- a/crosperf/suite_runner.py
+++ b/crosperf/suite_runner.py
@@ -10,11 +10,10 @@ from __future__ import print_function
 
 import json
 import os
+import pipes
 import shlex
 import time
 
-from contextlib import contextmanager
-
 from cros_utils import command_executer
 from cros_utils.device_setup_utils import DutWrapper
 
@@ -52,6 +51,10 @@ def GetProfilerArgs(profiler_args):
   return ' '.join(args_list)
 
 
+def GetDutConfigArgs(dut_config):
+  return 'dut_config={}'.format(pipes.quote(json.dumps(dut_config)))
+
+
 class SuiteRunner(object):
   """This defines the interface from crosperf to test script."""
 
@@ -72,8 +75,8 @@ class SuiteRunner(object):
 
   def Run(self, cros_machine, label, benchmark, test_args, profiler_args):
     machine_name = cros_machine.name
-    if not label.skylab:
-      # Initialize command executer on DUT.
+    if benchmark.suite != 'telemetry_Crosperf':
+      # Initialize command executer on DUT for test_that runs.
       run_on_dut = DutWrapper(
           label.chromeos_root,
           machine_name,
@@ -82,22 +85,23 @@ class SuiteRunner(object):
           ce=self._ce,
           dut_config=self.dut_config)
     for i in range(0, benchmark.retries + 1):
+      # TODO: For telemetry_Crosperf run, device setup has been moved into
+      # server test script; for client runs, need to figure out wrapper to do
+      # it before running, now it is still setup here.
+      if benchmark.suite != 'telemetry_Crosperf':
+        wait_time = run_on_dut.SetupDevice()
+        # This is for accumulating wait time for test_that runs only,
+        # for telemetry_Cropserf runs, please refer to result_cache.
+        cros_machine.AddCooldownWaitTime(wait_time)
+
       if label.skylab:
-        # TODO: need to migrate Device setups to autotest for skylab.
-        # Since in skylab mode, we may not know the DUT until one is assigned
-        # to the test. For telemetry_Crosperf run, we can move them into the
-        # server test script, for client runs, need to figure out wrapper to do
-        # it before running.
         ret_tup = self.Skylab_Run(label, benchmark, test_args, profiler_args)
+      elif benchmark.suite == 'telemetry_Crosperf':
+        ret_tup = self.Telemetry_Crosperf_Run(machine_name, label, benchmark,
+                                              test_args, profiler_args)
       else:
-        self.SetupDevice(run_on_dut, cros_machine)
-        if benchmark.suite == 'telemetry_Crosperf':
-          run_on_dut.DecreaseWaitTime()
-          ret_tup = self.Telemetry_Crosperf_Run(machine_name, label, benchmark,
-                                                test_args, profiler_args)
-        else:
-          ret_tup = self.Test_That_Run(machine_name, label, benchmark,
-                                       test_args, profiler_args)
+        ret_tup = self.Test_That_Run(machine_name, label, benchmark, test_args,
+                                     profiler_args)
 
       if ret_tup[0] != 0:
         self.logger.LogOutput('benchmark %s failed. Retries left: %s' %
@@ -112,69 +116,6 @@ class SuiteRunner(object):
         break
     return ret_tup
 
-  @contextmanager
-  def PauseUI(self, run_on_dut):
-    """Stop UI before and Start UI after the context block.
-
-    Context manager will make sure UI is always resumed at the end.
-    """
-    run_on_dut.StopUI()
-    try:
-      yield
-
-    finally:
-      run_on_dut.StartUI()
-
-  def SetupDevice(self, run_on_dut, cros_machine):
-    # Pause UI while configuring the DUT.
-    # This will accelerate setup (waiting for cooldown has x10 drop)
-    # and help to reset a Chrome state left after the previous test.
-    with self.PauseUI(run_on_dut):
-      # Unless the user turns on ASLR in the flag, we first disable ASLR
-      # before running the benchmarks
-      if not self.dut_config['enable_aslr']:
-        run_on_dut.DisableASLR()
-
-      # CPU usage setup comes first where we enable/disable cores.
-      run_on_dut.SetupCpuUsage()
-      cpu_online_status = run_on_dut.GetCpuOnline()
-      # List of online cores of type int (core number).
-      online_cores = [
-          core for core, status in cpu_online_status.items() if status
-      ]
-      if self.dut_config['cooldown_time']:
-        # Setup power conservative mode for effective cool down.
-        # Set ignore status since powersave may no be available
-        # on all platforms and we are going to handle it.
-        ret = run_on_dut.SetCpuGovernor('powersave', ignore_status=True)
-        if ret:
-          # "powersave" is not available, use "ondemand".
-          # Still not a fatal error if it fails.
-          ret = run_on_dut.SetCpuGovernor('ondemand', ignore_status=True)
-        # TODO(denik): Run comparison test for 'powersave' and 'ondemand'
-        # on scarlet and kevin64.
-        # We might have to consider reducing freq manually to the min
-        # if it helps to reduce waiting time.
-        wait_time = run_on_dut.WaitCooldown()
-        cros_machine.AddCooldownWaitTime(wait_time)
-
-      # Setup CPU governor for the benchmark run.
-      # It overwrites the previous governor settings.
-      governor = self.dut_config['governor']
-      # FIXME(denik): Pass online cores to governor setup.
-      run_on_dut.SetCpuGovernor(governor, ignore_status=False)
-
-      # Disable Turbo and Setup CPU freq should ALWAYS proceed governor setup
-      # since governor may change:
-      # - frequency;
-      # - turbo/boost.
-      run_on_dut.DisableTurbo()
-      run_on_dut.SetupCpuFreq(online_cores)
-      # FIXME(denik): Currently we are not recovering the previous cpufreq
-      # settings since we do reboot/setup every time anyway.
-      # But it may change in the future and then we have to recover the
-      # settings.
-
   def Test_That_Run(self, machine, label, benchmark, test_args, profiler_args):
     """Run the test_that test.."""
     options = ''
@@ -267,6 +208,8 @@ class SuiteRunner(object):
           test_args = test_args[1:-1]
       if profiler_args:
         test_args += GetProfilerArgs(profiler_args)
+      if self.dut_config:
+        test_args += GetDutConfigArgs(self.dut_config)
       test_args += ' run_local={} test={}'.format(
           benchmark.run_local,
           benchmark.test_name,
@@ -275,7 +218,7 @@ class SuiteRunner(object):
       if profiler_args:
         self.logger.LogFatal('Client tests do not support profiler.')
     if test_args:
-      options += ' -test-args="%s"' % test_args
+      options += ' -test-args=%s' % pipes.quote(test_args)
 
     dimensions = ''
     for dut in label.remote:
@@ -357,6 +300,7 @@ class SuiteRunner(object):
       autotest_dir_arg = '--autotest_dir %s' % label.autotest_path
 
     profiler_args = GetProfilerArgs(profiler_args)
+    dut_config_args = GetDutConfigArgs(self.dut_config)
     # --fast avoids unnecessary copies of syslogs.
     fast_arg = '--fast'
     args_string = ''
@@ -367,13 +311,13 @@ class SuiteRunner(object):
         test_args = test_args[1:-1]
       args_string = "test_args='%s'" % test_args
 
-    top_interval = self.dut_config['top_interval']
-    turbostat = self.dut_config['turbostat']
-    cmd = ('{} {} {} --board={} --args="{} run_local={} test={} '
-           'turbostat={} top_interval={} {}" {} telemetry_Crosperf'.format(
-               TEST_THAT_PATH, autotest_dir_arg, fast_arg, label.board,
-               args_string, benchmark.run_local, benchmark.test_name, turbostat,
-               top_interval, profiler_args, machine))
+    args = '{} run_local={} test={} {} {}'.format(
+        args_string, benchmark.run_local, benchmark.test_name, dut_config_args,
+        profiler_args)
+
+    cmd = ('{} {} {} --board={} --args={} {} telemetry_Crosperf'.format(
+        TEST_THAT_PATH, autotest_dir_arg, fast_arg, label.board,
+        pipes.quote(args), machine))
 
     # Use --no-ns-pid so that cros_sdk does not create a different
     # process namespace and we can kill process created easily by their
diff --git a/crosperf/suite_runner_unittest.py b/crosperf/suite_runner_unittest.py
index c2e33226..3523f14a 100755
--- a/crosperf/suite_runner_unittest.py
+++ b/crosperf/suite_runner_unittest.py
@@ -57,16 +57,6 @@ class SuiteRunnerTest(unittest.TestCase):
       False,  # rm_chroot_tmp
       'record -e cycles')  # perf_args
 
-  telemetry_bench = Benchmark(
-      'b3_test',  # name
-      'octane',  # test_name
-      '',  # test_args
-      3,  # iterations
-      False,  # rm_chroot_tmp
-      'record -e cycles',  # perf_args
-      'telemetry',  # suite
-      False)  # show_all_results
-
   def __init__(self, *args, **kwargs):
     super(SuiteRunnerTest, self).__init__(*args, **kwargs)
     self.call_test_that_run = False
@@ -129,13 +119,14 @@ class SuiteRunnerTest(unittest.TestCase):
     self.runner.Skylab_Run = FakeSkylabRun
     self.runner.Telemetry_Crosperf_Run = FakeTelemetryCrosperfRun
     self.runner.Test_That_Run = FakeTestThatRun
-    self.runner.SetupDevice = mock.Mock()
+    DutWrapper.SetupDevice = mock.Mock(return_value=0)
     DutWrapper.RunCommandOnDut = mock.Mock(return_value=FakeRunner)
 
     self.runner.dut_config['enable_aslr'] = False
     self.runner.dut_config['cooldown_time'] = 0
     self.runner.dut_config['governor'] = 'fake_governor'
     self.runner.dut_config['cpu_freq_pct'] = 65
+    self.runner.dut_config['intel_pstate'] = 'no_hwp'
     machine = 'fake_machine'
     cros_machine = MockCrosMachine(machine, self.mock_label.chromeos_root,
                                    self.mock_logger)
@@ -144,14 +135,14 @@ class SuiteRunnerTest(unittest.TestCase):
 
     reset()
     self.mock_label.skylab = True
-    self.runner.Run(cros_machine, self.mock_label, self.telemetry_bench,
+    self.runner.Run(cros_machine, self.mock_label, self.test_that_bench,
                     test_args, profiler_args)
     self.assertTrue(self.call_skylab_run)
     self.assertFalse(self.call_test_that_run)
     self.assertFalse(self.call_telemetry_crosperf_run)
     self.assertEqual(self.skylab_run_args,
-                     [self.mock_label, self.telemetry_bench, '', ''])
-    self.runner.SetupDevice.assert_not_called()
+                     [self.mock_label, self.test_that_bench, '', ''])
+    DutWrapper.SetupDevice.assert_called_once()
     self.mock_label.skylab = False
 
     reset()
@@ -172,101 +163,6 @@ class SuiteRunnerTest(unittest.TestCase):
         'fake_machine', self.mock_label, self.telemetry_crosperf_bench, '', ''
     ])
 
-  def test_setup_device(self):
-
-    def FakeRunner(command, ignore_status=False):
-      # pylint fix for unused variable.
-      del command, ignore_status
-      return 0, '', ''
-
-    machine = 'fake_machine'
-    mock_run_on_dut = mock.Mock(spec=DutWrapper)
-    cros_machine = MockCrosMachine(machine, self.mock_label.chromeos_root,
-                                   self.mock_logger)
-
-    mock_run_on_dut.RunCommandOnDut = mock.Mock(return_value=FakeRunner)
-    mock_run_on_dut.WaitCooldown = mock.Mock(return_value=0)
-    mock_run_on_dut.GetCpuOnline = mock.Mock(return_value={0: 1, 1: 1, 2: 0})
-
-    self.runner.dut_config['enable_aslr'] = False
-    self.runner.dut_config['cooldown_time'] = 0
-    self.runner.dut_config['governor'] = 'fake_governor'
-    self.runner.dut_config['cpu_freq_pct'] = 65
-
-    self.runner.SetupDevice(mock_run_on_dut, cros_machine)
-
-    mock_run_on_dut.SetupCpuUsage.assert_called_once_with()
-    mock_run_on_dut.SetupCpuFreq.assert_called_once_with([0, 1])
-    mock_run_on_dut.GetCpuOnline.assert_called_once_with()
-    mock_run_on_dut.SetCpuGovernor.assert_called_once_with(
-        'fake_governor', ignore_status=False)
-    mock_run_on_dut.DisableTurbo.assert_called_once_with()
-    mock_run_on_dut.StopUI.assert_called_once_with()
-    mock_run_on_dut.StartUI.assert_called_once_with()
-    mock_run_on_dut.WaitCooldown.assert_not_called()
-
-  def test_setup_device_with_cooldown(self):
-
-    def FakeRunner(command, ignore_status=False):
-      # pylint fix for unused variable.
-      del command, ignore_status
-      return 0, '', ''
-
-    machine = 'fake_machine'
-    mock_run_on_dut = mock.Mock(spec=DutWrapper)
-    cros_machine = MockCrosMachine(machine, self.mock_label.chromeos_root,
-                                   self.mock_logger)
-
-    mock_run_on_dut.RunCommandOnDut = mock.Mock(return_value=FakeRunner)
-    mock_run_on_dut.WaitCooldown = mock.Mock(return_value=0)
-    mock_run_on_dut.GetCpuOnline = mock.Mock(return_value={0: 0, 1: 1})
-
-    self.runner.dut_config['enable_aslr'] = False
-    self.runner.dut_config['cooldown_time'] = 10
-    self.runner.dut_config['governor'] = 'fake_governor'
-    self.runner.dut_config['cpu_freq_pct'] = 75
-
-    self.runner.SetupDevice(mock_run_on_dut, cros_machine)
-
-    mock_run_on_dut.WaitCooldown.assert_called_once_with()
-    mock_run_on_dut.DisableASLR.assert_called_once()
-    mock_run_on_dut.DisableTurbo.assert_called_once_with()
-    mock_run_on_dut.SetupCpuUsage.assert_called_once_with()
-    mock_run_on_dut.SetupCpuFreq.assert_called_once_with([1])
-    mock_run_on_dut.SetCpuGovernor.assert_called()
-    mock_run_on_dut.GetCpuOnline.assert_called_once_with()
-    mock_run_on_dut.StopUI.assert_called_once_with()
-    mock_run_on_dut.StartUI.assert_called_once_with()
-    self.assertGreater(mock_run_on_dut.SetCpuGovernor.call_count, 1)
-    self.assertEqual(mock_run_on_dut.SetCpuGovernor.call_args,
-                     mock.call('fake_governor', ignore_status=False))
-
-  def test_setup_device_with_exception(self):
-    """Test SetupDevice with an exception."""
-
-    machine = 'fake_machine'
-    mock_run_on_dut = mock.Mock(spec=DutWrapper)
-    cros_machine = MockCrosMachine(machine, self.mock_label.chromeos_root,
-                                   self.mock_logger)
-
-    mock_run_on_dut.SetupCpuUsage = mock.Mock(side_effect=RuntimeError())
-    self.runner.dut_config['enable_aslr'] = False
-
-    with self.assertRaises(RuntimeError):
-      self.runner.SetupDevice(mock_run_on_dut, cros_machine)
-
-    # This called injected an exception.
-    mock_run_on_dut.SetupCpuUsage.assert_called_once_with()
-    # Calls following the expeption are skipped.
-    mock_run_on_dut.WaitCooldown.assert_not_called()
-    mock_run_on_dut.DisableTurbo.assert_not_called()
-    mock_run_on_dut.SetupCpuFreq.assert_not_called()
-    mock_run_on_dut.SetCpuGovernor.assert_not_called()
-    mock_run_on_dut.GetCpuOnline.assert_not_called()
-    # Check that Stop/Start UI are always called.
-    mock_run_on_dut.StopUI.assert_called_once_with()
-    mock_run_on_dut.StartUI.assert_called_once_with()
-
   @mock.patch.object(command_executer.CommandExecuter, 'CrosRunCommand')
   @mock.patch.object(command_executer.CommandExecuter,
                      'ChrootRunCommandWOutput')
@@ -328,13 +224,14 @@ class SuiteRunnerTest(unittest.TestCase):
     args_list = mock_chroot_runcmd.call_args_list[0][0]
     args_dict = mock_chroot_runcmd.call_args_list[0][1]
     self.assertEqual(args_list[0], '/tmp/chromeos')
-    self.assertEqual(args_list[1],
-                     ('/usr/bin/test_that --autotest_dir '
-                      '~/trunk/src/third_party/autotest/files --fast '
-                      '--board=lumpy --args=" run_local=False test=octane '
-                      'turbostat=True top_interval=3 profiler=custom_perf '
-                      'profiler_args=\'record -a -e cycles,instructions\'" '
-                      'lumpy1.cros telemetry_Crosperf'))
+    self.assertEqual(
+        args_list[1],
+        ('/usr/bin/test_that --autotest_dir '
+         '~/trunk/src/third_party/autotest/files --fast '
+         "--board=lumpy --args=' run_local=False test=octane "
+         'dut_config=\'"\'"\'{"turbostat": true, "top_interval": 3}\'"\'"\' '
+         'profiler=custom_perf profiler_args=\'"\'"\'record -a -e '
+         'cycles,instructions\'"\'"\'\' lumpy1.cros telemetry_Crosperf'))
     self.assertEqual(args_dict['cros_sdk_options'],
                      ('--no-ns-pid --chrome_root= '
                       '--chrome_root_mount=/tmp/chrome_root '