1. Patchew
  2. linux
  3. View series

[PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit

Cyril Bur posted 1 patch 11 months ago
There is a newer version of this series
arch/riscv/kernel/entry.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
[PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit
Posted by Cyril Bur 11 months ago 
During switch to csrs will OR the value of the register into the
corresponding csr. In this case we're only interested in restoring the
SUM bit not the entire register.

Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches")
Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com>
---
I've put the Fixes tag in but I assume this will get squashed into the
patch. Either way I hope this works to fix the immediate issue.

 arch/riscv/kernel/entry.S | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 00bd0de9faa2..6ed3bd80903d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to)
 	REG_S s11, TASK_THREAD_S11_RA(a3)
 
 	/* save the user space access flag */
-	li    s0, SR_SUM
-	csrr  s1, CSR_STATUS
-	REG_S s1, TASK_THREAD_STATUS_RA(a3)
+	csrr  s0, CSR_STATUS
+	REG_S s0, TASK_THREAD_STATUS_RA(a3)
 
 	/* Save the kernel shadow call stack pointer */
 	scs_save_current
-	/* Restore context from next->thread */
+	/*
+	 * Restore context from next->thread. csrs will OR the bits from s0 and
+	 * only want to restore the SR_SUM bit
+	 */
 	REG_L s0,  TASK_THREAD_STATUS_RA(a4)
+	li    s1,  SR_SUM
+	and   s0,  s0, s1
 	csrs  CSR_STATUS, s0
 	REG_L ra,  TASK_THREAD_RA_RA(a4)
 	REG_L sp,  TASK_THREAD_SP_RA(a4)
-- 
2.34.1
Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit
Posted by Alexandre Ghiti 10 months, 3 weeks ago 
+cc linux-riscv, Andy, Deepak

On 5/22/25 18:09, Cyril Bur wrote:
> During switch to csrs will OR the value of the register into the
> corresponding csr. In this case we're only interested in restoring the
> SUM bit not the entire register.
>
> Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches")
> Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com>
> ---
> I've put the Fixes tag in but I assume this will get squashed into the
> patch. Either way I hope this works to fix the immediate issue.
>
>   arch/riscv/kernel/entry.S | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 00bd0de9faa2..6ed3bd80903d 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to)
>   	REG_S s11, TASK_THREAD_S11_RA(a3)
>   
>   	/* save the user space access flag */
> -	li    s0, SR_SUM
> -	csrr  s1, CSR_STATUS
> -	REG_S s1, TASK_THREAD_STATUS_RA(a3)
> +	csrr  s0, CSR_STATUS
> +	REG_S s0, TASK_THREAD_STATUS_RA(a3)
>   
>   	/* Save the kernel shadow call stack pointer */
>   	scs_save_current
> -	/* Restore context from next->thread */
> +	/*
> +	 * Restore context from next->thread. csrs will OR the bits from s0 and
> +	 * only want to restore the SR_SUM bit
> +	 */
>   	REG_L s0,  TASK_THREAD_STATUS_RA(a4)
> +	li    s1,  SR_SUM
> +	and   s0,  s0, s1
>   	csrs  CSR_STATUS, s0
>   	REG_L ra,  TASK_THREAD_RA_RA(a4)
>   	REG_L sp,  TASK_THREAD_SP_RA(a4)

To conclude the discussion we had here 
https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, 
in addition to Cyril's patch above, to me we only have to rename the 
status field into sum and we're good to go. @Andy, @Deepak @Samuel Do 
you agree?

As this is an important fix (along with 2 other fixes, one for thead 
vector and vdso static values), I'd like to send another PR soon for 
inclusion in 6.16-rc1, I did not want to delay the second PR any longer.

Thanks for your feedbacks,

