Xiaofeng Yuan <[email protected]> writes:
> +static __always_inline bool riscv_insn_is_lr(u32 code)
> +{
> + u32 funct5 = (code >> 27) & 0x1f;
> + u32 funct3 = (code >> 12) & 0x7;
> +
> + return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_AMO &&
> + funct5 == RVG_AMO_FUNCT5_LR &&
> + (funct3 == RVG_FUNCT3_AMO_W || funct3 == RVG_FUNCT3_AMO_D) &&
> + !(code & GENMASK(24, 20));
> +}
> +
> +static __always_inline bool riscv_insn_is_sc(u32 code)
> +{
> + u32 funct5 = (code >> 27) & 0x1f;
> + u32 funct3 = (code >> 12) & 0x7;
> +
> + return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_AMO &&
> + funct5 == RVG_AMO_FUNCT5_SC &&
> + (funct3 == RVG_FUNCT3_AMO_W || funct3 == RVG_FUNCT3_AMO_D);
> +}
There are already RV_EXTRACT_FUNCT3() and RV_EXTRACT_RS2_REG(), we
should use them to avoid code duplications.
And I think we should also have RV_EXTRACT_FUNCT5() to stay consistent.
> +static int __kprobes arch_check_kprobe(unsigned long addr)
> {
> - unsigned long tmp, offset;
> + unsigned long pc, offset;
> + int outstanding_lr = -1;
>
> - /* start iterating at the closest preceding symbol */
> if (!kallsyms_lookup_size_offset(addr, NULL, &offset))
> - return false;
> + return -EILSEQ;
>
> - tmp = addr - offset;
> + /* pc starts at the closest preceding symbol, a known instruction
> boundary */
> + pc = addr - offset;
>
> - while (tmp <= addr) {
> - if (tmp == addr)
> - return true;
> + while (pc < addr) {
> + u32 insn = get_unaligned((u32 *)pc);
> + unsigned int len = GET_INSN_LENGTH(insn);
>
> - tmp += GET_INSN_LENGTH(*(u16 *)tmp);
> + if (riscv_insn_is_lr(insn))
> + outstanding_lr = 0;
> + else if (riscv_insn_is_sc(insn))
> + outstanding_lr = -1;
> + else if (outstanding_lr >= 0 && ++outstanding_lr > MAX_LRSC_GAP)
> + outstanding_lr = -1;
> +
> + pc += len;
> }
The spec says "The loop comprises only an LR/SC sequence and code to
retry the sequence in the case of failure, and must comprise at most 16
instructions placed sequentially in memory". The 16 instructions limit
includes LR. So I think this implementation is off by one:
lr (outstanding_lr=0)
inst1 (outstanding_lr=1)
inst2 (outstanding_lr=2)
...
inst16 (outstanding_lr=16)
inst17 (outstanding_lr=-1)
As it is considering instruction from LR to inst16 to be a loop, so it
allows 17 instructions.
Nam
