Received-SPF: pass (google.com: domain of linux-kernel+bounces-13346-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <aa5d50f0-7696-4c96-b493-3c637183c516@collabora.com>
Date: Sat, 30 Dec 2023 12:04:35 +0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
 linux-kselftest@vger.kernel.org, loongarch@lists.linux.dev,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] selftests/vDSO: Fix runtime errors on LoongArch
Content-Language: en-US
To: Tiezhu Yang <yangtiezhu@loongson.cn>, Shuah Khan <shuah@kernel.org>,
 Shuah Khan <skhan@linuxfoundation.org>
References: <20231213012300.5640-1-yangtiezhu@loongson.cn>
 <20231213012300.5640-3-yangtiezhu@loongson.cn>
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
In-Reply-To: <20231213012300.5640-3-yangtiezhu@loongson.cn>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 12/13/23 6:23 AM, Tiezhu Yang wrote:
> It could not find __vdso_getcpu and __vdso_gettimeofday when test
> getcpu and gettimeofday on LoongArch.
> 
>   # make headers && cd tools/testing/selftests/vDSO && make
>   # ./vdso_test_getcpu
>   Could not find __vdso_getcpu
>   # ./vdso_test_gettimeofday
>   Could not find __vdso_gettimeofday
> 
> One simple way is to add LoongArch case to define version and name,
> just like commit d942f231afc0 ("selftests/vDSO: Add riscv getcpu &
> gettimeofday test"), but it is not the best way.
> 
> Since each architecture has already defined names and versions in
> vdso_config.h, it is proper to include vdso_config.h to get version
> and name for all archs.
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Tested on x86, works fine.
Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  .../testing/selftests/vDSO/vdso_test_getcpu.c | 16 +++++-------
>  .../selftests/vDSO/vdso_test_gettimeofday.c   | 26 +++++--------------
>  2 files changed, 13 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_getcpu.c b/tools/testing/selftests/vDSO/vdso_test_getcpu.c
> index 1df5d057d79f..b758f68c6c9c 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_getcpu.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_getcpu.c
> @@ -13,13 +13,7 @@
>  
>  #include "../kselftest.h"
>  #include "parse_vdso.h"
> -
> -#if defined(__riscv)
> -const char *version = "LINUX_4.15";
> -#else
> -const char *version = "LINUX_2.6";
> -#endif
> -const char *name = "__vdso_getcpu";
> +#include "vdso_config.h"
>  
>  struct getcpu_cache;
>  typedef long (*getcpu_t)(unsigned int *, unsigned int *,
> @@ -27,6 +21,8 @@ typedef long (*getcpu_t)(unsigned int *, unsigned int *,
>  
>  int main(int argc, char **argv)
>  {
> +	const char *version = versions[VDSO_VERSION];
> +	const char **name = (const char **)&names[VDSO_NAMES];
>  	unsigned long sysinfo_ehdr;
>  	unsigned int cpu, node;
>  	getcpu_t get_cpu;
> @@ -40,9 +36,9 @@ int main(int argc, char **argv)
>  
>  	vdso_init_from_sysinfo_ehdr(getauxval(AT_SYSINFO_EHDR));
>  
> -	get_cpu = (getcpu_t)vdso_sym(version, name);
> +	get_cpu = (getcpu_t)vdso_sym(version, name[4]);
>  	if (!get_cpu) {
> -		printf("Could not find %s\n", name);
> +		printf("Could not find %s\n", name[4]);
>  		return KSFT_SKIP;
>  	}
>  
> @@ -50,7 +46,7 @@ int main(int argc, char **argv)
>  	if (ret == 0) {
>  		printf("Running on CPU %u node %u\n", cpu, node);
>  	} else {
> -		printf("%s failed\n", name);
> +		printf("%s failed\n", name[4]);
>  		return KSFT_FAIL;
>  	}
>  
> diff --git a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c
> index e411f287a426..ee4f1ca56a71 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c
> @@ -18,25 +18,13 @@
>  
>  #include "../kselftest.h"
>  #include "parse_vdso.h"
> -
> -/*
> - * ARM64's vDSO exports its gettimeofday() implementation with a different
> - * name and version from other architectures, so we need to handle it as
> - * a special case.
> - */
> -#if defined(__aarch64__)
> -const char *version = "LINUX_2.6.39";
> -const char *name = "__kernel_gettimeofday";
> -#elif defined(__riscv)
> -const char *version = "LINUX_4.15";
> -const char *name = "__vdso_gettimeofday";
> -#else
> -const char *version = "LINUX_2.6";
> -const char *name = "__vdso_gettimeofday";
> -#endif
> +#include "vdso_config.h"
>  
>  int main(int argc, char **argv)
>  {
> +	const char *version = versions[VDSO_VERSION];
> +	const char **name = (const char **)&names[VDSO_NAMES];
> +
>  	unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
>  	if (!sysinfo_ehdr) {
>  		printf("AT_SYSINFO_EHDR is not present!\n");
> @@ -47,10 +35,10 @@ int main(int argc, char **argv)
>  
>  	/* Find gettimeofday. */
>  	typedef long (*gtod_t)(struct timeval *tv, struct timezone *tz);
> -	gtod_t gtod = (gtod_t)vdso_sym(version, name);
> +	gtod_t gtod = (gtod_t)vdso_sym(version, name[0]);
>  
>  	if (!gtod) {
> -		printf("Could not find %s\n", name);
> +		printf("Could not find %s\n", name[0]);
>  		return KSFT_SKIP;
>  	}
>  
> @@ -61,7 +49,7 @@ int main(int argc, char **argv)
>  		printf("The time is %lld.%06lld\n",
>  		       (long long)tv.tv_sec, (long long)tv.tv_usec);
>  	} else {
> -		printf("%s failed\n", name);
> +		printf("%s failed\n", name[0]);
>  		return KSFT_FAIL;
>  	}
>  

-- 
BR,
Muhammad Usama Anjum