* [PATCH 0/2] riscv: Correct the initialized flow of FP and __fstate_clean() @ 2019-08-08 7:58 Vincent Chen 2019-08-08 7:58 ` [PATCH 1/2] riscv: Correct the initialized flow of FP register Vincent Chen 2019-08-08 7:58 ` [PATCH 2/2] riscv: Make __fstate_clean() can work correctly Vincent Chen 0 siblings, 2 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-08 7:58 UTC (permalink / raw) To: paul.walmsley, palmer, aou; +Cc: Vincent Chen, linux-riscv, linux-kernel The following two reasons cause FP registers are sometimes not initialized before starting the user program. 1. Currently, the FP context is initialized in flush_thread() function and we expect these initial values to be restored to FP register when doing FP context switch. However, the FP context switch only occurs in switch_to function. Hence, if this process does not be scheduled out and scheduled in before entering the user space, the FP registers have no chance to initialize. 2. In flush_thread(), the state of reg->sstatus.FS inherits from the parent. Hence, the state of reg->sstatus.FS may be dirty. If this process is scheduled out during flush_thread() and initializing the FP register, the fstate_save() in switch_to will corrupt the FP context which has been initialized until flush_thread(). In addition, the __fstate_clean() function cannot correctly set the state of sstatus.FS to SR_FS_CLEAN. These problems will be solved in this patch set. Vincent Chen (2): riscv: Correct the initialized flow of FP register riscv: Make __fstate_clean() can work correctly. arch/riscv/include/asm/switch_to.h | 8 +++++++- arch/riscv/kernel/process.c | 13 +++++++++++-- 2 files changed, 18 insertions(+), 3 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] 13+ messages in thread
* [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-08 7:58 [PATCH 0/2] riscv: Correct the initialized flow of FP and __fstate_clean() Vincent Chen @ 2019-08-08 7:58 ` Vincent Chen 2019-08-08 10:15 ` Anup Patel 2019-08-12 14:58 ` Christoph Hellwig 2019-08-08 7:58 ` [PATCH 2/2] riscv: Make __fstate_clean() can work correctly Vincent Chen 1 sibling, 2 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-08 7:58 UTC (permalink / raw) To: paul.walmsley, palmer, aou; +Cc: Vincent Chen, linux-riscv, linux-kernel The following two reasons cause FP registers are sometimes not initialized before starting the user program. 1. Currently, the FP context is initialized in flush_thread() function and we expect these initial values to be restored to FP register when doing FP context switch. However, the FP context switch only occurs in switch_to function. Hence, if this process does not be scheduled out and scheduled in before entering the user space, the FP registers have no chance to initialize. 2. In flush_thread(), the state of reg->sstatus.FS inherits from the parent. Hence, the state of reg->sstatus.FS may be dirty. If this process is scheduled out during flush_thread() and initializing the FP register, the fstate_save() in switch_to will corrupt the FP context which has been initialized until flush_thread(). To solve the 1st case, the initialization of the FP register will be completed in start_thread(). It makes sure all FP registers are initialized before starting the user program. For the 2nd case, the state of reg->sstatus.FS in start_thread will be set to SR_FS_OFF to prevent this process from corrupting FP context in doing context save. The FP state is set to SR_FS_INITIAL in start_trhead(). Tested on both QEMU and HiFive Unleashed using BBL + Linux. Signed-off-by: Vincent Chen <vincent.chen@sifive.com> --- arch/riscv/include/asm/switch_to.h | 6 ++++++ arch/riscv/kernel/process.c | 13 +++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index 853b65e..d5fe573 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -19,6 +19,12 @@ static inline void __fstate_clean(struct pt_regs *regs) regs->sstatus |= (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; } +static inline void fstate_off(struct task_struct *task, + struct pt_regs *regs) +{ + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; +} + static inline void fstate_save(struct task_struct *task, struct pt_regs *regs) { diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index f23794b..e3077ee 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -64,8 +64,16 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp) { regs->sstatus = SR_SPIE; - if (has_fpu) + if (has_fpu) { regs->sstatus |= SR_FS_INITIAL; +#ifdef CONFIG_FPU + /* + * Restore the initial value to the FP register + * before starting the user program. + */ + fstate_restore(current, regs); +#endif + } regs->sepc = pc; regs->sp = sp; set_fs(USER_DS); @@ -75,10 +83,11 @@ void flush_thread(void) { #ifdef CONFIG_FPU /* - * Reset FPU context + * Reset FPU state and context * frm: round to nearest, ties to even (IEEE default) * fflags: accrued exceptions cleared */ + fstate_off(current, task_pt_regs(current)); memset(¤t->thread.fstate, 0, sizeof(current->thread.fstate)); #endif } -- 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] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-08 7:58 ` [PATCH 1/2] riscv: Correct the initialized flow of FP register Vincent Chen @ 2019-08-08 10:15 ` Anup Patel 2019-08-08 15:50 ` Paul Walmsley 2019-08-12 14:58 ` Christoph Hellwig 1 sibling, 1 reply; 13+ messages in thread From: Anup Patel @ 2019-08-08 10:15 UTC (permalink / raw) To: Vincent Chen Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > The following two reasons cause FP registers are sometimes not > initialized before starting the user program. > 1. Currently, the FP context is initialized in flush_thread() function > and we expect these initial values to be restored to FP register when > doing FP context switch. However, the FP context switch only occurs in > switch_to function. Hence, if this process does not be scheduled out > and scheduled in before entering the user space, the FP registers > have no chance to initialize. > 2. In flush_thread(), the state of reg->sstatus.FS inherits from the > parent. Hence, the state of reg->sstatus.FS may be dirty. If this > process is scheduled out during flush_thread() and initializing the > FP register, the fstate_save() in switch_to will corrupt the FP context > which has been initialized until flush_thread(). > > To solve the 1st case, the initialization of the FP register will be > completed in start_thread(). It makes sure all FP registers are initialized > before starting the user program. For the 2nd case, the state of > reg->sstatus.FS in start_thread will be set to SR_FS_OFF to prevent this > process from corrupting FP context in doing context save. The FP state is > set to SR_FS_INITIAL in start_trhead(). > > Tested on both QEMU and HiFive Unleashed using BBL + Linux. > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > arch/riscv/include/asm/switch_to.h | 6 ++++++ > arch/riscv/kernel/process.c | 13 +++++++++++-- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > index 853b65e..d5fe573 100644 > --- a/arch/riscv/include/asm/switch_to.h > +++ b/arch/riscv/include/asm/switch_to.h > @@ -19,6 +19,12 @@ static inline void __fstate_clean(struct pt_regs *regs) > regs->sstatus |= (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > } > > +static inline void fstate_off(struct task_struct *task, > + struct pt_regs *regs) > +{ > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; The SR_FS_OFF is 0x0 so no need for ORing it. > +} > + > static inline void fstate_save(struct task_struct *task, > struct pt_regs *regs) > { > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > index f23794b..e3077ee 100644 > --- a/arch/riscv/kernel/process.c > +++ b/arch/riscv/kernel/process.c > @@ -64,8 +64,16 @@ void start_thread(struct pt_regs *regs, unsigned long pc, > unsigned long sp) > { > regs->sstatus = SR_SPIE; > - if (has_fpu) > + if (has_fpu) { > regs->sstatus |= SR_FS_INITIAL; > +#ifdef CONFIG_FPU > + /* > + * Restore the initial value to the FP register > + * before starting the user program. > + */ > + fstate_restore(current, regs); > +#endif > + } > regs->sepc = pc; > regs->sp = sp; > set_fs(USER_DS); > @@ -75,10 +83,11 @@ void flush_thread(void) > { > #ifdef CONFIG_FPU > /* > - * Reset FPU context > + * Reset FPU state and context > * frm: round to nearest, ties to even (IEEE default) > * fflags: accrued exceptions cleared > */ > + fstate_off(current, task_pt_regs(current)); > memset(¤t->thread.fstate, 0, sizeof(current->thread.fstate)); > #endif > } > -- > 2.7.4 > Apart from above minor comment, looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-08 10:15 ` Anup Patel @ 2019-08-08 15:50 ` Paul Walmsley 2019-08-14 1:45 ` Vincent Chen 0 siblings, 1 reply; 13+ messages in thread From: Paul Walmsley @ 2019-08-08 15:50 UTC (permalink / raw) To: Anup Patel Cc: Vincent Chen, linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou On Thu, 8 Aug 2019, Anup Patel wrote: > On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > > > +static inline void fstate_off(struct task_struct *task, > > + struct pt_regs *regs) > > +{ > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; > > The SR_FS_OFF is 0x0 so no need for ORing it. That one looks OK to me, since it makes it more obvious to humans what's happening here - reviewers won't need to know that "off" is 0x0. The compiler should drop it internally, so it won't affect the generated code. > Apart from above minor comment, looks good to me. > > Reviewed-by: Anup Patel <anup@brainfault.org> Will add your Reviewed-by: tag - let us know if you want me to drop it or caveat it. - Paul _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-08 15:50 ` Paul Walmsley @ 2019-08-14 1:45 ` Vincent Chen 2019-08-14 3:51 ` Anup Patel 0 siblings, 1 reply; 13+ messages in thread From: Vincent Chen @ 2019-08-14 1:45 UTC (permalink / raw) To: Paul Walmsley, Anup Patel Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou On Thu, Aug 8, 2019 at 11:50 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > On Thu, 8 Aug 2019, Anup Patel wrote: > > > On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > > > > > +static inline void fstate_off(struct task_struct *task, > > > + struct pt_regs *regs) > > > +{ > > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; > > > > The SR_FS_OFF is 0x0 so no need for ORing it. > > That one looks OK to me, since it makes it more obvious to humans what's > happening here - reviewers won't need to know that "off" is 0x0. The > compiler should drop it internally, so it won't affect the generated > code. > Thanks for Paul's comment My thought is the same as Paul. > > Apart from above minor comment, looks good to me. > > > > Reviewed-by: Anup Patel <anup@brainfault.org> > > Will add your Reviewed-by: tag - let us know if you want me to drop it or > caveat it. > > > - Paul Dear Anup, I suppose you can accept our thought about using the SR_FS_OFF flag because I didn't receive any reply from you. Thanks for your review and comments. Regards, Vincent _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-14 1:45 ` Vincent Chen @ 2019-08-14 3:51 ` Anup Patel 0 siblings, 0 replies; 13+ messages in thread From: Anup Patel @ 2019-08-14 3:51 UTC (permalink / raw) To: Vincent Chen Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Wed, Aug 14, 2019 at 7:15 AM Vincent Chen <vincent.chen@sifive.com> wrote: > > On Thu, Aug 8, 2019 at 11:50 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > On Thu, 8 Aug 2019, Anup Patel wrote: > > > > > On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > > > > > > > +static inline void fstate_off(struct task_struct *task, > > > > + struct pt_regs *regs) > > > > +{ > > > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; > > > > > > The SR_FS_OFF is 0x0 so no need for ORing it. > > > > That one looks OK to me, since it makes it more obvious to humans what's > > happening here - reviewers won't need to know that "off" is 0x0. The > > compiler should drop it internally, so it won't affect the generated > > code. > > > Thanks for Paul's comment > My thought is the same as Paul. > > > > > Apart from above minor comment, looks good to me. > > > > > > Reviewed-by: Anup Patel <anup@brainfault.org> > > > > Will add your Reviewed-by: tag - let us know if you want me to drop it or > > caveat it. > > > > > > - Paul > > Dear Anup, > I suppose you can accept our thought about using the SR_FS_OFF flag > because I didn't receive any reply from you. > Thanks for your review and comments. No problem, go ahead without dropping SR_FS_OFF flag. You can include my Reviewed-by. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-08 7:58 ` [PATCH 1/2] riscv: Correct the initialized flow of FP register Vincent Chen 2019-08-08 10:15 ` Anup Patel @ 2019-08-12 14:58 ` Christoph Hellwig 2019-08-14 1:52 ` Vincent Chen 1 sibling, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2019-08-12 14:58 UTC (permalink / raw) To: Vincent Chen; +Cc: linux-riscv, palmer, linux-kernel, aou, paul.walmsley > +static inline void fstate_off(struct task_struct *task, > + struct pt_regs *regs) > +{ > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; No need for the inner braces here. > +} > + > static inline void fstate_save(struct task_struct *task, > struct pt_regs *regs) > { > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > index f23794b..e3077ee 100644 > --- a/arch/riscv/kernel/process.c > +++ b/arch/riscv/kernel/process.c > @@ -64,8 +64,16 @@ void start_thread(struct pt_regs *regs, unsigned long pc, > unsigned long sp) > { > regs->sstatus = SR_SPIE; > - if (has_fpu) > + if (has_fpu) { > regs->sstatus |= SR_FS_INITIAL; > +#ifdef CONFIG_FPU > + /* > + * Restore the initial value to the FP register > + * before starting the user program. > + */ > + fstate_restore(current, regs); > +#endif fstate_restore has a no-op stub for the !CONFIG_FPU case, so the ifdef here is not needed. Otherwise this looks good to me: Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: Correct the initialized flow of FP register 2019-08-12 14:58 ` Christoph Hellwig @ 2019-08-14 1:52 ` Vincent Chen 0 siblings, 0 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-14 1:52 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Mon, Aug 12, 2019 at 10:58 PM Christoph Hellwig <hch@infradead.org> wrote: > > > +static inline void fstate_off(struct task_struct *task, > > + struct pt_regs *regs) > > +{ > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_OFF; > > No need for the inner braces here. Ok. > > > +} > > + > > static inline void fstate_save(struct task_struct *task, > > struct pt_regs *regs) > > { > > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > > index f23794b..e3077ee 100644 > > --- a/arch/riscv/kernel/process.c > > +++ b/arch/riscv/kernel/process.c > > @@ -64,8 +64,16 @@ void start_thread(struct pt_regs *regs, unsigned long pc, > > unsigned long sp) > > { > > regs->sstatus = SR_SPIE; > > - if (has_fpu) > > + if (has_fpu) { > > regs->sstatus |= SR_FS_INITIAL; > > +#ifdef CONFIG_FPU > > + /* > > + * Restore the initial value to the FP register > > + * before starting the user program. > > + */ > > + fstate_restore(current, regs); > > +#endif > > fstate_restore has a no-op stub for the !CONFIG_FPU case, so the ifdef > here is not needed. > You are right. I will remove the Ifdef condition. > Otherwise this looks good to me: > > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks for your comments. Regards, Vincent Chen _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] riscv: Make __fstate_clean() can work correctly. 2019-08-08 7:58 [PATCH 0/2] riscv: Correct the initialized flow of FP and __fstate_clean() Vincent Chen 2019-08-08 7:58 ` [PATCH 1/2] riscv: Correct the initialized flow of FP register Vincent Chen @ 2019-08-08 7:58 ` Vincent Chen 2019-08-08 10:17 ` Anup Patel 2019-08-12 14:59 ` Christoph Hellwig 1 sibling, 2 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-08 7:58 UTC (permalink / raw) To: paul.walmsley, palmer, aou; +Cc: Vincent Chen, linux-riscv, linux-kernel Make the __fstate_clean() function can correctly set the state of sstatus.FS in pt_regs to SR_FS_CLEAN. Tested on both QEMU and HiFive Unleashed using BBL + Linux. Signed-off-by: Vincent Chen <vincent.chen@sifive.com> --- arch/riscv/include/asm/switch_to.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index d5fe573..544f99a 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -16,7 +16,7 @@ extern void __fstate_restore(struct task_struct *restore_from); static inline void __fstate_clean(struct pt_regs *regs) { - regs->sstatus |= (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; } static inline void fstate_off(struct task_struct *task, -- 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] 13+ messages in thread
* Re: [PATCH 2/2] riscv: Make __fstate_clean() can work correctly. 2019-08-08 7:58 ` [PATCH 2/2] riscv: Make __fstate_clean() can work correctly Vincent Chen @ 2019-08-08 10:17 ` Anup Patel 2019-08-14 1:55 ` Vincent Chen 2019-08-12 14:59 ` Christoph Hellwig 1 sibling, 1 reply; 13+ messages in thread From: Anup Patel @ 2019-08-08 10:17 UTC (permalink / raw) To: Vincent Chen Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > Make the __fstate_clean() function can correctly set the > state of sstatus.FS in pt_regs to SR_FS_CLEAN. > > Tested on both QEMU and HiFive Unleashed using BBL + Linux. > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > arch/riscv/include/asm/switch_to.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > index d5fe573..544f99a 100644 > --- a/arch/riscv/include/asm/switch_to.h > +++ b/arch/riscv/include/asm/switch_to.h > @@ -16,7 +16,7 @@ extern void __fstate_restore(struct task_struct *restore_from); > > static inline void __fstate_clean(struct pt_regs *regs) > { > - regs->sstatus |= (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > } > > static inline void fstate_off(struct task_struct *task, > -- > 2.7.4 > Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> This should be a RC fix. Please add "Fixes:" in your commit description and CC stable kernel. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] riscv: Make __fstate_clean() can work correctly. 2019-08-08 10:17 ` Anup Patel @ 2019-08-14 1:55 ` Vincent Chen 0 siblings, 0 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-14 1:55 UTC (permalink / raw) To: Anup Patel Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Thu, Aug 8, 2019 at 6:17 PM Anup Patel <anup@brainfault.org> wrote: > > On Thu, Aug 8, 2019 at 1:30 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > > > Make the __fstate_clean() function can correctly set the > > state of sstatus.FS in pt_regs to SR_FS_CLEAN. > > > > Tested on both QEMU and HiFive Unleashed using BBL + Linux. > > > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > --- > > arch/riscv/include/asm/switch_to.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > > index d5fe573..544f99a 100644 > > --- a/arch/riscv/include/asm/switch_to.h > > +++ b/arch/riscv/include/asm/switch_to.h > > @@ -16,7 +16,7 @@ extern void __fstate_restore(struct task_struct *restore_from); > > > > static inline void __fstate_clean(struct pt_regs *regs) > > { > > - regs->sstatus |= (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > > } > > > > static inline void fstate_off(struct task_struct *task, > > -- > > 2.7.4 > > > > Looks good to me. > > Reviewed-by: Anup Patel <anup@brainfault.org> > > This should be a RC fix. > > Please add "Fixes:" in your commit description and > CC stable kernel. > OK, I will follow your suggestions and resend this patch Thanks for your comments. Regards, Vincent Chen _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] riscv: Make __fstate_clean() can work correctly. 2019-08-08 7:58 ` [PATCH 2/2] riscv: Make __fstate_clean() can work correctly Vincent Chen 2019-08-08 10:17 ` Anup Patel @ 2019-08-12 14:59 ` Christoph Hellwig 2019-08-14 1:55 ` Vincent Chen 1 sibling, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2019-08-12 14:59 UTC (permalink / raw) To: Vincent Chen; +Cc: linux-riscv, palmer, linux-kernel, aou, paul.walmsley Maybe s/can //g in the subject? > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; No need for the inner braces here either. Otherwise: Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] riscv: Make __fstate_clean() can work correctly. 2019-08-12 14:59 ` Christoph Hellwig @ 2019-08-14 1:55 ` Vincent Chen 0 siblings, 0 replies; 13+ messages in thread From: Vincent Chen @ 2019-08-14 1:55 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-riscv, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Albert Ou, Paul Walmsley On Mon, Aug 12, 2019 at 10:59 PM Christoph Hellwig <hch@infradead.org> wrote: > > Maybe s/can //g in the subject? > > > + regs->sstatus = (regs->sstatus & ~(SR_FS)) | SR_FS_CLEAN; > > No need for the inner braces here either. OK, I will remove them. > > Otherwise: > > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks for your comments Regards, Vincent Chen _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-14 3:52 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-08 7:58 [PATCH 0/2] riscv: Correct the initialized flow of FP and __fstate_clean() Vincent Chen 2019-08-08 7:58 ` [PATCH 1/2] riscv: Correct the initialized flow of FP register Vincent Chen 2019-08-08 10:15 ` Anup Patel 2019-08-08 15:50 ` Paul Walmsley 2019-08-14 1:45 ` Vincent Chen 2019-08-14 3:51 ` Anup Patel 2019-08-12 14:58 ` Christoph Hellwig 2019-08-14 1:52 ` Vincent Chen 2019-08-08 7:58 ` [PATCH 2/2] riscv: Make __fstate_clean() can work correctly Vincent Chen 2019-08-08 10:17 ` Anup Patel 2019-08-14 1:55 ` Vincent Chen 2019-08-12 14:59 ` Christoph Hellwig 2019-08-14 1:55 ` Vincent Chen
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).