mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Zhang Rui <rui.zhang@intel.com>
To: linux-pm@vger.kernel.org, rafael.j.wysocki@intel.com,
	daniel.lezcano@linaro.org
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] powercap/intel_rapl: Fix handling for large time window
Date: Thu,  2 Feb 2023 09:31:40 +0800	[thread overview]
Message-ID: <20230202013140.30416-1-rui.zhang@intel.com> (raw)

When setting the power limit time window, software updates the 'y' bits
and 'f' bits in the power limit register, and the value hardware takes
follows the formula below

	Time window = 2 ^ y * (1 + f / 4) * Time_Unit

When handling large time window input from userspace, using left
shifting breaks in two cases,
1. when ilog2(value) is bigger than 31, in expression "1 << y", left
   shifting by more than 31 bits has undefined behavior. This breaks
   'y'. For example, on an Alderlake platform, "1 << 32" returns 1.
2. when ilog2(value) equals 31, "1 << 31" returns negative value
   because '1' is recognized as signed int. And this breaks 'f'.

Given that 'y' has 5 bits and hardware can never take a value larger
than 31, fix the first problem by clamp the time window to the maximum
possible value that the hardware can take.

Fix the second problem by using unsigned bit left shift.

Note that hardware has its own maximum time window limitation, which
may be lower than the time window value retrieved from the power limit
register. When this happens, hardware clamps the input to its maximum
time window limitation. That is why a software clamp is preferred to
handle the problem on hand.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_common.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index 26d00b1853b4..8b30e5259d3b 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -999,7 +999,12 @@ static u64 rapl_compute_time_window_core(struct rapl_package *rp, u64 value,
 
 		do_div(value, rp->time_unit);
 		y = ilog2(value);
-		f = div64_u64(4 * (value - (1 << y)), 1 << y);
+		if (y > 0x1f) {
+			pr_warn("%s: time window too large, clamped\n", rp->name);
+			return 0x7f;
+		}
+
+		f = div64_u64(4 * (value - (1ULL << y)), 1ULL << y);
 		value = (y & 0x1f) | ((f & 0x3) << 5);
 	}
 	return value;
-- 
2.25.1


             reply	other threads:[~2023-02-02  1:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-02  1:31 Zhang Rui [this message]
2023-02-09 17:06 ` Rafael J. Wysocki
2023-02-11  3:20   ` Zhang, Rui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230202013140.30416-1-rui.zhang@intel.com \
    --to=rui.zhang@intel.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®