Re: [PATCH bpf-next 2/7] arm64: insn: Add encoder for ADD/SUB (extended register)

On 8/11/2026 3:09 AM, Puranjay Mohan wrote:
> From: Tejun Heo <[email protected]>
> 
> The insn library encodes the immediate and shifted-register forms of
> ADD/SUB but not the extended-register form. The BPF JIT wants it to
> rebase a 32-bit arena offset onto the arena kernel base in a single
> instruction, add xN, xBASE, wN, uxtw, instead of a separate zero-extend
> followed by a plain add.
> 
> Add aarch64_insn_gen_add_sub_extended_reg(), modeled on the
> shifted-register generator. The option and imm3 fields occupy the same
> bits as the shifted form's shift amount, so they are encoded through the
> existing IMM_6 field type.
> 
> Note that register 31 does not mean the same thing in the two forms: in
> the extended-register encoding it is SP for Rn, and for Rd unless the
> instruction sets the flags, while it stays XZR for Rm. Callers porting a
> shifted-register site that passes A64_ZR need to be aware of that, so
> say so above the function.
> 
> Signed-off-by: Tejun Heo <[email protected]>
> Signed-off-by: Puranjay Mohan <[email protected]>
> ---
>   arch/arm64/include/asm/insn.h | 23 ++++++++++++++
>   arch/arm64/lib/insn.c         | 60 +++++++++++++++++++++++++++++++++++
>   2 files changed, 83 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index cc0702fa64a79..4548e8015808d 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -205,6 +205,18 @@ enum aarch64_insn_adsb_type {
>   	AARCH64_INSN_ADSB_SUB_SETFLAGS
>   };
>   
> +/* option field of add/sub (extended register) */
> +enum aarch64_insn_extend_type {
> +	AARCH64_INSN_EXTEND_UXTB,
> +	AARCH64_INSN_EXTEND_UXTH,
> +	AARCH64_INSN_EXTEND_UXTW,
> +	AARCH64_INSN_EXTEND_UXTX,
> +	AARCH64_INSN_EXTEND_SXTB,
> +	AARCH64_INSN_EXTEND_SXTH,
> +	AARCH64_INSN_EXTEND_SXTW,
> +	AARCH64_INSN_EXTEND_SXTX,
> +};
> +

I checked the Arm Architecture Reference Manual(ARM DDI 0487 M.c).
The enum values correctly match the 3-bit "extend" values defined in
the manual.

