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

[Mark Rutland]

On Wed, Apr 22, 2026 at 03:43:39PM +0100, Vladimir Murzin wrote:
> 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@xxxxxxxxxx>
> >> ---
> >>  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.

FWIW, I agree. I think it'd be better to leave this as-is for now.

Mark.