Re: [PATCH] kselftest/arm64: pac: Fix skipping of tests on systems without PAC

From: Amit Kachhap <amit.kachhap@arm.com>
To: Mark Brown <broonie@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Shuah Khan <shuah@kernel.org>
Cc: Boyan Karatotev <boyan.karatotev@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH] kselftest/arm64: pac: Fix skipping of tests on systems without PAC
Date: Fri, 20 Aug 2021 12:39:39 +0530	[thread overview]
Message-ID: <a988b3e2-5f88-fbd5-99da-2066a65a17b2@arm.com> (raw)
In-Reply-To: <20210819165723.43903-1-broonie@kernel.org>


On 8/19/21 10:27 PM, Mark Brown wrote:
> The PAC tests check to see if the system supports the relevant PAC features
> but instead of skipping the tests if they can't be executed they fail the
> tests which makes things look like they're not working when they are.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/arm64/pauth/pac.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/pauth/pac.c b/tools/testing/selftests/arm64/pauth/pac.c
> index 592fe538506e..b743daa772f5 100644
> --- a/tools/testing/selftests/arm64/pauth/pac.c
> +++ b/tools/testing/selftests/arm64/pauth/pac.c
> @@ -25,13 +25,15 @@
>   do { \
>   	unsigned long hwcaps = getauxval(AT_HWCAP); \
>   	/* data key instructions are not in NOP space. This prevents a SIGILL */ \
> -	ASSERT_NE(0, hwcaps & HWCAP_PACA) TH_LOG("PAUTH not enabled"); \
> +	if (!(hwcaps & HWCAP_PACA))					\
> +		SKIP(return, "PAUTH not enabled"); \
>   } while (0)
>   #define ASSERT_GENERIC_PAUTH_ENABLED() \

May be ASSERT_GENERIC_PAUTH_ENABLED can be replaced with
something like VERIFY_GENERIC_PAUTH_ENABLED

or can be modified like below and instead of failing it skips with a
message

   -	ASSERT_NE(0, hwcaps & HWCAP_PACA) TH_LOG("PAUTH not enabled"); \
   +	ASSERT_NE(0, hwcaps & HWCAP_PACA) SKIP(return, "PAUTH not 
enabled"); \					


>   do { \
>   	unsigned long hwcaps = getauxval(AT_HWCAP); \
>   	/* generic key instructions are not in NOP space. This prevents a SIGILL */ \
> -	ASSERT_NE(0, hwcaps & HWCAP_PACG) TH_LOG("Generic PAUTH not enabled"); \
> +	if (!(hwcaps & HWCAP_PACG)) \
> +		SKIP(return, "Generic PAUTH not enabled");	\
>   } while (0)
>   
>   void sign_specific(struct signatures *sign, size_t val)
> @@ -256,7 +258,7 @@ TEST(single_thread_different_keys)
>   	unsigned long hwcaps = getauxval(AT_HWCAP);
>   
>   	/* generic and data key instructions are not in NOP space. This prevents a SIGILL */
> -	ASSERT_NE(0, hwcaps & HWCAP_PACA) TH_LOG("PAUTH not enabled");
> +	ASSERT_PAUTH_ENABLED();
>   	if (!(hwcaps & HWCAP_PACG)) {
>   		TH_LOG("WARNING: Generic PAUTH not enabled. Skipping generic key checks");
>   		nkeys = NKEYS - 1;
> @@ -299,7 +301,7 @@ TEST(exec_changed_keys)
>   	unsigned long hwcaps = getauxval(AT_HWCAP);
>   
>   	/* generic and data key instructions are not in NOP space. This prevents a SIGILL */
> -	ASSERT_NE(0, hwcaps & HWCAP_PACA) TH_LOG("PAUTH not enabled");
> +	ASSERT_PAUTH_ENABLED();
>   	if (!(hwcaps & HWCAP_PACG)) {
>   		TH_LOG("WARNING: Generic PAUTH not enabled. Skipping generic key checks");
>   		nkeys = NKEYS - 1;
>