linux-kernel - RE: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default

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: Tue, 14 May 2024 04:54:31 +0000
From: "Yuan, Perry" <Perry.Yuan@....com>
To: "Ugwekar, Dhananjay" <Dhananjay.Ugwekar@....com>, "Limonciello, Mario"
	<Mario.Limonciello@....com>, "Shenoy, Gautham Ranjal"
	<gautham.shenoy@....com>, "Huang, Ray" <Ray.Huang@....com>, "Petkov,
 Borislav" <Borislav.Petkov@....com>
CC: "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>, "Deucher,
 Alexander" <Alexander.Deucher@....com>, "Huang, Shimmer"
	<Shimmer.Huang@....com>, "Du, Xiaojian" <Xiaojian.Du@....com>, "Meng, Li
 (Jassmine)" <Li.Meng@....com>, "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate
 driver by default

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Dhananjay

> -----Original Message-----
> From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@....com>
> Sent: Monday, May 13, 2024 5:37 PM
> To: Yuan, Perry <Perry.Yuan@....com>; Limonciello, Mario
> <Mario.Limonciello@....com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@....com>; Huang, Ray <Ray.Huang@....com>; Petkov,
> Borislav <Borislav.Petkov@....com>
> Cc: rafael.j.wysocki@...el.com; Deucher, Alexander
> <Alexander.Deucher@....com>; Huang, Shimmer
> <Shimmer.Huang@....com>; Du, Xiaojian <Xiaojian.Du@....com>; Meng,
> Li (Jassmine) <Li.Meng@....com>; linux-pm@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Subject: Re: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate
> driver by default
>
> Hello Perry,
>
> On 5/13/2024 7:37 AM, Perry Yuan wrote:
> > If the `amd-pstate` driver is not loaded automatically by default, it
> > is because the kernel command line parameter has not been added.
> > To resolve this issue, it is necessary to call the
> > `amd_pstate_set_driver()` function to enable the desired mode
> > (passive/active/guided) before registering the driver instance.
> > This ensures that the driver is loaded correctly without relying on
> > the kernel command line parameter.
> >
> > [    0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-
> v1 xhci-hcd
> > [    0.982579] amd_pstate: failed to register with return -22
> >
> > Reported-by: Andrei Amuraritei <andamu@...teo.net>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705
> > Signed-off-by: Perry Yuan <perry.yuan@....com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 36
> > +++++++++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index dce901a403c9..03342fef7d94
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1785,28 +1785,30 @@ static int __init amd_pstate_init(void)
> >     /* check if this machine need CPPC quirks */
> >     dmi_check_system(amd_pstate_quirks_table);
> >
> > +   /* get default driver mode for loading */
> > +   if (cppc_state == AMD_PSTATE_UNDEFINED)
> > +           cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE;
> > +
> > +   /* Disable on the following configs by default:
> > +    * 1. Undefined platforms
> > +    * 2. Server platforms
> > +    */
> > +   if (amd_pstate_acpi_pm_profile_undefined() ||
> > +           amd_pstate_acpi_pm_profile_server()) {
> > +           pr_info("driver load is disabled for server or undefined
> platform\n");
> > +           return -ENODEV;
> > +   }
> > +
>
> Wont this change make it impossible to use amd-pstate on server platforms? It
> wont work even if we pass "amd-pstate=active" in cmdline, because this check
> is before the switch case.
>
> Thanks,
> Dhananjay
>

Good catch,  the original checking is disabling server platform by default, it should not block the server platform when using "amd-pstate=active" in cmdline,  will fix it in v3 to allow parameter input for server platfrm from command line.


Regards.
Perry

> >     switch (cppc_state) {
> > -   case AMD_PSTATE_UNDEFINED:
> > -           /* Disable on the following configs by default:
> > -            * 1. Undefined platforms
> > -            * 2. Server platforms
> > -            * 3. Shared memory designs
> > -            */
> > -           if (amd_pstate_acpi_pm_profile_undefined() ||
> > -               amd_pstate_acpi_pm_profile_server() ||
> > -               !cpu_feature_enabled(X86_FEATURE_CPPC)) {
> > -                   pr_info("driver load is disabled, boot with specific
> mode to enable this\n");
> > -                   return -ENODEV;
> > -           }
> > -           ret =
> amd_pstate_set_driver(CONFIG_X86_AMD_PSTATE_DEFAULT_MODE);
> > -           if (ret)
> > -                   return ret;
> > -           break;
> >     case AMD_PSTATE_DISABLE:
> > +           pr_info("driver load is disabled, boot with specific mode to
> enable
> > +this\n");
> >             return -ENODEV;
> >     case AMD_PSTATE_PASSIVE:
> >     case AMD_PSTATE_ACTIVE:
> >     case AMD_PSTATE_GUIDED:
> > +           ret = amd_pstate_set_driver(cppc_state);
> > +           if (ret)
> > +                   return ret;
> >             break;
> >     default:
> >             return -EINVAL;
> > @@ -1827,7 +1829,7 @@ static int __init amd_pstate_init(void)
> >     /* enable amd pstate feature */
> >     ret = amd_pstate_enable(true);
> >     if (ret) {
> > -           pr_err("failed to enable with return %d\n", ret);
> > +           pr_err("failed to enable driver mode(%d)\n", cppc_state);
> >             return ret;
> >     }
> >