All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yanteng Si <si.yanteng@linux.dev>
To: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"WANG Xuerui" <kernel@xen0n.name>,
	"Theodore Ts'o" <tytso@mit.edu>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <nick.desaulniers+lkml@gmail.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
	"Xi Ruoyao" <xry111@xry111.site>
Cc: loongarch@lists.linux.dev, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev, stable@vger.kernel.org
Subject: Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers
Date: Wed, 4 Jun 2025 09:51:17 +0800	[thread overview]
Message-ID: <f661db68-4271-4fea-8309-47aa83c1078a@linux.dev> (raw)
In-Reply-To: <20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de>

在 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,


  parent reply	other threads:[~2025-06-04  1:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03 11:48 [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers Thomas Weißschuh
2025-06-03 21:54 ` Nathan Chancellor
2025-06-04  1:51 ` Yanteng Si [this message]
2025-06-04 14:05 ` Huacai Chen
2025-06-04 14:30   ` Xi Ruoyao
2025-06-05  7:37     ` Thomas Weißschuh
2025-06-05 11:18       ` Huacai Chen
2025-06-06  9:13       ` Xi Ruoyao
2025-06-04 16:39   ` WANG Xuerui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f661db68-4271-4fea-8309-47aa83c1078a@linux.dev \
    --to=si.yanteng@linux.dev \
    --cc=Jason@zx2c4.com \
    --cc=chenhuacai@kernel.org \
    --cc=jiaxun.yang@flygoat.com \
    --cc=justinstitt@google.com \
    --cc=kernel@xen0n.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=loongarch@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=thomas.weissschuh@linutronix.de \
    --cc=tytso@mit.edu \
    --cc=xry111@xry111.site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.