[PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Thomas Weißschuh]

The syscall wrappers use the "a0" register for two different register
variables, both the first argument and the return value. The "ret"
variable is used as both input and output while the argument register is
only used as input. Clang treats the conflicting input parameters as
undefined behaviour and optimizes away the argument assignment.

The code seems to work by chance for the most part today but that may
change in the future. Specifically clock_gettime_fallback() fails with
clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.

Switch the "ret" register variable to a pure output, similar to the other
architectures' vDSO code. This works in both clang and GCC.

Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd291a@linutronix.de/
Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/
Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 arch/loongarch/include/asm/vdso/getrandom.h    | 2 +-
 arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h
index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644
--- a/arch/loongarch/include/asm/vdso/getrandom.h
+++ b/arch/loongarch/include/asm/vdso/getrandom.h
@@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
 
 	asm volatile(
 	"      syscall 0\n"
-	: "+r" (ret)
+	: "=r" (ret)
 	: "r" (nr), "r" (buffer), "r" (len), "r" (flags)
 	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8",
 	  "memory");
diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h
index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644
--- a/arch/loongarch/include/asm/vdso/gettimeofday.h
+++ b/arch/loongarch/include/asm/vdso/gettimeofday.h
@@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
 
 	asm volatile(
 	"       syscall 0\n"
-	: "+r" (ret)
+	: "=r" (ret)
 	: "r" (nr), "r" (tv), "r" (tz)
 	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
 	  "$t8", "memory");
@@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
 
 	asm volatile(
 	"       syscall 0\n"
-	: "+r" (ret)
+	: "=r" (ret)
 	: "r" (nr), "r" (clkid), "r" (ts)
 	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
 	  "$t8", "memory");
@@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
 
 	asm volatile(
 	"       syscall 0\n"
-	: "+r" (ret)
+	: "=r" (ret)
 	: "r" (nr), "r" (clkid), "r" (ts)
 	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
 	  "$t8", "memory");

---
base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
change-id: 20250603-loongarch-vdso-syscall-f585a99bea03

Best regards,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Nathan Chancellor]

On Tue, Jun 03, 2025 at 01:48:54PM +0200, Thomas Weißschuh wrote:
> The syscall wrappers use the "a0" register for two different register
> variables, both the first argument and the return value. The "ret"
> variable is used as both input and output while the argument register is
> only used as input. Clang treats the conflicting input parameters as
> undefined behaviour and optimizes away the argument assignment.
> 
> The code seems to work by chance for the most part today but that may
> change in the future. Specifically clock_gettime_fallback() fails with
> clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> 
> Switch the "ret" register variable to a pure output, similar to the other
> architectures' vDSO code. This works in both clang and GCC.
> 
> Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd291a@linutronix.de/
> Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/
> Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
> Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

This is definitely an odd interaction because of the register variables
using the same value.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/loongarch/include/asm/vdso/getrandom.h    | 2 +-
>  arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h
> index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644
> --- a/arch/loongarch/include/asm/vdso/getrandom.h
> +++ b/arch/loongarch/include/asm/vdso/getrandom.h
> @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  
>  	asm volatile(
>  	"      syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>  	: "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8",
>  	  "memory");
> diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h
> index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644
> --- a/arch/loongarch/include/asm/vdso/gettimeofday.h
> +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h
> @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
>  
>  	asm volatile(
>  	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>  	: "r" (nr), "r" (tv), "r" (tz)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>  	  "$t8", "memory");
> @@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
>  
>  	asm volatile(
>  	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>  	: "r" (nr), "r" (clkid), "r" (ts)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>  	  "$t8", "memory");
> @@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
>  
>  	asm volatile(
>  	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>  	: "r" (nr), "r" (clkid), "r" (ts)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>  	  "$t8", "memory");
> 
> ---
> base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
> change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
> 
> Best regards,
> -- 
> Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> 

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Yanteng Si]

