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]
Message-ID:
 <CYYPR12MB86559CB690A186BD0F87B7739CCF2@CYYPR12MB8655.namprd12.prod.outlook.com>
Date: Wed, 19 Jun 2024 03:46:38 +0000
From: "Yuan, Perry" <Perry.Yuan@....com>
To: "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>,
	"viresh.kumar@...aro.org" <viresh.kumar@...aro.org>, "Shenoy, Gautham Ranjal"
	<gautham.shenoy@....com>, "Petkov, Borislav" <Borislav.Petkov@....com>,
	"Limonciello, Mario" <Mario.Limonciello@....com>
CC: "viresh.kumar@...aro.org" <viresh.kumar@...aro.org>, "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>, "Yuan, Perry"
	<Perry.Yuan@....com>
Subject: RE: [PATCH v11 3/9] cpufreq: introduce init_boost callback to
 initialize boost state for pstate drivers

[Public]

Hi Rafael

> -----Original Message-----
> From: Yuan, Perry <Perry.Yuan@....com>
> Sent: Friday, June 14, 2024 10:43 AM
> To: Limonciello, Mario <Mario.Limonciello@....com>; Shenoy, Gautham
> Ranjal <gautham.shenoy@....com>; Petkov, Borislav
> <Borislav.Petkov@....com>
> Cc: rafael.j.wysocki@...el.com; viresh.kumar@...aro.org; 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 v11 3/9] cpufreq: introduce init_boost callback to
> initialize boost state for pstate drivers
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> > -----Original Message-----
> > From: Limonciello, Mario <Mario.Limonciello@....com>
> > Sent: Friday, June 14, 2024 1:55 AM
> > To: Yuan, Perry <Perry.Yuan@....com>; Shenoy, Gautham Ranjal
> > <gautham.shenoy@....com>; Petkov, Borislav
> <Borislav.Petkov@....com>
> > Cc: rafael.j.wysocki@...el.com; viresh.kumar@...aro.org; 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 v11 3/9] cpufreq: introduce init_boost callback to
> > initialize boost state for pstate drivers
> >
> > On 6/13/2024 02:25, Perry Yuan wrote:
> > > Introduce a new init_boost callback in cpufreq to initialize the
> > > boost state for specific pstate drivers. This initialization is
> > > required before calling the set_boost interface for each CPU.
> > >
> > > The init_boost callback will set up and synchronize each CPU's
> > > current boost state before invoking the set_boost function. Without
> > > this step, the boost state may be inconsistent across CPUs.
> > >
> > > Signed-off-by: Perry Yuan <perry.yuan@....com>
> > > ---
> > >   drivers/cpufreq/cpufreq.c | 12 ++++++++++--
> > >   include/linux/cpufreq.h   |  2 ++
> > >   2 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index 1fdabb660231..e1a4730f4f8c 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -1429,8 +1429,16 @@ static int cpufreq_online(unsigned int cpu)
> > >                     goto out_free_policy;
> > >             }
> > >
> > > -           /* Let the per-policy boost flag mirror the cpufreq_driver
> > boost during init */
> > > -           policy->boost_enabled = cpufreq_boost_enabled() &&
> > policy_has_boost_freq(policy);
> > > +           /* init boost state to prepare set_boost callback for
> > > + each CPU
> > */
> > > +           if (cpufreq_driver->init_boost) {
> > > +                   ret = cpufreq_driver->init_boost(policy);
> > > +                   if (ret)
> > > +                           pr_debug("%s: %d: initialization boost
> > failed\n", __func__,
> > > +                                   __LINE__);
> >
> > The message should have the subject at the beginning.  IE:
> >
> > "boost initialization failed\n"
> >
> > Also, isn't this fatal if init failed?  IE shouldn't failing have a:
>
>
> Firstly, I also add the " goto out_free_policy", and removed later,  because I
> think it is a little risky to fail the whole online process if driver init boost
> callback failed.
> If driver init boost failed, it just let boost control function lost, but online
> initialize failed and go to free the policy,  it is a big problem.
> I am ok to add the error handling if maintainer agree to see the potential
> online function aborting. 😊
>
> Perry.
>
> >
> >       goto out_free_policy;
> >
> > > +           } else {
> > > +                   /* Let the per-policy boost flag mirror the
> > cpufreq_driver boost during init */
> > > +                   policy->boost_enabled = cpufreq_boost_enabled()
> > && policy_has_boost_freq(policy);
> > > +           }
> > >
> > >             /*
> > >              * The initialization has succeeded and the policy is online.
> > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index
> > > 20f7e98ee8af..0698c0292d8f 100644
> > > --- a/include/linux/cpufreq.h
> > > +++ b/include/linux/cpufreq.h
> > > @@ -409,6 +409,8 @@ struct cpufreq_driver {
> > >     bool            boost_enabled;
> > >     int             (*set_boost)(struct cpufreq_policy *policy, int state);
> > >
> > > +   /* initialize boost state to be consistent before calling set_boost */
> > > +   int             (*init_boost)(struct cpufreq_policy *policy);
> > >     /*
> > >      * Set by drivers that want to register with the energy model after the
> > >      * policy is properly initialized, but before the governor is started.

Could you please help to review this cpufreq change to add init_boost callback for the pstate driver?
We need to add this to initialize boost_supported and boost_state according to SBIOS , then user could change the boost setting as preference.

Perry.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