On Mon, Aug 15, 2022 at 10:34 AM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 10.08.2022 21:29, Jason Andryuk wrote:
> > Export feature_detect as intel_feature_detect so it can be re-used by
> > HWP.
> >
> > Signed-off-by: Jason Andryuk <jandr...@gmail.com>
> > ---
> > v2
> > export intel_feature_detect with typed pointer
> > Move intel_feature_detect to acpi/cpufreq/cpufreq.h since the
> > declaration now contains struct cpufreq_policy *.
>
> I don't mind the new placement, but I don't follow the reasoning.
v1 added
void intel_feature_detect(void *info);
to acpi/cpufreq/processor_perf.h
v2 adds
void intel_feature_detect(struct cpufreq_policy *policy)
to acpi/cpufreq/cpufreq.h, which was selected to have the type available.
> > --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> > +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> > @@ -340,9 +340,8 @@ static unsigned int cf_check
> > get_cur_freq_on_cpu(unsigned int cpu)
> > return extract_freq(get_cur_val(cpumask_of(cpu)), data);
> > }
> >
> > -static void cf_check feature_detect(void *info)
> > +void intel_feature_detect(struct cpufreq_policy *policy)
> > {
> > - struct cpufreq_policy *policy = info;
> > unsigned int eax;
> >
> > eax = cpuid_eax(6);
> > @@ -354,6 +353,11 @@ static void cf_check feature_detect(void *info)
> > }
> > }
> >
> > +static void cf_check feature_detect(void *info)
>
> This function is invoked indirectly via on_selected_cpus() (hence
> the cf_check attribute) - I wonder how you get away without that for
> HWP. Or else why we need this as a wrapper here when then you'd have
> another similar wrapper elsewhere.
HWP calls hwp_init_msrs via on_selected_cpus, which then directly
calls intel_feature_detect(). ACPI needs the cf_check on
feature_detect, but intel_feature_detect is only called directly.
> > +{
> > + intel_feature_detect((struct cpufreq_policy *)info);
>
> Why the cast?
I thought it was necessary to keep the compiler happy. Double
checking, you are correct that it is not needed.
Thanks,
Jason
