All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] HyperV equivalent of pvpanic driver
@ 2015-06-11 13:18 ` Denis V. Lunev
  0 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, Gleb Natapov, qemu-devel, Paolo Bonzini, Andrey Smetanin,
	Denis V. Lunev

Windows 2012 guests can notify hypervisor about occurred guest crash
(Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
handling of this MSR's by KVM and sending notification to user space that
allows to gather Windows guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 0/2] HyperV equivalent of pvpanic driver
@ 2015-06-11 13:18 ` Denis V. Lunev
  0 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, Gleb Natapov, qemu-devel, Paolo Bonzini, Andrey Smetanin,
	Denis V. Lunev

Windows 2012 guests can notify hypervisor about occurred guest crash
(Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
handling of this MSR's by KVM and sending notification to user space that
allows to gather Windows guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-11 13:18 ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-11 13:18   ` Denis V. Lunev
  -1 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, qemu-devel, Andrey Smetanin, Denis V. Lunev, Gleb Natapov,
	Paolo Bonzini

From: Andrey Smetanin <asmetanin@virtuozzo.com>

Windows 2012 guests can notify hypervisor about occurred guest crash
(Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
handling of this MSR's by KVM and sending notification to user space that
allows to gather Windows guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 10 +++++
 arch/x86/kvm/Makefile              |  2 +-
 arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/mshv.h                | 32 +++++++++++++++
 arch/x86/kvm/x86.c                 | 25 ++++++++++++
 include/linux/kvm_host.h           | 17 ++++++++
 include/uapi/linux/kvm.h           | 11 +++++
 7 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kvm/mshv.c
 create mode 100644 arch/x86/kvm/mshv.h

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index ce6068d..25f3064 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -199,6 +199,16 @@
 #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
 #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
 
+
+/* Hypev-V guest crash notification MSR's */
+#define HV_X64_MSR_CRASH_P0			0x40000100
+#define HV_X64_MSR_CRASH_P1			0x40000101
+#define HV_X64_MSR_CRASH_P2			0x40000102
+#define HV_X64_MSR_CRASH_P3			0x40000103
+#define HV_X64_MSR_CRASH_P4			0x40000104
+#define HV_X64_MSR_CRASH_CTL			0x40000105
+#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
+
 #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 16e8f96..b1ec24d 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
 kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
 
 kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
-			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
+			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
 kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
 kvm-intel-y		+= vmx.o
 kvm-amd-y		+= svm.o
diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
new file mode 100644
index 0000000..ad367c44
--- /dev/null
+++ b/arch/x86/kvm/mshv.c
@@ -0,0 +1,84 @@
+/*
+ * KVM Microsoft Hyper-V extended paravirtualization
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
+ *
+ * Authors: Andrey Smetanin asmetanin@virtuozzo.com
+ */
+
+#include <linux/kvm_host.h>
+#include "mshv.h"
+
+int kvm_mshv_ctx_create(struct kvm *kvm)
+{
+	struct kvm_mshv_ctx *ctx;
+
+	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->kvm = kvm;
+	atomic_set(&ctx->crash_pending, 0);
+	kvm->mshv_ctx = ctx;
+	return 0;
+}
+
+void kvm_mshv_ctx_destroy(struct kvm *kvm)
+{
+	kfree(kvm->mshv_ctx);
+}
+
+int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	atomic_set(&ctx->crash_pending, 1);
+
+	/* Response that KVM ready to receive crash data */
+	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
+	return 0;
+}
+
+int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	if (atomic_dec_and_test(&ctx->crash_pending)) {
+		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
+			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
+			 ctx->crash_p3, ctx->crash_p4);
+
+		/* Crash data almost gathered so notify user space */
+		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
+	}
+
+	return 0;
+}
+
+int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	switch (msr) {
+	case HV_X64_MSR_CRASH_P0:
+		ctx->crash_p0 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P1:
+		ctx->crash_p1 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P2:
+		ctx->crash_p2 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P3:
+		ctx->crash_p3 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P4:
+		ctx->crash_p4 = data;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
new file mode 100644
index 0000000..ce8e7fa
--- /dev/null
+++ b/arch/x86/kvm/mshv.h
@@ -0,0 +1,32 @@
+/*
+ * KVM Microsoft Hyper-V extended paravirtualization
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
+ *
+ * Authors: Andrey Smetanin asmetanin@virtuozzo.com
+ */
+
+#ifndef __ARCH_X86_KVM_MSHV_H__
+#define __ARCH_X86_KVM_MSHV_H__
+
+static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
+{
+	return vm->mshv_ctx;
+}
+
+static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
+{
+	return vcpu->kvm->mshv_ctx;
+}
+
+int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
+int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
+int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
+
+int kvm_mshv_ctx_create(struct kvm *kvm);
+void kvm_mshv_ctx_destroy(struct kvm *kvm);
+
+#endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ea306ad..388b58f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -28,6 +28,7 @@
 #include "x86.h"
 #include "cpuid.h"
 #include "assigned-dev.h"
+#include "mshv.h"
 
 #include <linux/clocksource.h>
 #include <linux/interrupt.h>
@@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		} else
 			return set_msr_hyperv(vcpu, msr, data);
 		break;
+	case HV_X64_MSR_CRASH_CTL:
+		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
+	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
+		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
 	case MSR_IA32_BBL_CR_CTL3:
 		/* Drop writes to this legacy MSR -- see rdmsr
 		 * counterpart for further detail.
@@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		} else
 			return get_msr_hyperv(vcpu, msr, pdata);
 		break;
+	case HV_X64_MSR_CRASH_CTL:
+		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
 	case MSR_IA32_BBL_CR_CTL3:
 		/* This legacy MSR exists but isn't fully documented in current
 		 * silicon.  It is however accessed by winxp in very narrow
@@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 			vcpu_scan_ioapic(vcpu);
 		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
 			kvm_vcpu_reload_apic_access_page(vcpu);
+		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
+			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
+			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
+			vcpu->run->system_event.flags = 0;
+			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
+			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
+			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
+			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
+			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
+			r = 0;
+			goto out;
+		}
 	}
 
 	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
@@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	if (type)
 		return -EINVAL;
 
+	if (kvm_mshv_ctx_create(kvm))
+		return -ENOMEM;
+
 	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
@@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
+	kvm_mshv_ctx_destroy(kvm);
 	if (current->mm == kvm->mm) {
 		/*
 		 * Free memory regions allocated on behalf of userspace,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ad45054..83bd7bf 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
 #define KVM_REQ_ENABLE_IBS        23
 #define KVM_REQ_DISABLE_IBS       24
 #define KVM_REQ_APIC_PAGE_RELOAD  25
+#define KVM_REQ_MSHV_CRASH        26
 
 #define KVM_USERSPACE_IRQ_SOURCE_ID		0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
@@ -343,6 +344,21 @@ struct kvm_memslots {
 	int used_slots;
 };
 
+/*
+ * Ms hyperv paravirt context
+ */
+struct kvm_mshv_ctx {
+	struct kvm	*kvm;
+	atomic_t	crash_pending;
+
+	/* Guest crash related parameters */
+	u64		crash_p0;
+	u64		crash_p1;
+	u64		crash_p2;
+	u64		crash_p3;
+	u64		crash_p4;
+};
+
 struct kvm {
 	spinlock_t mmu_lock;
 	struct mutex slots_lock;
@@ -395,6 +411,7 @@ struct kvm {
 #endif
 	long tlbs_dirty;
 	struct list_head devices;
+	struct kvm_mshv_ctx *mshv_ctx;
 };
 
 #define kvm_err(fmt, ...) \
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b60056..12f481b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -317,8 +317,19 @@ struct kvm_run {
 		struct {
 #define KVM_SYSTEM_EVENT_SHUTDOWN       1
 #define KVM_SYSTEM_EVENT_RESET          2
+#define KVM_SYSTEM_EVENT_CRASH          3
 			__u32 type;
 			__u64 flags;
+			union {
+				struct {
+					/* Guest crash related parameters */
+					__u64 p0;
+					__u64 p1;
+					__u64 p2;
+					__u64 p3;
+					__u64 p4;
+				} crash;
+			};
 		} system_event;
 		/* KVM_EXIT_S390_STSI */
 		struct {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-11 13:18   ` Denis V. Lunev
  0 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, Gleb Natapov, qemu-devel, Paolo Bonzini, Andrey Smetanin,
	Denis V. Lunev

From: Andrey Smetanin <asmetanin@virtuozzo.com>

Windows 2012 guests can notify hypervisor about occurred guest crash
(Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
handling of this MSR's by KVM and sending notification to user space that
allows to gather Windows guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 10 +++++
 arch/x86/kvm/Makefile              |  2 +-
 arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/mshv.h                | 32 +++++++++++++++
 arch/x86/kvm/x86.c                 | 25 ++++++++++++
 include/linux/kvm_host.h           | 17 ++++++++
 include/uapi/linux/kvm.h           | 11 +++++
 7 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kvm/mshv.c
 create mode 100644 arch/x86/kvm/mshv.h

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index ce6068d..25f3064 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -199,6 +199,16 @@
 #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
 #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
 
+
+/* Hypev-V guest crash notification MSR's */
+#define HV_X64_MSR_CRASH_P0			0x40000100
+#define HV_X64_MSR_CRASH_P1			0x40000101
+#define HV_X64_MSR_CRASH_P2			0x40000102
+#define HV_X64_MSR_CRASH_P3			0x40000103
+#define HV_X64_MSR_CRASH_P4			0x40000104
+#define HV_X64_MSR_CRASH_CTL			0x40000105
+#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
+
 #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 16e8f96..b1ec24d 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
 kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
 
 kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
-			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
+			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
 kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
 kvm-intel-y		+= vmx.o
 kvm-amd-y		+= svm.o
diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
new file mode 100644
index 0000000..ad367c44
--- /dev/null
+++ b/arch/x86/kvm/mshv.c
@@ -0,0 +1,84 @@
+/*
+ * KVM Microsoft Hyper-V extended paravirtualization
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
+ *
+ * Authors: Andrey Smetanin asmetanin@virtuozzo.com
+ */
+
+#include <linux/kvm_host.h>
+#include "mshv.h"
+
+int kvm_mshv_ctx_create(struct kvm *kvm)
+{
+	struct kvm_mshv_ctx *ctx;
+
+	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->kvm = kvm;
+	atomic_set(&ctx->crash_pending, 0);
+	kvm->mshv_ctx = ctx;
+	return 0;
+}
+
+void kvm_mshv_ctx_destroy(struct kvm *kvm)
+{
+	kfree(kvm->mshv_ctx);
+}
+
+int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	atomic_set(&ctx->crash_pending, 1);
+
+	/* Response that KVM ready to receive crash data */
+	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
+	return 0;
+}
+
+int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	if (atomic_dec_and_test(&ctx->crash_pending)) {
+		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
+			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
+			 ctx->crash_p3, ctx->crash_p4);
+
+		/* Crash data almost gathered so notify user space */
+		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
+	}
+
+	return 0;
+}
+
+int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+	switch (msr) {
+	case HV_X64_MSR_CRASH_P0:
+		ctx->crash_p0 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P1:
+		ctx->crash_p1 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P2:
+		ctx->crash_p2 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P3:
+		ctx->crash_p3 = data;
+		return 0;
+	case HV_X64_MSR_CRASH_P4:
+		ctx->crash_p4 = data;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
new file mode 100644
index 0000000..ce8e7fa
--- /dev/null
+++ b/arch/x86/kvm/mshv.h
@@ -0,0 +1,32 @@
+/*
+ * KVM Microsoft Hyper-V extended paravirtualization
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
+ *
+ * Authors: Andrey Smetanin asmetanin@virtuozzo.com
+ */
+
+#ifndef __ARCH_X86_KVM_MSHV_H__
+#define __ARCH_X86_KVM_MSHV_H__
+
+static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
+{
+	return vm->mshv_ctx;
+}
+
+static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
+{
+	return vcpu->kvm->mshv_ctx;
+}
+
+int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
+int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
+int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
+
+int kvm_mshv_ctx_create(struct kvm *kvm);
+void kvm_mshv_ctx_destroy(struct kvm *kvm);
+
+#endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ea306ad..388b58f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -28,6 +28,7 @@
 #include "x86.h"
 #include "cpuid.h"
 #include "assigned-dev.h"
+#include "mshv.h"
 
 #include <linux/clocksource.h>
 #include <linux/interrupt.h>
@@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		} else
 			return set_msr_hyperv(vcpu, msr, data);
 		break;
