Re: [Qemu-devel] [PATCH v3 22/50] *-user: plugin syscalls

[Alex Bennée]

Pranith Kumar <address@hidden> writes:

> Minor nits.
>
> On Fri, Jun 14, 2019 at 11:41 AM Alex Bennée <address@hidden> wrote:
>>
>> From: "Emilio G. Cota" <address@hidden>
>>
>> Signed-off-by: Emilio G. Cota <address@hidden>
>> ---
>>  bsd-user/syscall.c   | 9 +++++++++
>>  linux-user/syscall.c | 3 +++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
>> index 84a983a9a1..50e47d217c 100644
>> --- a/bsd-user/syscall.c
>> +++ b/bsd-user/syscall.c
>> @@ -323,6 +323,8 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      gemu_log("freebsd syscall %d\n", num);
>>  #endif
>>      trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> arg7, arg8);
>> +    qemu_plugin_vcpu_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> arg7,
>> +                             arg8);
>
> Looking at the previous line, seems like you can avoid splitting this
> line into 2. Keeps it more consistent that way.

Technically the trace line is over-long... I can make them both
consistent with each other but it would be nicer to wrap them both into
a common function.

Unfortunately this seems a little tricky for *-user as they are kept
pretty separate.

>
>>      if(do_strace)
>>          print_freebsd_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
>>
>> @@ -404,6 +406,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      if (do_strace)
>>          print_freebsd_syscall_ret(num, ret);
>>      trace_guest_user_syscall_ret(cpu, num, ret);
>> +    qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
>>      return ret;
>>   efault:
>>      ret = -TARGET_EFAULT;
>> @@ -422,6 +425,8 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      gemu_log("netbsd syscall %d\n", num);
>>  #endif
>>      trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> 0, 0);
>> +    qemu_plugin_vcpu_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> 0,
>> +                             0);
>
> ditto.
>
>>      if(do_strace)
>>          print_netbsd_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
>>
>> @@ -480,6 +485,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      if (do_strace)
>>          print_netbsd_syscall_ret(num, ret);
>>      trace_guest_user_syscall_ret(cpu, num, ret);
>> +    qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
>>      return ret;
>>   efault:
>>      ret = -TARGET_EFAULT;
>> @@ -498,6 +504,8 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      gemu_log("openbsd syscall %d\n", num);
>>  #endif
>>      trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> 0, 0);
>> +    qemu_plugin_vcpu_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> 0,
>> +                             0);
>
> ditto.
>
>>      if(do_strace)
>>          print_openbsd_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
>>
>> @@ -556,6 +564,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, 
>> abi_long arg1,
>>      if (do_strace)
>>          print_openbsd_syscall_ret(num, ret);
>>      trace_guest_user_syscall_ret(cpu, num, ret);
>> +    qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
>>      return ret;
>>   efault:
>>      ret = -TARGET_EFAULT;
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index b187c1281d..7f3cfdee84 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -11724,6 +11724,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>
>>      trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4,
>>                               arg5, arg6, arg7, arg8);
>> +    qemu_plugin_vcpu_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
>> arg7,
>> +                             arg8);
>
> This I am not sure.
>
>
>>
>>      if (unlikely(do_strace)) {
>>          print_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
>> @@ -11736,5 +11738,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>      }
>>
>>      trace_guest_user_syscall_ret(cpu, num, ret);
>> +    qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
>>      return ret;
>>  }
>> --
>> 2.20.1
>>
>>


--
Alex Bennée