linux-kernel - Re: [RFC PATCH] PM / OPP: Don't support OPP if it provides supported-hw but platform does not

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57E58BF8.5050506@ti.com>
Date:   Fri, 23 Sep 2016 15:09:28 -0500
From:   Dave Gerlach <d-gerlach@...com>
To:     <linux-kernel@...r.kernel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>
CC:     <linux-pm@...r.kernel.org>, Nishanth Menon <nm@...com>,
        Lee Jones <lee.jones@...aro.org>
Subject: Re: [RFC PATCH] PM / OPP: Don't support OPP if it provides
 supported-hw but platform does not

Hi,
On 09/23/2016 03:07 PM, Dave Gerlach wrote:
> The OPP framework allows each OPP to set a opp-supported-hw property
> which provides values that are matched against supported_hw values
> provided by the platform to limit support for certain OPPs on specific
> hardware. Currently, if the platform does not set supported_hw values,
> all OPPs are interpreted as supported, even if they have provided their
> own opp-supported-hw values.
>
> If an OPP has provided opp-supported-hw, it is indicating that there is
> some specific hardware configuration it is supported by. These constraints
> should be honored, and if no supported_hw has been provided by the
> platform, there is no way to determine if that OPP is actually supported,
> so it should be marked as not supported.
>
> Signed-off-by: Dave Gerlach <d-gerlach@...com>
> ---

Currently only the sti-cpufreq and forthcoming ti-cpufreq [1] driver are 
making use of dev_pm_opp_set_supported_hw so maybe nobody has seen this 
yet or the framework was designed to work as it does.

I would think that if an OPP provides a set of constraints that define 
when it is supported and we can't tell if we can meet those, we should 
disable an OPP rather than enable it. Otherwise, what was the point of 
providing constraints?

Regards,
Dave

[1] http://www.spinics.net/lists/arm-kernel/msg527921.html

>   drivers/base/power/opp/of.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
> index 1dfd3dd92624..ccccaf9f8968 100644
> --- a/drivers/base/power/opp/of.c
> +++ b/drivers/base/power/opp/of.c
> @@ -71,8 +71,18 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
>   	u32 version;
>   	int ret;
>
> -	if (!opp_table->supported_hw)
> -		return true;
> +	if (!opp_table->supported_hw) {
> +		/*
> +		 * In the case that no supported_hw has been set by the
> +		 * platform but there is an opp-supported-hw value set for
> +		 * an OPP then the OPP should not be enabled as there is
> +		 * no way to see if the hardware supports it.
> +		 */
> +		if (of_find_property(np, "opp-supported-hw", NULL))
> +			return false;
> +		else
> +			return true;
> +	}
>
>   	while (count--) {
>   		ret = of_property_read_u32_index(np, "opp-supported-hw", count,
>