Re: [PATCH] arm64: traps: Add a macro to simplify the condition codes check

[Vladimir Murzin <vladimir.murzin@arm.com>]

Hi Jinjie,

On 4/22/26 04:06, Jinjie Ruan wrote:
> 
> On 3/20/2026 4:28 PM, Jinjie Ruan wrote:
>> Add DEFINE_COND_CHECK macro to define the simple __check_* functions
>> to simplify the condition codes check.
>>
>> No functional changes.
> Gentle ping.
> 
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  arch/arm64/kernel/traps.c | 59 ++++++++++-----------------------------
>>  1 file changed, 15 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index 914282016069..6216fe9e8e42 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -49,45 +49,21 @@
>>  #include <asm/system_misc.h>
>>  #include <asm/sysreg.h>
>>  
>> -static bool __kprobes __check_eq(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_Z_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_ne(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_Z_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_cs(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_C_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_cc(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_C_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_mi(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_N_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_pl(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_N_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_vs(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_V_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_vc(unsigned long pstate)
>> -{
>> -	return (pstate & PSR_V_BIT) == 0;
>> -}
>> +#define DEFINE_COND_CHECK(name, flag, expected)			\
>> +static bool __kprobes __check_##name(unsigned long pstate)	\
>> +{								\
>> +	return ((pstate & (flag)) != 0) == (expected);		\
>> +}
>> +
>> +DEFINE_COND_CHECK(eq, PSR_Z_BIT, true)
>> +DEFINE_COND_CHECK(ne, PSR_Z_BIT, false)
>> +DEFINE_COND_CHECK(cs, PSR_C_BIT, true)
>> +DEFINE_COND_CHECK(cc, PSR_C_BIT, false)
>> +DEFINE_COND_CHECK(mi, PSR_N_BIT, true)
>> +DEFINE_COND_CHECK(pl, PSR_N_BIT, false)
>> +DEFINE_COND_CHECK(vs, PSR_V_BIT, true)
>> +DEFINE_COND_CHECK(vc, PSR_V_BIT, false)
>> +DEFINE_COND_CHECK(al, 0, false)		/* Always true */
>>  
>>  static bool __kprobes __check_hi(unsigned long pstate)
>>  {
>> @@ -131,11 +107,6 @@ static bool __kprobes __check_le(unsigned long pstate)
>>  	return (temp & PSR_N_BIT) != 0;
>>  }
>>  
>> -static bool __kprobes __check_al(unsigned long pstate)
>> -{
>> -	return true;
>> -}
>> -
>>  /*
>>   * Note that the ARMv8 ARM calls condition code 0b1111 "nv", but states that
>>   * it behaves identically to 0b1110 ("al").
> 

It looks like we now have a mix of checks implemented via macros
and others written out explicitly. The existing approach has the
advantage of being consistent and easy to follow, whereas
introducing macros here, even if it reduces some duplication,
adds a bit of cognitive overhead when reading the code.

This may come down to preference, but I think sticking to a
single, consistent style would make the code easier to scan and
maintain.

Just my 2p.

Cheers
Vladimir