riscv: vector: Fix data pointer constraints in context save/restore

[PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Troy Mitchell 2 weeks, 3 days ago

The standard vector save/restore asm advances datap but declares it as
input-only. An inlined caller reusing the original pointer may therefore
use the advanced address instead.

Declare datap as read-write so the compiler can preserve the original
pointer when needed.

Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
 arch/riscv/include/asm/vector.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index fffe72a772080..c7fd6d50a7a47 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -230,7 +230,7 @@ static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state *save_to,
 			"add		%1, %1, %0\n\t"
 			"vse8.v		v24, (%1)\n\t"
 			".option pop\n\t"
-			: "=&r" (vl) : "r" (datap) : "memory");
+			: "=&r" (vl), "+r" (datap) : : "memory");
 	}
 	riscv_v_disable();
 }
@@ -266,7 +266,7 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
 			"add		%1, %1, %0\n\t"
 			"vle8.v		v24, (%1)\n\t"
 			".option pop\n\t"
-			: "=&r" (vl) : "r" (datap) : "memory");
+			: "=&r" (vl), "+r" (datap) : : "memory");
 	}
 	__vstate_csr_restore(restore_from);
 	riscv_v_disable();

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260908-riscv-vector-asm-fix-27ed36623934

Best regards,
--  
Troy Mitchell <troy.mitchell@linux.dev>

Re: [PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Paul Walmsley 6 days, 20 hours ago

On Tue, 8 Sep 2026, Troy Mitchell wrote:

> The standard vector save/restore asm advances datap but declares it as
> input-only. An inlined caller reusing the original pointer may therefore
> use the advanced address instead.
> 
> Declare datap as read-write so the compiler can preserve the original
> pointer when needed.
> 
> Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>

Thanks, queued for v7.3-rc.


- Paul

Re: [PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Andy Chiu 2 weeks ago

Hi Troy,

On Tue, Sep 8, 2026 at 8:03 AM Troy Mitchell <troy.mitchell@linux.dev> wrote:
>
> The standard vector save/restore asm advances datap but declares it as
> input-only. An inlined caller reusing the original pointer may therefore
> use the advanced address instead.
>
> Declare datap as read-write so the compiler can preserve the original
> pointer when needed.
>
> Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>

Reviewed-by: Andy Chiu <tchiu@tenstorrent.com>

Thank you,
Andy Chiu

Re: [PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Aurelien Jarno 2 weeks ago

On 2026-09-08 21:03, Troy Mitchell wrote:
> The standard vector save/restore asm advances datap but declares it as
> input-only. An inlined caller reusing the original pointer may therefore
> use the advanced address instead.
> 
> Declare datap as read-write so the compiler can preserve the original
> pointer when needed.
> 
> Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  arch/riscv/include/asm/vector.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

Re: [PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Guo Ren 2 weeks ago

On Tue, Sep 8, 2026 at 9:03 PM Troy Mitchell <troy.mitchell@linux.dev> wrote:
>
> The standard vector save/restore asm advances datap but declares it as
> input-only. An inlined caller reusing the original pointer may therefore
> use the advanced address instead.
>
> Declare datap as read-write so the compiler can preserve the original
> pointer when needed.
>
> Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  arch/riscv/include/asm/vector.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index fffe72a772080..c7fd6d50a7a47 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -230,7 +230,7 @@ static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state *save_to,
>                         "add            %1, %1, %0\n\t"
Yes, %1 is modified. Good catch!

Reviewed-by: GUO Ren (XuanTie) <guoren@kernel.org>

PS:
Don't forget "if (has_xtheadvector()) {" part, thx :)

>                         "vse8.v         v24, (%1)\n\t"
>                         ".option pop\n\t"
> -                       : "=&r" (vl) : "r" (datap) : "memory");
> +                       : "=&r" (vl), "+r" (datap) : : "memory");
>         }
>         riscv_v_disable();
>  }
> @@ -266,7 +266,7 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
>                         "add            %1, %1, %0\n\t"
>                         "vle8.v         v24, (%1)\n\t"
>                         ".option pop\n\t"
> -                       : "=&r" (vl) : "r" (datap) : "memory");
> +                       : "=&r" (vl), "+r" (datap) : : "memory");
>         }
>         __vstate_csr_restore(restore_from);
>         riscv_v_disable();
>
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260908-riscv-vector-asm-fix-27ed36623934
>
> Best regards,
> --
> Troy Mitchell <troy.mitchell@linux.dev>
>


-- 
Best Regards
 Guo Ren

Re: [PATCH] riscv: vector: Fix data pointer constraints in context save/restore

Posted by Troy Mitchell 1 week, 6 days ago

On Fri Sep 11, 2026 at 2:56 PM +08, Guo Ren wrote:
> On Tue, Sep 8, 2026 at 9:03 PM Troy Mitchell <troy.mitchell@linux.dev> wrote:
>>
>> The standard vector save/restore asm advances datap but declares it as
>> input-only. An inlined caller reusing the original pointer may therefore
>> use the advanced address instead.
>>
>> Declare datap as read-write so the compiler can preserve the original
>> pointer when needed.
>>
>> Fixes: 03c3fcd9941a ("riscv: Introduce struct/helpers to save/restore per-task Vector state")
>> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
>> ---
>>  arch/riscv/include/asm/vector.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
>> index fffe72a772080..c7fd6d50a7a47 100644
>> --- a/arch/riscv/include/asm/vector.h
>> +++ b/arch/riscv/include/asm/vector.h
>> @@ -230,7 +230,7 @@ static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state *save_to,
>>                         "add            %1, %1, %0\n\t"
> Yes, %1 is modified. Good catch!
>
> Reviewed-by: GUO Ren (XuanTie) <guoren@kernel.org>
>
> PS:
> Don't forget "if (has_xtheadvector()) {" part, thx :)
The XTheadVector path copies datap into t0 and lists t0/t4 as
clobbers, so it doesn't have this issue. Thanks for the reminder!

-- 
Troy Mitchell