* Re: [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems [not found] <CAAhSdy1gNB0sMAq4mtGSNJ96BND4tMxHShq==3B1hzL9ebs=oQ@mail.gmail.com> @ 2018-12-08 20:25 ` Palmer Dabbelt 0 siblings, 0 replies; 4+ messages in thread From: Palmer Dabbelt @ 2018-12-08 20:25 UTC (permalink / raw) To: anup Cc: mark.rutland, devicetree, Damien.LeMoal, aou, dmitriy, daniel.lezcano, linux-kernel, atish.patra, robh+dt, linux-riscv, tglx On Fri, 07 Dec 2018 09:20:57 PST (-0800), anup@brainfault.org wrote: > On Fri, 7 Dec, 2018, 10:30 PM Palmer Dabbelt <palmer@sifive.com wrote: > >> On Mon, 03 Dec 2018 12:57:31 PST (-0800), atish.patra@wdc.com wrote: >> > Currently, clocksource registration happens for an invalid cpu >> > for non-smp kernels. This lead to kernel panic as cpu hotplug >> > registration will fail for those cpus. >> > >> > Do not proceed if hartid is invalid. Take this opprtunity to >> > print appropriate error strings for different failure cases. >> > >> > Signed-off-by: Atish Patra <atish.patra@wdc.com> >> > --- >> > drivers/clocksource/riscv_timer.c | 13 ++++++++++--- >> > 1 file changed, 10 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/clocksource/riscv_timer.c >> b/drivers/clocksource/riscv_timer.c >> > index 39de6e49..4af4af47 100644 >> > --- a/drivers/clocksource/riscv_timer.c >> > +++ b/drivers/clocksource/riscv_timer.c >> > @@ -108,6 +108,8 @@ static int __init riscv_timer_init_dt(struct >> device_node *n) >> > int cpuid, hartid, error; >> > >> > hartid = riscv_of_processor_hartid(n); >> > + if (hartid < 0) >> > + return hartid; >> >> This seems like it's just hiding a bug somewhere else. We should at least >> put >> out a WARN here, as I'm not sure the error will propagate anywhere useful. >> > > We need separate DT node for riscv_timer. The riscv_timer is nothing but > SOC timer accessed via rdtime and SBI calls. It can be viewed as one > device. In fact, this is how it's done in ARM/ARM64. We had that at some point, but this was changed. The logic was that, since the RISC-V ISA mandates the presence of this timer for all harts, the RISC-V CPU node is sufficient to encode the presence of a RISC-V timer. I'm OK changing this, but you should look at the old thread (which I can't find) to make sure all the arguments are taken into account. >> > cpuid = riscv_hartid_to_cpuid(hartid); >> > >> > if (cpuid != smp_processor_id()) >> > @@ -115,14 +117,19 @@ static int __init riscv_timer_init_dt(struct >> device_node *n) >> > >> > /* This should be called only for boot cpu */ >> > riscv_timebase = riscv_timebase_frequency(n); >> > - clocksource_register_hz(&riscv_clocksource, riscv_timebase); >> > + error = clocksource_register_hz(&riscv_clocksource, >> 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; >> > } >> > > Regards, > Anup > >> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/4] Timer code cleanup. @ 2018-12-03 20:57 Atish Patra 2018-12-03 20:57 ` [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra 0 siblings, 1 reply; 4+ messages in thread From: Atish Patra @ 2018-12-03 20:57 UTC (permalink / raw) To: linux-kernel Cc: Mark Rutland, devicetree, Damien Le Moal, Albert Ou, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano, Atish Patra, Rob Herring, Palmer Dabbelt, linux-riscv, Thomas Gleixner This patch series provides an assorted timer cleanups in RISC-V. Atish Patra (3): RISC-V: Support per-hart timebase-frequency RISC-V: Remove per cpu clocksource RISC-V: Fix non-smp kernel boot on SMP systems Palmer Dabbelt (1): dt-bindings: Correct RISC-V's timebase-frequency Documentation/devicetree/bindings/riscv/cpus.txt | 4 ++- arch/riscv/kernel/time.c | 9 +----- drivers/clocksource/riscv_timer.c | 39 ++++++++++++++++++++---- 3 files changed, 37 insertions(+), 15 deletions(-) -- 2.7.4 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems 2018-12-03 20:57 [PATCH 0/4] Timer code cleanup Atish Patra @ 2018-12-03 20:57 ` Atish Patra 2018-12-07 17:00 ` Palmer Dabbelt 0 siblings, 1 reply; 4+ messages in thread From: Atish Patra @ 2018-12-03 20:57 UTC (permalink / raw) To: linux-kernel Cc: Mark Rutland, devicetree, Damien Le Moal, Albert Ou, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano, Atish Patra, Rob Herring, Palmer Dabbelt, linux-riscv, Thomas Gleixner Currently, clocksource registration happens for an invalid cpu for non-smp kernels. This lead to kernel panic as cpu hotplug registration will fail for those cpus. Do not proceed if hartid is invalid. Take this opprtunity to print appropriate error strings for different failure cases. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- drivers/clocksource/riscv_timer.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c index 39de6e49..4af4af47 100644 --- a/drivers/clocksource/riscv_timer.c +++ b/drivers/clocksource/riscv_timer.c @@ -108,6 +108,8 @@ static int __init riscv_timer_init_dt(struct device_node *n) int cpuid, hartid, error; hartid = riscv_of_processor_hartid(n); + if (hartid < 0) + return hartid; cpuid = riscv_hartid_to_cpuid(hartid); if (cpuid != smp_processor_id()) @@ -115,14 +117,19 @@ static int __init riscv_timer_init_dt(struct device_node *n) /* This should be called only for boot cpu */ riscv_timebase = riscv_timebase_frequency(n); - clocksource_register_hz(&riscv_clocksource, riscv_timebase); + error = clocksource_register_hz(&riscv_clocksource, 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; } -- 2.7.4 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems 2018-12-03 20:57 ` [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra @ 2018-12-07 17:00 ` Palmer Dabbelt 2018-12-07 23:31 ` Atish Patra 0 siblings, 1 reply; 4+ messages in thread From: Palmer Dabbelt @ 2018-12-07 17:00 UTC (permalink / raw) To: atish.patra Cc: mark.rutland, devicetree, Damien.LeMoal, aou, dmitriy, anup, daniel.lezcano, linux-kernel, atish.patra, robh+dt, linux-riscv, tglx On Mon, 03 Dec 2018 12:57:31 PST (-0800), atish.patra@wdc.com wrote: > Currently, clocksource registration happens for an invalid cpu > for non-smp kernels. This lead to kernel panic as cpu hotplug > registration will fail for those cpus. > > Do not proceed if hartid is invalid. Take this opprtunity to > print appropriate error strings for different failure cases. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > drivers/clocksource/riscv_timer.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c > index 39de6e49..4af4af47 100644 > --- a/drivers/clocksource/riscv_timer.c > +++ b/drivers/clocksource/riscv_timer.c > @@ -108,6 +108,8 @@ static int __init riscv_timer_init_dt(struct device_node *n) > int cpuid, hartid, error; > > hartid = riscv_of_processor_hartid(n); > + if (hartid < 0) > + return hartid; This seems like it's just hiding a bug somewhere else. We should at least put out a WARN here, as I'm not sure the error will propagate anywhere useful. > cpuid = riscv_hartid_to_cpuid(hartid); > > if (cpuid != smp_processor_id()) > @@ -115,14 +117,19 @@ static int __init riscv_timer_init_dt(struct device_node *n) > > /* This should be called only for boot cpu */ > riscv_timebase = riscv_timebase_frequency(n); > - clocksource_register_hz(&riscv_clocksource, riscv_timebase); > + error = clocksource_register_hz(&riscv_clocksource, 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; > } _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems 2018-12-07 17:00 ` Palmer Dabbelt @ 2018-12-07 23:31 ` Atish Patra 0 siblings, 0 replies; 4+ messages in thread From: Atish Patra @ 2018-12-07 23:31 UTC (permalink / raw) To: Palmer Dabbelt Cc: mark.rutland, devicetree, Damien Le Moal, aou, dmitriy, anup, daniel.lezcano, linux-kernel, robh+dt, linux-riscv, tglx On 12/7/18 9:00 AM, Palmer Dabbelt wrote: > On Mon, 03 Dec 2018 12:57:31 PST (-0800), atish.patra@wdc.com wrote: >> Currently, clocksource registration happens for an invalid cpu >> for non-smp kernels. This lead to kernel panic as cpu hotplug >> registration will fail for those cpus. >> >> Do not proceed if hartid is invalid. Take this opprtunity to >> print appropriate error strings for different failure cases. >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> --- >> drivers/clocksource/riscv_timer.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c >> index 39de6e49..4af4af47 100644 >> --- a/drivers/clocksource/riscv_timer.c >> +++ b/drivers/clocksource/riscv_timer.c >> @@ -108,6 +108,8 @@ static int __init riscv_timer_init_dt(struct device_node *n) >> int cpuid, hartid, error; >> >> hartid = riscv_of_processor_hartid(n); >> + if (hartid < 0) >> + return hartid; > > This seems like it's just hiding a bug somewhere else. We should at least put > out a WARN here, as I'm not sure the error will propagate anywhere useful. > ok. I will add a warning here. That's what we are doing in plic as well. Regards, Atish >> cpuid = riscv_hartid_to_cpuid(hartid); >> >> if (cpuid != smp_processor_id()) >> @@ -115,14 +117,19 @@ static int __init riscv_timer_init_dt(struct device_node *n) >> >> /* This should be called only for boot cpu */ >> riscv_timebase = riscv_timebase_frequency(n); >> - clocksource_register_hz(&riscv_clocksource, riscv_timebase); >> + error = clocksource_register_hz(&riscv_clocksource, 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; >> } > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-08 20:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAAhSdy1gNB0sMAq4mtGSNJ96BND4tMxHShq==3B1hzL9ebs=oQ@mail.gmail.com>
2018-12-08 20:25 ` [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems Palmer Dabbelt
2018-12-03 20:57 [PATCH 0/4] Timer code cleanup Atish Patra
2018-12-03 20:57 ` [PATCH 4/4] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra
2018-12-07 17:00 ` Palmer Dabbelt
2018-12-07 23:31 ` Atish Patra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).