Re: [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
On 12/26/18 7:58 PM, Anup Patel wrote: On Thu, Dec 27, 2018 at 4:39 AM Atish Patra wrote: In non-smp configuration, hartid can be higher that NR_CPUS. riscv_of_processor_hartid should not be compared to hartid to NR_CPUS in that case. Moreover, this function checks all the DT properties of a hart node. NR_CPUS comparison seems out of place. This only explains change in arch/riscv/kernel/cpu.c Create separate patch for it. Do cpuid comparison with NR_CPUs in smp setup code. Update the Create separate patch for change in arch/riscv/kernel/smp.c drivers to handle appropriate code as well. Create separate patches for riscv_timer and irq-sifive-plic.c because they will probably go via different gitrepos. Signed-off-by: Atish Patra --- arch/riscv/kernel/cpu.c | 4 arch/riscv/kernel/smp.c | 1 - arch/riscv/kernel/smpboot.c | 5 + drivers/clocksource/riscv_timer.c | 21 ++--- drivers/irqchip/irq-sifive-plic.c | 5 + 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index b4a7d442..251ffab6 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node) pr_warn("Found CPU without hart ID\n"); return -(ENODEV); } - if (hart >= NR_CPUS) { - pr_info("Found hart ID %d, which is above NR_CPUs. Disabling this hart\n", hart); - return -(ENODEV); - } if (of_property_read_string(node, "status", &status)) { pr_warn("CPU with hartid=%d has no \"status\" property\n", hart); diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c index 57b1383e..9ea7ac7d 100644 --- a/arch/riscv/kernel/smp.c +++ b/arch/riscv/kernel/smp.c @@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid) return i; pr_err("Couldn't find cpu id for hartid [%d]\n", hartid); - BUG(); Have a separate patch with explanation about why we don't need BUG() here. Ok. I will split the patch into multiple ones as suggested. Regards, Atish return i; } diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index bb8cd242..05291840 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -66,6 +66,11 @@ void __init setup_smp(void) found_boot_cpu = 1; continue; } + if (cpuid >= NR_CPUS) { + pr_warn("Invalid cpuid [%d] for hartid [%d]\n", + cpuid, hart); + break; + } cpuid_to_hartid_map(cpuid) = hart; set_cpu_possible(cpuid, true); diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c index 084e97dc..acf2af10 100644 --- a/drivers/clocksource/riscv_timer.c +++ b/drivers/clocksource/riscv_timer.c @@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node *n) struct clocksource *cs; hartid = riscv_of_processor_hartid(n); + if (hartid < 0) { + pr_warn("Not valid hartid for node [%pOF] error = [%d]\n", + n, hartid); + return hartid; + } cpuid = riscv_hartid_to_cpuid(hartid); + if (cpuid < 0) + pr_warn("Invalid cpuid for hartid [%d]\n", hartid); + if (cpuid != smp_processor_id()) return 0; + pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n", + __func__, cpuid, hartid); cs = per_cpu_ptr(&riscv_clocksource, cpuid); - clocksource_register_hz(cs, riscv_timebase); + error = clocksource_register_hz(cs, riscv_timebase); + if (error) { + pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", + error, cpuid); + return error; + } error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING, "clockevents/riscv/timer:starting", riscv_timer_starting_cpu, riscv_timer_dying_cpu); if (error) - pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", - error, cpuid); + pr_err("cpu hp setup state failed for RISCV timer [%d]\n", + error); return error; } diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 357e9daf..254ecd76 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node, } cpu = riscv_hartid_to_cpuid(hartid); + if (cpu < 0) { + pr_warn("Invalid cpuid for context %d\n", i); +
Re: [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
On Thu, Dec 27, 2018 at 4:39 AM Atish Patra wrote: > > In non-smp configuration, hartid can be higher that NR_CPUS. > riscv_of_processor_hartid should not be compared to hartid to > NR_CPUS in that case. Moreover, this function checks all the > DT properties of a hart node. NR_CPUS comparison seems out of > place. This only explains change in arch/riscv/kernel/cpu.c Create separate patch for it. > > Do cpuid comparison with NR_CPUs in smp setup code. Update the Create separate patch for change in arch/riscv/kernel/smp.c > drivers to handle appropriate code as well. Create separate patches for riscv_timer and irq-sifive-plic.c because they will probably go via different gitrepos. > > Signed-off-by: Atish Patra > --- > arch/riscv/kernel/cpu.c | 4 > arch/riscv/kernel/smp.c | 1 - > arch/riscv/kernel/smpboot.c | 5 + > drivers/clocksource/riscv_timer.c | 21 ++--- > drivers/irqchip/irq-sifive-plic.c | 5 + > 5 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index b4a7d442..251ffab6 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node) > pr_warn("Found CPU without hart ID\n"); > return -(ENODEV); > } > - if (hart >= NR_CPUS) { > - pr_info("Found hart ID %d, which is above NR_CPUs. Disabling > this hart\n", hart); > - return -(ENODEV); > - } > > if (of_property_read_string(node, "status", &status)) { > pr_warn("CPU with hartid=%d has no \"status\" property\n", > hart); > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c > index 57b1383e..9ea7ac7d 100644 > --- a/arch/riscv/kernel/smp.c > +++ b/arch/riscv/kernel/smp.c > @@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid) > return i; > > pr_err("Couldn't find cpu id for hartid [%d]\n", hartid); > - BUG(); Have a separate patch with explanation about why we don't need BUG() here. > return i; > } > > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c > index bb8cd242..05291840 100644 > --- a/arch/riscv/kernel/smpboot.c > +++ b/arch/riscv/kernel/smpboot.c > @@ -66,6 +66,11 @@ void __init setup_smp(void) > found_boot_cpu = 1; > continue; > } > + if (cpuid >= NR_CPUS) { > + pr_warn("Invalid cpuid [%d] for hartid [%d]\n", > + cpuid, hart); > + break; > + } > > cpuid_to_hartid_map(cpuid) = hart; > set_cpu_possible(cpuid, true); > diff --git a/drivers/clocksource/riscv_timer.c > b/drivers/clocksource/riscv_timer.c > index 084e97dc..acf2af10 100644 > --- a/drivers/clocksource/riscv_timer.c > +++ b/drivers/clocksource/riscv_timer.c > @@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node > *n) > struct clocksource *cs; > > hartid = riscv_of_processor_hartid(n); > + if (hartid < 0) { > + pr_warn("Not valid hartid for node [%pOF] error = [%d]\n", > + n, hartid); > + return hartid; > + } > cpuid = riscv_hartid_to_cpuid(hartid); > > + if (cpuid < 0) > + pr_warn("Invalid cpuid for hartid [%d]\n", hartid); > + > if (cpuid != smp_processor_id()) > return 0; > > + pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n", > + __func__, cpuid, hartid); > cs = per_cpu_ptr(&riscv_clocksource, cpuid); > - clocksource_register_hz(cs, riscv_timebase); > + error = clocksource_register_hz(cs, riscv_timebase); > > + if (error) { > + pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > + error, cpuid); > + return error; > + } > error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING, > "clockevents/riscv/timer:starting", > riscv_timer_starting_cpu, riscv_timer_dying_cpu); > if (error) > - pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > - error, cpuid); > + pr_err("cpu hp setup state failed for RISCV timer [%d]\n", > + error); > return error; > } > > diff --git a/drivers/irqchip/irq-sifive-plic.c > b/drivers/irqchip/irq-sifive-plic.c > index 357e9daf..254ecd76 100644 > --- a/drivers/irqchip/irq-sifive-plic.c > +++ b/drivers/irqchip/irq-sifive-plic.c > @@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node, > } > > cpu = riscv_hartid_to_cpuid(hartid); > + if (cpu < 0) { > + pr_w
[PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
In non-smp configuration, hartid can be higher that NR_CPUS. riscv_of_processor_hartid should not be compared to hartid to NR_CPUS in that case. Moreover, this function checks all the DT properties of a hart node. NR_CPUS comparison seems out of place. Do cpuid comparison with NR_CPUs in smp setup code. Update the drivers to handle appropriate code as well. Signed-off-by: Atish Patra --- arch/riscv/kernel/cpu.c | 4 arch/riscv/kernel/smp.c | 1 - arch/riscv/kernel/smpboot.c | 5 + drivers/clocksource/riscv_timer.c | 21 ++--- drivers/irqchip/irq-sifive-plic.c | 5 + 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index b4a7d442..251ffab6 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node) pr_warn("Found CPU without hart ID\n"); return -(ENODEV); } - if (hart >= NR_CPUS) { - pr_info("Found hart ID %d, which is above NR_CPUs. Disabling this hart\n", hart); - return -(ENODEV); - } if (of_property_read_string(node, "status", &status)) { pr_warn("CPU with hartid=%d has no \"status\" property\n", hart); diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c index 57b1383e..9ea7ac7d 100644 --- a/arch/riscv/kernel/smp.c +++ b/arch/riscv/kernel/smp.c @@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid) return i; pr_err("Couldn't find cpu id for hartid [%d]\n", hartid); - BUG(); return i; } diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index bb8cd242..05291840 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -66,6 +66,11 @@ void __init setup_smp(void) found_boot_cpu = 1; continue; } + if (cpuid >= NR_CPUS) { + pr_warn("Invalid cpuid [%d] for hartid [%d]\n", + cpuid, hart); + break; + } cpuid_to_hartid_map(cpuid) = hart; set_cpu_possible(cpuid, true); diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c index 084e97dc..acf2af10 100644 --- a/drivers/clocksource/riscv_timer.c +++ b/drivers/clocksource/riscv_timer.c @@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node *n) struct clocksource *cs; hartid = riscv_of_processor_hartid(n); + if (hartid < 0) { + pr_warn("Not valid hartid for node [%pOF] error = [%d]\n", + n, hartid); + return hartid; + } cpuid = riscv_hartid_to_cpuid(hartid); + if (cpuid < 0) + pr_warn("Invalid cpuid for hartid [%d]\n", hartid); + if (cpuid != smp_processor_id()) return 0; + pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n", + __func__, cpuid, hartid); cs = per_cpu_ptr(&riscv_clocksource, cpuid); - clocksource_register_hz(cs, riscv_timebase); + error = clocksource_register_hz(cs, riscv_timebase); + if (error) { + pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", + error, cpuid); + return error; + } error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING, "clockevents/riscv/timer:starting", riscv_timer_starting_cpu, riscv_timer_dying_cpu); if (error) - pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", - error, cpuid); + pr_err("cpu hp setup state failed for RISCV timer [%d]\n", + error); return error; } diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 357e9daf..254ecd76 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node, } cpu = riscv_hartid_to_cpuid(hartid); + if (cpu < 0) { + pr_warn("Invalid cpuid for context %d\n", i); + continue; + } + handler = per_cpu_ptr(&plic_handlers, cpu); handler->present = true; handler->ctxid = i; -- 2.7.4