Messages in this thread | | | Date | Fri, 30 Jun 2023 11:02:18 +0800 | Subject | Re: [PATCH] riscv: BUG_ON() for no cpu nodes in setup_smp | From | Song Shuai <> |
| |
在 2023/6/29 20:33, Conor Dooley 写道: > Hey, > > On Thu, Jun 29, 2023 at 06:58:39PM +0800, Song Shuai wrote: >> When booting with ACPI tables, the tiny devictree created by >> EFI Stub doesn't provide cpu nodes. >
"When only the ACPI tables are passed to kernel, the tiny..." That seems more accurate. We can use "acpi=" kernel parameter to manually enable/disable ACPI.
> What are the conditions that are required to reproduce this issue? > When booting with ACPI, why is acpi_disabled true? > In my naivety, that seems like a bigger problem to address. >
Actually, I appended the "acpi=off" to kernel cmdline for testing the "off" option. That would set acpi_disabled as true.
>> In setup_smp(), of_parse_and_init_cpus() will bug on !found_boot_cpu > > Please, s/on !found_boot_cpu/if the boot cpu is not found in the > devicetree/, or similar. > ok >> if acpi_disabled. > > Why would of_parse_and_init_cpus() be called in any other case? There's > only this one caller, right? yes > >> That's unclear, so bug for no cpu nodes before >> of_parse_and_init_cpus(). > > What is unclear? That the reason for the BUG() was that there were no > cpu nodes, since it could also be that there were CPU nodes but they > were disabled etc?
The BUG() in of_parse_and_init_cpus() indicates there was no boot cpu found in the devicetree , not there were no cpu nodes in the devices. That's the "unclear" I mean.
> >> Signed-off-by: Song Shuai <suagrfillet@gmail.com> >> --- >> arch/riscv/kernel/smpboot.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c >> index 6ca2b5309aab..243a7b533ad7 100644 >> --- a/arch/riscv/kernel/smpboot.c >> +++ b/arch/riscv/kernel/smpboot.c >> @@ -187,8 +187,13 @@ static void __init of_parse_and_init_cpus(void) >> >> void __init setup_smp(void) >> { >> - if (acpi_disabled) >> + if (acpi_disabled) { >> + /* When booting with ACPI tables, the devictree created by EFI Stub > > This is not netdev, please use the correct comment style :/ > >> + * doesn't provide cpu nodes. So BUG here for any acpi_disabled. >> + */ >> + BUG_ON(!of_get_next_cpu_node(NULL)); >> of_parse_and_init_cpus(); >> + } >> else >> acpi_parse_and_init_cpus(); > > checkpatch should have told you that you now need to add braces on all > arms of this statement. ok, > > Or, better yet, move the whole thing into of_parse_and_init_cpus() in > the first place? You could drop most of the comment in the process, > since I think the details of how you hit this problem would likely not > be helpful to anyone that hit it under different conditions. > ok, I'll apply these comments if you're ok with my replies.
> Cheers, > Conor. > >> } >> -- >> 2.20.1 >>
-- Thanks Song Shuai
| |