Hi,
Thanks for the report. ->
On Tuesday, March 05, 2013 01:39:19 PM Konrad Rzeszutek Wilk wrote:
> In all fairness, the commit d5aaffa9dd531c978c6f3fea06a2972653bd7fc8
> (cpufreq: handle cpufreq being disabled for all exported function.) is not
> at fault - it just that it exposes an assumption that before v3.9-rc1
> was not true. And git bisection points to it :-(
>
> The problem I am hitting is that the module xen-acpi-processor which
> uses the ACPI's functions: acpi_processor_register_performance,
> acpi_processor_preregister_performance, and acpi_processor_notify_smm
> fails at acpi_processor_register_performance with -22.
>
> Note that earlier during bootup in arch/x86/xen/setup.c there is also
> an call to cpufreq's API: disable_cpufreq().
>
> This is b/c we want the Linux kernel to parse the ACPI data, but leave
> the cpufreq decisions to the hypervisor.
>
> In v3.9 all the checks that d5aaffa9dd531c978c6f3fea06a2972653bd7fc8
> added are now hit and the calls to cpufreq_register_notifier will now
> fail. This means that acpi_processor_ppc_init ends up printing:
>
> "Warning: Processor Platform Limit not supported"
>
> and the acpi_processor_ppc_status is not set.
>
> The repercussions of that is that the call to
> acpi_processor_register_performance fails right away at:
>
> if (!(acpi_processor_ppc_status & PPC_REGISTERED))
>
> and we don't progress any further on parsing and extracting the _P*
> objects.
>
>
> I am not really sure how to solve this. One thought I had was to write
> a quick and dirty nop-cpufreq driver, but then I run in the problems
> of having it being installed all the others and also to make sure it
> is the one by default when booting under Xen. I think I explored that
> idea a year ago and Dave Jones at that point suggested to just bypass
> cpufreq API altogether and just use the ACPI API by itself. That is
> where the disable_cpufreq() came from.
>
> The other idea would be to make acpi_processor_get_performance_info
> be exported and not use acpi_processor_register_performance, like so:
>
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 7672c37..9aecad2 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -472,7 +472,7 @@ static int acpi_processor_get_performance_states(struct acpi_processor *pr)
> return result;
> }
>
> -static int acpi_processor_get_performance_info(struct acpi_processor *pr)
> +int acpi_processor_get_performance_info(struct acpi_processor *pr)
> {
> int result = 0;
> acpi_status status = AE_OK;
> @@ -524,7 +524,7 @@ static int acpi_processor_get_performance_info(struct acpi_processor *pr)
> pr_info("%s:%d: RC:%d\n", __func__, __LINE__, result);
> return result;
> }
> -
> +EXPORT_SYMBOL_GPL(acpi_processor_get_performance_info);
> int acpi_processor_notify_smm(struct module *calling_module)
> {
> acpi_status status;
> diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
> index f4b7270..8c85d33 100644
> --- a/drivers/xen/xen-acpi-processor.c
> +++ b/drivers/xen/xen-acpi-processor.c
> @@ -502,18 +502,18 @@ static int __init xen_acpi_processor_init(void)
> pr_debug(DRV_NAME "pre-register: %d\n", rc);
>
> for_each_possible_cpu(i) {
> + struct acpi_processor *pr;
> struct acpi_processor_performance *perf;
>
> + pr = per_cpu(processors, i);
> perf = per_cpu_ptr(acpi_perf_data, i);
> - rc = acpi_processor_register_performance(perf, i);
> + pr->performance = perf;
> + rc = acpi_processor_get_performance_info(pr);
> if (rc) {
> pr_debug(DRV_NAME "register_perf: %d, got %d\n", i, rc);
> goto err_out;
> }
> }
> - rc = acpi_processor_notify_smm(THIS_MODULE);
> - if (rc)
> - goto err_unregister;
>
> for_each_possible_cpu(i) {
> struct acpi_processor *_pr;
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 555d033..b327b5a 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -235,6 +235,9 @@ extern void acpi_processor_unregister_performance(struct
> if a _PPC object exists, rmmod is disallowed then */
> int acpi_processor_notify_smm(struct module *calling_module);
>
> +/* parsing the _P* objects. */
> +extern int acpi_processor_get_performance_info(struct acpi_processor *pr);
> +
> /* for communication between multiple parts of the processor kernel module */
> DECLARE_PER_CPU(struct acpi_processor *, processors);
> extern struct acpi_processor_errata errata;
>
>
> (which works BTW).
>
> The third option is to restrict the usage of acpi_processor_ppc_status or
> export the acpi_processor_ppc_status. But that sounds hacky to me.
>
> Thoughts?
Well, since your patch above seems to only affect Xen, I'm basically fine with
it.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