+	case HV_X64_MSR_CRASH_CTL:
+		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
+	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
+		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
 	case MSR_IA32_BBL_CR_CTL3:
 		/* Drop writes to this legacy MSR -- see rdmsr
 		 * counterpart for further detail.
@@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		} else
 			return get_msr_hyperv(vcpu, msr, pdata);
 		break;
+	case HV_X64_MSR_CRASH_CTL:
+		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
 	case MSR_IA32_BBL_CR_CTL3:
 		/* This legacy MSR exists but isn't fully documented in current
 		 * silicon.  It is however accessed by winxp in very narrow
@@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 			vcpu_scan_ioapic(vcpu);
 		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
 			kvm_vcpu_reload_apic_access_page(vcpu);
+		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
+			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
+
+			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
+			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
+			vcpu->run->system_event.flags = 0;
+			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
+			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
+			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
+			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
+			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
+			r = 0;
+			goto out;
+		}
 	}
 
 	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
@@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	if (type)
 		return -EINVAL;
 
+	if (kvm_mshv_ctx_create(kvm))
+		return -ENOMEM;
+
 	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
@@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
+	kvm_mshv_ctx_destroy(kvm);
 	if (current->mm == kvm->mm) {
 		/*
 		 * Free memory regions allocated on behalf of userspace,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ad45054..83bd7bf 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
 #define KVM_REQ_ENABLE_IBS        23
 #define KVM_REQ_DISABLE_IBS       24
 #define KVM_REQ_APIC_PAGE_RELOAD  25
+#define KVM_REQ_MSHV_CRASH        26
 
 #define KVM_USERSPACE_IRQ_SOURCE_ID		0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
@@ -343,6 +344,21 @@ struct kvm_memslots {
 	int used_slots;
 };
 
+/*
+ * Ms hyperv paravirt context
+ */
+struct kvm_mshv_ctx {
+	struct kvm	*kvm;
+	atomic_t	crash_pending;
+
+	/* Guest crash related parameters */
+	u64		crash_p0;
+	u64		crash_p1;
+	u64		crash_p2;
+	u64		crash_p3;
+	u64		crash_p4;
+};
+
 struct kvm {
 	spinlock_t mmu_lock;
 	struct mutex slots_lock;
@@ -395,6 +411,7 @@ struct kvm {
 #endif
 	long tlbs_dirty;
 	struct list_head devices;
+	struct kvm_mshv_ctx *mshv_ctx;
 };
 
 #define kvm_err(fmt, ...) \
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b60056..12f481b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -317,8 +317,19 @@ struct kvm_run {
 		struct {
 #define KVM_SYSTEM_EVENT_SHUTDOWN       1
 #define KVM_SYSTEM_EVENT_RESET          2
+#define KVM_SYSTEM_EVENT_CRASH          3
 			__u32 type;
 			__u64 flags;
+			union {
+				struct {
+					/* Guest crash related parameters */
+					__u64 p0;
+					__u64 p1;
+					__u64 p2;
+					__u64 p3;
+					__u64 p4;
+				} crash;
+			};
 		} system_event;
 		/* KVM_EXIT_S390_STSI */
 		struct {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] qemu/kvm: kvm guest crash event handling
  2015-06-11 13:18 ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-11 13:18   ` Denis V. Lunev
  -1 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, qemu-devel, Andrey Smetanin, Denis V. Lunev, Gleb Natapov,
	Paolo Bonzini

From: Andrey Smetanin <asmetanin@virtuozzo.com>

KVM Hyper-V based guests can notify hypervisor about
occurred guest crash. This patch does handling of KVM crash event
by sending to libvirt guest panic event that allows to gather
guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/sysemu.h        |  2 ++
 kvm-all.c                      |  8 ++++++++
 linux-headers/asm-x86/hyperv.h |  2 ++
 linux-headers/linux/kvm.h      | 11 +++++++++++
 target-i386/cpu-qom.h          |  1 +
 target-i386/cpu.c              |  1 +
 target-i386/kvm.c              |  4 ++++
 vl.c                           | 31 +++++++++++++++++++++++++++++++
 8 files changed, 60 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 853d90a..82d3213 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -61,6 +61,8 @@ void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
+void qemu_system_crash_request(uint64_t p0, uint64_t p1, uint64_t p2,
+                                uint64_t p3, uint64_t p4);
 void qemu_system_vmstop_request(RunState reason);
 void qemu_system_vmstop_request_prepare(void);
 int qemu_shutdown_requested_get(void);
diff --git a/kvm-all.c b/kvm-all.c
index 53e01d4..cee23bc 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1844,6 +1844,14 @@ int kvm_cpu_exec(CPUState *cpu)
                 qemu_system_reset_request();
                 ret = EXCP_INTERRUPT;
                 break;
+            case KVM_SYSTEM_EVENT_CRASH:
+                qemu_system_crash_request(run->system_event.crash.p0,
+                                            run->system_event.crash.p1,
+                                            run->system_event.crash.p2,
+                                            run->system_event.crash.p3,
+                                            run->system_event.crash.p4);
+                ret = 0;
+                break;
             default:
                 DPRINTF("kvm_arch_handle_exit\n");
                 ret = kvm_arch_handle_exit(cpu, run);
diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h
index ce6068d..a5df1ab 100644
--- a/linux-headers/asm-x86/hyperv.h
+++ b/linux-headers/asm-x86/hyperv.h
@@ -108,6 +108,8 @@
 #define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE		(1 << 4)
 /* Support for a virtual guest idle state is available */
 #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
+/* Guest crash data handler available */
+#define HV_X64_GUEST_CRASH_MSR_AVAILABLE                (1 << 10)
 
 /*
  * Implementation recommendations. Indicates which behaviors the hypervisor
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index fad9e5c..e169602 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -317,8 +317,19 @@ struct kvm_run {
 		struct {
 #define KVM_SYSTEM_EVENT_SHUTDOWN       1
 #define KVM_SYSTEM_EVENT_RESET          2
+#define KVM_SYSTEM_EVENT_CRASH          3
 			__u32 type;
 			__u64 flags;
+                        union {
+                                struct {
+                                        /* Guest crash related parameters */
+                                        __u64 p0;
+                                        __u64 p1;
+                                        __u64 p2;
+                                        __u64 p3;
+                                        __u64 p4;
+                                } crash;
+                        };
 		} system_event;
 		/* KVM_EXIT_S390_STSI */
 		struct {
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 7a4fddd..c35b624 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -89,6 +89,7 @@ typedef struct X86CPU {
     bool hyperv_relaxed_timing;
     int hyperv_spinlock_attempts;
     bool hyperv_time;
+    bool hyperv_crash;
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4e7cdaa..af0552a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3117,6 +3117,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
     DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
     DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
+    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5a236e3..b63e4bb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -516,6 +516,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
             c->eax |= 0x200;
             has_msr_hv_tsc = true;
         }
+        if (cpu->hyperv_crash) {
+            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
+        }
+
         c = &cpuid_data.entries[cpuid_i++];
         c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
         if (cpu->hyperv_relaxed_timing) {
diff --git a/vl.c b/vl.c
index 66ccd06..76b5484 100644
--- a/vl.c
+++ b/vl.c
@@ -1505,6 +1505,14 @@ static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
 static int suspend_requested;
+
+static int crash_requested;
+static uint64_t crash_p0;
+static uint64_t crash_p1;
+static uint64_t crash_p2;
+static uint64_t crash_p3;
+static uint64_t crash_p4;
+
 static WakeupReason wakeup_reason;
 static NotifierList powerdown_notifiers =
     NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
@@ -1578,6 +1586,13 @@ static int qemu_debug_requested(void)
     return r;
 }
 
+static int qemu_crash_requested(void)
+{
+    int r = crash_requested;
+    crash_requested = 0;
+    return r;
+}
+
 void qemu_register_reset(QEMUResetHandler *func, void *opaque)
 {
     QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
@@ -1729,9 +1744,25 @@ void qemu_system_debug_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_crash_request(uint64_t p0, uint64_t  p1, uint64_t p2,
+                                uint64_t p3, uint64_t p4)
+{
+    crash_p0 = p0;
+    crash_p1 = p1;
+    crash_p2 = p2;
+    crash_p3 = p3;
+    crash_p4 = p4;
+    crash_requested = 1;
+    qemu_notify_event();
+}
+
 static bool main_loop_should_exit(void)
 {
     RunState r;
+    if (qemu_crash_requested()) {
+        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+    }
     if (qemu_debug_requested()) {
         vm_stop(RUN_STATE_DEBUG);
     }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 2/2] qemu/kvm: kvm guest crash event handling
@ 2015-06-11 13:18   ` Denis V. Lunev
  0 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2015-06-11 13:18 UTC (permalink / raw)
  Cc: kvm, Gleb Natapov, qemu-devel, Paolo Bonzini, Andrey Smetanin,
	Denis V. Lunev

From: Andrey Smetanin <asmetanin@virtuozzo.com>

KVM Hyper-V based guests can notify hypervisor about
occurred guest crash. This patch does handling of KVM crash event
by sending to libvirt guest panic event that allows to gather
guest crash dump by QEMU/LIBVIRT.

The idea is to provide functionality equal to pvpanic device without
QEMU guest agent for Windows.

The idea is borrowed from Linux HyperV bus driver and validated against
Windows 2k12.

Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gleb Natapov <gleb@kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/sysemu.h        |  2 ++
 kvm-all.c                      |  8 ++++++++
 linux-headers/asm-x86/hyperv.h |  2 ++
 linux-headers/linux/kvm.h      | 11 +++++++++++
 target-i386/cpu-qom.h          |  1 +
 target-i386/cpu.c              |  1 +
 target-i386/kvm.c              |  4 ++++
 vl.c                           | 31 +++++++++++++++++++++++++++++++
 8 files changed, 60 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 853d90a..82d3213 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -61,6 +61,8 @@ void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
+void qemu_system_crash_request(uint64_t p0, uint64_t p1, uint64_t p2,
+                                uint64_t p3, uint64_t p4);
 void qemu_system_vmstop_request(RunState reason);
 void qemu_system_vmstop_request_prepare(void);
 int qemu_shutdown_requested_get(void);
diff --git a/kvm-all.c b/kvm-all.c
index 53e01d4..cee23bc 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1844,6 +1844,14 @@ int kvm_cpu_exec(CPUState *cpu)
                 qemu_system_reset_request();
                 ret = EXCP_INTERRUPT;
                 break;
+            case KVM_SYSTEM_EVENT_CRASH:
+                qemu_system_crash_request(run->system_event.crash.p0,
+                                            run->system_event.crash.p1,
+                                            run->system_event.crash.p2,
+                                            run->system_event.crash.p3,
+                                            run->system_event.crash.p4);
+                ret = 0;
+                break;
             default:
                 DPRINTF("kvm_arch_handle_exit\n");
                 ret = kvm_arch_handle_exit(cpu, run);
diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h
index ce6068d..a5df1ab 100644
--- a/linux-headers/asm-x86/hyperv.h
+++ b/linux-headers/asm-x86/hyperv.h
@@ -108,6 +108,8 @@
 #define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE		(1 << 4)
 /* Support for a virtual guest idle state is available */
 #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
+/* Guest crash data handler available */
+#define HV_X64_GUEST_CRASH_MSR_AVAILABLE                (1 << 10)
 
 /*
  * Implementation recommendations. Indicates which behaviors the hypervisor
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index fad9e5c..e169602 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -317,8 +317,19 @@ struct kvm_run {
 		struct {
 #define KVM_SYSTEM_EVENT_SHUTDOWN       1
 #define KVM_SYSTEM_EVENT_RESET          2
+#define KVM_SYSTEM_EVENT_CRASH          3
 			__u32 type;
 			__u64 flags;
+                        union {
+                                struct {
+                                        /* Guest crash related parameters */
+                                        __u64 p0;
+                                        __u64 p1;
+                                        __u64 p2;
+                                        __u64 p3;
+                                        __u64 p4;
+                                } crash;
+                        };
 		} system_event;
 		/* KVM_EXIT_S390_STSI */
 		struct {
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 7a4fddd..c35b624 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -89,6 +89,7 @@ typedef struct X86CPU {
     bool hyperv_relaxed_timing;
     int hyperv_spinlock_attempts;
     bool hyperv_time;
+    bool hyperv_crash;
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4e7cdaa..af0552a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3117,6 +3117,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
     DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
     DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
+    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5a236e3..b63e4bb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -516,6 +516,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
             c->eax |= 0x200;
             has_msr_hv_tsc = true;
         }
+        if (cpu->hyperv_crash) {
+            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
+        }
+
         c = &cpuid_data.entries[cpuid_i++];
         c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
         if (cpu->hyperv_relaxed_timing) {
diff --git a/vl.c b/vl.c
index 66ccd06..76b5484 100644
--- a/vl.c
+++ b/vl.c
@@ -1505,6 +1505,14 @@ static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
 static int suspend_requested;
+
+static int crash_requested;
+static uint64_t crash_p0;
+static uint64_t crash_p1;
+static uint64_t crash_p2;
+static uint64_t crash_p3;
+static uint64_t crash_p4;
+
 static WakeupReason wakeup_reason;
 static NotifierList powerdown_notifiers =
     NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
@@ -1578,6 +1586,13 @@ static int qemu_debug_requested(void)
     return r;
 }
 
+static int qemu_crash_requested(void)
+{
+    int r = crash_requested;
+    crash_requested = 0;
+    return r;
+}
+
 void qemu_register_reset(QEMUResetHandler *func, void *opaque)
 {
     QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
@@ -1729,9 +1744,25 @@ void qemu_system_debug_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_crash_request(uint64_t p0, uint64_t  p1, uint64_t p2,
+                                uint64_t p3, uint64_t p4)
+{
+    crash_p0 = p0;
+    crash_p1 = p1;
+    crash_p2 = p2;
+    crash_p3 = p3;
+    crash_p4 = p4;
+    crash_requested = 1;
+    qemu_notify_event();
+}
+
 static bool main_loop_should_exit(void)
 {
     RunState r;
+    if (qemu_crash_requested()) {
+        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+    }
     if (qemu_debug_requested()) {
         vm_stop(RUN_STATE_DEBUG);
     }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-12 22:55     ` Peter Hornyack
  -1 siblings, 0 replies; 20+ messages in thread
From: Peter Hornyack @ 2015-06-12 22:55 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 14509 bytes --]

Hi Denis, Andrey, I have a few comments and questions.

On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:

> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
>
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
>
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84
> ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
>
> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
>
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0                    0x40000100
> +#define HV_X64_MSR_CRASH_P1                    0x40000101
> +#define HV_X64_MSR_CRASH_P2                    0x40000102
> +#define HV_X64_MSR_CRASH_P3                    0x40000103
> +#define HV_X64_MSR_CRASH_P4                    0x40000104
> +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o
> $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
>
>  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
>  kvm-intel-y            += vmx.o
>  kvm-amd-y              += svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +       struct kvm_mshv_ctx *ctx;
> +
> +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->kvm = kvm;
> +       atomic_set(&ctx->crash_pending, 0);
> +       kvm->mshv_ctx = ctx;
> +       return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +       kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       atomic_set(&ctx->crash_pending, 1);
>

Can you explain why crash_pending is needed?

Do you know what the Windows guest behavior is here? From my reading of the
Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at some point
(not necessarily at the time of the crash, potentially at boot time?) to
determine the crash actions that the hypervisor supports. I'm not sure why
kvm needs to remember any state on reads from HV_X64_MSR_CRASH_CTL.


> +
> +       /* Response that KVM ready to receive crash data */

+       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;
>

The TLFS says that CrashNotify is the only current action that is supported
on a guest crash, but other actions may be supported in the future. I
suggest defining something like HV_X64_MSR_CRASH_CTL_CONTENTS (see below),
which will only have HV_CRASH_CTL_CRASH_NOTIFY set for now but may have
other bits set in the future, and then setting *pdata =
HV_X64_MSR_CRASH_CTL_CONTENTS here.

+       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       if (atomic_dec_and_test(&ctx->crash_pending)) {
>

The TLFS says: "To invoke a supported hypervisor guest crash action, a
child partition writes to the HV_X64_MSR_CRASH_CTL register, specifying the
desired action." However, this implementation doesn't actually check the
data that's written to this register. The condition here should check for
"data & HV_CRASH_CTL_CRASH_NOTIFY", right?

I'm still not clear on the need for crash_pending here... maybe there are
concurrency reasons that I haven't thought through.

+               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +                        ctx->crash_p3, ctx->crash_p4);
> +
> +               /* Crash data almost gathered so notify user space */
> +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +       }
> +
> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       switch (msr) {
> +       case HV_X64_MSR_CRASH_P0:
> +               ctx->crash_p0 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P1:
> +               ctx->crash_p1 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P2:
> +               ctx->crash_p2 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P3:
> +               ctx->crash_p3 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P4:
> +               ctx->crash_p4 = data;
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +
>

I suggest here:

#define HV_X64_MSR_CRASH_CTL_CONTENTS  \
        (HV_CRASH_CTL_CRASH_NOTIFY)

To allow for more crash actions to be added in the future.


> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +       return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu
> *vcpu)
> +{
> +       return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64
> *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
>                 } else
>                         return set_msr_hyperv(vcpu, msr, data);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>         case MSR_IA32_BBL_CR_CTL3:
>                 /* Drop writes to this legacy MSR -- see rdmsr
>                  * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32
> msr, u64 *pdata)
>                 } else
>                         return get_msr_hyperv(vcpu, msr, pdata);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
>

Have the Windows guests that you've tested ever tried to read from the data
registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there any harm in
implementing them for reads? The TLFS doesn't seem to say if reads are
permitted or not. Not terribly important, but might be nice to have.


>         case MSR_IA32_BBL_CR_CTL3:
>                 /* This legacy MSR exists but isn't fully documented in
> current
>                  * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>                         vcpu_scan_ioapic(vcpu);
>                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>                         kvm_vcpu_reload_apic_access_page(vcpu);
> +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +                       struct kvm_mshv_ctx *ctx =
> kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +                       vcpu->run->system_event.type =
> KVM_SYSTEM_EVENT_CRASH;
> +                       vcpu->run->system_event.flags = 0;
> +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +                       r = 0;
> +                       goto out;
> +               }
>         }
>
>         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long
> type)
>         if (type)
>                 return -EINVAL;
>
> +       if (kvm_mshv_ctx_create(kvm))
> +               return -ENOMEM;
> +
>         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +       kvm_mshv_ctx_destroy(kvm);
>         if (current->mm == kvm->mm) {
>                 /*
>                  * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>
>  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>         int used_slots;
>  };
>
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {
> +       struct kvm      *kvm;
> +       atomic_t        crash_pending;
> +
> +       /* Guest crash related parameters */
> +       u64             crash_p0;
> +       u64             crash_p1;
> +       u64             crash_p2;
> +       u64             crash_p3;
> +       u64             crash_p4;
> +};
> +
>  struct kvm {
>         spinlock_t mmu_lock;
>         struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>         long tlbs_dirty;
>         struct list_head devices;
> +       struct kvm_mshv_ctx *mshv_ctx;
>  };
>
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>                 struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>                         __u32 type;
>                         __u64 flags;
> +                       union {
> +                               struct {
> +                                       /* Guest crash related parameters
> */
> +                                       __u64 p0;
> +                                       __u64 p1;
> +                                       __u64 p2;
> +                                       __u64 p3;
> +                                       __u64 p4;
> +                               } crash;
> +                       };

                } system_event;
>                 /* KVM_EXIT_S390_STSI */
>                 struct {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Pete

[-- Attachment #2: Type: text/html, Size: 19652 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-12 22:55     ` Peter Hornyack
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Hornyack @ 2015-06-12 22:55 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 14509 bytes --]

Hi Denis, Andrey, I have a few comments and questions.

On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:

> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
>
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
>
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84
> ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
>
> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
>
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0                    0x40000100
> +#define HV_X64_MSR_CRASH_P1                    0x40000101
> +#define HV_X64_MSR_CRASH_P2                    0x40000102
> +#define HV_X64_MSR_CRASH_P3                    0x40000103
> +#define HV_X64_MSR_CRASH_P4                    0x40000104
> +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o
> $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
>
>  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
>  kvm-intel-y            += vmx.o
>  kvm-amd-y              += svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +       struct kvm_mshv_ctx *ctx;
> +
> +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->kvm = kvm;
> +       atomic_set(&ctx->crash_pending, 0);
> +       kvm->mshv_ctx = ctx;
> +       return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +       kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       atomic_set(&ctx->crash_pending, 1);
>

Can you explain why crash_pending is needed?

Do you know what the Windows guest behavior is here? From my reading of the
Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at some point
(not necessarily at the time of the crash, potentially at boot time?) to
determine the crash actions that the hypervisor supports. I'm not sure why
kvm needs to remember any state on reads from HV_X64_MSR_CRASH_CTL.


> +
> +       /* Response that KVM ready to receive crash data */

+       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;
>

The TLFS says that CrashNotify is the only current action that is supported
on a guest crash, but other actions may be supported in the future. I
suggest defining something like HV_X64_MSR_CRASH_CTL_CONTENTS (see below),
which will only have HV_CRASH_CTL_CRASH_NOTIFY set for now but may have
other bits set in the future, and then setting *pdata =
HV_X64_MSR_CRASH_CTL_CONTENTS here.

+       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       if (atomic_dec_and_test(&ctx->crash_pending)) {
>

The TLFS says: "To invoke a supported hypervisor guest crash action, a
child partition writes to the HV_X64_MSR_CRASH_CTL register, specifying the
desired action." However, this implementation doesn't actually check the
data that's written to this register. The condition here should check for
"data & HV_CRASH_CTL_CRASH_NOTIFY", right?

I'm still not clear on the need for crash_pending here... maybe there are
concurrency reasons that I haven't thought through.

+               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +                        ctx->crash_p3, ctx->crash_p4);
> +
> +               /* Crash data almost gathered so notify user space */
> +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +       }
> +
> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       switch (msr) {
> +       case HV_X64_MSR_CRASH_P0:
> +               ctx->crash_p0 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P1:
> +               ctx->crash_p1 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P2:
> +               ctx->crash_p2 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P3:
> +               ctx->crash_p3 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P4:
> +               ctx->crash_p4 = data;
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +
>

I suggest here:

#define HV_X64_MSR_CRASH_CTL_CONTENTS  \
        (HV_CRASH_CTL_CRASH_NOTIFY)

To allow for more crash actions to be added in the future.


> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +       return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu
> *vcpu)
> +{
> +       return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64
> *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
>                 } else
>                         return set_msr_hyperv(vcpu, msr, data);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>         case MSR_IA32_BBL_CR_CTL3:
>                 /* Drop writes to this legacy MSR -- see rdmsr
>                  * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32
> msr, u64 *pdata)
>                 } else
>                         return get_msr_hyperv(vcpu, msr, pdata);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
>

Have the Windows guests that you've tested ever tried to read from the data
registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there any harm in
implementing them for reads? The TLFS doesn't seem to say if reads are
permitted or not. Not terribly important, but might be nice to have.


>         case MSR_IA32_BBL_CR_CTL3:
>                 /* This legacy MSR exists but isn't fully documented in
> current
>                  * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>                         vcpu_scan_ioapic(vcpu);
>                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>                         kvm_vcpu_reload_apic_access_page(vcpu);
> +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +                       struct kvm_mshv_ctx *ctx =
> kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +                       vcpu->run->system_event.type =
> KVM_SYSTEM_EVENT_CRASH;
> +                       vcpu->run->system_event.flags = 0;
> +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +                       r = 0;
> +                       goto out;
> +               }
>         }
>
>         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long
> type)
>         if (type)
>                 return -EINVAL;
>
> +       if (kvm_mshv_ctx_create(kvm))
> +               return -ENOMEM;
> +
>         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +       kvm_mshv_ctx_destroy(kvm);
>         if (current->mm == kvm->mm) {
>                 /*
>                  * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>
>  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>         int used_slots;
>  };
>
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {
> +       struct kvm      *kvm;
> +       atomic_t        crash_pending;
> +
> +       /* Guest crash related parameters */
> +       u64             crash_p0;
> +       u64             crash_p1;
> +       u64             crash_p2;
> +       u64             crash_p3;
> +       u64             crash_p4;
> +};
> +
>  struct kvm {
>         spinlock_t mmu_lock;
>         struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>         long tlbs_dirty;
>         struct list_head devices;
> +       struct kvm_mshv_ctx *mshv_ctx;
>  };
>
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>                 struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>                         __u32 type;
>                         __u64 flags;
> +                       union {
> +                               struct {
> +                                       /* Guest crash related parameters
> */
> +                                       __u64 p0;
> +                                       __u64 p1;
> +                                       __u64 p2;
> +                                       __u64 p3;
> +                                       __u64 p4;
> +                               } crash;
> +                       };

                } system_event;
