arm64: skip register_cpufreq_notifier on ACPI-based systems
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
On ACPI based systems where the topology is setup using the API store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.
Skip register_cpufreq_notifier if raw_capacity is not allocated as part of topology initialization.
Stack:
init_cpu_capacity_callback+0xb4/0x1c8
notifier_call_chain+0x5c/0xa0
__blocking_notifier_call_chain+0x58/0xa0
blocking_notifier_call_chain+0x3c/0x50
cpufreq_set_policy+0xe4/0x328
cpufreq_init_policy+0x80/0x100
cpufreq_online+0x418/0x710
cpufreq_add_dev+0x118/0x180
subsys_interface_register+0xa4/0xf8
cpufreq_register_driver+0x1c0/0x298
cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq]
do_one_initcall+0x5c/0x168
do_init_module+0x64/0x1e4
load_module+0x130c/0x14d0
SyS_finit_module+0x108/0x120
el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/
--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/topology.c +1 -1
@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)return -EINVAL ;if (!alloc_cpumask_var (&cpus_to_visit,GFP_KERNEL )) {
Signature · 3 lines
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Hi,
Quoted 38 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.Skip
register_cpufreq_notifierifraw_capacityis not allocated as part of topology initialization.Stack: init_cpu_capacity_callback+0xb4/0x1c8 notifier_call_chain+0x5c/0xa0 __blocking_notifier_call_chain+0x58/0xa0 blocking_notifier_call_chain+0x3c/0x50 cpufreq_set_policy+0xe4/0x328 cpufreq_init_policy+0x80/0x100 cpufreq_online+0x418/0x710 cpufreq_add_dev+0x118/0x180 subsys_interface_register+0xa4/0xf8 cpufreq_register_driver+0x1c0/0x298 cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq] do_one_initcall+0x5c/0x168 do_init_module+0x64/0x1e4 load_module+0x130c/0x14d0 SyS_finit_module+0x108/0x120 el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/Signed-off-by Prashanth Prakash <[email protected]>--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/topology.c +0
-0
arch/arm64/kernel/topology.c static void normalize_cpu_capacity(void) · line 212
Quoted 5 lines
arch/arm64/kernel/topology.c +1
-1@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)
Looks good. I couldn't really test it to check if it fixes your problem (I assume it does), but it doesn't seem to introduce regressions on my boxes.
Thanks,
- Juri
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Quoted 30 lines · Juri Lelli
Hi,
Quoted 27 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.Skip
register_cpufreq_notifierifraw_capacityis not allocated as part of topology initialization.Stack: init_cpu_capacity_callback+0xb4/0x1c8 notifier_call_chain+0x5c/0xa0 __blocking_notifier_call_chain+0x58/0xa0 blocking_notifier_call_chain+0x3c/0x50 cpufreq_set_policy+0xe4/0x328 cpufreq_init_policy+0x80/0x100 cpufreq_online+0x418/0x710 cpufreq_add_dev+0x118/0x180 subsys_interface_register+0xa4/0xf8 cpufreq_register_driver+0x1c0/0x298 cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq] do_one_initcall+0x5c/0x168 do_init_module+0x64/0x1e4 load_module+0x130c/0x14d0 SyS_finit_module+0x108/0x120 el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/
In other words,
Quoted 11 lines
Quoted 11 lines
Signed-off-by Prashanth Prakash <[email protected]>--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/topology.c +0
-0@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)
Quoted 8 lines
arch/arm64/kernel/topology.c static void normalize_cpu_capacity(void) · line 212
Quoted 4 lines
arch/arm64/kernel/topology.c +1
-1static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)Looks good. I couldn't really test it to check if it fixes your problem (I assume it does), but it doesn't seem to introduce regressions on my boxes.
Are you testing on QEMU or Fast Models? I was under the impression that the former had ACPI support. For the latter, wouldn't it be nice to have ACPI support in the bootwrapper. Out of curiosity, is Mark's bootwrapper repository on kernel.org the best one to use these days?
http://git.kernel.org/cgit/linux/kernel/git/mark/boot-wrapper-aarch64.git/
Thanks,
Cov
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Quoted 56 lines · Christopher Covington
Quoted 30 lines · Juri Lelli
Hi,
Quoted 27 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.Skip
register_cpufreq_notifierifraw_capacityis not allocated as part of topology initialization.Stack: init_cpu_capacity_callback+0xb4/0x1c8 notifier_call_chain+0x5c/0xa0 __blocking_notifier_call_chain+0x58/0xa0 blocking_notifier_call_chain+0x3c/0x50 cpufreq_set_policy+0xe4/0x328 cpufreq_init_policy+0x80/0x100 cpufreq_online+0x418/0x710 cpufreq_add_dev+0x118/0x180 subsys_interface_register+0xa4/0xf8 cpufreq_register_driver+0x1c0/0x298 cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq] do_one_initcall+0x5c/0x168 do_init_module+0x64/0x1e4 load_module+0x130c/0x14d0 SyS_finit_module+0x108/0x120 el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/In other words,
Fixes 7202bde8b7ae ("arm64: parse cpu capacity-dmips-mhz from DT")Quoted 11 lines
Quoted 11 lines
Signed-off-by Prashanth Prakash <[email protected]>--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/topology.c +0
-0@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)Quoted 8 lines
arch/arm64/kernel/topology.c static void normalize_cpu_capacity(void) · line 212
Quoted 4 lines
arch/arm64/kernel/topology.c +1
-1static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)Looks good. I couldn't really test it to check if it fixes your problem (I assume it does), but it doesn't seem to introduce regressions on my boxes.
Quoted 1 line
Are you testing on QEMU or Fast Models?
Neither, Juno R2. Let me see if I can quickly test it in ACPI mode.
Quoted 7 lines
I was under the impression that the
former had ACPI support. For the latter, wouldn't it be nice to have ACPI support in the bootwrapper. Out of curiosity, is Mark's bootwrapper repository on kernel.org the best one to use these days?http://git.kernel.org/cgit/linux/kernel/git/mark/boot-wrapper-aarch64.git/
Not sure, Mark, others?
Thanks,
- Juri
Mark Rutland
reviewerPATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
In reply to Christopher CovingtonQuoted 3 lines · Christopher Covington
Are you testing on QEMU or Fast Models? I was under the impression that the former had ACPI support. For the latter, wouldn't it be nice to have ACPI support in the bootwrapper.
One goal of the boot wrapper is to be as simple as reasonably possible. ACPI support in the boot wrapper would be far from trivial, at minimum requiring EFI support (requiring either a whole reimplementation, or work to make EDK2 run atop of the boot wrapper).
Given that it is already possible to use ARM Trusted Firmware and EDK2 in a model environment, and this can already be used to handover ACPI tables, one would be served better by those.
Quoted 4 lines
Out of curiosity, is Mark's bootwrapper repository on kernel.org the best one to use these days?
http://git.kernel.org/cgit/linux/kernel/git/mark/boot-wrapper-aarch64.git/
That is the canonical repo these days, yes.
Thanks,
Mark.
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
In reply to Prashanth Prakash[adding Sudeep and Lorenzo for comment]
Quoted 3 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.
What is the "necessary code" that we're missing? Wouldn't it be better to add that, or explicitly avoid the cpufreq notifier registration if we're using ACPI?
Will
Quoted 47 lines
Skip
register_cpufreq_notifierifraw_capacityis not allocated as part of topology initialization.Stack: init_cpu_capacity_callback+0xb4/0x1c8 notifier_call_chain+0x5c/0xa0 __blocking_notifier_call_chain+0x58/0xa0 blocking_notifier_call_chain+0x3c/0x50 cpufreq_set_policy+0xe4/0x328 cpufreq_init_policy+0x80/0x100 cpufreq_online+0x418/0x710 cpufreq_add_dev+0x118/0x180 subsys_interface_register+0xa4/0xf8 cpufreq_register_driver+0x1c0/0x298 cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq] do_one_initcall+0x5c/0x168 do_init_module+0x64/0x1e4 load_module+0x130c/0x14d0 SyS_finit_module+0x108/0x120 el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/Signed-off-by Prashanth Prakash <[email protected]>--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/topology.c +1
-1@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)return -EINVAL ;if (!alloc_cpumask_var (&cpus_to_visit,GFP_KERNEL )) {Signature · 3 lines
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Quoted 10 lines · Will Deacon
[adding Sudeep and Lorenzo for comment]
Quoted 3 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.What is the "necessary code" that we're missing? Wouldn't it be better to add that, or explicitly avoid the cpufreq notifier registration if we're using ACPI?
Necessary code is, in DT, code parsing bindings to provide capacity values and allocate the raw_capacity array; there is no ACPI counterpart for those bindings (well..there is a byte length field in the GICC MADT entry "Processor Power Efficiency Class" but as far as I understand, currently, it would be more reliable as a random seed than a useful capacity scale, I just do not know why it is there) so the whole CPUfreq notifier thing is basically useless when booting with ACPI.
How about using (or put the ACPI bit in a separate line with a comment):
if (!acpi_disabled || cap_parsing_failed)
to prevent registering the CPUfreq notifier ? Using raw_capacity check this patch achieves the same in a much more opaque way (you will have to include <linux/acpi.h> to make use of acpi_disabled).
We can work to turn "Processor Power Efficiency Class" into something useful but that's for another series anyway.
Lorenzo
Quoted 51 lines
Will
Quoted 47 lines
Skip
register_cpufreq_notifierifraw_capacityis not allocated as part of topology initialization.Stack: init_cpu_capacity_callback+0xb4/0x1c8 notifier_call_chain+0x5c/0xa0 __blocking_notifier_call_chain+0x58/0xa0 blocking_notifier_call_chain+0x3c/0x50 cpufreq_set_policy+0xe4/0x328 cpufreq_init_policy+0x80/0x100 cpufreq_online+0x418/0x710 cpufreq_add_dev+0x118/0x180 subsys_interface_register+0xa4/0xf8 cpufreq_register_driver+0x1c0/0x298 cppc_cpufreq_init+0xdc/0x1000 [cppc_cpufreq] do_one_initcall+0x5c/0x168 do_init_module+0x64/0x1e4 load_module+0x130c/0x14d0 SyS_finit_module+0x108/0x120 el0_svc_naked+0x24/0x28Patch that added support for popultaing cpu capacity for DT:
https://patchwork.codeaurora.org/patch/98353/Signed-off-by Prashanth Prakash <[email protected]>--- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/topology.c +1
-1@@ -209,7 +209,7 @@ static void normalize_cpu_capacity(void)static int __init register_cpufreq_notifier (void ) { -if (cap_parsing_failed) +if (cap_parsing_failed || !raw_capacity)return -EINVAL ;if (!alloc_cpumask_var (&cpus_to_visit,GFP_KERNEL )) {Signature · 3 lines
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Quoted 12 lines · Lorenzo Pieralisi
Quoted 10 lines · Will Deacon
[adding Sudeep and Lorenzo for comment]
Quoted 3 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.What is the "necessary code" that we're missing? Wouldn't it be better to add that, or explicitly avoid the cpufreq notifier registration if we're using ACPI?
Quoted 7 lines
Necessary code is, in DT, code parsing bindings to provide capacity values and allocate the
raw_capacityarray; there is no ACPI counterpart for those bindings (well..there is a byte length field in the GICC MADT entry "Processor Power Efficiency Class" but as far as I understand, currently, it would be more reliable as a random seed than a useful capacity scale, I just do not know why it is there) so the whole CPUfreq notifier thing is basically useless when booting with ACPI.
Ok, thanks for the explanation.
Quoted 8 lines
How about using (or put the ACPI bit in a separate line with a comment):
if (!acpi_disabled || cap_parsing_failed)to prevent registering the CPUfreq notifier ? Using
raw_capacitycheck this patch achieves the same in a much more opaque way (you will have to include <linux/acpi.h> to make use ofacpi_disabled).
That looks good to me. If somebody sends a v2, I can apply it for 4.11.
Thanks,
Will
PATCH 1/1 arm64: skip register_cpufreq_notifier on ACPI-based systems
Quoted 23 lines · Will Deacon
Quoted 10 lines · Lorenzo Pieralisi
Quoted 9 lines · Will Deacon
[adding Sudeep and Lorenzo for comment]
Quoted 3 lines · Prashanth Prakash
On ACPI based systems where the topology is setup using the API
store_cpu_topology, at the moment we do not have necessary code to handle a cpufreq notifier, thus resulting in a crash.What is the "necessary code" that we're missing? Wouldn't it be better to add that, or explicitly avoid the cpufreq notifier registration if we're using ACPI?
Quoted 7 lines
Necessary code is, in DT, code parsing bindings to provide capacity values and allocate the
raw_capacityarray; there is no ACPI counterpart for those bindings (well..there is a byte length field in the GICC MADT entry "Processor Power Efficiency Class" but as far as I understand, currently, it would be more reliable as a random seed than a useful capacity scale, I just do not know why it is there) so the whole CPUfreq notifier thing is basically useless when booting with ACPI.Ok, thanks for the explanation.
Quoted 3 lines
How about using (or put the ACPI bit in a separate line with a comment):
Quoted 6 lines
Quoted 5 lines
if (!acpi_disabled || cap_parsing_failed)to prevent registering the CPUfreq notifier ? Using
raw_capacitycheck this patch achieves the same in a much more opaque way (you will have to include <linux/acpi.h> to make use ofacpi_disabled).That looks good to me. If somebody sends a v2, I can apply it for 4.11.
Thanks for the inputs. I will post v2 in next couple of hours.
Any chances this can make it to one of the 4-10 release candidates as it results in a panic without this change?
--
Thanks,
Prashanth