RE: [PATCH] KVM: x86: disable halt polling when powersave governor is used

[Mi, Dapeng1]

> 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.