在 6/3/25 7:48 PM, Thomas WeiÃschuh 写道:
> The syscall wrappers use the "a0" register for two different register
> variables, both the first argument and the return value. The "ret"
> variable is used as both input and output while the argument register is
> only used as input. Clang treats the conflicting input parameters as
> undefined behaviour and optimizes away the argument assignment.
> 
> The code seems to work by chance for the most part today but that may
> change in the future. Specifically clock_gettime_fallback() fails with
> clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> 
> Switch the "ret" register variable to a pure output, similar to the other
> architectures' vDSO code. This works in both clang and GCC.

I don't know Clang and GCC, but I think it's great to do so.

Reviewed-by: Yanteng Si <si.yanteng@linux.dev>

Thanks,
Yanteng
> 
> Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd291a@linutronix.de/
> Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/
> Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
> Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   arch/loongarch/include/asm/vdso/getrandom.h    | 2 +-
>   arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h
> index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644
> --- a/arch/loongarch/include/asm/vdso/getrandom.h
> +++ b/arch/loongarch/include/asm/vdso/getrandom.h
> @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>   
>   	asm volatile(
>   	"      syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>   	: "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>   	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8",
>   	  "memory");
> diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h
> index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644
> --- a/arch/loongarch/include/asm/vdso/gettimeofday.h
> +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h
> @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
>   
>   	asm volatile(
>   	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>   	: "r" (nr), "r" (tv), "r" (tz)
>   	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>   	  "$t8", "memory");
> @@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
>   
>   	asm volatile(
>   	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>   	: "r" (nr), "r" (clkid), "r" (ts)
>   	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>   	  "$t8", "memory");
> @@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
>   
>   	asm volatile(
>   	"       syscall 0\n"
> -	: "+r" (ret)
> +	: "=r" (ret)
>   	: "r" (nr), "r" (clkid), "r" (ts)
>   	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>   	  "$t8", "memory");
> 
> ---
> base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
> change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
> 
> Best regards,

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Huacai Chen]

On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> The syscall wrappers use the "a0" register for two different register
> variables, both the first argument and the return value. The "ret"
> variable is used as both input and output while the argument register is
> only used as input. Clang treats the conflicting input parameters as
> undefined behaviour and optimizes away the argument assignment.
>
> The code seems to work by chance for the most part today but that may
> change in the future. Specifically clock_gettime_fallback() fails with
> clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
>
> Switch the "ret" register variable to a pure output, similar to the other
> architectures' vDSO code. This works in both clang and GCC.
Hmmm, at first the constraint is "=r", during the progress of
upstream, Xuerui suggested me to use "+r" instead [1].
[1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/

Huacai

>
> Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd291a@linutronix.de/
> Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/
> Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
> Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  arch/loongarch/include/asm/vdso/getrandom.h    | 2 +-
>  arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h
> index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644
> --- a/arch/loongarch/include/asm/vdso/getrandom.h
> +++ b/arch/loongarch/include/asm/vdso/getrandom.h
> @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>
>         asm volatile(
>         "      syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8",
>           "memory");
> diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h
> index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644
> --- a/arch/loongarch/include/asm/vdso/gettimeofday.h
> +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h
> @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (tv), "r" (tz)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
> @@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (clkid), "r" (ts)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
> @@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (clkid), "r" (ts)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
>
> ---
> base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
> change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
>
> Best regards,
> --
> Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Xi Ruoyao]

