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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The upper 20 bits of the jump offset are discarded when calculating the immediate for the `auipc` instruction, completely breaking alternative patching for `auipc`+`jalr` sequences.
--

--- Patch [2]: [PATCH v3 02/17] riscv: alternatives: Use generated instruction headers for patching code ---
commit 2a83fc3ccb6ec3374819154c22898986ddd592af
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: alternatives: Use generated instruction headers for patching code

Migrate the alternatives patching code to use the generated instruction
headers instead of the hand-written instruction composition functions.

> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -78,14 +79,24 @@ static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 auipc_insn,

[ ... ]

>  	/* 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);

[Severity: Critical]
Does this code drop the upper 20 bits of the jump offset?

The original code combined the upper 20 bits with the sign-compensation bit 
like this:

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

It appears that auipc_imm now only contains the sign-compensation bit but 
leaves out the base imm value. Will this cause the auipc instruction to be 
patched with a 0 upper immediate (or 1 if sign extension bleeds), destroying 
the target address for any jump offset outside the +/-4KB range?

> +	riscv_insn_jalr_insert_imm(&call[1], imm);
>  
>  	/* patch the call place again */
>  	patch_text_nosync(ptr, call, sizeof(u32) * 2);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-riscv_insn_table-v3-0-3f8e5f75ba97@gmail.com?part=2