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

[Yonghong Song <yhs@fb.com>]

On 7/29/22 4:10 AM, Jiri Olsa wrote:
> 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?

Thanks for pointing out. Yes, we can just do break and later call
__save_normal_arg_regs(...) will handle this automatically.
The same for another place you pointed below.

> 
> 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