Messages in this thread | | | From | Nick Hu <> | Date | Wed, 23 Aug 2023 11:08:04 +0800 | Subject | Re: [PATCH v2] riscv: suspend: Add syscore ops for suspend |
| |
Hi Anup
Thanks for your feedbacks On Tue, Aug 22, 2023 at 5:17 PM Anup Patel <anup@brainfault.org> wrote: > > On Tue, Aug 22, 2023 at 1:28 PM Nick Hu <nick.hu@sifive.com> wrote: > > > > Hi Anup > > > > > > On Tue, Aug 22, 2023 at 2:16 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Tue, Aug 22, 2023 at 11:17 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > > > > > If a task is the one who performs the suspend flow and it also do some > > > > floating or vector operations before the suspend, we might lose the FPU > > > > and vector states when it goes to the non-retention system suspend state. > > > > Add syscore ops to save and restore the FPU and vector states of the > > > > current task to fix the issue above. > > > > > > This only applies to non-retentive system suspend so why do we need > > > this before SBI system suspend is merged ? > > > > > > Regards, > > > Anup > > > > > > > > How about hibernation? > > If this is for hibernation then the commit description should say so. > Actually this commit is for the suspend-to-ram. I ask about hibernation here because I think hibernation may also have the same issue too. If hibernation doesn't need it, I can defer this patch until we have system suspend. I send this patch because I believe we will support system suspend in the kernel soon or later and the hibernation might need it now.
> Adding syscore_ops would mean these callbacks will be called for > both suspend-to-ram and hibernate cases. Other architectures don't > have this save/restore for suspend-to-ram because it is generally > called from idle task. > Do you think we don't have to consider the case in the patch?
Regards, Nick > Why not do the save/restore in save_processor_state() and > restore_processor_state() just like arch/mips/power/cpu.c ? > > Regards, > Anup > > > > > > Regards, > > Nick > > > > > > > > Co-developed-by: Andy Chiu <andy.chiu@sifive.com> > > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > > > --- > > > > Changes in v2: > > > > a) Add Co-developed-by and adjust the order of signed-off > > > > b) Rephrase the commit message > > > > > > > > arch/riscv/kernel/suspend.c | 45 +++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 45 insertions(+) > > > > > > > > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c > > > > index 3c89b8ec69c4..ff69ff8a1974 100644 > > > > --- a/arch/riscv/kernel/suspend.c > > > > +++ b/arch/riscv/kernel/suspend.c > > > > @@ -4,9 +4,14 @@ > > > > * Copyright (c) 2022 Ventana Micro Systems Inc. > > > > */ > > > > > > > > +#include <linux/cpu_pm.h> > > > > #include <linux/ftrace.h> > > > > +#include <linux/thread_info.h> > > > > +#include <linux/syscore_ops.h> > > > > #include <asm/csr.h> > > > > #include <asm/suspend.h> > > > > +#include <asm/switch_to.h> > > > > +#include <asm/vector.h> > > > > > > > > void suspend_save_csrs(struct suspend_context *context) > > > > { > > > > @@ -85,3 +90,43 @@ int cpu_suspend(unsigned long arg, > > > > > > > > return rc; > > > > } > > > > + > > > > +static int riscv_cpu_suspend(void) > > > > +{ > > > > + struct task_struct *cur_task = get_current(); > > > > + struct pt_regs *regs = task_pt_regs(cur_task); > > > > + > > > > + if (has_fpu()) { > > > > + if (unlikely(regs->status & SR_SD)) > > > > + fstate_save(cur_task, regs); > > > > + } > > > > + if (has_vector()) { > > > > + if (unlikely(regs->status & SR_SD)) > > > > + riscv_v_vstate_save(cur_task, regs); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static void riscv_cpu_resume(void) > > > > +{ > > > > + struct task_struct *cur_task = get_current(); > > > > + struct pt_regs *regs = task_pt_regs(cur_task); > > > > + > > > > + if (has_fpu()) > > > > + fstate_restore(cur_task, regs); > > > > + if (has_vector()) > > > > + riscv_v_vstate_restore(cur_task, regs); > > > > +} > > > > + > > > > +static struct syscore_ops riscv_cpu_syscore_ops = { > > > > + .suspend = riscv_cpu_suspend, > > > > + .resume = riscv_cpu_resume, > > > > +}; > > > > + > > > > +static int __init riscv_cpu_suspend_init(void) > > > > +{ > > > > + register_syscore_ops(&riscv_cpu_syscore_ops); > > > > + return 0; > > > > +} > > > > +arch_initcall(riscv_cpu_suspend_init); > > > > -- > > > > 2.34.1 > > > >
| |