linux-kernel - RE: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

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] [day] [month] [year] [list]

Date:   Fri, 8 Jul 2022 11:48:50 +0000
From:   "Yuan, Perry" <Perry.Yuan@....com>
To:     "Fontenot, Nathan" <Nathan.Fontenot@....com>,
        "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>,
        "viresh.kumar@...aro.org" <viresh.kumar@...aro.org>,
        "Huang, Ray" <Ray.Huang@....com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
CC:     "Sharma, Deepak" <Deepak.Sharma@....com>,
        "Limonciello, Mario" <Mario.Limonciello@....com>,
        "Deucher, Alexander" <Alexander.Deucher@....com>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@....com>,
        "Huang, Shimmer" <Shimmer.Huang@....com>,
        "Du, Xiaojian" <Xiaojian.Du@....com>,
        "Meng, Li (Jassmine)" <Li.Meng@....com>
Subject: RE: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

[AMD Official Use Only - General]

Hi Nathan.

> -----Original Message-----
> From: Fontenot, Nathan <Nathan.Fontenot@....com>
> Sent: Friday, July 8, 2022 3:46 AM
> To: Yuan, Perry <Perry.Yuan@....com>; rafael.j.wysocki@...el.com;
> viresh.kumar@...aro.org; Huang, Ray <Ray.Huang@....com>; Rafael J.
> Wysocki <rafael@...nel.org>; Len Brown <lenb@...nel.org>; linux-
> acpi@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> pm@...r.kernel.org
> Cc: Sharma, Deepak <Deepak.Sharma@....com>; Limonciello, Mario
> <Mario.Limonciello@....com>; Fontenot, Nathan
> <Nathan.Fontenot@....com>; Deucher, Alexander
> <Alexander.Deucher@....com>; Su, Jinzhou (Joe) <Jinzhou.Su@....com>;
> Huang, Shimmer <Shimmer.Huang@....com>; Du, Xiaojian
> <Xiaojian.Du@....com>; Meng, Li (Jassmine) <Li.Meng@....com>
> Subject: Re: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check
> 
> On 7/7/22 12:01, Perry Yuan wrote:
> > Add acpi function check in case ACPI is not enabled, that will cause
> > pstate driver failed to call cppc acpi to change perf or update epp
> > value for shared memory solution processors.
> >
> > When CPPC is invalid, warning log will be needed to be printed to tell
> > user what is wrong.
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@....com>
> > ---
> >  drivers/acpi/cppc_acpi.c     | 3 +++
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index
> > 6ff1901d7d43..17d67e3ededf 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
> >  	struct cpc_desc *cpc_ptr;
> >  	int cpu;
> >
> > +	if (acpi_disabled)
> > +		return false> +
> 
> This seems ok, the only other places I find that call acpi_cpc_valid() also
> check for acpi_disabled.
> 
> If the acpi_disabled check is added to acpi_cpc_valid() the other calling sites
> should be updated to remove their check for acpi_disabled.
> 
> -Nathan

You are correct. It needs to remove the same check code from other driver file.
Will add this feedback into V2 patch. 

Perry. 

> 
> >  	for_each_present_cpu(cpu) {
> >  		cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
> >  		if (!cpc_ptr)
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index b54b3b559993..6d81a9a94dde
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -684,7 +684,7 @@ static int __init amd_pstate_init(void)
> >  		return -ENODEV;
> >
> >  	if (!acpi_cpc_valid()) {
> > -		pr_debug("the _CPC object is not present in SBIOS\n");
> > +		pr_warn_once("the _CPC object is not present in SBIOS or
> ACPI
> > +disabled\n");
> >  		return -ENODEV;
> >  	}
> >