> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Sent: Wednesday, October 26, 2022 1:07 AM
> To: Mi, Dapeng1 <dapeng1.mi@xxxxxxxxx>
> Cc: pbonzini@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx;
> dave.hansen@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; zhenyuw@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] KVM: x86: disable halt polling when powersave
> governor is used
>
> On Sat, Oct 08, 2022, Mi, Dapeng1 wrote:
> > > > + !strncmp(policy->governor->name,
> > > "powersave",
> > >
> > > KVM should not be comparing magic strings. If the cpufreq subsystem
> > > can't get
> > > policy->policy right, then that needs to be fixed.
> >
> > Yeah, using magic strings looks a little bit strange, but this is what
> > is cpufreq doing. Currently cpufreq mechanism supports two kinds of
> > drivers, one is the driver which has the built-in governor, like intel_pstate
> driver.
> > For this kind of driver, the cpufreq governor is saved in the
> > policy->policy field. The other is the traditional driver which is
> > independent with cpufreq governor and the cpufreq governor type is
> saved in the governor->name field.
> > For the second kind of cpufreq driver, the policy->policy field is
> > meaningless and we have to read the governor name.
>
> That doesn't mean it's ok to bleed those internal details into KVM. I would
> much rather cpufreq provide a helper to get the effective policy, e.g.
>
> unsigned int cpufreq_cpu_get_policy(unsigned int cpu)
> {
> struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> unsigned int pol;
>
> if (!policy)
> return CPUFREQ_POLICY_UNKNOWN;
>
> pol = policy->policy
> if (pol == CPUFREQ_POLICY_UNKNOWN && policy->governor)
> pol = cpufreq_parse_policy(policy->governor->name);
>
> cpufreq_cpu_put(policy);
> }
Thanks Sean for reviewing. Would do in next version.
