Re: [PATCH v3 02/17] riscv: alternatives: Use generated instruction headers for patching code

[Nam Cao <namcao@linutronix.de>]

Charlie Jenkins <thecharlesjenkins@gmail.com> writes:
> @@ -78,14 +79,24 @@ static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 auipc_insn,
>  					     u32 jalr_insn, int patch_offset)
>  {
>  	u32 call[2] = { auipc_insn, jalr_insn };
> +	u32 auipc_imm;
>  	s32 imm;
>  
>  	/* get and adjust new target address */
> -	imm = riscv_insn_extract_utype_itype_imm(auipc_insn, jalr_insn);
> +	imm = riscv_insn_auipc_extract_imm(auipc_insn) + riscv_insn_jalr_extract_imm(jalr_insn);
>  	imm -= patch_offset;
>  
> +	/*
> +	 * When the 32-bit immediate is split across auipc and jalr, the
> +	 * constructed immediates need to be treated as individually sign
> +	 * extended numbers. Add the sign bit of the lower 12 bits to the upper
> +	 * 20 bits to undo the bleeding of the sign.
> +	 */
> +	auipc_imm = (imm & BIT(11)) << 1;
> +
>  	/* update instructions */
> -	riscv_insn_insert_utype_itype_imm(&call[0], &call[1], imm);
> +	riscv_insn_auipc_insert_imm(&call[0], auipc_imm);
> +	riscv_insn_jalr_insert_imm(&call[1], imm);

Looking at the old riscv_insn_insert_utype_itype_imm(), for auipc we have

	*utype_insn &= ~(RV_U_IMM_31_12_MASK);
	*utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1);

but the new code does

	auipc_imm = (imm & BIT(11)) << 1;
	*_insn &= ~GENMASK(31, 12);
	*_insn |= (((auipc_imm >> 12) & GENMASK(19, 0)) << 12);

which is not equivalent. The new code is missing (imm & RV_U_IMM_31_12_MASK)
from the old code.

Or am I confused somewhere?

Nam