[PATCH v3 3/4] kvm: decode the argument for KVM_{SET, GET}_SREGS ioctl command
Dmitry V. Levin
ldv at altlinux.org
Mon Dec 4 11:40:09 UTC 2017
On Mon, Dec 04, 2017 at 08:04:51PM +0900, Masatake YAMATO wrote:
> * configure.ac (AC_CHECK_TYPES): Add struct kvm_sregs.
> * kvm.c (kvm_ioctl_decode_regs_segment): New function.
> (kvm_ioctl) <KVM_SET_SREGS, KVM_GET_SREGS>: Use it.
> * linux/arch_kvm.c (arch_print_kvm_sregs): New function.
> * linux/x86_64/arch_kvm.c (arch_print_kvm_sregs): New function.
> (kvm_ioctl_decode_regs_dtable): Ditto.
> (kvm_ioctl_decode_regs_segment): Ditto.
>
> Changes in v2:
> * Decode only if struct kvm_sregs is available.
> * Put arch-specific and generic stub decoders to arch_kvm.c.
> * Use umove_or_printaddr instead of umove.
>
> Above 3 items are suggested by ldv.
>
> * Use more const modifiers.
>
> Change in v3:
> * Put ifdef guards around codes using kvm_sregs.
> Suggested by ldv.
>
> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> ---
> configure.ac | 4 ++-
> kvm.c | 23 +++++++++++++++++
> linux/arch_kvm.c | 10 +++++++
> linux/x86_64/arch_kvm.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index e637ea95..3f18e0c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -543,9 +543,11 @@ AC_CHECK_TYPES([struct blk_user_trace_setup],,, [#include <linux/blktrace_api.h>
>
> AC_CHECK_TYPES(m4_normalize([
> struct kvm_userspace_memory_region,
> - struct kvm_regs
> + struct kvm_regs,
> + struct kvm_sregs
> ]),,, [#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 9d42b22a..12ea8c6b 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -88,6 +88,26 @@ kvm_ioctl_decode_regs(struct tcb *const tcp, const unsigned int code, const kern
> #endif
> }
>
> +static int
> +kvm_ioctl_decode_sregs(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t arg)
> +{
> +#ifdef HAVE_STRUCT_KVM_REGS
s/HAVE_STRUCT_KVM_REGS/HAVE_STRUCT_KVM_SREGS/
> + struct kvm_sregs sregs;
> +
> + if (code == KVM_GET_SREGS && entering(tcp))
> + return 0;
> +
> + tprints(", ");
> + if (umove_or_printaddr(tcp, arg, &sregs) < 0)
> + return RVAL_DECODED;
> +
> + arch_print_kvm_sregs(tcp, arg, &sregs);
> + 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)
> {
> @@ -99,6 +119,9 @@ kvm_ioctl(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t a
> case KVM_SET_REGS:
> case KVM_GET_REGS:
> return kvm_ioctl_decode_regs(tcp, code, arg);
> + case KVM_SET_SREGS:
> + case KVM_GET_SREGS:
> + return kvm_ioctl_decode_sregs(tcp, code, arg);
The same issue here as with HAVE_STRUCT_KVM_USERSPACE_MEMORY_REGION
and HAVE_STRUCT_KVM_REGS.
> case KVM_CREATE_VM:
> return RVAL_DECODED | RVAL_FD;
> case KVM_RUN:
> diff --git a/linux/arch_kvm.c b/linux/arch_kvm.c
> index ef4b7958..64ea0aca 100644
> --- a/linux/arch_kvm.c
> +++ b/linux/arch_kvm.c
> @@ -7,3 +7,13 @@ arch_print_kvm_regs(struct tcb *const tcp,
> printaddr(addr);
> }
> #endif
> +
> +#ifdef HAVE_STRUCT_KVM_SREGS
> +static void
> +arch_print_kvm_sregs(struct tcb *const tcp,
> + const kernel_ulong_t addr,
> + const struct kvm_regs *const sregs)
s/kvm_regs/kvm_sregs/
> +{
> + printaddr(addr);
> +}
> +#endif
> diff --git a/linux/x86_64/arch_kvm.c b/linux/x86_64/arch_kvm.c
> index 30110491..ba28d0cb 100644
> --- a/linux/x86_64/arch_kvm.c
> +++ b/linux/x86_64/arch_kvm.c
> @@ -36,3 +36,72 @@ arch_print_kvm_regs(struct tcb *const tcp,
> tprints("}");
> }
> #endif
> +
> +#ifdef HAVE_STRUCT_KVM_SREGS
> +static void
> +kvm_ioctl_decode_regs_segment(const char *prefix,
> + const struct kvm_segment *const segment)
> +{
> + tprints(prefix);
> + PRINT_FIELD_X("={", *segment, base);
> + PRINT_FIELD_U(", ", *segment, limit);
> + PRINT_FIELD_U(", ", *segment, selector);
> + PRINT_FIELD_U(", ", *segment, type);
> + PRINT_FIELD_U(", ", *segment, present);
> + PRINT_FIELD_U(", ", *segment, dpl);
> + PRINT_FIELD_U(", ", *segment, db);
> + PRINT_FIELD_U(", ", *segment, s);
> + PRINT_FIELD_U(", ", *segment, l);
> + PRINT_FIELD_U(", ", *segment, g);
> + PRINT_FIELD_U(", ", *segment, avl);
> + tprints("}");
> +}
> +
> +static void
> +kvm_ioctl_decode_regs_dtable(const char *prefix,
> + const struct kvm_dtable *const dtable)
> +{
> + tprints(prefix);
> + PRINT_FIELD_X("={", *dtable, base);
> + PRINT_FIELD_U(", ", *dtable, limit);
> + tprints("}");
> +}
What if you also introduced a PRINT_FIELD-style macro, e.g.
#define PRINT_FIELD_KVM_SREGS_STRUCT(prefix_, where_, type_, field_) \
kvm_ioctl_decode_regs_ ## type_(prefix_ #field_, &(where_)->field_)
?
> +
> +static void
> +arch_print_kvm_sregs(struct tcb *const tcp,
> + const kernel_ulong_t addr,
> + const struct kvm_sregs *const sregs)
> +{
> + tprints("{");
> + kvm_ioctl_decode_regs_segment("cs", &sregs->cs);
Then you'd be able to use
PRINT_FIELD_KVM_SREGS_STRUCT("{", sregs, segment, cs);
> + if (abbrev(tcp)) {
> + tprints(", ...}");
> + return;
> + }
> +
> + kvm_ioctl_decode_regs_segment(", ds", &sregs->ds);
PRINT_FIELD_KVM_SREGS_STRUCT(", ", sregs, segment, ds);
and so on.
> + kvm_ioctl_decode_regs_segment(", es", &sregs->es);
> + kvm_ioctl_decode_regs_segment(", fs", &sregs->fs);
> + kvm_ioctl_decode_regs_segment(", gs", &sregs->gs);
> + kvm_ioctl_decode_regs_segment(", ss", &sregs->ss);
> + kvm_ioctl_decode_regs_segment(", tr", &sregs->tr);
> + kvm_ioctl_decode_regs_segment(", ldt", &sregs->ldt);
> + kvm_ioctl_decode_regs_dtable(", gdt", &sregs->gdt);
> + kvm_ioctl_decode_regs_dtable(", idt", &sregs->idt);
PRINT_FIELD_KVM_SREGS_STRUCT(", ", sregs, dtable, gdt);
and so on.
> + PRINT_FIELD_U(", ", *sregs, cr0);
> + PRINT_FIELD_U(", ", *sregs, cr2);
> + PRINT_FIELD_U(", ", *sregs, cr3);
> + PRINT_FIELD_U(", ", *sregs, cr4);
> + PRINT_FIELD_U(", ", *sregs, cr8);
> + PRINT_FIELD_U(", ", *sregs, efer);
> + PRINT_FIELD_X(", ", *sregs, apic_base);
> + tprints(", interrupt_bitmap=[");
> + for (int i = 0; i < (KVM_NR_INTERRUPTS + 63) / 64; i++) {
Please move definition of the index variable out of the loop,
and please use ARRAY_SIZE, e.g.
unsigned int i;
for (i = 0; i < ARRAY_SIZE(sregs->interrupt_bitmap); ++i) {
> + if (i != 0)
> + tprints(", ");
> + tprintf("%" PRI__u64, sregs->interrupt_bitmap[i]);
If it's a bitmap, then "%#" PRI__x64 format would be more appropriate.
> + }
> + tprints("]");
> + tprints("}");
You can safely merge these two tprints calls.
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20171204/0566bbde/attachment.bin>
More information about the Strace-devel
mailing list