linux-kernel - Re: [PATCH][RFC] ACPI: add "processor.broadcast_ppc" hook to broadcast

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190214070648.pvsavc46iivub4hy@vireshk-i7>
Date:   Thu, 14 Feb 2019 12:36:48 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Yu Chen <yu.c.chen@...el.com>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        linux-acpi@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH][RFC] ACPI: add "processor.broadcast_ppc" hook to
 broadcast  _PPC to all online CPUs

On 14-02-19, 00:55, Yu Chen wrote:
> Hi Viresh,
> On Mon, Feb 11, 2019 at 04:03:07PM +0530, Viresh Kumar wrote:
> > On 09-02-19, 20:02, Chen Yu wrote:
> > > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > > the power source changes), the maximum frequency of the
> > > CPU is not updated accordingly. This is due to the policy's
> > > cpuinfo.max is not updated when _PPC notifier fires.
> > > 
> > > Fix this problem by updating the policy's cpuinfo.max
> > > and broadcast the _PPC notifier to all online CPUs.
> > > 
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@...il.com>
> > > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> > > ---
> > >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> > >  drivers/cpufreq/cpufreq.c        |  2 ++
> > >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > > index a303fd0e108c..737dbf5aa7f7 100644
> > > --- a/drivers/acpi/processor_perflib.c
> > > +++ b/drivers/acpi/processor_perflib.c
> > > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> > >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> > >  		 "limited by BIOS, this should help");
> > >  
> > > +static int broadcast_ppc;
> > > +module_param(broadcast_ppc, int, 0644);
> > > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > > +
> > >  #define PPC_REGISTERED   1
> > >  #define PPC_IN_USE       2
> > >  
> > > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> > >  		else
> > >  			acpi_processor_ppc_ost(pr->handle, 0);
> > >  	}
> > > -	if (ret >= 0)
> > > -		cpufreq_update_policy(pr->id);
> > > +	if (ret >= 0) {
> > > +		if (broadcast_ppc) {
> > > +			int cpu;
> > > +
> > > +			for_each_possible_cpu(cpu)
> > > +				cpufreq_update_policy(cpu);
> > > +		} else {
> > > +			cpufreq_update_policy(pr->id);
> > > +		}
> > > +	}
> > >  }
> > >  
> > >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index e35a886e00bc..95e08816b512 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> > >  
> > >  	policy->min = new_policy->min;
> > >  	policy->max = new_policy->max;
> > > +	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > > +	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> > >  	trace_cpu_frequency_limits(policy);
> > >  
> > >  	policy->cached_target_freq = UINT_MAX;
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index dd66decf2087..e1881313c396 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> > >  
> > >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> > >  {
> > > +	int max_freq;
> > >  	struct cpudata *cpu = all_cpu_data[policy->cpu];
> > >  
> > >  	update_turbo_state();
> > > +	max_freq = intel_pstate_get_max_freq(cpu);
> > > +
> > > +	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > 
> > Why do have this check for policy->max here ?
> >
> Thanks for looking at this change, I've replied to another email in detail of
> the scenario that, this is due to corner case that if the system boots
> with battery and plug the AC after boot up, the cpufreq max limit will not
> increase even the turbo has been enabled after the AC plugged.

Yeah, but I asked a different question I believe, why is this
comparison necessary ?

policy->max == policy->cpuinfo.max_freq

-- 
viresh