>   enum aarch64_insn_movewide_type {
>   	AARCH64_INSN_MOVEWIDE_ZERO,
>   	AARCH64_INSN_MOVEWIDE_KEEP,
> @@ -378,6 +390,10 @@ __AARCH64_INSN_FUNCS(add,	0x7F200000, 0x0B000000)
>   __AARCH64_INSN_FUNCS(adds,	0x7F200000, 0x2B000000)
>   __AARCH64_INSN_FUNCS(sub,	0x7F200000, 0x4B000000)
>   __AARCH64_INSN_FUNCS(subs,	0x7F200000, 0x6B000000)
> +__AARCH64_INSN_FUNCS(add_ext,	0x7F200000, 0x0B200000)
> +__AARCH64_INSN_FUNCS(adds_ext,	0x7F200000, 0x2B200000)
> +__AARCH64_INSN_FUNCS(sub_ext,	0x7F200000, 0x4B200000)
> +__AARCH64_INSN_FUNCS(subs_ext,	0x7F200000, 0x6B200000)

Page C4-814 of the manual ARM DDI 0487 M.c says the bits 22~23
are fixed to 0 for ADD/SUB(extended register) instructions, so
the mask should be 0x7FE00000, not 0x7F200000.

>   __AARCH64_INSN_FUNCS(madd,	0x7FE08000, 0x1B000000)
>   __AARCH64_INSN_FUNCS(msub,	0x7FE08000, 0x1B008000)
>   __AARCH64_INSN_FUNCS(udiv,	0x7FE0FC00, 0x1AC00800)
> @@ -637,6 +653,13 @@ u32 aarch64_insn_gen_add_sub_shifted_reg(enum aarch64_insn_register dst,
>   					 int shift,
>   					 enum aarch64_insn_variant variant,
>   					 enum aarch64_insn_adsb_type type);
> +u32 aarch64_insn_gen_add_sub_extended_reg(enum aarch64_insn_register dst,
> +					  enum aarch64_insn_register src,
> +					  enum aarch64_insn_register reg,
> +					  enum aarch64_insn_extend_type extend,
> +					  int shift,
> +					  enum aarch64_insn_variant variant,
> +					  enum aarch64_insn_adsb_type type);
>   u32 aarch64_insn_gen_data1(enum aarch64_insn_register dst,
>   			   enum aarch64_insn_register src,
>   			   enum aarch64_insn_variant variant,
> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
> index 37ce75f7f1f08..e70ac02385153 100644
> --- a/arch/arm64/lib/insn.c
> +++ b/arch/arm64/lib/insn.c
> @@ -986,6 +986,66 @@ u32 aarch64_insn_gen_add_sub_shifted_reg(enum aarch64_insn_register dst,
>   	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_6, insn, shift);
>   }
>   
> +/*
> + * Unlike the shifted-register form, register 31 is not XZR everywhere here:
> + * it encodes SP for @src, and for @dst too unless @type sets the flags. Only
> + * @reg keeps the XZR meaning.
> + */
> +u32 aarch64_insn_gen_add_sub_extended_reg(enum aarch64_insn_register dst,
> +					  enum aarch64_insn_register src,
> +					  enum aarch64_insn_register reg,
> +					  enum aarch64_insn_extend_type extend,
> +					  int shift,
> +					  enum aarch64_insn_variant variant,
> +					  enum aarch64_insn_adsb_type type)
> +{
> +	u32 insn;
> +
> +	switch (type) {
> +	case AARCH64_INSN_ADSB_ADD:
> +		insn = aarch64_insn_get_add_ext_value();
> +		break;
> +	case AARCH64_INSN_ADSB_SUB:
> +		insn = aarch64_insn_get_sub_ext_value();
> +		break;
> +	case AARCH64_INSN_ADSB_ADD_SETFLAGS:
> +		insn = aarch64_insn_get_adds_ext_value();
> +		break;
> +	case AARCH64_INSN_ADSB_SUB_SETFLAGS:
> +		insn = aarch64_insn_get_subs_ext_value();
> +		break;
> +	default:
> +		pr_err("%s: unknown add/sub encoding %d\n", __func__, type);
> +		return AARCH64_BREAK_FAULT;
> +	}
> +
> +	switch (variant) {
> +	case AARCH64_INSN_VARIANT_32BIT:
> +		break;
> +	case AARCH64_INSN_VARIANT_64BIT:
> +		insn |= AARCH64_INSN_SF_BIT;
> +		break;
> +	default:
> +		pr_err("%s: unknown variant encoding %d\n", __func__, variant);
> +		return AARCH64_BREAK_FAULT;
> +	}
> +
> +	if (shift < 0 || shift > 4) {
> +		pr_err("%s: invalid shift encoding %d\n", __func__, shift);
> +		return AARCH64_BREAK_FAULT;
> +	}
> +
> +	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RD, insn, dst);
> +
> +	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn, src);
> +
> +	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, reg);
> +
> +	/* option in bits [15:13] and imm3 in [12:10] together fill IMM_6 */
> +	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_6, insn,
> +					     (extend << 3) | shift);
> +}
> +
>   u32 aarch64_insn_gen_data1(enum aarch64_insn_register dst,
>   			   enum aarch64_insn_register src,
>   			   enum aarch64_insn_variant variant,

Except for the mask value mentioned above, the rest lgtm.

Reviewed-by: Xu Kuohai <[email protected]>