Charlie Jenkins <[email protected]> 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