>                 /* KVM_EXIT_S390_STSI */
>                 struct {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Pete

[-- Attachment #2: Type: text/html, Size: 19652 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-12 23:03     ` Peter Hornyack
  -1 siblings, 0 replies; 20+ messages in thread
From: Peter Hornyack @ 2015-06-12 23:03 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: kvm, qemu-devel, Andrey Smetanin, Gleb Natapov, Paolo Bonzini

Hi Denis, Andrey, I have a few comments and questions. (re-sending in
plain-text mode, apologies for sending twice.)

On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
>
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
>
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
>
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
>
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0                    0x40000100
> +#define HV_X64_MSR_CRASH_P1                    0x40000101
> +#define HV_X64_MSR_CRASH_P2                    0x40000102
> +#define HV_X64_MSR_CRASH_P3                    0x40000103
> +#define HV_X64_MSR_CRASH_P4                    0x40000104
> +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
>
>  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
>  kvm-intel-y            += vmx.o
>  kvm-amd-y              += svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +       struct kvm_mshv_ctx *ctx;
> +
> +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->kvm = kvm;
> +       atomic_set(&ctx->crash_pending, 0);
> +       kvm->mshv_ctx = ctx;
> +       return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +       kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       atomic_set(&ctx->crash_pending, 1);

Can you explain why crash_pending is needed?

Do you know what the Windows guest behavior is here? From my reading
of the Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at
some point (not necessarily at the time of the crash, potentially at
boot time?) to determine the crash actions that the hypervisor
supports. I'm not sure why kvm needs to remember any state on reads
from HV_X64_MSR_CRASH_CTL.

 > +
> +       /* Response that KVM ready to receive crash data */
> +       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;

The TLFS says that CrashNotify is the only current action that is
supported on a guest crash, but other actions may be supported in the
future. I suggest defining something like
HV_X64_MSR_CRASH_CTL_CONTENTS (see below), which will only have
HV_CRASH_CTL_CRASH_NOTIFY set for now but may have other bits set in
the future, and then setting *pdata = HV_X64_MSR_CRASH_CTL_CONTENTS
here.

> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       if (atomic_dec_and_test(&ctx->crash_pending)) {

The TLFS says: "To invoke a supported hypervisor guest crash action, a
child partition writes to the HV_X64_MSR_CRASH_CTL register,
specifying the desired action." However, this implementation doesn't
actually check the data that's written to this register. The condition
here should check for "data & HV_CRASH_CTL_CRASH_NOTIFY", right?

I'm still not clear on the need for crash_pending here... maybe there
are concurrency reasons that I haven't thought through.

> +               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +                        ctx->crash_p3, ctx->crash_p4);
> +
> +               /* Crash data almost gathered so notify user space */
> +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +       }
> +
> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       switch (msr) {
> +       case HV_X64_MSR_CRASH_P0:
> +               ctx->crash_p0 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P1:
> +               ctx->crash_p1 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P2:
> +               ctx->crash_p2 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P3:
> +               ctx->crash_p3 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P4:
> +               ctx->crash_p4 = data;
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +

I suggest here:

#define HV_X64_MSR_CRASH_CTL_CONTENTS  \
        (HV_CRASH_CTL_CRASH_NOTIFY)

To allow for more crash actions to be added in the future.

> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +       return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> +{
> +       return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>                 } else
>                         return set_msr_hyperv(vcpu, msr, data);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>         case MSR_IA32_BBL_CR_CTL3:
>                 /* Drop writes to this legacy MSR -- see rdmsr
>                  * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>                 } else
>                         return get_msr_hyperv(vcpu, msr, pdata);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);

Have the Windows guests that you've tested ever tried to read from the
data registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there
any harm in implementing them for reads? The TLFS doesn't seem to say
if reads are permitted or not. Not terribly important, but might be
nice to have.

>         case MSR_IA32_BBL_CR_CTL3:
>                 /* This legacy MSR exists but isn't fully documented in current
>                  * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>                         vcpu_scan_ioapic(vcpu);
>                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>                         kvm_vcpu_reload_apic_access_page(vcpu);
> +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +                       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +                       vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> +                       vcpu->run->system_event.flags = 0;
> +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +                       r = 0;
> +                       goto out;
> +               }
>         }
>
>         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>         if (type)
>                 return -EINVAL;
>
> +       if (kvm_mshv_ctx_create(kvm))
> +               return -ENOMEM;
> +
>         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +       kvm_mshv_ctx_destroy(kvm);
>         if (current->mm == kvm->mm) {
>                 /*
>                  * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>
>  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>         int used_slots;
>  };
>
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {
> +       struct kvm      *kvm;
> +       atomic_t        crash_pending;
> +
> +       /* Guest crash related parameters */
> +       u64             crash_p0;
> +       u64             crash_p1;
> +       u64             crash_p2;
> +       u64             crash_p3;
> +       u64             crash_p4;
> +};
> +
>  struct kvm {
>         spinlock_t mmu_lock;
>         struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>         long tlbs_dirty;
>         struct list_head devices;
> +       struct kvm_mshv_ctx *mshv_ctx;
>  };
>
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>                 struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>                         __u32 type;
>                         __u64 flags;
> +                       union {
> +                               struct {
> +                                       /* Guest crash related parameters */
> +                                       __u64 p0;
> +                                       __u64 p1;
> +                                       __u64 p2;
> +                                       __u64 p3;
> +                                       __u64 p4;
> +                               } crash;
> +                       };
>                 } system_event;
>                 /* KVM_EXIT_S390_STSI */
>                 struct {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
Pete

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-12 23:03     ` Peter Hornyack
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Hornyack @ 2015-06-12 23:03 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm, Paolo Bonzini

Hi Denis, Andrey, I have a few comments and questions. (re-sending in
plain-text mode, apologies for sending twice.)

On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
>
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
>
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
>
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
>
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0                    0x40000100
> +#define HV_X64_MSR_CRASH_P1                    0x40000101
> +#define HV_X64_MSR_CRASH_P2                    0x40000102
> +#define HV_X64_MSR_CRASH_P3                    0x40000103
> +#define HV_X64_MSR_CRASH_P4                    0x40000104
> +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
>
>  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
>  kvm-intel-y            += vmx.o
>  kvm-amd-y              += svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +       struct kvm_mshv_ctx *ctx;
> +
> +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->kvm = kvm;
> +       atomic_set(&ctx->crash_pending, 0);
> +       kvm->mshv_ctx = ctx;
> +       return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +       kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       atomic_set(&ctx->crash_pending, 1);

Can you explain why crash_pending is needed?

Do you know what the Windows guest behavior is here? From my reading
of the Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at
some point (not necessarily at the time of the crash, potentially at
boot time?) to determine the crash actions that the hypervisor
supports. I'm not sure why kvm needs to remember any state on reads
from HV_X64_MSR_CRASH_CTL.

 > +
> +       /* Response that KVM ready to receive crash data */
> +       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;

The TLFS says that CrashNotify is the only current action that is
supported on a guest crash, but other actions may be supported in the
future. I suggest defining something like
HV_X64_MSR_CRASH_CTL_CONTENTS (see below), which will only have
HV_CRASH_CTL_CRASH_NOTIFY set for now but may have other bits set in
the future, and then setting *pdata = HV_X64_MSR_CRASH_CTL_CONTENTS
here.

> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       if (atomic_dec_and_test(&ctx->crash_pending)) {

The TLFS says: "To invoke a supported hypervisor guest crash action, a
child partition writes to the HV_X64_MSR_CRASH_CTL register,
specifying the desired action." However, this implementation doesn't
actually check the data that's written to this register. The condition
here should check for "data & HV_CRASH_CTL_CRASH_NOTIFY", right?

I'm still not clear on the need for crash_pending here... maybe there
are concurrency reasons that I haven't thought through.

> +               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +                        ctx->crash_p3, ctx->crash_p4);
> +
> +               /* Crash data almost gathered so notify user space */
> +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +       }
> +
> +       return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +       switch (msr) {
> +       case HV_X64_MSR_CRASH_P0:
> +               ctx->crash_p0 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P1:
> +               ctx->crash_p1 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P2:
> +               ctx->crash_p2 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P3:
> +               ctx->crash_p3 = data;
> +               return 0;
> +       case HV_X64_MSR_CRASH_P4:
> +               ctx->crash_p4 = data;
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +

I suggest here:

#define HV_X64_MSR_CRASH_CTL_CONTENTS  \
        (HV_CRASH_CTL_CRASH_NOTIFY)

To allow for more crash actions to be added in the future.

> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +       return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> +{
> +       return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>                 } else
>                         return set_msr_hyperv(vcpu, msr, data);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>         case MSR_IA32_BBL_CR_CTL3:
>                 /* Drop writes to this legacy MSR -- see rdmsr
>                  * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>                 } else
>                         return get_msr_hyperv(vcpu, msr, pdata);
>                 break;
> +       case HV_X64_MSR_CRASH_CTL:
> +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);

Have the Windows guests that you've tested ever tried to read from the
data registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there
any harm in implementing them for reads? The TLFS doesn't seem to say
if reads are permitted or not. Not terribly important, but might be
nice to have.

>         case MSR_IA32_BBL_CR_CTL3:
>                 /* This legacy MSR exists but isn't fully documented in current
>                  * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>                         vcpu_scan_ioapic(vcpu);
>                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>                         kvm_vcpu_reload_apic_access_page(vcpu);
> +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +                       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +                       vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> +                       vcpu->run->system_event.flags = 0;
> +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +                       r = 0;
> +                       goto out;
> +               }
>         }
>
>         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>         if (type)
>                 return -EINVAL;
>
> +       if (kvm_mshv_ctx_create(kvm))
> +               return -ENOMEM;
> +
>         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +       kvm_mshv_ctx_destroy(kvm);
>         if (current->mm == kvm->mm) {
>                 /*
>                  * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>
>  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>         int used_slots;
>  };
>
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {
> +       struct kvm      *kvm;
> +       atomic_t        crash_pending;
> +
> +       /* Guest crash related parameters */
> +       u64             crash_p0;
> +       u64             crash_p1;
> +       u64             crash_p2;
> +       u64             crash_p3;
> +       u64             crash_p4;
> +};
> +
>  struct kvm {
>         spinlock_t mmu_lock;
>         struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>         long tlbs_dirty;
>         struct list_head devices;
> +       struct kvm_mshv_ctx *mshv_ctx;
>  };
>
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>                 struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>                         __u32 type;
>                         __u64 flags;
> +                       union {
> +                               struct {
> +                                       /* Guest crash related parameters */
> +                                       __u64 p0;
> +                                       __u64 p1;
> +                                       __u64 p2;
> +                                       __u64 p3;
> +                                       __u64 p4;
> +                               } crash;
> +                       };
>                 } system_event;
>                 /* KVM_EXIT_S390_STSI */
>                 struct {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
Pete

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-12 23:03     ` [Qemu-devel] " Peter Hornyack
@ 2015-06-15 10:21       ` Andrey Smetanin
  -1 siblings, 0 replies; 20+ messages in thread
From: Andrey Smetanin @ 2015-06-15 10:21 UTC (permalink / raw)
  To: Peter Hornyack
  Cc: Denis V. Lunev, kvm, qemu-devel, Gleb Natapov, Paolo Bonzini

Hi Peter,
thank you for comments. See answers below.
On Fri, 2015-06-12 at 16:03 -0700, Peter Hornyack wrote:
> Hi Denis, Andrey, I have a few comments and questions. (re-sending in
> plain-text mode, apologies for sending twice.)
> 
> On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:
> > From: Andrey Smetanin <asmetanin@virtuozzo.com>
> >
> > Windows 2012 guests can notify hypervisor about occurred guest crash
> > (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> > handling of this MSR's by KVM and sending notification to user space that
> > allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> >
> > The idea is to provide functionality equal to pvpanic device without
> > QEMU guest agent for Windows.
> >
> > The idea is borrowed from Linux HyperV bus driver and validated against
> > Windows 2k12.
> >
> > Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Gleb Natapov <gleb@kernel.org>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
> >  arch/x86/kvm/Makefile              |  2 +-
> >  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++
> >  include/linux/kvm_host.h           | 17 ++++++++
> >  include/uapi/linux/kvm.h           | 11 +++++
> >  7 files changed, 180 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/kvm/mshv.c
> >  create mode 100644 arch/x86/kvm/mshv.h
> >
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index ce6068d..25f3064 100644
> > --- a/arch/x86/include/uapi/asm/hyp	erv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -199,6 +199,16 @@
> >  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
> >  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
> >
> > +
> > +/* Hypev-V guest crash notification MSR's */
> > +#define HV_X64_MSR_CRASH_P0                    0x40000100
> > +#define HV_X64_MSR_CRASH_P1                    0x40000101
> > +#define HV_X64_MSR_CRASH_P2                    0x40000102
> > +#define HV_X64_MSR_CRASH_P3                    0x40000103
> > +#define HV_X64_MSR_CRASH_P4                    0x40000104
> > +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> > +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> > +
> >  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index 16e8f96..b1ec24d 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> >  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
> >
> >  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> > -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> > +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
> >  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
> >  kvm-intel-y            += vmx.o
> >  kvm-amd-y              += svm.o
> > diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> > new file mode 100644
> > index 0000000..ad367c44
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#include <linux/kvm_host.h>
> > +#include "mshv.h"
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm)
> > +{
> > +       struct kvm_mshv_ctx *ctx;
> > +
> > +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> > +       if (!ctx)
> > +               return -ENOMEM;
> > +
> > +       ctx->kvm = kvm;
> > +       atomic_set(&ctx->crash_pending, 0);
> > +       kvm->mshv_ctx = ctx;
> > +       return 0;
> > +}
> > +
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> > +{
> > +       kfree(kvm->mshv_ctx);
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       atomic_set(&ctx->crash_pending, 1);
> 
> Can you explain why crash_pending is needed?
> 
> Do you know what the Windows guest behavior is here? From my reading
> of the Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at
> some point (not necessarily at the time of the crash, potentially at
> boot time?) to determine the crash actions that the hypervisor
> supports. I'm not sure why kvm needs to remember any state on reads
> from HV_X64_MSR_CRASH_CTL.
Thank you for notice, I agree that crash_pending is useless. We'll drop
it and will send new version of patch.
> 
>  > +
> > +       /* Response that KVM ready to receive crash data */
> > +       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> 
> The TLFS says that CrashNotify is the only current action that is
> supported on a guest crash, but other actions may be supported in the
> future. I suggest defining something like
> HV_X64_MSR_CRASH_CTL_CONTENTS (see below), which will only have
> HV_CRASH_CTL_CRASH_NOTIFY set for now but may have other bits set in
> the future, and then setting *pdata = HV_X64_MSR_CRASH_CTL_CONTENTS
> here.
> 
Ok, will do
> > +       return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       if (atomic_dec_and_test(&ctx->crash_pending)) {
> 
> The TLFS says: "To invoke a supported hypervisor guest crash action, a
> child partition writes to the HV_X64_MSR_CRASH_CTL register,
> specifying the desired action." However, this implementation doesn't
> actually check the data that's written to this register. The condition
> here should check for "data & HV_CRASH_CTL_CRASH_NOTIFY", right?
> 
Agree, will be done in new version of patch
> I'm still not clear on the need for crash_pending here... maybe there
> are concurrency reasons that I haven't thought through.
> 
Will drop it
> > +               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> > +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> > +                        ctx->crash_p3, ctx->crash_p4);
> > +
> > +               /* Crash data almost gathered so notify user space */
> > +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       switch (msr) {
> > +       case HV_X64_MSR_CRASH_P0:
> > +               ctx->crash_p0 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P1:
> > +               ctx->crash_p1 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P2:
> > +               ctx->crash_p2 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P3:
> > +               ctx->crash_p3 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P4:
> > +               ctx->crash_p4 = data;
> > +               return 0;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> > new file mode 100644
> > index 0000000..ce8e7fa
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.h
> > @@ -0,0 +1,32 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#ifndef __ARCH_X86_KVM_MSHV_H__
> > +#define __ARCH_X86_KVM_MSHV_H__
> > +
> 
> I suggest here:
> 
> #define HV_X64_MSR_CRASH_CTL_CONTENTS  \
>         (HV_CRASH_CTL_CRASH_NOTIFY)
> 
> To allow for more crash actions to be added in the future.
> 
Ok, will do
> > +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> > +{
> > +       return vm->mshv_ctx;
> > +}
> > +
> > +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> > +{
> > +       return vcpu->kvm->mshv_ctx;
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm);
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> > +
> > +#endif
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ea306ad..388b58f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -28,6 +28,7 @@
> >  #include "x86.h"
> >  #include "cpuid.h"
> >  #include "assigned-dev.h"
> > +#include "mshv.h"
> >
> >  #include <linux/clocksource.h>
> >  #include <linux/interrupt.h>
> > @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >                 } else
> >                         return set_msr_hyperv(vcpu, msr, data);
> >                 break;
> > +       case HV_X64_MSR_CRASH_CTL:
> > +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> > +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> > +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
> >         case MSR_IA32_BBL_CR_CTL3:
> >                 /* Drop writes to this legacy MSR -- see rdmsr
> >                  * counterpart for further detail.
> > @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >                 } else
> >                         return get_msr_hyperv(vcpu, msr, pdata);
> >                 break;
> > +       case HV_X64_MSR_CRASH_CTL:
> > +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
> 
> Have the Windows guests that you've tested ever tried to read from the
> data registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there
> any harm in implementing them for reads? The TLFS doesn't seem to say
> if reads are permitted or not. Not terribly important, but might be
> nice to have.
> 
This MSR's are not read by w2k12 guest, but we'll make placeholder
handler that returns error in that case.
> >         case MSR_IA32_BBL_CR_CTL3:
> >                 /* This legacy MSR exists but isn't fully documented in current
> >                  * silicon.  It is however accessed by winxp in very narrow
> > @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >                         vcpu_scan_ioapic(vcpu);
> >                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
> >                         kvm_vcpu_reload_apic_access_page(vcpu);
> > +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> > +                       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> > +                       vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> > +                       vcpu->run->system_event.flags = 0;
> > +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> > +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> > +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> > +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> > +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> > +                       r = 0;
> > +                       goto out;
> > +               }
> >         }
> >
> >         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >         if (type)
> >                 return -EINVAL;
> >
> > +       if (kvm_mshv_ctx_create(kvm))
> > +               return -ENOMEM;
> > +
> >         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
> >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> > @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
> >
> >  void kvm_arch_destroy_vm(struct kvm *kvm)
> >  {
> > +       kvm_mshv_ctx_destroy(kvm);
> >         if (current->mm == kvm->mm) {
> >                 /*
> >                  * Free memory regions allocated on behalf of userspace,
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index ad45054..83bd7bf 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
> >  #define KVM_REQ_ENABLE_IBS        23
> >  #define KVM_REQ_DISABLE_IBS       24
> >  #define KVM_REQ_APIC_PAGE_RELOAD  25
> > +#define KVM_REQ_MSHV_CRASH        26
> >
> >  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
> >  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> > @@ -343,6 +344,21 @@ struct kvm_memslots {
> >         int used_slots;
> >  };
> >
> > +/*
> > + * Ms hyperv paravirt context
> > + */
> > +struct kvm_mshv_ctx {
> > +       struct kvm      *kvm;
> > +       atomic_t        crash_pending;
> > +
> > +       /* Guest crash related parameters */
> > +       u64             crash_p0;
> > +       u64             crash_p1;
> > +       u64             crash_p2;
> > +       u64             crash_p3;
> > +       u64             crash_p4;
> > +};
> > +
> >  struct kvm {
> >         spinlock_t mmu_lock;
> >         struct mutex slots_lock;
> > @@ -395,6 +411,7 @@ struct kvm {
> >  #endif
> >         long tlbs_dirty;
> >         struct list_head devices;
> > +       struct kvm_mshv_ctx *mshv_ctx;
> >  };
> >
> >  #define kvm_err(fmt, ...) \
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b60056..12f481b 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -317,8 +317,19 @@ struct kvm_run {
> >                 struct {
> >  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
> >  #define KVM_SYSTEM_EVENT_RESET          2
> > +#define KVM_SYSTEM_EVENT_CRASH          3
> >                         __u32 type;
> >                         __u64 flags;
> > +                       union {
> > +                               struct {
> > +                                       /* Guest crash related parameters */
> > +                                       __u64 p0;
> > +                                       __u64 p1;
> > +                                       __u64 p2;
> > +                                       __u64 p3;
> > +                                       __u64 p4;
> > +                               } crash;
> > +                       };
> >                 } system_event;
> >                 /* KVM_EXIT_S390_STSI */
> >                 struct {
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Thanks,
> Pete
Thanks,
Andrey



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-15 10:21       ` Andrey Smetanin
  0 siblings, 0 replies; 20+ messages in thread
From: Andrey Smetanin @ 2015-06-15 10:21 UTC (permalink / raw)
  To: Peter Hornyack
  Cc: Gleb Natapov, Denis V. Lunev, qemu-devel, kvm, Paolo Bonzini

Hi Peter,
thank you for comments. See answers below.
On Fri, 2015-06-12 at 16:03 -0700, Peter Hornyack wrote:
> Hi Denis, Andrey, I have a few comments and questions. (re-sending in
> plain-text mode, apologies for sending twice.)
> 
> On Thu, Jun 11, 2015 at 6:18 AM, Denis V. Lunev <den@openvz.org> wrote:
> > From: Andrey Smetanin <asmetanin@virtuozzo.com>
> >
> > Windows 2012 guests can notify hypervisor about occurred guest crash
> > (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> > handling of this MSR's by KVM and sending notification to user space that
> > allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> >
> > The idea is to provide functionality equal to pvpanic device without
> > QEMU guest agent for Windows.
> >
> > The idea is borrowed from Linux HyperV bus driver and validated against
> > Windows 2k12.
> >
> > Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Gleb Natapov <gleb@kernel.org>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
> >  arch/x86/kvm/Makefile              |  2 +-
> >  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++
> >  include/linux/kvm_host.h           | 17 ++++++++
> >  include/uapi/linux/kvm.h           | 11 +++++
> >  7 files changed, 180 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/kvm/mshv.c
> >  create mode 100644 arch/x86/kvm/mshv.h
> >
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index ce6068d..25f3064 100644
> > --- a/arch/x86/include/uapi/asm/hyp	erv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -199,6 +199,16 @@
> >  #define HV_X64_MSR_STIMER3_CONFIG              0x400000B6
> >  #define HV_X64_MSR_STIMER3_COUNT               0x400000B7
> >
> > +
> > +/* Hypev-V guest crash notification MSR's */
> > +#define HV_X64_MSR_CRASH_P0                    0x40000100
> > +#define HV_X64_MSR_CRASH_P1                    0x40000101
> > +#define HV_X64_MSR_CRASH_P2                    0x40000102
> > +#define HV_X64_MSR_CRASH_P3                    0x40000103
> > +#define HV_X64_MSR_CRASH_P4                    0x40000104
> > +#define HV_X64_MSR_CRASH_CTL                   0x40000105
> > +#define HV_CRASH_CTL_CRASH_NOTIFY              (1ULL << 63)
> > +
> >  #define HV_X64_MSR_HYPERCALL_ENABLE            0x00000001
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT        12
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK \
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index 16e8f96..b1ec24d 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -12,7 +12,7 @@ kvm-y                 += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> >  kvm-$(CONFIG_KVM_ASYNC_PF)     += $(KVM)/async_pf.o
> >
> >  kvm-y                  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> > -                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> > +                          i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
> >  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)    += assigned-dev.o iommu.o
> >  kvm-intel-y            += vmx.o
> >  kvm-amd-y              += svm.o
> > diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> > new file mode 100644
> > index 0000000..ad367c44
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#include <linux/kvm_host.h>
> > +#include "mshv.h"
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm)
> > +{
> > +       struct kvm_mshv_ctx *ctx;
> > +
> > +       ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> > +       if (!ctx)
> > +               return -ENOMEM;
> > +
> > +       ctx->kvm = kvm;
> > +       atomic_set(&ctx->crash_pending, 0);
> > +       kvm->mshv_ctx = ctx;
> > +       return 0;
> > +}
> > +
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> > +{
> > +       kfree(kvm->mshv_ctx);
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       atomic_set(&ctx->crash_pending, 1);
> 
> Can you explain why crash_pending is needed?
> 
> Do you know what the Windows guest behavior is here? From my reading
> of the Hyper-V TLFS 4.0, the guest will read HV_X64_MSR_CRASH_CTL at
> some point (not necessarily at the time of the crash, potentially at
> boot time?) to determine the crash actions that the hypervisor
> supports. I'm not sure why kvm needs to remember any state on reads
> from HV_X64_MSR_CRASH_CTL.
Thank you for notice, I agree that crash_pending is useless. We'll drop
it and will send new version of patch.
> 
>  > +
> > +       /* Response that KVM ready to receive crash data */
> > +       *pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> 
> The TLFS says that CrashNotify is the only current action that is
> supported on a guest crash, but other actions may be supported in the
> future. I suggest defining something like
> HV_X64_MSR_CRASH_CTL_CONTENTS (see below), which will only have
> HV_CRASH_CTL_CRASH_NOTIFY set for now but may have other bits set in
> the future, and then setting *pdata = HV_X64_MSR_CRASH_CTL_CONTENTS
> here.
> 
Ok, will do
> > +       return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       if (atomic_dec_and_test(&ctx->crash_pending)) {
> 
> The TLFS says: "To invoke a supported hypervisor guest crash action, a
> child partition writes to the HV_X64_MSR_CRASH_CTL register,
> specifying the desired action." However, this implementation doesn't
> actually check the data that's written to this register. The condition
> here should check for "data & HV_CRASH_CTL_CRASH_NOTIFY", right?
> 
Agree, will be done in new version of patch
> I'm still not clear on the need for crash_pending here... maybe there
> are concurrency reasons that I haven't thought through.
> 
Will drop it
> > +               pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> > +                        vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> > +                        ctx->crash_p3, ctx->crash_p4);
> > +
> > +               /* Crash data almost gathered so notify user space */
> > +               kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +       switch (msr) {
> > +       case HV_X64_MSR_CRASH_P0:
> > +               ctx->crash_p0 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P1:
> > +               ctx->crash_p1 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P2:
> > +               ctx->crash_p2 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P3:
> > +               ctx->crash_p3 = data;
> > +               return 0;
> > +       case HV_X64_MSR_CRASH_P4:
> > +               ctx->crash_p4 = data;
> > +               return 0;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> > new file mode 100644
> > index 0000000..ce8e7fa
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.h
> > @@ -0,0 +1,32 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#ifndef __ARCH_X86_KVM_MSHV_H__
> > +#define __ARCH_X86_KVM_MSHV_H__
> > +
> 
> I suggest here:
> 
> #define HV_X64_MSR_CRASH_CTL_CONTENTS  \
>         (HV_CRASH_CTL_CRASH_NOTIFY)
> 
> To allow for more crash actions to be added in the future.
> 
Ok, will do
> > +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> > +{
> > +       return vm->mshv_ctx;
> > +}
> > +
> > +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> > +{
> > +       return vcpu->kvm->mshv_ctx;
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm);
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> > +
> > +#endif
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ea306ad..388b58f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -28,6 +28,7 @@
> >  #include "x86.h"
> >  #include "cpuid.h"
> >  #include "assigned-dev.h"
> > +#include "mshv.h"
> >
> >  #include <linux/clocksource.h>
> >  #include <linux/interrupt.h>
> > @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >                 } else
> >                         return set_msr_hyperv(vcpu, msr, data);
> >                 break;
> > +       case HV_X64_MSR_CRASH_CTL:
> > +               return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> > +       case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> > +               return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
> >         case MSR_IA32_BBL_CR_CTL3:
> >                 /* Drop writes to this legacy MSR -- see rdmsr
> >                  * counterpart for further detail.
> > @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >                 } else
> >                         return get_msr_hyperv(vcpu, msr, pdata);
> >                 break;
> > +       case HV_X64_MSR_CRASH_CTL:
> > +               return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
> 
> Have the Windows guests that you've tested ever tried to read from the
> data registers HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4? Is there
> any harm in implementing them for reads? The TLFS doesn't seem to say
> if reads are permitted or not. Not terribly important, but might be
> nice to have.
> 
This MSR's are not read by w2k12 guest, but we'll make placeholder
handler that returns error in that case.
> >         case MSR_IA32_BBL_CR_CTL3:
> >                 /* This legacy MSR exists but isn't fully documented in current
> >                  * silicon.  It is however accessed by winxp in very narrow
> > @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >                         vcpu_scan_ioapic(vcpu);
> >                 if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
> >                         kvm_vcpu_reload_apic_access_page(vcpu);
> > +               if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> > +                       struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +                       vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> > +                       vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> > +                       vcpu->run->system_event.flags = 0;
> > +                       vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> > +                       vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> > +                       vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> > +                       vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> > +                       vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> > +                       r = 0;
> > +                       goto out;
> > +               }
> >         }
> >
> >         if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >         if (type)
> >                 return -EINVAL;
> >
> > +       if (kvm_mshv_ctx_create(kvm))
> > +               return -ENOMEM;
> > +
> >         INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
> >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >         INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> > @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
> >
> >  void kvm_arch_destroy_vm(struct kvm *kvm)
> >  {
> > +       kvm_mshv_ctx_destroy(kvm);
> >         if (current->mm == kvm->mm) {
> >                 /*
> >                  * Free memory regions allocated on behalf of userspace,
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index ad45054..83bd7bf 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
> >  #define KVM_REQ_ENABLE_IBS        23
> >  #define KVM_REQ_DISABLE_IBS       24
> >  #define KVM_REQ_APIC_PAGE_RELOAD  25
> > +#define KVM_REQ_MSHV_CRASH        26
> >
> >  #define KVM_USERSPACE_IRQ_SOURCE_ID            0
> >  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID       1
> > @@ -343,6 +344,21 @@ struct kvm_memslots {
> >         int used_slots;
> >  };
> >
> > +/*
> > + * Ms hyperv paravirt context
> > + */
> > +struct kvm_mshv_ctx {
> > +       struct kvm      *kvm;
> > +       atomic_t        crash_pending;
> > +
> > +       /* Guest crash related parameters */
> > +       u64             crash_p0;
> > +       u64             crash_p1;
> > +       u64             crash_p2;
> > +       u64             crash_p3;
> > +       u64             crash_p4;
> > +};
> > +
> >  struct kvm {
> >         spinlock_t mmu_lock;
> >         struct mutex slots_lock;
> > @@ -395,6 +411,7 @@ struct kvm {
> >  #endif
> >         long tlbs_dirty;
> >         struct list_head devices;
> > +       struct kvm_mshv_ctx *mshv_ctx;
> >  };
> >
> >  #define kvm_err(fmt, ...) \
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b60056..12f481b 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -317,8 +317,19 @@ struct kvm_run {
> >                 struct {
> >  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
> >  #define KVM_SYSTEM_EVENT_RESET          2
> > +#define KVM_SYSTEM_EVENT_CRASH          3
> >                         __u32 type;
> >                         __u64 flags;
> > +                       union {
> > +                               struct {
> > +                                       /* Guest crash related parameters */
> > +                                       __u64 p0;
> > +                                       __u64 p1;
> > +                                       __u64 p2;
> > +                                       __u64 p3;
> > +                                       __u64 p4;
> > +                               } crash;
> > +                       };
> >                 } system_event;
> >                 /* KVM_EXIT_S390_STSI */
> >                 struct {
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Thanks,
> Pete
Thanks,
Andrey

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-17 12:44     ` Paolo Bonzini
  -1 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-17 12:44 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm



On 11/06/2015 15:18, Denis V. Lunev wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> 
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
> 
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++

Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
 We can later move more functions from x86.c to the new file, so it's
better to keep the names consistent.

>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
>  
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0			0x40000100
> +#define HV_X64_MSR_CRASH_P1			0x40000101
> +#define HV_X64_MSR_CRASH_P2			0x40000102
> +#define HV_X64_MSR_CRASH_P3			0x40000103
> +#define HV_X64_MSR_CRASH_P4			0x40000104
> +#define HV_X64_MSR_CRASH_CTL			0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
>  
>  kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
>  kvm-intel-y		+= vmx.o
>  kvm-amd-y		+= svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +	struct kvm_mshv_ctx *ctx;
> +
> +	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->kvm = kvm;
> +	atomic_set(&ctx->crash_pending, 0);
> +	kvm->mshv_ctx = ctx;
> +	return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +	kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	atomic_set(&ctx->crash_pending, 1);
> +
> +	/* Response that KVM ready to receive crash data */
> +	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> +	return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	if (atomic_dec_and_test(&ctx->crash_pending)) {
> +		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +			 ctx->crash_p3, ctx->crash_p4);
> +
> +		/* Crash data almost gathered so notify user space */

Why "almost" gathered?

> +		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +	}
> +
> +	return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	switch (msr) {
> +	case HV_X64_MSR_CRASH_P0:
> +		ctx->crash_p0 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P1:
> +		ctx->crash_p1 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P2:
> +		ctx->crash_p2 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P3:
> +		ctx->crash_p3 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P4:
> +		ctx->crash_p4 = data;
> +		return 0;

Please use an array (with a WARN_ON_ONCE check that the index is in bounds).

> +	default:
> +		return -EINVAL;
> +	}
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +
> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +	return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>  
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		} else
>  			return set_msr_hyperv(vcpu, msr, data);
>  		break;
> +	case HV_X64_MSR_CRASH_CTL:
> +		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>  	case MSR_IA32_BBL_CR_CTL3:
>  		/* Drop writes to this legacy MSR -- see rdmsr
>  		 * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  		} else
>  			return get_msr_hyperv(vcpu, msr, pdata);
>  		break;
> +	case HV_X64_MSR_CRASH_CTL:
> +		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);

Please implement get_crash_data as well.  Userspace may want to retrieve
this value and stash it somewhere for post-mortem analysis, and
KVM_GET_MSR is very handy for this purpose.

Do not return an error, just return the last written datum.

>  	case MSR_IA32_BBL_CR_CTL3:
>  		/* This legacy MSR exists but isn't fully documented in current
>  		 * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  			vcpu_scan_ioapic(vcpu);
>  		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>  			kvm_vcpu_reload_apic_access_page(vcpu);
> +		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> +			vcpu->run->system_event.flags = 0;
> +			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +			r = 0;
> +			goto out;
> +		}
>  	}
>  
>  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	if (type)
>  		return -EINVAL;
>  
> +	if (kvm_mshv_ctx_create(kvm))
> +		return -ENOMEM;
> +
>  	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>  	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>  
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +	kvm_mshv_ctx_destroy(kvm);
>  	if (current->mm == kvm->mm) {
>  		/*
>  		 * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>  
>  #define KVM_USERSPACE_IRQ_SOURCE_ID		0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>  	int used_slots;
>  };
>  
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {

This should be in an x86-specific file.  Please name it "struct
kvm_arch_hyperv hv" and stick it inside struct kvm_arch (so it's
accessed as kvm->arch.hv).  We can also move other fields, e.g.
kvm->arch.hv_hypercall inside this new struct.

> +	struct kvm	*kvm;

Not needed if you avoid the pointer: then you can just use container_of.

> +	atomic_t	crash_pending;
> +
> +	/* Guest crash related parameters */
> +	u64		crash_p0;
> +	u64		crash_p1;
> +	u64		crash_p2;
> +	u64		crash_p3;
> +	u64		crash_p4;
> +};
> +
>  struct kvm {
>  	spinlock_t mmu_lock;
>  	struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>  	long tlbs_dirty;
>  	struct list_head devices;
> +	struct kvm_mshv_ctx *mshv_ctx;
>  };
>  
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>  		struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>  			__u32 type;
>  			__u64 flags;
> +			union {
> +				struct {
> +					/* Guest crash related parameters */
> +					__u64 p0;
> +					__u64 p1;
> +					__u64 p2;
> +					__u64 p3;
> +					__u64 p4;
> +				} crash;

No need to return the parameters here.  Userspace can use KVM_GET_MSR to
read them.

Paolo

> +			};
>  		} system_event;
>  		/* KVM_EXIT_S390_STSI */
>  		struct {
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-17 12:44     ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-17 12:44 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm



On 11/06/2015 15:18, Denis V. Lunev wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> Windows 2012 guests can notify hypervisor about occurred guest crash
> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> handling of this MSR's by KVM and sending notification to user space that
> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> 
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
> 
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>  arch/x86/kvm/Makefile              |  2 +-
>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++

Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
 We can later move more functions from x86.c to the new file, so it's
better to keep the names consistent.

>  arch/x86/kvm/x86.c                 | 25 ++++++++++++
>  include/linux/kvm_host.h           | 17 ++++++++
>  include/uapi/linux/kvm.h           | 11 +++++
>  7 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kvm/mshv.c
>  create mode 100644 arch/x86/kvm/mshv.h
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index ce6068d..25f3064 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -199,6 +199,16 @@
>  #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
>  #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
>  
> +
> +/* Hypev-V guest crash notification MSR's */
> +#define HV_X64_MSR_CRASH_P0			0x40000100
> +#define HV_X64_MSR_CRASH_P1			0x40000101
> +#define HV_X64_MSR_CRASH_P2			0x40000102
> +#define HV_X64_MSR_CRASH_P3			0x40000103
> +#define HV_X64_MSR_CRASH_P4			0x40000104
> +#define HV_X64_MSR_CRASH_CTL			0x40000105
> +#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
> +
>  #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 16e8f96..b1ec24d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
>  kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
>  
>  kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> -			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> +			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
>  kvm-intel-y		+= vmx.o
>  kvm-amd-y		+= svm.o
> diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> new file mode 100644
> index 0000000..ad367c44
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.c
> @@ -0,0 +1,84 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#include <linux/kvm_host.h>
> +#include "mshv.h"
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm)
> +{
> +	struct kvm_mshv_ctx *ctx;
> +
> +	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->kvm = kvm;
> +	atomic_set(&ctx->crash_pending, 0);
> +	kvm->mshv_ctx = ctx;
> +	return 0;
> +}
> +
> +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> +{
> +	kfree(kvm->mshv_ctx);
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	atomic_set(&ctx->crash_pending, 1);
> +
> +	/* Response that KVM ready to receive crash data */
> +	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> +	return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	if (atomic_dec_and_test(&ctx->crash_pending)) {
> +		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> +			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> +			 ctx->crash_p3, ctx->crash_p4);
> +
> +		/* Crash data almost gathered so notify user space */

Why "almost" gathered?

> +		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> +	}
> +
> +	return 0;
> +}
> +
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +	switch (msr) {
> +	case HV_X64_MSR_CRASH_P0:
> +		ctx->crash_p0 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P1:
> +		ctx->crash_p1 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P2:
> +		ctx->crash_p2 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P3:
> +		ctx->crash_p3 = data;
> +		return 0;
> +	case HV_X64_MSR_CRASH_P4:
> +		ctx->crash_p4 = data;
> +		return 0;

Please use an array (with a WARN_ON_ONCE check that the index is in bounds).

> +	default:
> +		return -EINVAL;
> +	}
> +}
> diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> new file mode 100644
> index 0000000..ce8e7fa
> --- /dev/null
> +++ b/arch/x86/kvm/mshv.h
> @@ -0,0 +1,32 @@
> +/*
> + * KVM Microsoft Hyper-V extended paravirtualization
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> + *
> + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> + */
> +
> +#ifndef __ARCH_X86_KVM_MSHV_H__
> +#define __ARCH_X86_KVM_MSHV_H__
> +
> +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> +{
> +	return vm->mshv_ctx;
> +}
> +
> +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->kvm->mshv_ctx;
> +}
> +
> +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> +
> +int kvm_mshv_ctx_create(struct kvm *kvm);
> +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> +
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ea306ad..388b58f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include "x86.h"
>  #include "cpuid.h"
>  #include "assigned-dev.h"
> +#include "mshv.h"
>  
>  #include <linux/clocksource.h>
>  #include <linux/interrupt.h>
> @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		} else
>  			return set_msr_hyperv(vcpu, msr, data);
>  		break;
> +	case HV_X64_MSR_CRASH_CTL:
> +		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> +	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> +		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
>  	case MSR_IA32_BBL_CR_CTL3:
>  		/* Drop writes to this legacy MSR -- see rdmsr
>  		 * counterpart for further detail.
> @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  		} else
>  			return get_msr_hyperv(vcpu, msr, pdata);
>  		break;
> +	case HV_X64_MSR_CRASH_CTL:
> +		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);

Please implement get_crash_data as well.  Userspace may want to retrieve
this value and stash it somewhere for post-mortem analysis, and
KVM_GET_MSR is very handy for this purpose.

Do not return an error, just return the last written datum.

>  	case MSR_IA32_BBL_CR_CTL3:
>  		/* This legacy MSR exists but isn't fully documented in current
>  		 * silicon.  It is however accessed by winxp in very narrow
> @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  			vcpu_scan_ioapic(vcpu);
>  		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
>  			kvm_vcpu_reload_apic_access_page(vcpu);
> +		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> +			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> +
> +			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> +			vcpu->run->system_event.flags = 0;
> +			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> +			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> +			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> +			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> +			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> +			r = 0;
> +			goto out;
> +		}
>  	}
>  
>  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	if (type)
>  		return -EINVAL;
>  
> +	if (kvm_mshv_ctx_create(kvm))
> +		return -ENOMEM;
> +
>  	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
>  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>  	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
>  
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +	kvm_mshv_ctx_destroy(kvm);
>  	if (current->mm == kvm->mm) {
>  		/*
>  		 * Free memory regions allocated on behalf of userspace,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..83bd7bf 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_MSHV_CRASH        26
>  
>  #define KVM_USERSPACE_IRQ_SOURCE_ID		0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
> @@ -343,6 +344,21 @@ struct kvm_memslots {
>  	int used_slots;
>  };
>  
> +/*
> + * Ms hyperv paravirt context
> + */
> +struct kvm_mshv_ctx {

This should be in an x86-specific file.  Please name it "struct
kvm_arch_hyperv hv" and stick it inside struct kvm_arch (so it's
accessed as kvm->arch.hv).  We can also move other fields, e.g.
kvm->arch.hv_hypercall inside this new struct.

> +	struct kvm	*kvm;

Not needed if you avoid the pointer: then you can just use container_of.

> +	atomic_t	crash_pending;
> +
> +	/* Guest crash related parameters */
> +	u64		crash_p0;
> +	u64		crash_p1;
> +	u64		crash_p2;
> +	u64		crash_p3;
> +	u64		crash_p4;
> +};
> +
>  struct kvm {
>  	spinlock_t mmu_lock;
>  	struct mutex slots_lock;
> @@ -395,6 +411,7 @@ struct kvm {
>  #endif
>  	long tlbs_dirty;
>  	struct list_head devices;
> +	struct kvm_mshv_ctx *mshv_ctx;
>  };
>  
>  #define kvm_err(fmt, ...) \
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..12f481b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>  		struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>  			__u32 type;
>  			__u64 flags;
> +			union {
> +				struct {
> +					/* Guest crash related parameters */
> +					__u64 p0;
> +					__u64 p1;
> +					__u64 p2;
> +					__u64 p3;
> +					__u64 p4;
> +				} crash;

No need to return the parameters here.  Userspace can use KVM_GET_MSR to
read them.

Paolo

> +			};
>  		} system_event;
>  		/* KVM_EXIT_S390_STSI */
>  		struct {
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] qemu/kvm: kvm guest crash event handling
  2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
@ 2015-06-17 12:47     ` Paolo Bonzini
  -1 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-17 12:47 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: kvm, qemu-devel, Andrey Smetanin, Gleb Natapov



On 11/06/2015 15:18, Denis V. Lunev wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> KVM Hyper-V based guests can notify hypervisor about
> occurred guest crash. This patch does handling of KVM crash event
> by sending to libvirt guest panic event that allows to gather
> guest crash dump by QEMU/LIBVIRT.
> 
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
> 
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/sysemu.h        |  2 ++
>  kvm-all.c                      |  8 ++++++++
>  linux-headers/asm-x86/hyperv.h |  2 ++
>  linux-headers/linux/kvm.h      | 11 +++++++++++
>  target-i386/cpu-qom.h          |  1 +
>  target-i386/cpu.c              |  1 +
>  target-i386/kvm.c              |  4 ++++
>  vl.c                           | 31 +++++++++++++++++++++++++++++++
>  8 files changed, 60 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 853d90a..82d3213 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -61,6 +61,8 @@ void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_system_debug_request(void);
> +void qemu_system_crash_request(uint64_t p0, uint64_t p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4);
>  void qemu_system_vmstop_request(RunState reason);
>  void qemu_system_vmstop_request_prepare(void);
>  int qemu_shutdown_requested_get(void);
> diff --git a/kvm-all.c b/kvm-all.c
> index 53e01d4..cee23bc 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1844,6 +1844,14 @@ int kvm_cpu_exec(CPUState *cpu)
>                  qemu_system_reset_request();
>                  ret = EXCP_INTERRUPT;
>                  break;
> +            case KVM_SYSTEM_EVENT_CRASH:
> +                qemu_system_crash_request(run->system_event.crash.p0,
> +                                            run->system_event.crash.p1,
> +                                            run->system_event.crash.p2,
> +                                            run->system_event.crash.p3,
> +                                            run->system_event.crash.p4);

This needs to be synchronous, so you can do it here:

        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
        vm_stop(RUN_STATE_GUEST_PANICKED);

The five values are never read back.  Please include them in the CPU
state and in the migration stream.  Migration to file is commonly used
to gather post mortem dumps, and there are even tools to convert the
migration file format to Windows crash dump format.  The tools could be
improved to find the crash data and populate the appropriate fields in
the dump file's header.

Paolo

> +                ret = 0;
> +                break;
>              default:
>                  DPRINTF("kvm_arch_handle_exit\n");
>                  ret = kvm_arch_handle_exit(cpu, run);
> diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h
> index ce6068d..a5df1ab 100644
> --- a/linux-headers/asm-x86/hyperv.h
> +++ b/linux-headers/asm-x86/hyperv.h
> @@ -108,6 +108,8 @@
>  #define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE		(1 << 4)
>  /* Support for a virtual guest idle state is available */
>  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
> +/* Guest crash data handler available */
> +#define HV_X64_GUEST_CRASH_MSR_AVAILABLE                (1 << 10)
>  
>  /*
>   * Implementation recommendations. Indicates which behaviors the hypervisor
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index fad9e5c..e169602 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>  		struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>  			__u32 type;
>  			__u64 flags;
> +                        union {
> +                                struct {
> +                                        /* Guest crash related parameters */
> +                                        __u64 p0;
> +                                        __u64 p1;
> +                                        __u64 p2;
> +                                        __u64 p3;
> +                                        __u64 p4;
> +                                } crash;
> +                        };
>  		} system_event;
>  		/* KVM_EXIT_S390_STSI */
>  		struct {
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 7a4fddd..c35b624 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -89,6 +89,7 @@ typedef struct X86CPU {
>      bool hyperv_relaxed_timing;
>      int hyperv_spinlock_attempts;
>      bool hyperv_time;
> +    bool hyperv_crash;
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4e7cdaa..af0552a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3117,6 +3117,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
>      DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
>      DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
> +    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 5a236e3..b63e4bb 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -516,6 +516,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              c->eax |= 0x200;
>              has_msr_hv_tsc = true;
>          }
> +        if (cpu->hyperv_crash) {
> +            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> +        }
> +
>          c = &cpuid_data.entries[cpuid_i++];
>          c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
>          if (cpu->hyperv_relaxed_timing) {
> diff --git a/vl.c b/vl.c
> index 66ccd06..76b5484 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1505,6 +1505,14 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> +
> +static int crash_requested;
> +static uint64_t crash_p0;
> +static uint64_t crash_p1;
> +static uint64_t crash_p2;
> +static uint64_t crash_p3;
> +static uint64_t crash_p4;
> +
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1578,6 +1586,13 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>  
> +static int qemu_crash_requested(void)
> +{
> +    int r = crash_requested;
> +    crash_requested = 0;
> +    return r;
> +}
> +
>  void qemu_register_reset(QEMUResetHandler *func, void *opaque)
>  {
>      QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> @@ -1729,9 +1744,25 @@ void qemu_system_debug_request(void)
>      qemu_notify_event();
>  }
>  
> +void qemu_system_crash_request(uint64_t p0, uint64_t  p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4)
> +{
> +    crash_p0 = p0;
> +    crash_p1 = p1;
> +    crash_p2 = p2;
> +    crash_p3 = p3;
> +    crash_p4 = p4;
> +    crash_requested = 1;
> +    qemu_notify_event();
> +}
> +
>  static bool main_loop_should_exit(void)
>  {
>      RunState r;
> +    if (qemu_crash_requested()) {
> +        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +    }
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] qemu/kvm: kvm guest crash event handling
@ 2015-06-17 12:47     ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-17 12:47 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: Andrey Smetanin, Gleb Natapov, qemu-devel, kvm



On 11/06/2015 15:18, Denis V. Lunev wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> KVM Hyper-V based guests can notify hypervisor about
> occurred guest crash. This patch does handling of KVM crash event
> by sending to libvirt guest panic event that allows to gather
> guest crash dump by QEMU/LIBVIRT.
> 
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
> 
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/sysemu.h        |  2 ++
>  kvm-all.c                      |  8 ++++++++
>  linux-headers/asm-x86/hyperv.h |  2 ++
>  linux-headers/linux/kvm.h      | 11 +++++++++++
>  target-i386/cpu-qom.h          |  1 +
>  target-i386/cpu.c              |  1 +
>  target-i386/kvm.c              |  4 ++++
>  vl.c                           | 31 +++++++++++++++++++++++++++++++
>  8 files changed, 60 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 853d90a..82d3213 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -61,6 +61,8 @@ void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_system_debug_request(void);
> +void qemu_system_crash_request(uint64_t p0, uint64_t p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4);
>  void qemu_system_vmstop_request(RunState reason);
>  void qemu_system_vmstop_request_prepare(void);
>  int qemu_shutdown_requested_get(void);
> diff --git a/kvm-all.c b/kvm-all.c
> index 53e01d4..cee23bc 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1844,6 +1844,14 @@ int kvm_cpu_exec(CPUState *cpu)
>                  qemu_system_reset_request();
>                  ret = EXCP_INTERRUPT;
>                  break;
> +            case KVM_SYSTEM_EVENT_CRASH:
> +                qemu_system_crash_request(run->system_event.crash.p0,
> +                                            run->system_event.crash.p1,
> +                                            run->system_event.crash.p2,
> +                                            run->system_event.crash.p3,
> +                                            run->system_event.crash.p4);

This needs to be synchronous, so you can do it here:

        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
        vm_stop(RUN_STATE_GUEST_PANICKED);

The five values are never read back.  Please include them in the CPU
state and in the migration stream.  Migration to file is commonly used
to gather post mortem dumps, and there are even tools to convert the
migration file format to Windows crash dump format.  The tools could be
improved to find the crash data and populate the appropriate fields in
the dump file's header.

Paolo

> +                ret = 0;
> +                break;
>              default:
>                  DPRINTF("kvm_arch_handle_exit\n");
>                  ret = kvm_arch_handle_exit(cpu, run);
> diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h
> index ce6068d..a5df1ab 100644
> --- a/linux-headers/asm-x86/hyperv.h
> +++ b/linux-headers/asm-x86/hyperv.h
> @@ -108,6 +108,8 @@
>  #define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE		(1 << 4)
>  /* Support for a virtual guest idle state is available */
>  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
> +/* Guest crash data handler available */
> +#define HV_X64_GUEST_CRASH_MSR_AVAILABLE                (1 << 10)
>  
>  /*
>   * Implementation recommendations. Indicates which behaviors the hypervisor
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index fad9e5c..e169602 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>  		struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>  			__u32 type;
>  			__u64 flags;
> +                        union {
> +                                struct {
> +                                        /* Guest crash related parameters */
> +                                        __u64 p0;
> +                                        __u64 p1;
> +                                        __u64 p2;
> +                                        __u64 p3;
> +                                        __u64 p4;
> +                                } crash;
> +                        };
>  		} system_event;
>  		/* KVM_EXIT_S390_STSI */
>  		struct {
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 7a4fddd..c35b624 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -89,6 +89,7 @@ typedef struct X86CPU {
>      bool hyperv_relaxed_timing;
>      int hyperv_spinlock_attempts;
>      bool hyperv_time;
> +    bool hyperv_crash;
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4e7cdaa..af0552a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3117,6 +3117,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
>      DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
>      DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
> +    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 5a236e3..b63e4bb 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -516,6 +516,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              c->eax |= 0x200;
>              has_msr_hv_tsc = true;
>          }
> +        if (cpu->hyperv_crash) {
> +            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> +        }
> +
>          c = &cpuid_data.entries[cpuid_i++];
>          c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
>          if (cpu->hyperv_relaxed_timing) {
> diff --git a/vl.c b/vl.c
> index 66ccd06..76b5484 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1505,6 +1505,14 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> +
> +static int crash_requested;
> +static uint64_t crash_p0;
> +static uint64_t crash_p1;
> +static uint64_t crash_p2;
> +static uint64_t crash_p3;
> +static uint64_t crash_p4;
> +
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1578,6 +1586,13 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>  
> +static int qemu_crash_requested(void)
> +{
> +    int r = crash_requested;
> +    crash_requested = 0;
> +    return r;
> +}
> +
>  void qemu_register_reset(QEMUResetHandler *func, void *opaque)
>  {
>      QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> @@ -1729,9 +1744,25 @@ void qemu_system_debug_request(void)
>      qemu_notify_event();
>  }
>  
> +void qemu_system_crash_request(uint64_t p0, uint64_t  p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4)
> +{
> +    crash_p0 = p0;
> +    crash_p1 = p1;
> +    crash_p2 = p2;
> +    crash_p3 = p3;
> +    crash_p4 = p4;
> +    crash_requested = 1;
> +    qemu_notify_event();
> +}
> +
>  static bool main_loop_should_exit(void)
>  {
>      RunState r;
> +    if (qemu_crash_requested()) {
> +        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +    }
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-17 12:44     ` [Qemu-devel] " Paolo Bonzini
@ 2015-06-19 10:28       ` Andrey Smetanin
  -1 siblings, 0 replies; 20+ messages in thread
From: Andrey Smetanin @ 2015-06-19 10:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Denis V. Lunev, kvm, qemu-devel, Gleb Natapov

On Wed, 2015-06-17 at 14:44 +0200, Paolo Bonzini wrote:
> 
> On 11/06/2015 15:18, Denis V. Lunev wrote:
> > From: Andrey Smetanin <asmetanin@virtuozzo.com>
> > 
> > Windows 2012 guests can notify hypervisor about occurred guest crash
> > (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> > handling of this MSR's by KVM and sending notification to user space that
> > allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> > 
> > The idea is to provide functionality equal to pvpanic device without
> > QEMU guest agent for Windows.
> > 
> > The idea is borrowed from Linux HyperV bus driver and validated against
> > Windows 2k12.
> > 
> > Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Gleb Natapov <gleb@kernel.org>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
> >  arch/x86/kvm/Makefile              |  2 +-
> >  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
> 
> Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
>  We can later move more functions from x86.c to the new file, so it's
> better to keep the names consistent.
> 
Should we prepare a 1st patch in this series where
we move all hyper-v related code from x86.c into hyperv.c new file with
copyright extension ?
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++
> >  include/linux/kvm_host.h           | 17 ++++++++
> >  include/uapi/linux/kvm.h           | 11 +++++
> >  7 files changed, 180 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/kvm/mshv.c
> >  create mode 100644 arch/x86/kvm/mshv.h
> > 
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index ce6068d..25f3064 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -199,6 +199,16 @@
> >  #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
> >  #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
> >  
> > +
> > +/* Hypev-V guest crash notification MSR's */
> > +#define HV_X64_MSR_CRASH_P0			0x40000100
> > +#define HV_X64_MSR_CRASH_P1			0x40000101
> > +#define HV_X64_MSR_CRASH_P2			0x40000102
> > +#define HV_X64_MSR_CRASH_P3			0x40000103
> > +#define HV_X64_MSR_CRASH_P4			0x40000104
> > +#define HV_X64_MSR_CRASH_CTL			0x40000105
> > +#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
> > +
> >  #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index 16e8f96..b1ec24d 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> >  kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
> >  
> >  kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> > -			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> > +			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
> >  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
> >  kvm-intel-y		+= vmx.o
> >  kvm-amd-y		+= svm.o
> > diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> > new file mode 100644
> > index 0000000..ad367c44
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#include <linux/kvm_host.h>
> > +#include "mshv.h"
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm)
> > +{
> > +	struct kvm_mshv_ctx *ctx;
> > +
> > +	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->kvm = kvm;
> > +	atomic_set(&ctx->crash_pending, 0);
> > +	kvm->mshv_ctx = ctx;
> > +	return 0;
> > +}
> > +
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> > +{
> > +	kfree(kvm->mshv_ctx);
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	atomic_set(&ctx->crash_pending, 1);
> > +
> > +	/* Response that KVM ready to receive crash data */
> > +	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> > +	return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	if (atomic_dec_and_test(&ctx->crash_pending)) {
> > +		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> > +			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> > +			 ctx->crash_p3, ctx->crash_p4);
> > +
> > +		/* Crash data almost gathered so notify user space */
> 
> Why "almost" gathered?
> 
> > +		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	switch (msr) {
> > +	case HV_X64_MSR_CRASH_P0:
> > +		ctx->crash_p0 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P1:
> > +		ctx->crash_p1 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P2:
> > +		ctx->crash_p2 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P3:
> > +		ctx->crash_p3 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P4:
> > +		ctx->crash_p4 = data;
> > +		return 0;
> 
> Please use an array (with a WARN_ON_ONCE check that the index is in bounds).
> 
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> > new file mode 100644
> > index 0000000..ce8e7fa
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.h
> > @@ -0,0 +1,32 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#ifndef __ARCH_X86_KVM_MSHV_H__
> > +#define __ARCH_X86_KVM_MSHV_H__
> > +
> > +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> > +{
> > +	return vm->mshv_ctx;
> > +}
> > +
> > +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> > +{
> > +	return vcpu->kvm->mshv_ctx;
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm);
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> > +
> > +#endif
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ea306ad..388b58f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -28,6 +28,7 @@
> >  #include "x86.h"
> >  #include "cpuid.h"
> >  #include "assigned-dev.h"
> > +#include "mshv.h"
> >  
> >  #include <linux/clocksource.h>
> >  #include <linux/interrupt.h>
> > @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		} else
> >  			return set_msr_hyperv(vcpu, msr, data);
> >  		break;
> > +	case HV_X64_MSR_CRASH_CTL:
> > +		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> > +	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> > +		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
> >  	case MSR_IA32_BBL_CR_CTL3:
> >  		/* Drop writes to this legacy MSR -- see rdmsr
> >  		 * counterpart for further detail.
> > @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >  		} else
> >  			return get_msr_hyperv(vcpu, msr, pdata);
> >  		break;
> > +	case HV_X64_MSR_CRASH_CTL:
> > +		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
> 
> Please implement get_crash_data as well.  Userspace may want to retrieve
> this value and stash it somewhere for post-mortem analysis, and
> KVM_GET_MSR is very handy for this purpose.
> 
> Do not return an error, just return the last written datum.
> 
> >  	case MSR_IA32_BBL_CR_CTL3:
> >  		/* This legacy MSR exists but isn't fully documented in current
> >  		 * silicon.  It is however accessed by winxp in very narrow
> > @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  			vcpu_scan_ioapic(vcpu);
> >  		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
> >  			kvm_vcpu_reload_apic_access_page(vcpu);
> > +		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> > +			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> > +			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> > +			vcpu->run->system_event.flags = 0;
> > +			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> > +			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> > +			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> > +			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> > +			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> > +			r = 0;
> > +			goto out;
> > +		}
> >  	}
> >  
> >  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >  	if (type)
> >  		return -EINVAL;
> >  
> > +	if (kvm_mshv_ctx_create(kvm))
> > +		return -ENOMEM;
> > +
> >  	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
> >  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >  	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> > @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
> >  
> >  void kvm_arch_destroy_vm(struct kvm *kvm)
> >  {
> > +	kvm_mshv_ctx_destroy(kvm);
> >  	if (current->mm == kvm->mm) {
> >  		/*
> >  		 * Free memory regions allocated on behalf of userspace,
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index ad45054..83bd7bf 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
> >  #define KVM_REQ_ENABLE_IBS        23
> >  #define KVM_REQ_DISABLE_IBS       24
> >  #define KVM_REQ_APIC_PAGE_RELOAD  25
> > +#define KVM_REQ_MSHV_CRASH        26
> >  
> >  #define KVM_USERSPACE_IRQ_SOURCE_ID		0
> >  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
> > @@ -343,6 +344,21 @@ struct kvm_memslots {
> >  	int used_slots;
> >  };
> >  
> > +/*
> > + * Ms hyperv paravirt context
> > + */
> > +struct kvm_mshv_ctx {
> 
> This should be in an x86-specific file.  Please name it "struct
> kvm_arch_hyperv hv" and stick it inside struct kvm_arch (so it's
> accessed as kvm->arch.hv).  We can also move other fields, e.g.
> kvm->arch.hv_hypercall inside this new struct.
> 
> > +	struct kvm	*kvm;
> 
> Not needed if you avoid the pointer: then you can just use container_of.
> 
> > +	atomic_t	crash_pending;
> > +
> > +	/* Guest crash related parameters */
> > +	u64		crash_p0;
> > +	u64		crash_p1;
> > +	u64		crash_p2;
> > +	u64		crash_p3;
> > +	u64		crash_p4;
> > +};
> > +
> >  struct kvm {
> >  	spinlock_t mmu_lock;
> >  	struct mutex slots_lock;
> > @@ -395,6 +411,7 @@ struct kvm {
> >  #endif
> >  	long tlbs_dirty;
> >  	struct list_head devices;
> > +	struct kvm_mshv_ctx *mshv_ctx;
> >  };
> >  
> >  #define kvm_err(fmt, ...) \
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b60056..12f481b 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -317,8 +317,19 @@ struct kvm_run {
> >  		struct {
> >  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
> >  #define KVM_SYSTEM_EVENT_RESET          2
> > +#define KVM_SYSTEM_EVENT_CRASH          3
> >  			__u32 type;
> >  			__u64 flags;
> > +			union {
> > +				struct {
> > +					/* Guest crash related parameters */
> > +					__u64 p0;
> > +					__u64 p1;
> > +					__u64 p2;
> > +					__u64 p3;
> > +					__u64 p4;
> > +				} crash;
> 
> No need to return the parameters here.  Userspace can use KVM_GET_MSR to
> read them.
> 
> Paolo
> 
> > +			};
> >  		} system_event;
> >  		/* KVM_EXIT_S390_STSI */
> >  		struct {
> > 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-19 10:28       ` Andrey Smetanin
  0 siblings, 0 replies; 20+ messages in thread
From: Andrey Smetanin @ 2015-06-19 10:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Gleb Natapov, Denis V. Lunev, qemu-devel, kvm

On Wed, 2015-06-17 at 14:44 +0200, Paolo Bonzini wrote:
> 
> On 11/06/2015 15:18, Denis V. Lunev wrote:
> > From: Andrey Smetanin <asmetanin@virtuozzo.com>
> > 
> > Windows 2012 guests can notify hypervisor about occurred guest crash
> > (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
> > handling of this MSR's by KVM and sending notification to user space that
> > allows to gather Windows guest crash dump by QEMU/LIBVIRT.
> > 
> > The idea is to provide functionality equal to pvpanic device without
> > QEMU guest agent for Windows.
> > 
> > The idea is borrowed from Linux HyperV bus driver and validated against
> > Windows 2k12.
> > 
> > Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Gleb Natapov <gleb@kernel.org>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
> >  arch/x86/kvm/Makefile              |  2 +-
> >  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
> 
> Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
>  We can later move more functions from x86.c to the new file, so it's
> better to keep the names consistent.
> 
Should we prepare a 1st patch in this series where
we move all hyper-v related code from x86.c into hyperv.c new file with
copyright extension ?
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++
> >  include/linux/kvm_host.h           | 17 ++++++++
> >  include/uapi/linux/kvm.h           | 11 +++++
> >  7 files changed, 180 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/kvm/mshv.c
> >  create mode 100644 arch/x86/kvm/mshv.h
> > 
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index ce6068d..25f3064 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -199,6 +199,16 @@
> >  #define HV_X64_MSR_STIMER3_CONFIG		0x400000B6
> >  #define HV_X64_MSR_STIMER3_COUNT		0x400000B7
> >  
> > +
> > +/* Hypev-V guest crash notification MSR's */
> > +#define HV_X64_MSR_CRASH_P0			0x40000100
> > +#define HV_X64_MSR_CRASH_P1			0x40000101
> > +#define HV_X64_MSR_CRASH_P2			0x40000102
> > +#define HV_X64_MSR_CRASH_P3			0x40000103
> > +#define HV_X64_MSR_CRASH_P4			0x40000104
> > +#define HV_X64_MSR_CRASH_CTL			0x40000105
> > +#define HV_CRASH_CTL_CRASH_NOTIFY		(1ULL << 63)
> > +
> >  #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
> >  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index 16e8f96..b1ec24d 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> >  kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
> >  
> >  kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> > -			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
> > +			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mshv.o
> >  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
> >  kvm-intel-y		+= vmx.o
> >  kvm-amd-y		+= svm.o
> > diff --git a/arch/x86/kvm/mshv.c b/arch/x86/kvm/mshv.c
> > new file mode 100644
> > index 0000000..ad367c44
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#include <linux/kvm_host.h>
> > +#include "mshv.h"
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm)
> > +{
> > +	struct kvm_mshv_ctx *ctx;
> > +
> > +	ctx = kzalloc(sizeof(struct kvm_mshv_ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->kvm = kvm;
> > +	atomic_set(&ctx->crash_pending, 0);
> > +	kvm->mshv_ctx = ctx;
> > +	return 0;
> > +}
> > +
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm)
> > +{
> > +	kfree(kvm->mshv_ctx);
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	atomic_set(&ctx->crash_pending, 1);
> > +
> > +	/* Response that KVM ready to receive crash data */
> > +	*pdata = HV_CRASH_CTL_CRASH_NOTIFY;
> > +	return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	if (atomic_dec_and_test(&ctx->crash_pending)) {
> > +		pr_debug("vcpu %p 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx",
> > +			 vcpu, ctx->crash_p0, ctx->crash_p1, ctx->crash_p2,
> > +			 ctx->crash_p3, ctx->crash_p4);
> > +
> > +		/* Crash data almost gathered so notify user space */
> 
> Why "almost" gathered?
> 
> > +		kvm_make_request(KVM_REQ_MSHV_CRASH, vcpu);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > +	struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +	switch (msr) {
> > +	case HV_X64_MSR_CRASH_P0:
> > +		ctx->crash_p0 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P1:
> > +		ctx->crash_p1 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P2:
> > +		ctx->crash_p2 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P3:
> > +		ctx->crash_p3 = data;
> > +		return 0;
> > +	case HV_X64_MSR_CRASH_P4:
> > +		ctx->crash_p4 = data;
> > +		return 0;
> 
> Please use an array (with a WARN_ON_ONCE check that the index is in bounds).
> 
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > diff --git a/arch/x86/kvm/mshv.h b/arch/x86/kvm/mshv.h
> > new file mode 100644
> > index 0000000..ce8e7fa
> > --- /dev/null
> > +++ b/arch/x86/kvm/mshv.h
> > @@ -0,0 +1,32 @@
> > +/*
> > + * KVM Microsoft Hyper-V extended paravirtualization
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Copyright (C) 2015 Andrey Smetanin <asmetanin@virtuozzo.com>
> > + *
> > + * Authors: Andrey Smetanin asmetanin@virtuozzo.com
> > + */
> > +
> > +#ifndef __ARCH_X86_KVM_MSHV_H__
> > +#define __ARCH_X86_KVM_MSHV_H__
> > +
> > +static inline struct kvm_mshv_ctx *kvm_get_mshv_ctx(struct kvm *vm)
> > +{
> > +	return vm->mshv_ctx;
> > +}
> > +
> > +static inline struct kvm_mshv_ctx *kvm_vcpu_get_mshv_ctx(struct kvm_vcpu *vcpu)
> > +{
> > +	return vcpu->kvm->mshv_ctx;
> > +}
> > +
> > +int kvm_mshv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> > +int kvm_mshv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +int kvm_mshv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +
> > +int kvm_mshv_ctx_create(struct kvm *kvm);
> > +void kvm_mshv_ctx_destroy(struct kvm *kvm);
> > +
> > +#endif
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ea306ad..388b58f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -28,6 +28,7 @@
> >  #include "x86.h"
> >  #include "cpuid.h"
> >  #include "assigned-dev.h"
> > +#include "mshv.h"
> >  
> >  #include <linux/clocksource.h>
> >  #include <linux/interrupt.h>
> > @@ -2338,6 +2339,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		} else
> >  			return set_msr_hyperv(vcpu, msr, data);
> >  		break;
> > +	case HV_X64_MSR_CRASH_CTL:
> > +		return kvm_mshv_msr_set_crash_ctl(vcpu, msr, data);
> > +	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> > +		return kvm_mshv_msr_set_crash_data(vcpu, msr, data);
> >  	case MSR_IA32_BBL_CR_CTL3:
> >  		/* Drop writes to this legacy MSR -- see rdmsr
> >  		 * counterpart for further detail.
> > @@ -2650,6 +2655,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >  		} else
> >  			return get_msr_hyperv(vcpu, msr, pdata);
> >  		break;
> > +	case HV_X64_MSR_CRASH_CTL:
> > +		return kvm_mshv_msr_get_crash_ctl(vcpu, msr, pdata);
> 
> Please implement get_crash_data as well.  Userspace may want to retrieve
> this value and stash it somewhere for post-mortem analysis, and
> KVM_GET_MSR is very handy for this purpose.
> 
> Do not return an error, just return the last written datum.
> 
> >  	case MSR_IA32_BBL_CR_CTL3:
> >  		/* This legacy MSR exists but isn't fully documented in current
> >  		 * silicon.  It is however accessed by winxp in very narrow
> > @@ -6280,6 +6287,20 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  			vcpu_scan_ioapic(vcpu);
> >  		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
> >  			kvm_vcpu_reload_apic_access_page(vcpu);
> > +		if (kvm_check_request(KVM_REQ_MSHV_CRASH, vcpu)) {
> > +			struct kvm_mshv_ctx *ctx = kvm_vcpu_get_mshv_ctx(vcpu);
> > +
> > +			vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> > +			vcpu->run->system_event.type = KVM_SYSTEM_EVENT_CRASH;
> > +			vcpu->run->system_event.flags = 0;
> > +			vcpu->run->system_event.crash.p0 = ctx->crash_p0;
> > +			vcpu->run->system_event.crash.p1 = ctx->crash_p1;
> > +			vcpu->run->system_event.crash.p2 = ctx->crash_p2;
> > +			vcpu->run->system_event.crash.p3 = ctx->crash_p3;
> > +			vcpu->run->system_event.crash.p4 = ctx->crash_p4;
> > +			r = 0;
> > +			goto out;
> > +		}
> >  	}
> >  
> >  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > @@ -7418,6 +7439,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >  	if (type)
> >  		return -EINVAL;
> >  
> > +	if (kvm_mshv_ctx_create(kvm))
> > +		return -ENOMEM;
> > +
> >  	INIT_HLIST_HEAD(&kvm->arch.mask_notifier_list);
> >  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >  	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
> > @@ -7484,6 +7508,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
> >  
> >  void kvm_arch_destroy_vm(struct kvm *kvm)
> >  {
> > +	kvm_mshv_ctx_destroy(kvm);
> >  	if (current->mm == kvm->mm) {
> >  		/*
> >  		 * Free memory regions allocated on behalf of userspace,
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index ad45054..83bd7bf 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
> >  #define KVM_REQ_ENABLE_IBS        23
> >  #define KVM_REQ_DISABLE_IBS       24
> >  #define KVM_REQ_APIC_PAGE_RELOAD  25
> > +#define KVM_REQ_MSHV_CRASH        26
> >  
> >  #define KVM_USERSPACE_IRQ_SOURCE_ID		0
> >  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
> > @@ -343,6 +344,21 @@ struct kvm_memslots {
> >  	int used_slots;
> >  };
> >  
> > +/*
> > + * Ms hyperv paravirt context
> > + */
> > +struct kvm_mshv_ctx {
> 
> This should be in an x86-specific file.  Please name it "struct
> kvm_arch_hyperv hv" and stick it inside struct kvm_arch (so it's
> accessed as kvm->arch.hv).  We can also move other fields, e.g.
> kvm->arch.hv_hypercall inside this new struct.
> 
> > +	struct kvm	*kvm;
> 
> Not needed if you avoid the pointer: then you can just use container_of.
> 
> > +	atomic_t	crash_pending;
> > +
> > +	/* Guest crash related parameters */
> > +	u64		crash_p0;
> > +	u64		crash_p1;
> > +	u64		crash_p2;
> > +	u64		crash_p3;
> > +	u64		crash_p4;
> > +};
> > +
> >  struct kvm {
> >  	spinlock_t mmu_lock;
> >  	struct mutex slots_lock;
> > @@ -395,6 +411,7 @@ struct kvm {
> >  #endif
> >  	long tlbs_dirty;
> >  	struct list_head devices;
> > +	struct kvm_mshv_ctx *mshv_ctx;
> >  };
> >  
> >  #define kvm_err(fmt, ...) \
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b60056..12f481b 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -317,8 +317,19 @@ struct kvm_run {
> >  		struct {
> >  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
> >  #define KVM_SYSTEM_EVENT_RESET          2
> > +#define KVM_SYSTEM_EVENT_CRASH          3
> >  			__u32 type;
> >  			__u64 flags;
> > +			union {
> > +				struct {
> > +					/* Guest crash related parameters */
> > +					__u64 p0;
> > +					__u64 p1;
> > +					__u64 p2;
> > +					__u64 p3;
> > +					__u64 p4;
> > +				} crash;
> 
> No need to return the parameters here.  Userspace can use KVM_GET_MSR to
> read them.
> 
> Paolo
> 
> > +			};
> >  		} system_event;
> >  		/* KVM_EXIT_S390_STSI */
> >  		struct {
> > 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
  2015-06-19 10:28       ` [Qemu-devel] " Andrey Smetanin
@ 2015-06-19 10:32         ` Paolo Bonzini
  -1 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-19 10:32 UTC (permalink / raw)
  To: asmetanin; +Cc: Gleb Natapov, Denis V. Lunev, qemu-devel, kvm



On 19/06/2015 12:28, Andrey Smetanin wrote:
> On Wed, 2015-06-17 at 14:44 +0200, Paolo Bonzini wrote:
>>
>> On 11/06/2015 15:18, Denis V. Lunev wrote:
>>> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>>>
>>> Windows 2012 guests can notify hypervisor about occurred guest crash
>>> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
>>> handling of this MSR's by KVM and sending notification to user space that
>>> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>>>
>>> The idea is to provide functionality equal to pvpanic device without
>>> QEMU guest agent for Windows.
>>>
>>> The idea is borrowed from Linux HyperV bus driver and validated against
>>> Windows 2k12.
>>>
>>> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Gleb Natapov <gleb@kernel.org>
>>> CC: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>>>  arch/x86/kvm/Makefile              |  2 +-
>>>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>>
>> Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
>>  We can later move more functions from x86.c to the new file, so it's
>> better to keep the names consistent.
>>
> Should we prepare a 1st patch in this series where
> we move all hyper-v related code from x86.c into hyperv.c new file with
> copyright extension ?

If you want to do that, I certainly wouldn't complain.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling
@ 2015-06-19 10:32         ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-06-19 10:32 UTC (permalink / raw)
  To: asmetanin; +Cc: Gleb Natapov, Denis V. Lunev, qemu-devel, kvm



On 19/06/2015 12:28, Andrey Smetanin wrote:
> On Wed, 2015-06-17 at 14:44 +0200, Paolo Bonzini wrote:
>>
>> On 11/06/2015 15:18, Denis V. Lunev wrote:
>>> From: Andrey Smetanin <asmetanin@virtuozzo.com>
>>>
>>> Windows 2012 guests can notify hypervisor about occurred guest crash
>>> (Windows bugcheck(BSOD)) by writing specific Hyper-V msrs. This patch does
>>> handling of this MSR's by KVM and sending notification to user space that
>>> allows to gather Windows guest crash dump by QEMU/LIBVIRT.
>>>
>>> The idea is to provide functionality equal to pvpanic device without
>>> QEMU guest agent for Windows.
>>>
>>> The idea is borrowed from Linux HyperV bus driver and validated against
>>> Windows 2k12.
>>>
>>> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Gleb Natapov <gleb@kernel.org>
>>> CC: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  arch/x86/include/uapi/asm/hyperv.h | 10 +++++
>>>  arch/x86/kvm/Makefile              |  2 +-
>>>  arch/x86/kvm/mshv.c                | 84 ++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/kvm/mshv.h                | 32 +++++++++++++++
>>
>> Please use hyperv.[ch] or hyper-v.[ch] and name the functions kvm_hv_*.
>>  We can later move more functions from x86.c to the new file, so it's
>> better to keep the names consistent.
>>
> Should we prepare a 1st patch in this series where
> we move all hyper-v related code from x86.c into hyperv.c new file with
> copyright extension ?

If you want to do that, I certainly wouldn't complain.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-06-19 10:33 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-11 13:18 [PATCH 0/2] HyperV equivalent of pvpanic driver Denis V. Lunev
2015-06-11 13:18 ` [Qemu-devel] " Denis V. Lunev
2015-06-11 13:18 ` [PATCH 1/2] kvm/x86: Hyper-V based guest crash data handling Denis V. Lunev
2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
2015-06-12 22:55   ` Peter Hornyack
2015-06-12 22:55     ` [Qemu-devel] " Peter Hornyack
2015-06-12 23:03   ` Peter Hornyack
2015-06-12 23:03     ` [Qemu-devel] " Peter Hornyack
2015-06-15 10:21     ` Andrey Smetanin
2015-06-15 10:21       ` [Qemu-devel] " Andrey Smetanin
2015-06-17 12:44   ` Paolo Bonzini
2015-06-17 12:44     ` [Qemu-devel] " Paolo Bonzini
2015-06-19 10:28     ` Andrey Smetanin
2015-06-19 10:28       ` [Qemu-devel] " Andrey Smetanin
2015-06-19 10:32       ` Paolo Bonzini
2015-06-19 10:32         ` [Qemu-devel] " Paolo Bonzini
2015-06-11 13:18 ` [PATCH 2/2] qemu/kvm: kvm guest crash event handling Denis V. Lunev
2015-06-11 13:18   ` [Qemu-devel] " Denis V. Lunev
2015-06-17 12:47   ` Paolo Bonzini
2015-06-17 12:47     ` [Qemu-devel] " Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.