Re: [RFC PATCH 4/6] perf bpf: Convert arglist to bpf prologue

[Alexei Starovoitov]

On 5/5/15 3:10 AM, He Kuang wrote:
> When all arguments in bpf config section are collected in register and
> offset form, this patch will fetch them from bpf context register and
> place them as bpf input parameters.
>
> Bpf prologue is generated as the following steps:
> 1. alloc dst address in stack -> r1
> 2. set size -> r2
> 3. fetch base register and offset -> r3
> 4. call BPF_FUNC_probe_read
> 5. loop 1
> 6. save intermediate result and process next arg
> 7. restore intermediate result to arg2~5
>
> Signed-off-by: He Kuang <hekuang@xxxxxxxxxx>
...
> +#define BPF_STRBUF_ADD(buf, statement)		\
> +	strbuf_add(buf,				\
> +		&statement,			\
> +		sizeof(struct bpf_insn))
> +
> +#define BPF_STRBUF_ADD_BUF(statement) BPF_STRBUF_ADD(buf, statement)
> +
> +int synthesize_probe_trace_arg_bpf_begin(struct strbuf *buf)
> +{
> +	/* save arg1 to ctx */
> +	BPF_STRBUF_ADD_BUF(BPF_MOV64_REG(BPF_REG_CTX, BPF_REG_ARG1));
> +	return 0;
> +}

the macro approach looks a bit ugly.
Why not to do it similar to net/core/filter.c style:
*insn++ = BPF_ALU32_IMM(BPF_RSH, dst_reg, 5);
Looks more readable to me.

> +	for (i = 0; i < num; i++) {
> +		/* restore r7~10 to arg2~5*/
> +		BPF_STRBUF_ADD_BUF(
> +			BPF_MOV64_REG(BPF_REG_ARG2 + i, BPF_REG_7 + i));
> +	}

comment and/or logic is wrong. r10 is read only stack. you cannot use
it as callee-saved.

> +	/* store intermediate results to r7~10 */
> +	BPF_STRBUF_ADD_BUF(BPF_MOV64_REG(BPF_REG_7 + index, BPF_REG_3));

should be r7-r9.
Also is there a check somewhere that accepts only 3 debuginfo-backed
args?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/