On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
> > 
> > The syscall wrappers use the "a0" register for two different register
> > variables, both the first argument and the return value. The "ret"
> > variable is used as both input and output while the argument register is
> > only used as input. Clang treats the conflicting input parameters as
> > undefined behaviour and optimizes away the argument assignment.
> > 
> > The code seems to work by chance for the most part today but that may
> > change in the future. Specifically clock_gettime_fallback() fails with
> > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > 
> > Switch the "ret" register variable to a pure output, similar to the other
> > architectures' vDSO code. This works in both clang and GCC.
> Hmmm, at first the constraint is "=r", during the progress of
> upstream, Xuerui suggested me to use "+r" instead [1].
> [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/

Based on the example at
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:

   To force an operand into a register, create a local variable and specify
   the register name after the variable’s declaration. Then use the local
   variable for the asm operand and specify any constraint letter that
   matches the register:
   
   register int *p1 asm ("r0") = …;
   register int *p2 asm ("r1") = …;
   register int *result asm ("r0");
   asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
   
I think this should actually be written

 	asm volatile(
 	"      syscall 0\n"
	: "=r" (ret)
 	: "r" (nr), "0" (buffer), "r" (len), "r" (flags)
 	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
"$t8",
 	  "memory");

i.e. "=" should be used for the output operand 0, and "0" should be used
for the input operand 2 (buffer) to emphasis the same register as
operand 0 is used.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[WANG Xuerui]

On 6/4/25 22:05, Huacai Chen wrote:
> On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
>>
>> The syscall wrappers use the "a0" register for two different register
>> variables, both the first argument and the return value. The "ret"
>> variable is used as both input and output while the argument register is
>> only used as input. Clang treats the conflicting input parameters as
>> undefined behaviour and optimizes away the argument assignment.
>>
>> The code seems to work by chance for the most part today but that may
>> change in the future. Specifically clock_gettime_fallback() fails with
>> clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
>>
>> Switch the "ret" register variable to a pure output, similar to the other
>> architectures' vDSO code. This works in both clang and GCC.
> Hmmm, at first the constraint is "=r", during the progress of
> upstream, Xuerui suggested me to use "+r" instead [1].
> [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/

Oops, I've already completely forgotten! That said...

I didn't notice back then, that `ret` and the first parameter actually 
shared the same manually allocated register, so I replied as if the two 
shared one variable. If it were me to write the original code, I would 
re-used `ret` for arg0 (with a comment explaining the a0 situation) so 
that "+r" could be properly used there without UB.

As for the current situation -- both this patch's approach or my 
alternative above are OK to me. Feel free to take either; and have my 
R-b tag if you send a v2.

Reviewed-by: WANG Xuerui <git@xen0n.name>

Thanks!

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Thomas Weißschuh]

On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
> On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> > On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> > <thomas.weissschuh@linutronix.de> wrote:
> > > 
> > > The syscall wrappers use the "a0" register for two different register
> > > variables, both the first argument and the return value. The "ret"
> > > variable is used as both input and output while the argument register is
> > > only used as input. Clang treats the conflicting input parameters as
> > > undefined behaviour and optimizes away the argument assignment.
> > > 
> > > The code seems to work by chance for the most part today but that may
> > > change in the future. Specifically clock_gettime_fallback() fails with
> > > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > > 
> > > Switch the "ret" register variable to a pure output, similar to the other
> > > architectures' vDSO code. This works in both clang and GCC.
> > Hmmm, at first the constraint is "=r", during the progress of
> > upstream, Xuerui suggested me to use "+r" instead [1].
> > [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/
> 
> Based on the example at
> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
> 
>    To force an operand into a register, create a local variable and specify
>    the register name after the variable’s declaration. Then use the local
>    variable for the asm operand and specify any constraint letter that
>    matches the register:
>    
>    register int *p1 asm ("r0") = …;
>    register int *p2 asm ("r1") = …;
>    register int *result asm ("r0");
>    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
>    
> I think this should actually be written
> 
>  	asm volatile(
>  	"      syscall 0\n"
> 	: "=r" (ret)
>  	: "r" (nr), "0" (buffer), "r" (len), "r" (flags)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
> "$t8",
>  	  "memory");
> 
> i.e. "=" should be used for the output operand 0, and "0" should be used
> for the input operand 2 (buffer) to emphasis the same register as
> operand 0 is used.

I would have expected that matching constraints ("0") would only really make
sense if the compiler selects the specific register to use. When the register is
already selected manually it seems redundant.
But my inline ASM knowledge is limited and this is a real example from the GCC
docs, so it is probably more correct.
On the other hand all the other vDSO implementations use "r" over "0" for the
input operand 2 and I'd like to keep them consistent.


Thomas

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Huacai Chen]

On Thu, Jun 5, 2025 at 3:37 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
> > On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> > > On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> > > <thomas.weissschuh@linutronix.de> wrote:
> > > >
> > > > The syscall wrappers use the "a0" register for two different register
> > > > variables, both the first argument and the return value. The "ret"
> > > > variable is used as both input and output while the argument register is
> > > > only used as input. Clang treats the conflicting input parameters as
> > > > undefined behaviour and optimizes away the argument assignment.
> > > >
> > > > The code seems to work by chance for the most part today but that may
> > > > change in the future. Specifically clock_gettime_fallback() fails with
> > > > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > > >
> > > > Switch the "ret" register variable to a pure output, similar to the other
> > > > architectures' vDSO code. This works in both clang and GCC.
> > > Hmmm, at first the constraint is "=r", during the progress of
> > > upstream, Xuerui suggested me to use "+r" instead [1].
> > > [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/
> >
> > Based on the example at
> > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
> >
> >    To force an operand into a register, create a local variable and specify
> >    the register name after the variable’s declaration. Then use the local
> >    variable for the asm operand and specify any constraint letter that
> >    matches the register:
> >
> >    register int *p1 asm ("r0") = …;
> >    register int *p2 asm ("r1") = …;
> >    register int *result asm ("r0");
> >    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
> >
> > I think this should actually be written
> >
> >       asm volatile(
> >       "      syscall 0\n"
> >       : "=r" (ret)
> >       : "r" (nr), "0" (buffer), "r" (len), "r" (flags)
> >       : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
> > "$t8",
> >         "memory");
> >
> > i.e. "=" should be used for the output operand 0, and "0" should be used
> > for the input operand 2 (buffer) to emphasis the same register as
> > operand 0 is used.
>
> I would have expected that matching constraints ("0") would only really make
> sense if the compiler selects the specific register to use. When the register is
> already selected manually it seems redundant.
> But my inline ASM knowledge is limited and this is a real example from the GCC
> docs, so it is probably more correct.
> On the other hand all the other vDSO implementations use "r" over "0" for the
> input operand 2 and I'd like to keep them consistent.
OK, if there are no objections, I will take this patch.

Huacai

>
>
> Thomas

Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Xi Ruoyao]

On Thu, 2025-06-05 at 09:37 +0200, Thomas Weißschuh wrote:
> On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
> > On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> > > On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> > > <thomas.weissschuh@linutronix.de> wrote:
> > > > 
> > > > The syscall wrappers use the "a0" register for two different register
> > > > variables, both the first argument and the return value. The "ret"
> > > > variable is used as both input and output while the argument register is
> > > > only used as input. Clang treats the conflicting input parameters as
> > > > undefined behaviour and optimizes away the argument assignment.
> > > > 
> > > > The code seems to work by chance for the most part today but that may
> > > > change in the future. Specifically clock_gettime_fallback() fails with
> > > > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > > > 
> > > > Switch the "ret" register variable to a pure output, similar to the other
> > > > architectures' vDSO code. This works in both clang and GCC.
> > > Hmmm, at first the constraint is "=r", during the progress of
> > > upstream, Xuerui suggested me to use "+r" instead [1].
> > > [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/
> > 
> > Based on the example at
> > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
> > 
> >    To force an operand into a register, create a local variable and specify
> >    the register name after the variable’s declaration. Then use the local
> >    variable for the asm operand and specify any constraint letter that
> >    matches the register:
> >    
> >    register int *p1 asm ("r0") = …;
> >    register int *p2 asm ("r1") = …;
> >    register int *result asm ("r0");
> >    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
> >    
> > I think this should actually be written
> > 
> >  	asm volatile(
> >  	"      syscall 0\n"
> > 	: "=r" (ret)
> >  	: "r" (nr), "0" (buffer), "r" (len), "r" (flags)
> >  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
> > "$t8",
> >  	  "memory");
> > 
> > i.e. "=" should be used for the output operand 0, and "0" should be used
> > for the input operand 2 (buffer) to emphasis the same register as
> > operand 0 is used.
> 
> I would have expected that matching constraints ("0") would only really make
> sense if the compiler selects the specific register to use. When the register is
> already selected manually it seems redundant.
> But my inline ASM knowledge is limited and this is a real example from the GCC
> docs, so it is probably more correct.
> On the other hand all the other vDSO implementations use "r" over "0" for the
> input operand 2 and I'd like to keep them consistent.

Per https://gcc.gnu.org/pipermail/gcc-help/2025-June/144261.html and
https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html it should
be fine to just use "r".

Reviewed-by: Xi Ruoyao <xry111@xry111.site>

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University