Using an small value for the time window results in a
bogus value for the time window.  For example,

[root@intel-chiefriver-03 linux]# echo 950 >
/sys/devices/virtual/powercap/intel-rapl/intel-rapl\:0/constraint_0_time_window_us;
egrep ^ 
/sys/devices/virtual/powercap/intel-rapl/intel-rapl\:0/constraint_0_time_window_us
-bash: echo: write error: Invalid argument
/sys/devices/virtual/powercap/intel-rapl/intel-rapl:0/constraint_0_time_window_us:1:4501502475370496

The Intel ASDM doesn't explicitly define a minimum time window.
The MSR_RAPL_POWER_UNIT register, read during initialization, does
specify a minimum time window unit so that can be used as a lower
bound for error checking.

After this change the minimum time window is properly clamped:

[root@intel-chiefriver-03 linux]# echo 950 >
/sys/devices/virtual/powercap/intel-rapl/intel-rapl\:0/constraint_0_time_window_us;
egrep ^ 
/sys/devices/virtual/powercap/intel-rapl/intel-rapl\:0/constraint_0_time_window_us
-bash: echo: write error: Invalid argument
/sys/devices/virtual/powercap/intel-rapl/intel-rapl:0/constraint_0_time_window_us:1:976

Cc: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Radivoje Jovanovic <radivoje.jovano...@intel.com>
Cc: Seiichi Ikarashi <s.ikara...@jp.fujitsu.com>
Cc: Mathias Krause <mini...@googlemail.com>
Cc: Ajay Thomas <ajay.thomas.david.rajamanic...@intel.com>
Signed-off-by: Prarit Bhargava <pra...@redhat.com>
---
 drivers/powercap/intel_rapl.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index feb063d..cf89b3d 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -516,6 +516,7 @@ static int set_time_window(struct powercap_zone 
*power_zone, int id,
                                                                u64 window)
 {
        struct rapl_domain *rd;
+       struct rapl_package *rp;
        int ret = 0;
        u64 max_window;
 
@@ -524,12 +525,18 @@ static int set_time_window(struct powercap_zone 
*power_zone, int id,
        if (ret < 0)
                goto out;
 
-       if (window > max_window) {
+       rd = power_zone_to_rapl_domain(power_zone);
+       rp = find_package_by_id(rd->package_id);
+       /*
+        * The Intel ASDM doesn't explicitly define a minimum time window.
+        * The MSR_RAPL_POWER_UNIT register, read during initialization,
+        * does contain the smallest unit of time that can be measured.
+        */
+       if ((window > max_window) || (window < rp->time_unit)) {
                ret = -EINVAL;
                goto out;
        }
 
-       rd = power_zone_to_rapl_domain(power_zone);
        switch (rd->rpl[id].prim_id) {
        case PL1_ENABLE:
                rapl_write_data_raw(rd, TIME_WINDOW1, window);
-- 
1.7.9.3

Reply via email to