linux-kernel - RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

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]

Date:   Thu, 24 May 2018 17:10:32 +0300
From:   <ilialin@...eaurora.org>
To:     "'Sudeep Holla'" <sudeep.holla@....com>
Cc:     <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <vireshk@...nel.org>, <nm@...com>,
        <sboyd@...nel.org>, <robh@...nel.org>, <mark.rutland@....com>,
        <rjw@...ysocki.net>
Subject: RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

Thank you for the explanation. However, could you suggest, which condition should I check then? Device tree?

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@....com>
> Sent: Thursday, May 24, 2018 17:01
> To: ilialin@...eaurora.org; vireshk@...nel.org; nm@...com;
> sboyd@...nel.org; robh@...nel.org; mark.rutland@....com;
> rjw@...ysocki.net
> Cc: Sudeep Holla <sudeep.holla@....com>; linux-pm@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 14:03, ilialin@...eaurora.org wrote:
> >
> >
> 
> [...]
> 
> >>> +
> >>> +	ret = PTR_ERR_OR_ZERO(platform_device_register_simple(
> >>> +		"qcom-cpufreq-kryo", -1, NULL, 0));
> >>
> >>
> >> You simply can't do this unconditionally here. This will blow up on
> >> platforms where this driver is not supposed to work. The probe will
> >> be called on non- QCOM or non-Kryo QCOM platforms and I reckon it
> >> will crash trying to execute something in qcom_smem_get.
> >
> > What do you mean by 'unconditionally'?
> 
> Why should you even add/register a device "qcom-cpufreq-kryo" on other
> platforms. Drivers can get registered, but only devices that are present or
> required by the platform need to be registered.
> 
> > The driver depends on the smem and nvmem drivers, which depend on
> ARCH_QCOM:
> >  +	depends on QCOM_QFPROM
> >  +	depends on QCOM_SMEM
> >
> 
> Sure, but we have something called single image for all ARM64 platforms.
> May be QCOM still used to tweeking config to build binary for your particular
> mobile platform but the distro kernel need single binary to work on all
> platforms. We have moved far away from platform specific builds long back
> now IIRC.
> 
> > And if SMEM read in the probe returns something other than Kryo, it will
> exit.
> >
> 
> Check what this driver does ?
> 
> 	msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY,
> MSM_ID_SMEM, &len);
> 	msm_id++;
> 	switch ((enum _msm_id)*msm_id)
> 
> I think it *will and should* crash here ? You need to check the return value
> for sure. But since qcom_smem_get return -EPROBE_DEFER, we keep
> retrying even on non QCOM platforms which is something I would like to
> avoid.
> 
> Therefore that's not the main concern. Why do I have to see "qcom-cpufreq-
> kryo" device registered on my non QCOM platform ?
> 
> --
> Regards,
> Sudeep