| Message ID | 20240829033904.477200-2-nick.hu@sifive.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support SSTC while PM operations | expand |
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
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
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 >
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
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
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(+)