linux-kernel - Re: [RFC PATCH] cpufreq: dt-platdev: Fix module autoloading

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: <1c5e13b7472917b5fa303553da04ae16590f3105.camel@redhat.com>
Date: Fri, 22 Nov 2024 11:16:46 -0500
From: Radu Rendec <rrendec@...hat.com>
To: Javier Martinez Canillas <javierm@...hat.com>, Viresh Kumar
	 <viresh.kumar@...aro.org>
Cc: robh@...nel.org, arnd@...aro.org, linux-kernel@...r.kernel.org, Zhipeng
 Wang <zhipeng.wang_1@....com>, Maxime Ripard <mripard@...nel.org>, 
 javier@...hile0.org, "Rafael J. Wysocki" <rafael@...nel.org>, 
 linux-pm@...r.kernel.org
Subject: Re: [RFC PATCH] cpufreq: dt-platdev: Fix module autoloading

On Thu, 2024-11-21 at 10:13 +0100, Javier Martinez Canillas wrote:
> Viresh Kumar <viresh.kumar@...aro.org> writes:
> 
> > On 21-11-24, 09:52, Javier Martinez Canillas wrote:
> > > Will autload the driver for any platform that has a Device Tree node with a
> > > compatible = "operating-points-v2" (assuming that this node will be a phandle
> > > for the "operating-points-v2" property.
> > > 
> > > For example, in the case of the following DT snippet:
> > > 
> > > cpus {
> > >         cpu@0 {
> > >                 operating-points-v2     = <&cpu0_opp_table>;
> > >         };
> > > };
> > > 
> > > cpu0_opp_table: opp_table {
> > >         compatible = "operating-points-v2";
> > > ...
> > > };
> > > 
> > > It will autoload if OF finds the opp_table node, but it register the cpufreq-dt
> > > device only if there's a cpu@0 with a "operating-points-v2" property.
> > > 
> > > Yes, there may be false positives because the autload semantics don't exactly
> > > match the criteria for the driver to "match" but I believe is better to load it
> > > and not use for those cases, than needing the driver and not autoloading it.
> > > 
> > > > I am not sure what's the best way forward to fix this.
> > > > 
> > > 
> > > I couldn't find another way to solve it, if you have a better idea please let
> > > me know. But IMO we should either workaround like this or revert the commit 
> > > that changed the driver's Kconfig symbol to be tristate.
> > 
> > Yeah, this needs to be fixed and this patch is one of the ways. Lets see if Arnd
> > or Rob have something to add, else can apply this patch.
> > 
> 
> Ok. Please notice though that this is an RFC, since all my arm64 machines have
> their own CPUFreq driver and are not using cpufreq-dt-platdev. So I would not
> apply it until someone actually tested the patch.

I tested the patch on a Renesas R-Car S4 Spider (r8a779f0-spider.dts)
board, and it didn't work. I think the problem is that the OPP table DT
node does not have a corresponding device instance that is registered,
and therefore no modalias uevent is reported to udev/kmod.

FWIW, the OPP table is defined at the top of r8a779f0.dtsi and
referenced just a few more lines below, where the CPU nodes are
defined.

As far as I understand, there are two options to fix this:
   1. Revert the patch that allows the cpufreq-dt-platdev driver to be
      built as a module. There's little benefit in allowing that anyway
      because the overhead at init time is minimal when the driver is
      unused, and driver can't be unloaded.
   2. Modify the driver and create an explicit of_device_id table of
      supported platforms for v2 too (like the existing one for v1) and
      use that instead of the cpu0_node_has_opp_v2_prop() call and the
      blacklist. That would of course also eliminate the blacklist.

--
Best regards,
Radu Rendec