[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5673FADC.4030405@redhat.com>
Date: Fri, 18 Dec 2015 07:23:56 -0500
From: Prarit Bhargava <prarit@...hat.com>
To: linux-kernel@...r.kernel.org,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
CC: Seiichi Ikarashi <s.ikarashi@...fujitsu.com>,
Radivoje Jovanovic <radivoje.jovanovic@...el.com>,
Mathias Krause <minipli@...glemail.com>,
Ajay Thomas <ajay.thomas.david.rajamanickam@...el.com>
Subject: Re: [PATCH 3/3] powercap, intel_rapl, Add ignore_max_window_check
module parameter for broken BIOSes
On 12/18/2015 12:50 AM, Seiichi Ikarashi wrote:
> On 2015-12-15 22:02, Prarit Bhargava wrote:
>> Some systems erroneously set the maximum time window field of
>> MSR_PKG_POWER_INFO register to 0. This results in a user not being able
>> to set the time windows for the package. In some cases, however, RAPL
>> will still continue to work with a small window (albeit through some
>> trial and error). This patch adds a ignore_max_window_check module
>> parameter to avoid the maximum time window check in set_time_window().
>>
>> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
>> Cc: Prarit Bhargava <prarit@...hat.com>
>> Cc: Radivoje Jovanovic <radivoje.jovanovic@...el.com>
>> Cc: Seiichi Ikarashi <s.ikarashi@...fujitsu.com>
>> Cc: Mathias Krause <minipli@...glemail.com>
>> Cc: Ajay Thomas <ajay.thomas.david.rajamanickam@...el.com>
>> Signed-off-by: Prarit Bhargava <prarit@...hat.com>
>> ---
>> drivers/powercap/intel_rapl.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
>> index 14753e5..3cdb8ee 100644
>> --- a/drivers/powercap/intel_rapl.c
>> +++ b/drivers/powercap/intel_rapl.c
>> @@ -508,10 +508,22 @@ static int get_max_time_window(struct powercap_zone *power_zone, int id,
>> else
>> *data = val;
>>
>> + if (val == 0)
>
> If rapl_read_data_raw() fails, "val" becomes indefinite.
> So this check and warn should be performed only if rapl_read_data_raw() succeeds.
>
>> + pr_warn_once(FW_BUG "intel_rapl: Maximum Time Window is zero. This is a BIOS bug that should be reported to your hardware or BIOS vendor. The value of zero may prevent Intel RAPL from functioning properly. Most bugs can be avoided by setting the ignore_max_window_check module parameter.\n");
>> +
>> put_online_cpus();
>> return ret;
>> }
>>
>> +/* Some BIOSes incorrectly program the maximum time window in the
>> + * MSR_PKG_POWER_INFO register. Some of these systems still have functional
>> + * RAPL registers, etc., so give the user the option of disabling the maximum
>> + * time window check.
>> + */
>> +static int ignore_max_window_check;
>> +module_param(ignore_max_window_check, int, 0444);
>> +MODULE_PARM_DESC(ignore_max_window_check, "Ignore maximum window check. A bug should be reported to your hardware or BIOS vendor if this parameter is used.");
>
> Don't you need to use "time_window" instead of just "window" in these names?
Will submit a [v2] to cover the val error condition and s/time_window/window in
the module parameter description.
Rafael, I cannot remember ... will you take a 3/3 v2 or do you want me to repost
the whole thing as a v2?
P.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists