[PATCH 5/7] RISC-V: fix auipc-jalr addresses in patched alternatives
Lad, Prabhakar
prabhakar.csengg at gmail.com
Mon Nov 21 01:50:09 PST 2022
Hi Heiko,
On Thu, Nov 10, 2022 at 4:50 PM Heiko Stuebner <heiko at sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner at vrull.eu>
>
> Alternatives live in a different section, so addresses used by call
> functions will point to wrong locations after the patch got applied.
>
> Similar to arm64, adjust the location to consider that offset.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner at vrull.eu>
> ---
> arch/riscv/kernel/cpufeature.c | 79 +++++++++++++++++++++++++++++++++-
> 1 file changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 694267d1fe81..026512ca9c4c 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -298,6 +298,74 @@ static u32 __init_or_module cpufeature_probe(unsigned int stage)
> return cpu_req_feature;
> }
>
> +#include <asm/parse_asm.h>
> +
> +DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> +DECLARE_INSN(auipc, MATCH_AUIPC, MASK_AUIPC)
> +
> +static inline bool is_auipc_jalr_pair(long insn1, long insn2)
> +{
> + return is_auipc_insn(insn1) && is_jalr_insn(insn2);
> +}
> +
> +#define JALR_SIGN_MASK BIT(I_IMM_SIGN_OPOFF - I_IMM_11_0_OPOFF)
> +#define JALR_OFFSET_MASK I_IMM_11_0_MASK
> +#define AUIPC_OFFSET_MASK U_IMM_31_12_MASK
> +#define AUIPC_PAD (0x00001000)
> +#define JALR_SHIFT I_IMM_11_0_OPOFF
> +
> +#define to_jalr_imm(offset) \
> + ((offset & I_IMM_11_0_MASK) << I_IMM_11_0_OPOFF)
> +
> +#define to_auipc_imm(offset) \
> + ((offset & JALR_SIGN_MASK) ? \
> + ((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) : \
> + (offset & AUIPC_OFFSET_MASK))
> +
> +static void riscv_alternative_fix_auipc_jalr(unsigned int *alt_ptr,
> + unsigned int len, int patch_offset)
> +{
> + int num_instr = len / sizeof(u32);
> + unsigned int call[2];
> + int i;
> + int imm1;
> + u32 rd1;
> +
> + for (i = 0; i < num_instr; i++) {
> + /* is there a further instruction? */
> + if (i + 1 >= num_instr)
> + continue;
> +
> + if (!is_auipc_jalr_pair(*(alt_ptr + i), *(alt_ptr + i + 1)))
> + continue;
> +
> + /* call will use ra register */
> + rd1 = EXTRACT_RD_REG(*(alt_ptr + i));
> + if (rd1 != 1)
> + continue;
> +
> + /* get and adjust new target address */
> + imm1 = EXTRACT_UTYPE_IMM(*(alt_ptr + i));
> + imm1 += EXTRACT_ITYPE_IMM(*(alt_ptr + i + 1));
> + imm1 -= patch_offset;
> +
> + /* pick the original auipc + jalr */
> + call[0] = *(alt_ptr + i);
> + call[1] = *(alt_ptr + i + 1);
> +
> + /* drop the old IMMs */
> + call[0] &= ~(U_IMM_31_12_MASK);
> + call[1] &= ~(I_IMM_11_0_MASK << I_IMM_11_0_OPOFF);
> +
> + /* add the adapted IMMs */
> + call[0] |= to_auipc_imm(imm1);
> + call[1] |= to_jalr_imm(imm1);
> +
> + /* patch the call place again */
> + patch_text_nosync(alt_ptr + i * sizeof(u32), call, 8);
> + }
> +}
> +
I have the below assembly code which I have tested without the
alternatives for the RZ/Five CMO,
#define ALT_CMO_OP(_op, _start, _size, _cachesize, _dir, _ops) \
asm volatile(".option push\n\t\n\t" \
".option norvc\n\t" \
".option norelax\n\t" \
"addi sp,sp,-16\n\t" \
"sd s0,0(sp)\n\t" \
"sd ra,8(sp)\n\t" \
"addi s0,sp,16\n\t" \
"mv a4,%6\n\t" \
"mv a3,%5\n\t" \
"mv a2,%4\n\t" \
"mv a1,%3\n\t" \
"mv a0,%0\n\t" \
"call rzfive_cmo\n\t" \
"ld ra,8(sp)\n\t" \
"ld s0,0(sp)\n\t" \
"addi sp,sp,16\n\t" \
".option pop\n\t" \
: : "r"(_cachesize), \
"r"((unsigned long)(_start) & ~((_cachesize) - 1UL)), \
"r"((unsigned long)(_start) + (_size)), \
"r"((unsigned long) (_start)), \
"r"((unsigned long) (_size)), \
"r"((unsigned long) (_dir)), \
"r"((unsigned long) (_ops)) \
: "a0", "a1", "a2", "a3", "a4", "memory")
Now when integrate this with ALTERNATIVE_2() as below,
#define ALT_CMO_OP(_op, _start, _size, _cachesize, _dir, _ops) \
asm volatile(ALTERNATIVE_2( \
__nops(14), \
"mv a0, %1\n\t" \
"j 2f\n\t" \
"3:\n\t" \
"cbo." __stringify(_op) " (a0)\n\t" \
"add a0, a0, %0\n\t" \
"2:\n\t" \
"bltu a0, %2, 3b\n\t" \
__nops(8), 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
".option push\n\t\n\t" \
".option norvc\n\t" \
".option norelax\n\t" \
"addi sp,sp,-16\n\t" \
"sd s0,0(sp)\n\t" \
"sd ra,8(sp)\n\t" \
"addi s0,sp,16\n\t" \
"mv a4,%6\n\t" \
"mv a3,%5\n\t" \
"mv a2,%4\n\t" \
"mv a1,%3\n\t" \
"mv a0,%0\n\t" \
"call rzfive_cmo\n\t" \
"ld ra,8(sp)\n\t" \
"ld s0,0(sp)\n\t" \
"addi sp,sp,16\n\t" \
".option pop\n\t" \
, ANDESTECH_VENDOR_ID, \
ERRATA_ANDESTECH_NO_IOCP, CONFIG_ERRATA_RZFIVE_CMO) \
: : "r"(_cachesize), \
"r"((unsigned long)(_start) & ~((_cachesize) - 1UL)), \
"r"((unsigned long)(_start) + (_size)), \
"r"((unsigned long) (_start)), \
"r"((unsigned long) (_size)), \
"r"((unsigned long) (_dir)), \
"r"((unsigned long) (_ops)) \
: "a0", "a1", "a2", "a3", "a4", "memory")
I am seeing kernel panic with this change. Looking at the
riscv_alternative_fix_auipc_jalr() implementation it assumes the rest
of the alternative options are calls too. Is my understanding correct
here?
Do you think this is the correct approach in my case?
Note, I wanted to test with ALTERNATIVE_2() first to make sure
everything is okay and then later test my ALTERNATIVE_3()
implementation.
Cheers,
Prabhakar
More information about the linux-riscv
mailing list