* [PATCH 0/2] Support SSTC while PM operations @ 2024-08-29 3:38 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:38 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Samuel Holland, Conor Dooley, Sunil V L, Nick Hu, linux-pm, linux-riscv, linux-kernel When the cpu is going to be hotplug, stop the stimecmp to prevent pending interrupt. When the cpu is going to be suspended, save the stimecmp before entering the suspend state and restore it in the resume path. Nick Hu (2): riscv: Add stimecmp save and restore time-riscv: Stop stimecmp when cpu hotplug arch/riscv/include/asm/suspend.h | 4 ++++ arch/riscv/kernel/suspend.c | 13 +++++++++++++ drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- 3 files changed, 32 insertions(+), 7 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/2] Support SSTC while PM operations @ 2024-08-29 3:38 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:38 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Samuel Holland, Conor Dooley, Sunil V L, Nick Hu, linux-pm, linux-riscv, linux-kernel When the cpu is going to be hotplug, stop the stimecmp to prevent pending interrupt. When the cpu is going to be suspended, save the stimecmp before entering the suspend state and restore it in the resume path. Nick Hu (2): riscv: Add stimecmp save and restore time-riscv: Stop stimecmp when cpu hotplug arch/riscv/include/asm/suspend.h | 4 ++++ arch/riscv/kernel/suspend.c | 13 +++++++++++++ drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- 3 files changed, 32 insertions(+), 7 deletions(-) -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/2] riscv: Add stimecmp save and restore 2024-08-29 3:38 ` Nick Hu @ 2024-08-29 3:38 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:38 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, Nick Hu, linux-pm, linux-riscv, linux-kernel If the HW support the SSTC extension, we should save and restore the stimecmp register while cpu non retention suspend. Signed-off-by: Nick Hu <nick.hu@sifive.com> --- arch/riscv/include/asm/suspend.h | 4 ++++ arch/riscv/kernel/suspend.c | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h index 4ffb022b097f..ffaac2efabb5 100644 --- a/arch/riscv/include/asm/suspend.h +++ b/arch/riscv/include/asm/suspend.h @@ -16,6 +16,10 @@ struct suspend_context { unsigned long envcfg; unsigned long tvec; unsigned long ie; +#if __riscv_xlen < 64 + unsigned long stimecmph; +#endif + unsigned long stimecmp; #ifdef CONFIG_MMU unsigned long satp; #endif diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c index c8cec0cc5833..3afd86e1abf7 100644 --- a/arch/riscv/kernel/suspend.c +++ b/arch/riscv/kernel/suspend.c @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) context->tvec = csr_read(CSR_TVEC); context->ie = csr_read(CSR_IE); + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { + context->stimecmp = csr_read(CSR_STIMECMP); +#if __riscv_xlen < 64 + context->stimecmph = csr_read(CSR_STIMECMPH); +#endif + } /* * No need to save/restore IP CSR (i.e. MIP or SIP) because: * @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) csr_write(CSR_TVEC, context->tvec); csr_write(CSR_IE, context->ie); + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { + csr_write(CSR_STIMECMP, context->stimecmp); +#if __riscv_xlen < 64 + csr_write(CSR_STIMECMPH, context->stimecmph); +#endif + } + #ifdef CONFIG_MMU csr_write(CSR_SATP, context->satp); #endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/2] riscv: Add stimecmp save and restore @ 2024-08-29 3:38 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:38 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, Nick Hu, linux-pm, linux-riscv, linux-kernel If the HW support the SSTC extension, we should save and restore the stimecmp register while cpu non retention suspend. Signed-off-by: Nick Hu <nick.hu@sifive.com> --- arch/riscv/include/asm/suspend.h | 4 ++++ arch/riscv/kernel/suspend.c | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h index 4ffb022b097f..ffaac2efabb5 100644 --- a/arch/riscv/include/asm/suspend.h +++ b/arch/riscv/include/asm/suspend.h @@ -16,6 +16,10 @@ struct suspend_context { unsigned long envcfg; unsigned long tvec; unsigned long ie; +#if __riscv_xlen < 64 + unsigned long stimecmph; +#endif + unsigned long stimecmp; #ifdef CONFIG_MMU unsigned long satp; #endif diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c index c8cec0cc5833..3afd86e1abf7 100644 --- a/arch/riscv/kernel/suspend.c +++ b/arch/riscv/kernel/suspend.c @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) context->tvec = csr_read(CSR_TVEC); context->ie = csr_read(CSR_IE); + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { + context->stimecmp = csr_read(CSR_STIMECMP); +#if __riscv_xlen < 64 + context->stimecmph = csr_read(CSR_STIMECMPH); +#endif + } /* * No need to save/restore IP CSR (i.e. MIP or SIP) because: * @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) csr_write(CSR_TVEC, context->tvec); csr_write(CSR_IE, context->ie); + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { + csr_write(CSR_STIMECMP, context->stimecmp); +#if __riscv_xlen < 64 + csr_write(CSR_STIMECMPH, context->stimecmph); +#endif + } + #ifdef CONFIG_MMU csr_write(CSR_SATP, context->satp); #endif -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore 2024-08-29 3:38 ` Nick Hu @ 2024-08-29 5:18 ` Anup Patel -1 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 5:18 UTC (permalink / raw) To: Nick Hu Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29, 2024 at 9:09 AM Nick Hu <nick.hu@sifive.com> wrote: > > If the HW support the SSTC extension, we should save and restore the > stimecmp register while cpu non retention suspend. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > arch/riscv/include/asm/suspend.h | 4 ++++ > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > index 4ffb022b097f..ffaac2efabb5 100644 > --- a/arch/riscv/include/asm/suspend.h > +++ b/arch/riscv/include/asm/suspend.h > @@ -16,6 +16,10 @@ struct suspend_context { > unsigned long envcfg; > unsigned long tvec; > unsigned long ie; > +#if __riscv_xlen < 64 > + unsigned long stimecmph; > +#endif > + unsigned long stimecmp; > #ifdef CONFIG_MMU > unsigned long satp; > #endif > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > index c8cec0cc5833..3afd86e1abf7 100644 > --- a/arch/riscv/kernel/suspend.c > +++ b/arch/riscv/kernel/suspend.c > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > context->tvec = csr_read(CSR_TVEC); > context->ie = csr_read(CSR_IE); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + context->stimecmp = csr_read(CSR_STIMECMP); > +#if __riscv_xlen < 64 > + context->stimecmph = csr_read(CSR_STIMECMPH); > +#endif > + } The suspend save/restore is enabled for the NoMMU kernel as well (which runs in M-mode) so it is better to save/restore stimecmp CSR only for MMU enabled kernels (just like satp CSR). > /* > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > * > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > csr_write(CSR_TVEC, context->tvec); > csr_write(CSR_IE, context->ie); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + csr_write(CSR_STIMECMP, context->stimecmp); > +#if __riscv_xlen < 64 > + csr_write(CSR_STIMECMPH, context->stimecmph); > +#endif > + } > + > #ifdef CONFIG_MMU > csr_write(CSR_SATP, context->satp); > #endif > -- > 2.34.1 > > Regards, Anup ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore @ 2024-08-29 5:18 ` Anup Patel 0 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 5:18 UTC (permalink / raw) To: Nick Hu Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones On Thu, Aug 29, 2024 at 9:09 AM Nick Hu <nick.hu@sifive.com> wrote: > > If the HW support the SSTC extension, we should save and restore the > stimecmp register while cpu non retention suspend. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > arch/riscv/include/asm/suspend.h | 4 ++++ > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > index 4ffb022b097f..ffaac2efabb5 100644 > --- a/arch/riscv/include/asm/suspend.h > +++ b/arch/riscv/include/asm/suspend.h > @@ -16,6 +16,10 @@ struct suspend_context { > unsigned long envcfg; > unsigned long tvec; > unsigned long ie; > +#if __riscv_xlen < 64 > + unsigned long stimecmph; > +#endif > + unsigned long stimecmp; > #ifdef CONFIG_MMU > unsigned long satp; > #endif > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > index c8cec0cc5833..3afd86e1abf7 100644 > --- a/arch/riscv/kernel/suspend.c > +++ b/arch/riscv/kernel/suspend.c > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > context->tvec = csr_read(CSR_TVEC); > context->ie = csr_read(CSR_IE); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + context->stimecmp = csr_read(CSR_STIMECMP); > +#if __riscv_xlen < 64 > + context->stimecmph = csr_read(CSR_STIMECMPH); > +#endif > + } The suspend save/restore is enabled for the NoMMU kernel as well (which runs in M-mode) so it is better to save/restore stimecmp CSR only for MMU enabled kernels (just like satp CSR). > /* > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > * > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > csr_write(CSR_TVEC, context->tvec); > csr_write(CSR_IE, context->ie); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + csr_write(CSR_STIMECMP, context->stimecmp); > +#if __riscv_xlen < 64 > + csr_write(CSR_STIMECMPH, context->stimecmph); > +#endif > + } > + > #ifdef CONFIG_MMU > csr_write(CSR_SATP, context->satp); > #endif > -- > 2.34.1 > > Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore 2024-08-29 5:18 ` Anup Patel @ 2024-08-29 6:16 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 6:16 UTC (permalink / raw) To: Anup Patel Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel Hi Anup, On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 9:09 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > If the HW support the SSTC extension, we should save and restore the > > stimecmp register while cpu non retention suspend. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > arch/riscv/include/asm/suspend.h | 4 ++++ > > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > > index 4ffb022b097f..ffaac2efabb5 100644 > > --- a/arch/riscv/include/asm/suspend.h > > +++ b/arch/riscv/include/asm/suspend.h > > @@ -16,6 +16,10 @@ struct suspend_context { > > unsigned long envcfg; > > unsigned long tvec; > > unsigned long ie; > > +#if __riscv_xlen < 64 > > + unsigned long stimecmph; > > +#endif > > + unsigned long stimecmp; > > #ifdef CONFIG_MMU > > unsigned long satp; > > #endif > > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > > index c8cec0cc5833..3afd86e1abf7 100644 > > --- a/arch/riscv/kernel/suspend.c > > +++ b/arch/riscv/kernel/suspend.c > > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > > context->tvec = csr_read(CSR_TVEC); > > context->ie = csr_read(CSR_IE); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + context->stimecmp = csr_read(CSR_STIMECMP); > > +#if __riscv_xlen < 64 > > + context->stimecmph = csr_read(CSR_STIMECMPH); > > +#endif > > + } > > The suspend save/restore is enabled for the NoMMU kernel as well > (which runs in M-mode) so it is better to save/restore stimecmp CSR > only for MMU enabled kernels (just like satp CSR). > Good point. Will update that in the next version. Thanks for the feedback. > > /* > > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > > * > > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > > csr_write(CSR_TVEC, context->tvec); > > csr_write(CSR_IE, context->ie); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + csr_write(CSR_STIMECMP, context->stimecmp); > > +#if __riscv_xlen < 64 > > + csr_write(CSR_STIMECMPH, context->stimecmph); > > +#endif > > + } > > + > > #ifdef CONFIG_MMU > > csr_write(CSR_SATP, context->satp); > > #endif > > -- > > 2.34.1 > > > > > > Regards, > Anup Regards, Nick ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore @ 2024-08-29 6:16 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 6:16 UTC (permalink / raw) To: Anup Patel Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones Hi Anup, On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 9:09 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > If the HW support the SSTC extension, we should save and restore the > > stimecmp register while cpu non retention suspend. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > arch/riscv/include/asm/suspend.h | 4 ++++ > > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > > index 4ffb022b097f..ffaac2efabb5 100644 > > --- a/arch/riscv/include/asm/suspend.h > > +++ b/arch/riscv/include/asm/suspend.h > > @@ -16,6 +16,10 @@ struct suspend_context { > > unsigned long envcfg; > > unsigned long tvec; > > unsigned long ie; > > +#if __riscv_xlen < 64 > > + unsigned long stimecmph; > > +#endif > > + unsigned long stimecmp; > > #ifdef CONFIG_MMU > > unsigned long satp; > > #endif > > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > > index c8cec0cc5833..3afd86e1abf7 100644 > > --- a/arch/riscv/kernel/suspend.c > > +++ b/arch/riscv/kernel/suspend.c > > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > > context->tvec = csr_read(CSR_TVEC); > > context->ie = csr_read(CSR_IE); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + context->stimecmp = csr_read(CSR_STIMECMP); > > +#if __riscv_xlen < 64 > > + context->stimecmph = csr_read(CSR_STIMECMPH); > > +#endif > > + } > > The suspend save/restore is enabled for the NoMMU kernel as well > (which runs in M-mode) so it is better to save/restore stimecmp CSR > only for MMU enabled kernels (just like satp CSR). > Good point. Will update that in the next version. Thanks for the feedback. > > /* > > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > > * > > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > > csr_write(CSR_TVEC, context->tvec); > > csr_write(CSR_IE, context->ie); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + csr_write(CSR_STIMECMP, context->stimecmp); > > +#if __riscv_xlen < 64 > > + csr_write(CSR_STIMECMPH, context->stimecmph); > > +#endif > > + } > > + > > #ifdef CONFIG_MMU > > csr_write(CSR_SATP, context->satp); > > #endif > > -- > > 2.34.1 > > > > > > Regards, > Anup Regards, Nick _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore 2024-08-29 3:38 ` Nick Hu @ 2024-08-29 7:59 ` Andrew Jones -1 siblings, 0 replies; 26+ messages in thread From: Andrew Jones @ 2024-08-29 7:59 UTC (permalink / raw) To: Nick Hu Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29, 2024 at 11:38:59AM GMT, Nick Hu wrote: > If the HW support the SSTC extension, we should save and restore the > stimecmp register while cpu non retention suspend. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > arch/riscv/include/asm/suspend.h | 4 ++++ > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > index 4ffb022b097f..ffaac2efabb5 100644 > --- a/arch/riscv/include/asm/suspend.h > +++ b/arch/riscv/include/asm/suspend.h > @@ -16,6 +16,10 @@ struct suspend_context { > unsigned long envcfg; > unsigned long tvec; > unsigned long ie; > +#if __riscv_xlen < 64 > + unsigned long stimecmph; > +#endif I'm not sure the reduction in struct size is worth the #ifdeffery. If we just always add stimecmph, then we can also change the #ifdef's below to if's, i.e. if (__riscv_xlen < 64), which should still remove the code from 64-bit builds. Or maybe we need something like #if __riscv_xlen < 64 #define csrh_write(r, v) csr_write(r, v) #else #define csrh_write(r, v) #endif in asm/csr.h and then use it for all the *h csrs, but keep the #if in the struct. Thanks, drew > + unsigned long stimecmp; > #ifdef CONFIG_MMU > unsigned long satp; > #endif > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > index c8cec0cc5833..3afd86e1abf7 100644 > --- a/arch/riscv/kernel/suspend.c > +++ b/arch/riscv/kernel/suspend.c > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > context->tvec = csr_read(CSR_TVEC); > context->ie = csr_read(CSR_IE); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + context->stimecmp = csr_read(CSR_STIMECMP); > +#if __riscv_xlen < 64 > + context->stimecmph = csr_read(CSR_STIMECMPH); > +#endif > + } > /* > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > * > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > csr_write(CSR_TVEC, context->tvec); > csr_write(CSR_IE, context->ie); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + csr_write(CSR_STIMECMP, context->stimecmp); > +#if __riscv_xlen < 64 > + csr_write(CSR_STIMECMPH, context->stimecmph); > +#endif > + } > + > #ifdef CONFIG_MMU > csr_write(CSR_SATP, context->satp); > #endif > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore @ 2024-08-29 7:59 ` Andrew Jones 0 siblings, 0 replies; 26+ messages in thread From: Andrew Jones @ 2024-08-29 7:59 UTC (permalink / raw) To: Nick Hu Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv On Thu, Aug 29, 2024 at 11:38:59AM GMT, Nick Hu wrote: > If the HW support the SSTC extension, we should save and restore the > stimecmp register while cpu non retention suspend. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > arch/riscv/include/asm/suspend.h | 4 ++++ > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > index 4ffb022b097f..ffaac2efabb5 100644 > --- a/arch/riscv/include/asm/suspend.h > +++ b/arch/riscv/include/asm/suspend.h > @@ -16,6 +16,10 @@ struct suspend_context { > unsigned long envcfg; > unsigned long tvec; > unsigned long ie; > +#if __riscv_xlen < 64 > + unsigned long stimecmph; > +#endif I'm not sure the reduction in struct size is worth the #ifdeffery. If we just always add stimecmph, then we can also change the #ifdef's below to if's, i.e. if (__riscv_xlen < 64), which should still remove the code from 64-bit builds. Or maybe we need something like #if __riscv_xlen < 64 #define csrh_write(r, v) csr_write(r, v) #else #define csrh_write(r, v) #endif in asm/csr.h and then use it for all the *h csrs, but keep the #if in the struct. Thanks, drew > + unsigned long stimecmp; > #ifdef CONFIG_MMU > unsigned long satp; > #endif > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > index c8cec0cc5833..3afd86e1abf7 100644 > --- a/arch/riscv/kernel/suspend.c > +++ b/arch/riscv/kernel/suspend.c > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > context->tvec = csr_read(CSR_TVEC); > context->ie = csr_read(CSR_IE); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + context->stimecmp = csr_read(CSR_STIMECMP); > +#if __riscv_xlen < 64 > + context->stimecmph = csr_read(CSR_STIMECMPH); > +#endif > + } > /* > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > * > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > csr_write(CSR_TVEC, context->tvec); > csr_write(CSR_IE, context->ie); > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > + csr_write(CSR_STIMECMP, context->stimecmp); > +#if __riscv_xlen < 64 > + csr_write(CSR_STIMECMPH, context->stimecmph); > +#endif > + } > + > #ifdef CONFIG_MMU > csr_write(CSR_SATP, context->satp); > #endif > -- > 2.34.1 > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore 2024-08-29 7:59 ` Andrew Jones @ 2024-08-30 5:53 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:53 UTC (permalink / raw) To: Andrew Jones Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel Hi Andrew On Thu, Aug 29, 2024 at 3:59 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 11:38:59AM GMT, Nick Hu wrote: > > If the HW support the SSTC extension, we should save and restore the > > stimecmp register while cpu non retention suspend. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > arch/riscv/include/asm/suspend.h | 4 ++++ > > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > > index 4ffb022b097f..ffaac2efabb5 100644 > > --- a/arch/riscv/include/asm/suspend.h > > +++ b/arch/riscv/include/asm/suspend.h > > @@ -16,6 +16,10 @@ struct suspend_context { > > unsigned long envcfg; > > unsigned long tvec; > > unsigned long ie; > > +#if __riscv_xlen < 64 > > + unsigned long stimecmph; > > +#endif > > I'm not sure the reduction in struct size is worth the #ifdeffery. If we > just always add stimecmph, then we can also change the #ifdef's below to > if's, i.e. if (__riscv_xlen < 64), which should still remove the code from > 64-bit builds. > > Or maybe we need something like > > #if __riscv_xlen < 64 > #define csrh_write(r, v) csr_write(r, v) > #else > #define csrh_write(r, v) > #endif > > in asm/csr.h and then use it for all the *h csrs, but keep the #if in > the struct. > > Thanks, > drew > If no other comment, I'll choose the csrh_write() because it can save some memory and update in the next version. Thanks for the suggestion! > > + unsigned long stimecmp; > > #ifdef CONFIG_MMU > > unsigned long satp; > > #endif > > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > > index c8cec0cc5833..3afd86e1abf7 100644 > > --- a/arch/riscv/kernel/suspend.c > > +++ b/arch/riscv/kernel/suspend.c > > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > > context->tvec = csr_read(CSR_TVEC); > > context->ie = csr_read(CSR_IE); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + context->stimecmp = csr_read(CSR_STIMECMP); > > +#if __riscv_xlen < 64 > > + context->stimecmph = csr_read(CSR_STIMECMPH); > > +#endif > > + } > > /* > > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > > * > > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > > csr_write(CSR_TVEC, context->tvec); > > csr_write(CSR_IE, context->ie); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + csr_write(CSR_STIMECMP, context->stimecmp); > > +#if __riscv_xlen < 64 > > + csr_write(CSR_STIMECMPH, context->stimecmph); > > +#endif > > + } > > + > > #ifdef CONFIG_MMU > > csr_write(CSR_SATP, context->satp); > > #endif > > -- > > 2.34.1 > > Regards, Nick ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] riscv: Add stimecmp save and restore @ 2024-08-30 5:53 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:53 UTC (permalink / raw) To: Andrew Jones Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv Hi Andrew On Thu, Aug 29, 2024 at 3:59 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 11:38:59AM GMT, Nick Hu wrote: > > If the HW support the SSTC extension, we should save and restore the > > stimecmp register while cpu non retention suspend. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > arch/riscv/include/asm/suspend.h | 4 ++++ > > arch/riscv/kernel/suspend.c | 13 +++++++++++++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h > > index 4ffb022b097f..ffaac2efabb5 100644 > > --- a/arch/riscv/include/asm/suspend.h > > +++ b/arch/riscv/include/asm/suspend.h > > @@ -16,6 +16,10 @@ struct suspend_context { > > unsigned long envcfg; > > unsigned long tvec; > > unsigned long ie; > > +#if __riscv_xlen < 64 > > + unsigned long stimecmph; > > +#endif > > I'm not sure the reduction in struct size is worth the #ifdeffery. If we > just always add stimecmph, then we can also change the #ifdef's below to > if's, i.e. if (__riscv_xlen < 64), which should still remove the code from > 64-bit builds. > > Or maybe we need something like > > #if __riscv_xlen < 64 > #define csrh_write(r, v) csr_write(r, v) > #else > #define csrh_write(r, v) > #endif > > in asm/csr.h and then use it for all the *h csrs, but keep the #if in > the struct. > > Thanks, > drew > If no other comment, I'll choose the csrh_write() because it can save some memory and update in the next version. Thanks for the suggestion! > > + unsigned long stimecmp; > > #ifdef CONFIG_MMU > > unsigned long satp; > > #endif > > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > > index c8cec0cc5833..3afd86e1abf7 100644 > > --- a/arch/riscv/kernel/suspend.c > > +++ b/arch/riscv/kernel/suspend.c > > @@ -19,6 +19,12 @@ void suspend_save_csrs(struct suspend_context *context) > > context->tvec = csr_read(CSR_TVEC); > > context->ie = csr_read(CSR_IE); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + context->stimecmp = csr_read(CSR_STIMECMP); > > +#if __riscv_xlen < 64 > > + context->stimecmph = csr_read(CSR_STIMECMPH); > > +#endif > > + } > > /* > > * No need to save/restore IP CSR (i.e. MIP or SIP) because: > > * > > @@ -42,6 +48,13 @@ void suspend_restore_csrs(struct suspend_context *context) > > csr_write(CSR_TVEC, context->tvec); > > csr_write(CSR_IE, context->ie); > > > > + if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) { > > + csr_write(CSR_STIMECMP, context->stimecmp); > > +#if __riscv_xlen < 64 > > + csr_write(CSR_STIMECMPH, context->stimecmph); > > +#endif > > + } > > + > > #ifdef CONFIG_MMU > > csr_write(CSR_SATP, context->satp); > > #endif > > -- > > 2.34.1 > > Regards, Nick _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 3:38 ` Nick Hu @ 2024-08-29 3:39 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:39 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Nick Hu, Sunil V L, linux-pm, linux-riscv, linux-kernel Stop the stimecmp when the cpu is going to be off otherwise the timer interrupt may pending while performing power down operation. Signed-off-by: Nick Hu <nick.hu@sifive.com> --- drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 48ce50c5f5e6..9a6acaa8dfb0 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -32,15 +32,19 @@ static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); static bool riscv_timer_cannot_wake_cpu; +static void riscv_clock_stop_stimecmp(void) +{ + csr_write(CSR_STIMECMP, ULONG_MAX); + if (IS_ENABLED(CONFIG_32BIT)) + csr_write(CSR_STIMECMPH, ULONG_MAX); +} + static void riscv_clock_event_stop(void) { - if (static_branch_likely(&riscv_sstc_available)) { - csr_write(CSR_STIMECMP, ULONG_MAX); - if (IS_ENABLED(CONFIG_32BIT)) - csr_write(CSR_STIMECMPH, ULONG_MAX); - } else { + if (static_branch_likely(&riscv_sstc_available)) + riscv_clock_stop_stimecmp(); + else sbi_set_timer(U64_MAX); - } } static int riscv_clock_next_event(unsigned long delta, @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) static int riscv_timer_dying_cpu(unsigned int cpu) { - disable_percpu_irq(riscv_clock_event_irq); + if (static_branch_likely(&riscv_sstc_available)) + riscv_clock_stop_stimecmp(); + else + disable_percpu_irq(riscv_clock_event_irq); + return 0; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-29 3:39 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 3:39 UTC (permalink / raw) To: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Nick Hu, Sunil V L, linux-pm, linux-riscv, linux-kernel Stop the stimecmp when the cpu is going to be off otherwise the timer interrupt may pending while performing power down operation. Signed-off-by: Nick Hu <nick.hu@sifive.com> --- drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 48ce50c5f5e6..9a6acaa8dfb0 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -32,15 +32,19 @@ static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); static bool riscv_timer_cannot_wake_cpu; +static void riscv_clock_stop_stimecmp(void) +{ + csr_write(CSR_STIMECMP, ULONG_MAX); + if (IS_ENABLED(CONFIG_32BIT)) + csr_write(CSR_STIMECMPH, ULONG_MAX); +} + static void riscv_clock_event_stop(void) { - if (static_branch_likely(&riscv_sstc_available)) { - csr_write(CSR_STIMECMP, ULONG_MAX); - if (IS_ENABLED(CONFIG_32BIT)) - csr_write(CSR_STIMECMPH, ULONG_MAX); - } else { + if (static_branch_likely(&riscv_sstc_available)) + riscv_clock_stop_stimecmp(); + else sbi_set_timer(U64_MAX); - } } static int riscv_clock_next_event(unsigned long delta, @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) static int riscv_timer_dying_cpu(unsigned int cpu) { - disable_percpu_irq(riscv_clock_event_irq); + if (static_branch_likely(&riscv_sstc_available)) + riscv_clock_stop_stimecmp(); + else + disable_percpu_irq(riscv_clock_event_irq); + return 0; } -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 3:39 ` Nick Hu @ 2024-08-29 5:18 ` Anup Patel -1 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 5:18 UTC (permalink / raw) To: Nick Hu Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > Stop the stimecmp when the cpu is going to be off otherwise the timer > interrupt may pending while performing power down operation. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -32,15 +32,19 @@ > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > static bool riscv_timer_cannot_wake_cpu; > > +static void riscv_clock_stop_stimecmp(void) > +{ > + csr_write(CSR_STIMECMP, ULONG_MAX); > + if (IS_ENABLED(CONFIG_32BIT)) > + csr_write(CSR_STIMECMPH, ULONG_MAX); > +} > + > static void riscv_clock_event_stop(void) > { > - if (static_branch_likely(&riscv_sstc_available)) { > - csr_write(CSR_STIMECMP, ULONG_MAX); > - if (IS_ENABLED(CONFIG_32BIT)) > - csr_write(CSR_STIMECMPH, ULONG_MAX); > - } else { > + if (static_branch_likely(&riscv_sstc_available)) > + riscv_clock_stop_stimecmp(); > + else > sbi_set_timer(U64_MAX); > - } > } > > static int riscv_clock_next_event(unsigned long delta, > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > static int riscv_timer_dying_cpu(unsigned int cpu) > { > - disable_percpu_irq(riscv_clock_event_irq); > + if (static_branch_likely(&riscv_sstc_available)) > + riscv_clock_stop_stimecmp(); > + else > + disable_percpu_irq(riscv_clock_event_irq); > + Not disabling riscv_clock_event_irq here for Sstc would now cause riscv_timer_starting_cpu() to unnecessarily enable it when the CPU is powered-up. I think the below change is sufficient for this patch: diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 48ce50c5f5e6..546fd248f4ff 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) static int riscv_timer_dying_cpu(unsigned int cpu) { disable_percpu_irq(riscv_clock_event_irq); + /* + * Stop the timer when the cpu is going to be offline otherwise + * the timer interrupt may be pending while performing power-down. + */ + riscv_clock_event_stop(); return 0; } Regards, Anup ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-29 5:18 ` Anup Patel 0 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 5:18 UTC (permalink / raw) To: Nick Hu Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > Stop the stimecmp when the cpu is going to be off otherwise the timer > interrupt may pending while performing power down operation. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > --- > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -32,15 +32,19 @@ > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > static bool riscv_timer_cannot_wake_cpu; > > +static void riscv_clock_stop_stimecmp(void) > +{ > + csr_write(CSR_STIMECMP, ULONG_MAX); > + if (IS_ENABLED(CONFIG_32BIT)) > + csr_write(CSR_STIMECMPH, ULONG_MAX); > +} > + > static void riscv_clock_event_stop(void) > { > - if (static_branch_likely(&riscv_sstc_available)) { > - csr_write(CSR_STIMECMP, ULONG_MAX); > - if (IS_ENABLED(CONFIG_32BIT)) > - csr_write(CSR_STIMECMPH, ULONG_MAX); > - } else { > + if (static_branch_likely(&riscv_sstc_available)) > + riscv_clock_stop_stimecmp(); > + else > sbi_set_timer(U64_MAX); > - } > } > > static int riscv_clock_next_event(unsigned long delta, > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > static int riscv_timer_dying_cpu(unsigned int cpu) > { > - disable_percpu_irq(riscv_clock_event_irq); > + if (static_branch_likely(&riscv_sstc_available)) > + riscv_clock_stop_stimecmp(); > + else > + disable_percpu_irq(riscv_clock_event_irq); > + Not disabling riscv_clock_event_irq here for Sstc would now cause riscv_timer_starting_cpu() to unnecessarily enable it when the CPU is powered-up. I think the below change is sufficient for this patch: diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 48ce50c5f5e6..546fd248f4ff 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) static int riscv_timer_dying_cpu(unsigned int cpu) { disable_percpu_irq(riscv_clock_event_irq); + /* + * Stop the timer when the cpu is going to be offline otherwise + * the timer interrupt may be pending while performing power-down. + */ + riscv_clock_event_stop(); return 0; } Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 5:18 ` Anup Patel @ 2024-08-29 6:23 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 6:23 UTC (permalink / raw) To: Anup Patel Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel Hi Anup On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > interrupt may pending while performing power down operation. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -32,15 +32,19 @@ > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > static bool riscv_timer_cannot_wake_cpu; > > > > +static void riscv_clock_stop_stimecmp(void) > > +{ > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > + if (IS_ENABLED(CONFIG_32BIT)) > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > +} > > + > > static void riscv_clock_event_stop(void) > > { > > - if (static_branch_likely(&riscv_sstc_available)) { > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > - if (IS_ENABLED(CONFIG_32BIT)) > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > - } else { > > + if (static_branch_likely(&riscv_sstc_available)) > > + riscv_clock_stop_stimecmp(); > > + else > > sbi_set_timer(U64_MAX); > > - } > > } > > > > static int riscv_clock_next_event(unsigned long delta, > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > { > > - disable_percpu_irq(riscv_clock_event_irq); > > + if (static_branch_likely(&riscv_sstc_available)) > > + riscv_clock_stop_stimecmp(); > > + else > > + disable_percpu_irq(riscv_clock_event_irq); > > + > > Not disabling riscv_clock_event_irq here for Sstc would now > cause riscv_timer_starting_cpu() to unnecessarily enable it > when the CPU is powered-up. > > I think the below change is sufficient for this patch: > > diff --git a/drivers/clocksource/timer-riscv.c > b/drivers/clocksource/timer-riscv.c > index 48ce50c5f5e6..546fd248f4ff 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > static int riscv_timer_dying_cpu(unsigned int cpu) > { > disable_percpu_irq(riscv_clock_event_irq); > + /* > + * Stop the timer when the cpu is going to be offline otherwise > + * the timer interrupt may be pending while performing power-down. > + */ > + riscv_clock_event_stop(); > return 0; > } > The sbi_exit() of OpenSBI will disable mtimecmp when sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is no SSTC. How about adding a SSTC available check before the riscv_clock_event_stop? /* * Stop the timer when the cpu is going to be offline otherwise * the timer interrupt may be pending while performing power-down. */ if (static_branch_likely(&riscv_sstc_available)) riscv_clock_event_stop(); > Regards, > Anup ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-29 6:23 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-29 6:23 UTC (permalink / raw) To: Anup Patel Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones Hi Anup On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > interrupt may pending while performing power down operation. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > --- > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -32,15 +32,19 @@ > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > static bool riscv_timer_cannot_wake_cpu; > > > > +static void riscv_clock_stop_stimecmp(void) > > +{ > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > + if (IS_ENABLED(CONFIG_32BIT)) > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > +} > > + > > static void riscv_clock_event_stop(void) > > { > > - if (static_branch_likely(&riscv_sstc_available)) { > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > - if (IS_ENABLED(CONFIG_32BIT)) > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > - } else { > > + if (static_branch_likely(&riscv_sstc_available)) > > + riscv_clock_stop_stimecmp(); > > + else > > sbi_set_timer(U64_MAX); > > - } > > } > > > > static int riscv_clock_next_event(unsigned long delta, > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > { > > - disable_percpu_irq(riscv_clock_event_irq); > > + if (static_branch_likely(&riscv_sstc_available)) > > + riscv_clock_stop_stimecmp(); > > + else > > + disable_percpu_irq(riscv_clock_event_irq); > > + > > Not disabling riscv_clock_event_irq here for Sstc would now > cause riscv_timer_starting_cpu() to unnecessarily enable it > when the CPU is powered-up. > > I think the below change is sufficient for this patch: > > diff --git a/drivers/clocksource/timer-riscv.c > b/drivers/clocksource/timer-riscv.c > index 48ce50c5f5e6..546fd248f4ff 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > static int riscv_timer_dying_cpu(unsigned int cpu) > { > disable_percpu_irq(riscv_clock_event_irq); > + /* > + * Stop the timer when the cpu is going to be offline otherwise > + * the timer interrupt may be pending while performing power-down. > + */ > + riscv_clock_event_stop(); > return 0; > } > The sbi_exit() of OpenSBI will disable mtimecmp when sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is no SSTC. How about adding a SSTC available check before the riscv_clock_event_stop? /* * Stop the timer when the cpu is going to be offline otherwise * the timer interrupt may be pending while performing power-down. */ if (static_branch_likely(&riscv_sstc_available)) riscv_clock_event_stop(); > Regards, > Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 6:23 ` Nick Hu @ 2024-08-29 6:49 ` Anup Patel -1 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 6:49 UTC (permalink / raw) To: Nick Hu Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29, 2024 at 11:53 AM Nick Hu <nick.hu@sifive.com> wrote: > > Hi Anup > > On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > > interrupt may pending while performing power down operation. > > > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > > --- > > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > > --- a/drivers/clocksource/timer-riscv.c > > > +++ b/drivers/clocksource/timer-riscv.c > > > @@ -32,15 +32,19 @@ > > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > > static bool riscv_timer_cannot_wake_cpu; > > > > > > +static void riscv_clock_stop_stimecmp(void) > > > +{ > > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > > + if (IS_ENABLED(CONFIG_32BIT)) > > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > > +} > > > + > > > static void riscv_clock_event_stop(void) > > > { > > > - if (static_branch_likely(&riscv_sstc_available)) { > > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > > - if (IS_ENABLED(CONFIG_32BIT)) > > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > > - } else { > > > + if (static_branch_likely(&riscv_sstc_available)) > > > + riscv_clock_stop_stimecmp(); > > > + else > > > sbi_set_timer(U64_MAX); > > > - } > > > } > > > > > > static int riscv_clock_next_event(unsigned long delta, > > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > { > > > - disable_percpu_irq(riscv_clock_event_irq); > > > + if (static_branch_likely(&riscv_sstc_available)) > > > + riscv_clock_stop_stimecmp(); > > > + else > > > + disable_percpu_irq(riscv_clock_event_irq); > > > + > > > > Not disabling riscv_clock_event_irq here for Sstc would now > > cause riscv_timer_starting_cpu() to unnecessarily enable it > > when the CPU is powered-up. > > > > I think the below change is sufficient for this patch: > > > > diff --git a/drivers/clocksource/timer-riscv.c > > b/drivers/clocksource/timer-riscv.c > > index 48ce50c5f5e6..546fd248f4ff 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > static int riscv_timer_dying_cpu(unsigned int cpu) > > { > > disable_percpu_irq(riscv_clock_event_irq); > > + /* > > + * Stop the timer when the cpu is going to be offline otherwise > > + * the timer interrupt may be pending while performing power-down. > > + */ > > + riscv_clock_event_stop(); > > return 0; > > } > > > The sbi_exit() of OpenSBI will disable mtimecmp when > sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is > no SSTC. > How about adding a SSTC available check before the riscv_clock_event_stop? Currently, OpenSBI uses mtimecmp only for implementing SBI time extension but in the future, OpenSBI might implement a per-hart timer event list to allow M-mode timer events. Don't assume in what environment the driver is running. It is possible that the driver is running under a hypervisor which only provides SBI time extension and does not virtualize Sstc extension. > > /* > * Stop the timer when the cpu is going to be offline otherwise > * the timer interrupt may be pending while performing power-down. > */ > if (static_branch_likely(&riscv_sstc_available)) > riscv_clock_event_stop(); > Regards, Anup ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-29 6:49 ` Anup Patel 0 siblings, 0 replies; 26+ messages in thread From: Anup Patel @ 2024-08-29 6:49 UTC (permalink / raw) To: Nick Hu Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones On Thu, Aug 29, 2024 at 11:53 AM Nick Hu <nick.hu@sifive.com> wrote: > > Hi Anup > > On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > > interrupt may pending while performing power down operation. > > > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > > --- > > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > > --- a/drivers/clocksource/timer-riscv.c > > > +++ b/drivers/clocksource/timer-riscv.c > > > @@ -32,15 +32,19 @@ > > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > > static bool riscv_timer_cannot_wake_cpu; > > > > > > +static void riscv_clock_stop_stimecmp(void) > > > +{ > > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > > + if (IS_ENABLED(CONFIG_32BIT)) > > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > > +} > > > + > > > static void riscv_clock_event_stop(void) > > > { > > > - if (static_branch_likely(&riscv_sstc_available)) { > > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > > - if (IS_ENABLED(CONFIG_32BIT)) > > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > > - } else { > > > + if (static_branch_likely(&riscv_sstc_available)) > > > + riscv_clock_stop_stimecmp(); > > > + else > > > sbi_set_timer(U64_MAX); > > > - } > > > } > > > > > > static int riscv_clock_next_event(unsigned long delta, > > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > { > > > - disable_percpu_irq(riscv_clock_event_irq); > > > + if (static_branch_likely(&riscv_sstc_available)) > > > + riscv_clock_stop_stimecmp(); > > > + else > > > + disable_percpu_irq(riscv_clock_event_irq); > > > + > > > > Not disabling riscv_clock_event_irq here for Sstc would now > > cause riscv_timer_starting_cpu() to unnecessarily enable it > > when the CPU is powered-up. > > > > I think the below change is sufficient for this patch: > > > > diff --git a/drivers/clocksource/timer-riscv.c > > b/drivers/clocksource/timer-riscv.c > > index 48ce50c5f5e6..546fd248f4ff 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > static int riscv_timer_dying_cpu(unsigned int cpu) > > { > > disable_percpu_irq(riscv_clock_event_irq); > > + /* > > + * Stop the timer when the cpu is going to be offline otherwise > > + * the timer interrupt may be pending while performing power-down. > > + */ > > + riscv_clock_event_stop(); > > return 0; > > } > > > The sbi_exit() of OpenSBI will disable mtimecmp when > sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is > no SSTC. > How about adding a SSTC available check before the riscv_clock_event_stop? Currently, OpenSBI uses mtimecmp only for implementing SBI time extension but in the future, OpenSBI might implement a per-hart timer event list to allow M-mode timer events. Don't assume in what environment the driver is running. It is possible that the driver is running under a hypervisor which only provides SBI time extension and does not virtualize Sstc extension. > > /* > * Stop the timer when the cpu is going to be offline otherwise > * the timer interrupt may be pending while performing power-down. > */ > if (static_branch_likely(&riscv_sstc_available)) > riscv_clock_event_stop(); > Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 6:49 ` Anup Patel @ 2024-08-30 5:56 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:56 UTC (permalink / raw) To: Anup Patel Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Thomas Gleixner, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel Hi Anup On Thu, Aug 29, 2024 at 2:49 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 11:53 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > Hi Anup > > > > On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > > > interrupt may pending while performing power down operation. > > > > > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > > > --- > > > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > > > --- a/drivers/clocksource/timer-riscv.c > > > > +++ b/drivers/clocksource/timer-riscv.c > > > > @@ -32,15 +32,19 @@ > > > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > > > static bool riscv_timer_cannot_wake_cpu; > > > > > > > > +static void riscv_clock_stop_stimecmp(void) > > > > +{ > > > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > > > + if (IS_ENABLED(CONFIG_32BIT)) > > > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > > > +} > > > > + > > > > static void riscv_clock_event_stop(void) > > > > { > > > > - if (static_branch_likely(&riscv_sstc_available)) { > > > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > > > - if (IS_ENABLED(CONFIG_32BIT)) > > > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > > > - } else { > > > > + if (static_branch_likely(&riscv_sstc_available)) > > > > + riscv_clock_stop_stimecmp(); > > > > + else > > > > sbi_set_timer(U64_MAX); > > > > - } > > > > } > > > > > > > > static int riscv_clock_next_event(unsigned long delta, > > > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > > { > > > > - disable_percpu_irq(riscv_clock_event_irq); > > > > + if (static_branch_likely(&riscv_sstc_available)) > > > > + riscv_clock_stop_stimecmp(); > > > > + else > > > > + disable_percpu_irq(riscv_clock_event_irq); > > > > + > > > > > > Not disabling riscv_clock_event_irq here for Sstc would now > > > cause riscv_timer_starting_cpu() to unnecessarily enable it > > > when the CPU is powered-up. > > > > > > I think the below change is sufficient for this patch: > > > > > > diff --git a/drivers/clocksource/timer-riscv.c > > > b/drivers/clocksource/timer-riscv.c > > > index 48ce50c5f5e6..546fd248f4ff 100644 > > > --- a/drivers/clocksource/timer-riscv.c > > > +++ b/drivers/clocksource/timer-riscv.c > > > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > { > > > disable_percpu_irq(riscv_clock_event_irq); > > > + /* > > > + * Stop the timer when the cpu is going to be offline otherwise > > > + * the timer interrupt may be pending while performing power-down. > > > + */ > > > + riscv_clock_event_stop(); > > > return 0; > > > } > > > > > The sbi_exit() of OpenSBI will disable mtimecmp when > > sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is > > no SSTC. > > How about adding a SSTC available check before the riscv_clock_event_stop? > > Currently, OpenSBI uses mtimecmp only for implementing > SBI time extension but in the future, OpenSBI might implement > a per-hart timer event list to allow M-mode timer events. > > Don't assume in what environment the driver is running. > > It is possible that the driver is running under a hypervisor > which only provides SBI time extension and does not virtualize > Sstc extension. > I see, Thank you for the explanation! > > > > /* > > * Stop the timer when the cpu is going to be offline otherwise > > * the timer interrupt may be pending while performing power-down. > > */ > > if (static_branch_likely(&riscv_sstc_available)) > > riscv_clock_event_stop(); > > > I'll update this one in the next version. > Regards, > Anup Regards, Nick ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-30 5:56 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:56 UTC (permalink / raw) To: Anup Patel Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, Thomas Gleixner, linux-riscv, Andrew Jones Hi Anup On Thu, Aug 29, 2024 at 2:49 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Thu, Aug 29, 2024 at 11:53 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > Hi Anup > > > > On Thu, Aug 29, 2024 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > On Thu, Aug 29, 2024 at 9:10 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > > > > > Stop the stimecmp when the cpu is going to be off otherwise the timer > > > > interrupt may pending while performing power down operation. > > > > > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > > > --- > > > > drivers/clocksource/timer-riscv.c | 22 +++++++++++++++------- > > > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > > > index 48ce50c5f5e6..9a6acaa8dfb0 100644 > > > > --- a/drivers/clocksource/timer-riscv.c > > > > +++ b/drivers/clocksource/timer-riscv.c > > > > @@ -32,15 +32,19 @@ > > > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > > > static bool riscv_timer_cannot_wake_cpu; > > > > > > > > +static void riscv_clock_stop_stimecmp(void) > > > > +{ > > > > + csr_write(CSR_STIMECMP, ULONG_MAX); > > > > + if (IS_ENABLED(CONFIG_32BIT)) > > > > + csr_write(CSR_STIMECMPH, ULONG_MAX); > > > > +} > > > > + > > > > static void riscv_clock_event_stop(void) > > > > { > > > > - if (static_branch_likely(&riscv_sstc_available)) { > > > > - csr_write(CSR_STIMECMP, ULONG_MAX); > > > > - if (IS_ENABLED(CONFIG_32BIT)) > > > > - csr_write(CSR_STIMECMPH, ULONG_MAX); > > > > - } else { > > > > + if (static_branch_likely(&riscv_sstc_available)) > > > > + riscv_clock_stop_stimecmp(); > > > > + else > > > > sbi_set_timer(U64_MAX); > > > > - } > > > > } > > > > > > > > static int riscv_clock_next_event(unsigned long delta, > > > > @@ -126,7 +130,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > > > > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > > { > > > > - disable_percpu_irq(riscv_clock_event_irq); > > > > + if (static_branch_likely(&riscv_sstc_available)) > > > > + riscv_clock_stop_stimecmp(); > > > > + else > > > > + disable_percpu_irq(riscv_clock_event_irq); > > > > + > > > > > > Not disabling riscv_clock_event_irq here for Sstc would now > > > cause riscv_timer_starting_cpu() to unnecessarily enable it > > > when the CPU is powered-up. > > > > > > I think the below change is sufficient for this patch: > > > > > > diff --git a/drivers/clocksource/timer-riscv.c > > > b/drivers/clocksource/timer-riscv.c > > > index 48ce50c5f5e6..546fd248f4ff 100644 > > > --- a/drivers/clocksource/timer-riscv.c > > > +++ b/drivers/clocksource/timer-riscv.c > > > @@ -127,6 +127,11 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > > > static int riscv_timer_dying_cpu(unsigned int cpu) > > > { > > > disable_percpu_irq(riscv_clock_event_irq); > > > + /* > > > + * Stop the timer when the cpu is going to be offline otherwise > > > + * the timer interrupt may be pending while performing power-down. > > > + */ > > > + riscv_clock_event_stop(); > > > return 0; > > > } > > > > > The sbi_exit() of OpenSBI will disable mtimecmp when > > sbi_hsm_hart_stop() so the mtimecmp will be disabled twice if there is > > no SSTC. > > How about adding a SSTC available check before the riscv_clock_event_stop? > > Currently, OpenSBI uses mtimecmp only for implementing > SBI time extension but in the future, OpenSBI might implement > a per-hart timer event list to allow M-mode timer events. > > Don't assume in what environment the driver is running. > > It is possible that the driver is running under a hypervisor > which only provides SBI time extension and does not virtualize > Sstc extension. > I see, Thank you for the explanation! > > > > /* > > * Stop the timer when the cpu is going to be offline otherwise > > * the timer interrupt may be pending while performing power-down. > > */ > > if (static_branch_likely(&riscv_sstc_available)) > > riscv_clock_event_stop(); > > > I'll update this one in the next version. > Regards, > Anup Regards, Nick _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 3:39 ` Nick Hu @ 2024-08-29 13:43 ` Thomas Gleixner -1 siblings, 0 replies; 26+ messages in thread From: Thomas Gleixner @ 2024-08-29 13:43 UTC (permalink / raw) To: Nick Hu, greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Andrew Jones, Conor Dooley, Samuel Holland, Nick Hu, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29 2024 at 11:39, Nick Hu wrote: clocksource/drivers/timer-riscv: is the proper prefix Thanks, tglx ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-29 13:43 ` Thomas Gleixner 0 siblings, 0 replies; 26+ messages in thread From: Thomas Gleixner @ 2024-08-29 13:43 UTC (permalink / raw) To: Nick Hu, greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Andrew Jones, Conor Dooley, Samuel Holland, Nick Hu, Sunil V L, linux-pm, linux-riscv, linux-kernel On Thu, Aug 29 2024 at 11:39, Nick Hu wrote: clocksource/drivers/timer-riscv: is the proper prefix Thanks, tglx _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug 2024-08-29 13:43 ` Thomas Gleixner @ 2024-08-30 5:56 ` Nick Hu -1 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:56 UTC (permalink / raw) To: Thomas Gleixner Cc: greentime.hu, zong.li, Rafael J. Wysocki, Pavel Machek, Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano, Andrew Jones, Conor Dooley, Samuel Holland, Sunil V L, linux-pm, linux-riscv, linux-kernel Hi Thomas On Thu, Aug 29, 2024 at 9:43 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Thu, Aug 29 2024 at 11:39, Nick Hu wrote: > > clocksource/drivers/timer-riscv: is the proper prefix > > Thanks, > > tglx > Thanks, I'll update it in the next version Regards, Nick ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug @ 2024-08-30 5:56 ` Nick Hu 0 siblings, 0 replies; 26+ messages in thread From: Nick Hu @ 2024-08-30 5:56 UTC (permalink / raw) To: Thomas Gleixner Cc: Albert Ou, zong.li, Rafael J. Wysocki, linux-pm, Daniel Lezcano, linux-kernel, Samuel Holland, Conor Dooley, Palmer Dabbelt, Pavel Machek, Paul Walmsley, greentime.hu, linux-riscv, Andrew Jones Hi Thomas On Thu, Aug 29, 2024 at 9:43 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Thu, Aug 29 2024 at 11:39, Nick Hu wrote: > > clocksource/drivers/timer-riscv: is the proper prefix > > Thanks, > > tglx > Thanks, I'll update it in the next version Regards, Nick _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-08-30 7:01 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-29 3:38 [PATCH 0/2] Support SSTC while PM operations Nick Hu 2024-08-29 3:38 ` Nick Hu 2024-08-29 3:38 ` [PATCH 1/2] riscv: Add stimecmp save and restore Nick Hu 2024-08-29 3:38 ` Nick Hu 2024-08-29 5:18 ` Anup Patel 2024-08-29 5:18 ` Anup Patel 2024-08-29 6:16 ` Nick Hu 2024-08-29 6:16 ` Nick Hu 2024-08-29 7:59 ` Andrew Jones 2024-08-29 7:59 ` Andrew Jones 2024-08-30 5:53 ` Nick Hu 2024-08-30 5:53 ` Nick Hu 2024-08-29 3:39 ` [PATCH 2/2] time-riscv: Stop stimecmp when cpu hotplug Nick Hu 2024-08-29 3:39 ` Nick Hu 2024-08-29 5:18 ` Anup Patel 2024-08-29 5:18 ` Anup Patel 2024-08-29 6:23 ` Nick Hu 2024-08-29 6:23 ` Nick Hu 2024-08-29 6:49 ` Anup Patel 2024-08-29 6:49 ` Anup Patel 2024-08-30 5:56 ` Nick Hu 2024-08-30 5:56 ` Nick Hu 2024-08-29 13:43 ` Thomas Gleixner 2024-08-29 13:43 ` Thomas Gleixner 2024-08-30 5:56 ` Nick Hu 2024-08-30 5:56 ` Nick Hu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.