On Sat, Dec 02, 2017 at 04:05:30AM +0900, Masatake YAMATO wrote:
> * configure.ac (AC_CHECK_TYPES): Add struct kvm_regs.
> * linux/arck_kvm.c: New file.
> * linux/x86_64/arch_kvm.c: New file.

Could you please add linux/x32/arch_kvm.c containing just one line,
#include "x86_64/arch_kvm.c"?

> * kvm.c (kvm_ioctl_decode_regs): New function.
> (kvm_ioctl) <KVM_SET_REGS, KVM_GET_REGS>: Use it.
> (top-level): Include "arch_kvm.c".
> 
> Changes in v2:
> * Decode only if struct kvm_regs is available.
> * Introduce files in which arch-specific and generic stub decoders
>   are defined. Use them from kvm_ioctl_decode_regs.
> * Use umove_or_printaddr instead of umove.
> 
>   All items are suggested by ldv.
> 
> Signed-off-by: Masatake YAMATO <yam...@redhat.com>
> ---
>  configure.ac            |  2 ++
>  kvm.c                   | 28 ++++++++++++++++++++++++++--
>  linux/arch_kvm.c        |  7 +++++++
>  linux/x86_64/arch_kvm.c | 36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 2 deletions(-)
>  create mode 100644 linux/arch_kvm.c
>  create mode 100644 linux/x86_64/arch_kvm.c
> 
> diff --git a/configure.ac b/configure.ac
> index fa451d84..93d4bd73 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -541,6 +541,8 @@ AC_CHECK_TYPES([struct statfs64], [
>  
>  AC_CHECK_TYPES([struct blk_user_trace_setup],,, [#include 
> <linux/blktrace_api.h>])
>  
> +AC_CHECK_TYPES([struct kvm_regs],,, [#include <linux/kvm.h>])
> +
>  AC_CHECK_HEADERS([linux/btrfs.h], [
>       AC_CHECK_MEMBERS(m4_normalize([
>               struct btrfs_ioctl_feature_flags.compat_flags,
> diff --git a/kvm.c b/kvm.c
> index 19779c84..a4bab9d4 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_LINUX_KVM_H
>  #include <linux/kvm.h>
>  #include "print_fields.h"
> +#include "arch_kvm.c"
>  
>  static int
>  kvm_ioctl_create_vcpu(struct tcb *const tcp, const kernel_ulong_t arg)
> @@ -63,16 +64,39 @@ kvm_ioctl_set_user_memory_region(struct tcb *const tcp, 
> const kernel_ulong_t arg
>       return RVAL_IOCTL_DECODED;
>  }
>  
> +static int
> +kvm_ioctl_decode_regs(struct tcb *const tcp, const unsigned int code, const 
> kernel_ulong_t arg)
> +{
> +#ifdef HAVE_STRUCT_KVM_REGS
> +     struct kvm_regs regs;

Looks like struct kvm_regs was in linux/kvm.h from the beginning (kernel
commit v2.6.20-rc1~15^2~39), so the check might be redundant after all.
However, if you want to keep it, then ...

> +
> +     if (code == KVM_GET_REGS && entering(tcp))
> +             return 0;
> +
> +     tprints(", ");
> +     if (umove_or_printaddr(tcp, arg, &regs))
> +             return RVAL_DECODED;
> +
> +     arch_print_kvm_regs(tcp, arg, &regs);
> +     return RVAL_IOCTL_DECODED;
> +#else
> +     return RVAL_DECODED;
> +#endif
> +}
> +
>  int
>  kvm_ioctl(struct tcb *const tcp, const unsigned int code, const 
> kernel_ulong_t arg)
>  {
>       switch (code) {
>       case KVM_CREATE_VCPU:
>               return kvm_ioctl_create_vcpu(tcp, arg);
> -     case KVM_CREATE_VM:
> -             return RVAL_DECODED | RVAL_FD;
>       case KVM_SET_USER_MEMORY_REGION:
>               return kvm_ioctl_set_user_memory_region(tcp, arg);
> +     case KVM_SET_REGS:
> +     case KVM_GET_REGS:
> +             return kvm_ioctl_decode_regs(tcp, code, arg);
> +     case KVM_CREATE_VM:
> +             return RVAL_DECODED | RVAL_FD;
>       case KVM_RUN:
>       case KVM_GET_VCPU_MMAP_SIZE:
>       case KVM_GET_API_VERSION:
> diff --git a/linux/arch_kvm.c b/linux/arch_kvm.c
> new file mode 100644
> index 00000000..041f6715
> --- /dev/null
> +++ b/linux/arch_kvm.c
> @@ -0,0 +1,7 @@
> +static void
> +arch_print_kvm_regs(struct tcb *const tcp,
> +                 const kernel_ulong_t addr,
> +                 const struct kvm_regs *const regs)
> +{
> +     printaddr(addr);
> +}

... this definition would have to be guarded by HAVE_STRUCT_KVM_REGS, ...

> diff --git a/linux/x86_64/arch_kvm.c b/linux/x86_64/arch_kvm.c
> new file mode 100644
> index 00000000..8bd98acc
> --- /dev/null
> +++ b/linux/x86_64/arch_kvm.c
> @@ -0,0 +1,36 @@
> +static void
> +arch_print_kvm_regs(struct tcb *const tcp,
> +                 const kernel_ulong_t addr,
> +                 const struct kvm_regs *const regs)
> +{
> +     PRINT_FIELD_X("{", *regs, rax);
> +     if (abbrev(tcp))
> +             tprints(", ...");
> +     else {
> +             PRINT_FIELD_X(", ",  *regs, rbx);
> +             PRINT_FIELD_X(", ",  *regs, rcx);
> +             PRINT_FIELD_X(", ",  *regs, rdx);
> +             PRINT_FIELD_X(", ",  *regs, rsi);
> +             PRINT_FIELD_X(", ",  *regs, rdi);
> +     }
> +     PRINT_FIELD_X(", ",  *regs, rsp);
> +     PRINT_FIELD_X(", ",  *regs, rbp);
> +     if (abbrev(tcp))
> +             tprints(", ...");
> +     else {
> +             PRINT_FIELD_X(", ",  *regs, r8);
> +             PRINT_FIELD_X(", ",  *regs, r9);
> +             PRINT_FIELD_X(", ",  *regs, r10);
> +             PRINT_FIELD_X(", ",  *regs, r11);
> +             PRINT_FIELD_X(", ",  *regs, r12);
> +             PRINT_FIELD_X(", ",  *regs, r13);
> +             PRINT_FIELD_X(", ",  *regs, r14);
> +             PRINT_FIELD_X(", ",  *regs, r15);
> +     }
> +     PRINT_FIELD_X(", ",  *regs, rip);
> +
> +     /* TODO: we can decode this more */
> +     PRINT_FIELD_X(", ",  *regs, rflags);
> +
> +     tprints("}");
> +}

... as well as this one.  I'm inclined to think that we can rely
on struct kvm_regs being available and drop the check, though.


-- 
ldv

Attachment: signature.asc
Description: PGP signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to