Re: [PATCH v6 2/2] riscv: kprobes: add KUnit test for LR/SC sequence rejection

[Nam Cao <namcao@linutronix.de>]

Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> writes:
> +.section .rodata
> +SYM_DATA_START(test_kprobes_lrsc_offsets)
> +	RISCV_PTR .Llrsc_before
> +	RISCV_PTR .Llrsc_lr
> +	RISCV_PTR .Llrsc_inside
> +	RISCV_PTR .Llrsc_sc
> +	RISCV_PTR .Llrsc_after
> +SYM_DATA_END(test_kprobes_lrsc_offsets)

"_offsets" is not precise, as this array has absolute addresses. Maybe
rename to "_addresses"?

> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> index 027424a3f..c2d9c87f2 100644
> --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> @@ -43,8 +43,39 @@ static void test_kprobe_riscv(struct kunit *test)
>  	kfree(kp);
>  }
>  
> +static void test_kprobe_lrsc(struct kunit *test)
> +{
> +	/*
> +	 * test_kprobes_lrsc_offsets[] points, in order, at an instruction
> +	 * right before an LR/SC sequence, the LR itself, an instruction
> +	 * inside the sequence, the SC, and an instruction right after it.
> +	 * reject[] must have the same number of entries.
> +	 */
> +	bool reject[] = { false, true, true, true, false };

I am not a fan of coupled code. A change at test_kprobes_lrsc_offsets
mandates a change here.

Instead of this, how about we split test_kprobes_lrsc_offsets into
test_kprobes_lrsc_allowed_addresses and test_kprobes_lrsc_rejected_addresses?

Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv