Re: [Regression] 6.11.0-rc1: AMD CPU boot with error when CPPC feature disabled by BIOS

[Xiaojian Du]

On 2024/7/31 8:25, David Wang wrote:
> Hi Du,
>
> Thanks for the quick response
>
> At 2024-07-31 01:43:47, "Xiaojian Du" <xiaojidu@xxxxxxx> wrote:
> > Hi  David,
> >
> > CPPC feature is enabled by default in BIOS for new ZENX arch CPU, and
> > amd-pstate driver is enabled in new linux kernel.
> >
> > For your system ,why CPPC is disabled, is it for debug or some special case?
>   I do not have any special purpose, it is just the default BIOS setup for my old system.
>
> > If you want to use legacy acpi cpu driver, you can disable amd-pstate
> > driver module in your linux kernel config file and compile a new kernel.
> >
> > > Those warning message was introduced by commit:
> > >    bff7d13c190ad98cf4f877189b022c75df4cb383 ("cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS)
> > > , which make sense.
> > >
> > > Those error message was introduced by commit:
> > >    8f8b42c1fcc939a73b547b172a9ffcb65ef4bf47 ("cpufreq: amd-pstate: optimize the initial frequency values verification")
> > > , when CPPC is disabled by BIOS, this error message does not make sense, and the error return-code would abort the driver registeration,
> > > but this behavior could be handled earlier when detecting CPPC feature.
> > >
> > > I feel following changes would make a clean fix: do not register amd_pstate driver when CPPC disabled by BIOS.
> > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > > index 68c616b572f2..b06faea58fd4 100644
> > > --- a/drivers/cpufreq/amd-pstate.c
> > > +++ b/drivers/cpufreq/amd-pstate.c
> > > @@ -1837,8 +1837,6 @@ static bool amd_cppc_supported(void)
> > >            * If the CPPC feature is disabled in the BIOS for processors that support MSR-based CPPC,
> > >            * the AMD Pstate driver may not function correctly.
> > >            * Check the CPPC flag and display a warning message if the platform supports CPPC.
> > > -        * Note: below checking code will not abort the driver registeration process because of
> > > -        * the code is added for debugging purposes.
> > As you see, it is for debug purpose, in some corner case, if CPPC
> > feature is disabled, this debug info will help to guide user to
> > *re-enable* it.
> >
> > Target system, including CPU+baseboard+BIOS, is supposed to enable and
> > use CPPC feature for better Performance per Watt.
> I agree that the warning guide makes total sense,  but  the  *errors* does not, why bother trying and report error when the feature is disabled by BIOS.
It has to do that.
AMD strongly recommends users to enable this feature, but each OEM will 
have customized BIOS, some support CPPC but disable by default,
some support CPPC and enable by default, and some disable CPPC directly 
without providing switch option.

Since newer Linux kernel enables the amd-pstate driver by default, and 
newer AMD Zen X CPU supports this feature,
but user find that CPPC is not effective under their software and 
hardware combination,
if this driver does not try to load module and print no information, it 
will mislead users that the problem is caused by the amd-pstate driver.

Thanks,
Xiaojian

> > >            */
> > >           if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
> > >                   if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) {
> > > @@ -1856,6 +1854,7 @@ static bool amd_cppc_supported(void)
> > >           if (warn)
> > >                   pr_warn_once("The CPPC feature is supported but currently disabled by the BIOS.\n"
> > >                                           "Please enable it if your BIOS has the CPPC option.\n");
> > > +               return false;
> > Maybe need a pair of curly brace after this "if(warn)" for your change.
> > >           return true;
> > >    }
> > >
> > >
> > >
> > > Thanks
> > > David