Alex
Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit
Posted by Andy Chiu 10 months, 3 weeks ago 
On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> +cc linux-riscv, Andy, Deepak
>
> On 5/22/25 18:09, Cyril Bur wrote:
> > During switch to csrs will OR the value of the register into the
> > corresponding csr. In this case we're only interested in restoring the
> > SUM bit not the entire register.
> >
> > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches")
> > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com>
> > ---
> > I've put the Fixes tag in but I assume this will get squashed into the
> > patch. Either way I hope this works to fix the immediate issue.
> >
> >   arch/riscv/kernel/entry.S | 12 ++++++++----
> >   1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> > index 00bd0de9faa2..6ed3bd80903d 100644
> > --- a/arch/riscv/kernel/entry.S
> > +++ b/arch/riscv/kernel/entry.S
> > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to)
> >       REG_S s11, TASK_THREAD_S11_RA(a3)
> >
> >       /* save the user space access flag */
> > -     li    s0, SR_SUM
> > -     csrr  s1, CSR_STATUS
> > -     REG_S s1, TASK_THREAD_STATUS_RA(a3)
> > +     csrr  s0, CSR_STATUS
> > +     REG_S s0, TASK_THREAD_STATUS_RA(a3)
> >
> >       /* Save the kernel shadow call stack pointer */
> >       scs_save_current
> > -     /* Restore context from next->thread */
> > +     /*
> > +      * Restore context from next->thread. csrs will OR the bits from s0 and
> > +      * only want to restore the SR_SUM bit
> > +      */
> >       REG_L s0,  TASK_THREAD_STATUS_RA(a4)
> > +     li    s1,  SR_SUM
> > +     and   s0,  s0, s1
> >       csrs  CSR_STATUS, s0
> >       REG_L ra,  TASK_THREAD_RA_RA(a4)
> >       REG_L sp,  TASK_THREAD_SP_RA(a4)
>
> To conclude the discussion we had here
> https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t,
> in addition to Cyril's patch above, to me we only have to rename the
> status field into sum and we're good to go. @Andy, @Deepak @Samuel Do
> you agree?

LGTM, thanks!

Andy

>
> As this is an important fix (along with 2 other fixes, one for thead
> vector and vdso static values), I'd like to send another PR soon for
> inclusion in 6.16-rc1, I did not want to delay the second PR any longer.
>
> Thanks for your feedbacks,
>
> Alex
>
>
Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit
Posted by Deepak Gupta 10 months, 3 weeks ago 
On Thu, May 29, 2025 at 12:17:24AM +0800, Andy Chiu wrote:
>On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>>
>> +cc linux-riscv, Andy, Deepak
>>
>> On 5/22/25 18:09, Cyril Bur wrote:
>> > During switch to csrs will OR the value of the register into the
>> > corresponding csr. In this case we're only interested in restoring the
>> > SUM bit not the entire register.
>> >
>> > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches")
>> > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com>
>> > ---
>> > I've put the Fixes tag in but I assume this will get squashed into the
>> > patch. Either way I hope this works to fix the immediate issue.
>> >
>> >   arch/riscv/kernel/entry.S | 12 ++++++++----
>> >   1 file changed, 8 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
>> > index 00bd0de9faa2..6ed3bd80903d 100644
>> > --- a/arch/riscv/kernel/entry.S
>> > +++ b/arch/riscv/kernel/entry.S
>> > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to)
>> >       REG_S s11, TASK_THREAD_S11_RA(a3)
>> >
>> >       /* save the user space access flag */
>> > -     li    s0, SR_SUM
>> > -     csrr  s1, CSR_STATUS
>> > -     REG_S s1, TASK_THREAD_STATUS_RA(a3)
>> > +     csrr  s0, CSR_STATUS
>> > +     REG_S s0, TASK_THREAD_STATUS_RA(a3)
>> >
>> >       /* Save the kernel shadow call stack pointer */
>> >       scs_save_current
>> > -     /* Restore context from next->thread */
>> > +     /*
>> > +      * Restore context from next->thread. csrs will OR the bits from s0 and
>> > +      * only want to restore the SR_SUM bit
>> > +      */
>> >       REG_L s0,  TASK_THREAD_STATUS_RA(a4)
>> > +     li    s1,  SR_SUM
>> > +     and   s0,  s0, s1
>> >       csrs  CSR_STATUS, s0
>> >       REG_L ra,  TASK_THREAD_RA_RA(a4)
>> >       REG_L sp,  TASK_THREAD_SP_RA(a4)
>>
>> To conclude the discussion we had here
>> https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t,
>> in addition to Cyril's patch above, to me we only have to rename the
>> status field into sum and we're good to go. @Andy, @Deepak @Samuel Do
>> you agree?
>
>LGTM, thanks!
>
>Andy

Sounds good to me.

>
>>
>> As this is an important fix (along with 2 other fixes, one for thead
>> vector and vdso static values), I'd like to send another PR soon for
>> inclusion in 6.16-rc1, I did not want to delay the second PR any longer.
>>
>> Thanks for your feedbacks,
>>
>> Alex
>>
>>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.