Re: [RFC PATCH bpf-next 4/7] bpf: x86: Support in-register struct arguments

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Jul 26, 2022 at 10:11:51AM -0700, Yonghong Song wrote:

SNIP

>  
> -static void restore_regs(const struct btf_func_model *m, u8 **prog, int nr_args,
> -			 int regs_off)
> +static void __save_struct_arg_regs(u8 **prog, int curr_reg_idx, int nr_regs,
> +				   int struct_val_off, int stack_start_idx)
>  {
> -	int i;
> +	int i, reg_idx;
> +
> +	/* Save struct registers to stack.
> +	 * For example, argument 1 (second argument) size is 16 which occupies two
> +	 * registers, these two register values will be saved in stack.
> +	 * mov QWORD PTR [rbp-0x40],rsi
> +	 * mov QWORD PTR [rbp-0x38],rdx
> +	 */
> +	for (i = 0; i < nr_regs; i++) {
> +		reg_idx = curr_reg_idx + i;
> +		emit_stx(prog, bytes_to_bpf_size(8),
> +			 BPF_REG_FP,
> +			 reg_idx == 5 ? X86_REG_R9 : BPF_REG_1 + reg_idx,
> +			 -(struct_val_off - stack_start_idx * 8));
> +		stack_start_idx++;
> +	}
> +}
> +
> +static void save_regs(const struct btf_func_model *m, u8 **prog, int nr_args,
> +		      int regs_off, int struct_val_off)
> +{
> +	int curr_arg_idx, curr_reg_idx, curr_s_stack_off;
> +	int s_size, s_arg_idx, s_arg_nregs;
> +
> +	curr_arg_idx = curr_reg_idx = curr_s_stack_off = 0;
> +	for (int i = 0; i < MAX_BPF_FUNC_STRUCT_ARGS; i++) {
> +		s_size = m->struct_arg_bsize[i];
> +		if (!s_size)
> +			return __save_normal_arg_regs(m, prog, curr_arg_idx, nr_args - curr_arg_idx,
> +						      curr_reg_idx, regs_off);

could we just do break in here instead?

SNIP

> +
> +static void restore_regs(const struct btf_func_model *m, u8 **prog, int nr_args,
> +			 int regs_off, int struct_val_off)
> +{
> +	int curr_arg_idx, curr_reg_idx, curr_s_stack_off;
> +	int s_size, s_arg_idx, s_arg_nregs;
> +
> +	curr_arg_idx = curr_reg_idx = curr_s_stack_off = 0;
> +	for (int i = 0; i < MAX_BPF_FUNC_STRUCT_ARGS; i++) {
> +		s_size = m->struct_arg_bsize[i];
> +		if (!s_size)
> +			return __restore_normal_arg_regs(m, prog, curr_arg_idx,
> +							 nr_args - curr_arg_idx,
> +							 curr_reg_idx, regs_off);

same here

jirka

> +
> +		s_arg_idx = m->struct_arg_idx[i];
> +		s_arg_nregs = (s_size + 7) / 8;
> +
> +		__restore_normal_arg_regs(m, prog, curr_arg_idx, s_arg_idx - curr_arg_idx,
> +					  curr_reg_idx, regs_off);
> +		__restore_struct_arg_regs(prog, curr_reg_idx + s_arg_idx - curr_arg_idx,
> +					  s_arg_nregs, struct_val_off, curr_s_stack_off);
> +		curr_reg_idx += s_arg_idx - curr_arg_idx + s_arg_nregs;
> +		curr_s_stack_off += s_arg_nregs;
> +		curr_arg_idx = s_arg_idx + 1;
> +	}
> +
> +	__restore_normal_arg_regs(m, prog, curr_arg_idx, nr_args - curr_arg_idx, curr_reg_idx,
> +				  regs_off);
>  }
>  

SNIP