[PATCH v8] kvm: notify host when the guest is panicked

[PATCH v8] kvm: notify host when the guest is panicked

[Wen Congyang]

We can know the guest is panicked when the guest runs on xen.
But we do not have such feature on kvm.

Another purpose of this feature is: management app(for example:
libvirt) can do auto dump when the guest is panicked. If management
app does not do auto dump, the guest's user can do dump by hand if
he sees the guest is panicked.

We have three solutions to implement this feature:
1. use vmcall
2. use I/O port
3. use virtio-serial.

We have decided to avoid touching hypervisor. The reason why I choose
choose the I/O port is:
1. it is easier to implememt
2. it does not depend any virtual device
3. it can work when starting the kernel

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 Documentation/virtual/kvm/pv_event.txt |   32 ++++++++++++++++++++++++++++++++
 arch/ia64/include/asm/kvm_para.h       |   14 ++++++++++++++
 arch/powerpc/include/asm/kvm_para.h    |   14 ++++++++++++++
 arch/s390/include/asm/kvm_para.h       |   14 ++++++++++++++
 arch/x86/include/asm/kvm_para.h        |   27 +++++++++++++++++++++++++++
 arch/x86/kernel/kvm.c                  |   25 +++++++++++++++++++++++++
 include/linux/kvm_para.h               |   23 +++++++++++++++++++++++
 7 files changed, 149 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/virtual/kvm/pv_event.txt

diff --git a/Documentation/virtual/kvm/pv_event.txt b/Documentation/virtual/kvm/pv_event.txt
new file mode 100644
index 0000000..0ebc890
--- /dev/null
+++ b/Documentation/virtual/kvm/pv_event.txt
@@ -0,0 +1,32 @@
+The KVM paravirtual event interface
+=================================
+
+Initializing the paravirtual event interface
+======================
+kvm_pv_event_init()
+Argiments:
+	None
+
+Return Value:
+	0 : The guest kernel can't use paravirtual event interface.
+	-1: The guest kernel can use paravirtual event interface.
+
+Querying whether the event can be ejected
+======================
+kvm_pv_has_feature()
+Arguments:
+	feature: The bit value of this paravirtual event to query
+
+Return Value:
+	0: The guest kernel can't eject this paravirtual event.
+	1: The guest kernel can eject this paravirtual event.
+
+
+Ejecting paravirtual event
+======================
+kvm_pv_eject_event()
+Arguments:
+	event: The event to be ejected.
+
+Return Value:
+	None
diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 2019cb9..b5ec658 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -31,6 +31,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline int kvm_arch_pv_event_init(void)
+{
+	return 0;
+}
+
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
+static inline void kvm_arch_pv_eject_event(unsigned int event)
+{
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index c18916b..01b98c7 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -211,6 +211,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline int kvm_arch_pv_event_init(void)
+{
+	return 0;
+}
+
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
+static inline void kvm_arch_pv_eject_event(unsigned int event)
+{
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* __POWERPC_KVM_PARA_H__ */
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index da44867..00ce058 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -154,6 +154,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline int kvm_arch_pv_event_init(void)
+{
+	return 0;
+}
+
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
+static inline void kvm_arch_pv_eject_event(unsigned int event)
+{
+}
+
 #endif
 
 #endif /* __S390_KVM_PARA_H */
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 2f7712e..7d297f0 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -96,8 +96,11 @@ struct kvm_vcpu_pv_apf_data {
 #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
 #define KVM_PV_EOI_DISABLED 0x0
 
+#define KVM_PV_EVENT_PORT	(0x505UL)
+
 #ifdef __KERNEL__
 #include <asm/processor.h>
+#include <linux/ioport.h>
 
 extern void kvmclock_init(void);
 extern int kvm_register_clock(char *txt);
@@ -228,6 +231,30 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+static inline int kvm_arch_pv_event_init(void)
+{
+	if (!request_region(KVM_PV_EVENT_PORT, 1, "KVM_PV_EVENT"))
+		return -1;
+
+	return 0;
+}
+
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	unsigned int features = inl(KVM_PV_EVENT_PORT);
+
+	/* Reading from an invalid I/O port will return -1 */
+	if (features == ~0)
+		features = 0;
+
+	return features;
+}
+
+static inline void kvm_arch_pv_eject_event(unsigned int event)
+{
+	outl(event, KVM_PV_EVENT_PORT);
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index c1d61ee..6129459 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -368,6 +368,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
 	.notifier_call = kvm_pv_reboot_notify,
 };
 
+static int
+kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
+{
+	kvm_pv_eject_event(KVM_PV_EVENT_PANICKED);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block kvm_pv_panic_nb = {
+	.notifier_call = kvm_pv_panic_notify,
+};
+
 static u64 kvm_steal_clock(int cpu)
 {
 	u64 steal;
@@ -447,6 +458,20 @@ static void __init kvm_apf_trap_init(void)
 	set_intr_gate(14, &async_page_fault);
 }
 
+static void __init kvm_pv_panicked_event_init(void)
+{
+	if (!kvm_para_available())
+		return;
+
+	if (kvm_pv_event_init())
+		return;
+
+	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
+		atomic_notifier_chain_register(&panic_notifier_list,
+			&kvm_pv_panic_nb);
+}
+arch_initcall(kvm_pv_panicked_event_init);
+
 void __init kvm_guest_init(void)
 {
 	int i;
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index ff476dd..8e0fb81 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -20,6 +20,12 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+/* The bit of supported pv event */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The pv event value */
+#define KVM_PV_EVENT_PANICKED	1
+
 /*
  * hypercalls use architecture specific
  */
@@ -33,5 +39,22 @@ static inline int kvm_para_has_feature(unsigned int feature)
 		return 1;
 	return 0;
 }
+
+static inline int kvm_pv_event_init(void)
+{
+	return kvm_arch_pv_event_init();
+}
+
+static inline int kvm_pv_has_feature(unsigned int feature)
+{
+	if (kvm_arch_pv_features() & (1UL << feature))
+		return 1;
+	return 0;
+}
+
+static inline void kvm_pv_eject_event(unsigned int event)
+{
+	kvm_arch_pv_eject_event(event);
+}
 #endif /* __KERNEL__ */
 #endif /* __LINUX_KVM_PARA_H */
-- 
1.7.1

[PATCH v8 1/6] start vm after reseting it

[Wen Congyang]

The guest should run after reseting it, but it does not run if its
old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.

We don't set runstate to RUN_STATE_PAUSED when reseting the guest,
so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 block.h |    2 ++
 qmp.c   |    2 +-
 vl.c    |    7 ++++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block.h b/block.h
index 650d872..29449bd 100644
--- a/block.h
+++ b/block.h
@@ -338,6 +338,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
+void iostatus_bdrv_it(void *opaque, BlockDriverState *bs);
+
 enum BlockAcctType {
     BDRV_ACCT_READ,
     BDRV_ACCT_WRITE,
diff --git a/qmp.c b/qmp.c
index fee9fb2..a111dff 100644
--- a/qmp.c
+++ b/qmp.c
@@ -125,7 +125,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
 };
 #endif
 
-static void iostatus_bdrv_it(void *opaque, BlockDriverState *bs)
+void iostatus_bdrv_it(void *opaque, BlockDriverState *bs)
 {
     bdrv_iostatus_reset(bs);
 }
diff --git a/vl.c b/vl.c
index e71cb30..856e089 100644
--- a/vl.c
+++ b/vl.c
@@ -333,7 +333,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH },
 
-    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_RUNNING },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
@@ -366,7 +366,7 @@ static const RunStateTransition runstate_transitions_def[] = {
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
-    { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
@@ -1531,7 +1531,8 @@ static bool main_loop_should_exit(void)
         resume_all_vcpus();
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
             runstate_check(RUN_STATE_SHUTDOWN)) {
-            runstate_set(RUN_STATE_PAUSED);
+            bdrv_iterate(iostatus_bdrv_it, NULL);
+            vm_start();
         }
     }
     if (qemu_powerdown_requested()) {
-- 
1.7.1

[PATCH v8 2/6] kvm: Update kernel headers

[Wen Congyang]

Corresponding kvm.git hash: 439793d4 with my patch for kvm
---
 linux-headers/asm-s390/kvm.h      |    2 +-
 linux-headers/asm-s390/kvm_para.h |    2 +-
 linux-headers/asm-x86/kvm.h       |    1 +
 linux-headers/asm-x86/kvm_para.h  |    9 +++++++++
 linux-headers/linux/kvm.h         |    3 +++
 linux-headers/linux/kvm_para.h    |    6 ++++++
 6 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index bdcbe0f..d25da59 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_KVM_S390_H
 #define __LINUX_KVM_S390_H
 /*
- * asm-s390/kvm.h - KVM s390 specific structures and definitions
+ * KVM s390 specific structures and definitions
  *
  * Copyright IBM Corp. 2008
  *
diff --git a/linux-headers/asm-s390/kvm_para.h b/linux-headers/asm-s390/kvm_para.h
index 8e2dd67..870051f 100644
--- a/linux-headers/asm-s390/kvm_para.h
+++ b/linux-headers/asm-s390/kvm_para.h
@@ -1,5 +1,5 @@
 /*
- * asm-s390/kvm_para.h - definition for paravirtual devices on s390
+ * definition for paravirtual devices on s390
  *
  * Copyright IBM Corp. 2008
  *
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index e7d1c19..246617e 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -12,6 +12,7 @@
 /* Select x86 specific features in <linux/kvm.h> */
 #define __KVM_HAVE_PIT
 #define __KVM_HAVE_IOAPIC
+#define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_DEVICE_ASSIGNMENT
 #define __KVM_HAVE_MSI
 #define __KVM_HAVE_USER_NMI
diff --git a/linux-headers/asm-x86/kvm_para.h b/linux-headers/asm-x86/kvm_para.h
index f2ac46a..53aca59 100644
--- a/linux-headers/asm-x86/kvm_para.h
+++ b/linux-headers/asm-x86/kvm_para.h
@@ -22,6 +22,7 @@
 #define KVM_FEATURE_CLOCKSOURCE2        3
 #define KVM_FEATURE_ASYNC_PF		4
 #define KVM_FEATURE_STEAL_TIME		5
+#define KVM_FEATURE_PV_EOI		6
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -37,6 +38,7 @@
 #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
+#define MSR_KVM_PV_EOI_EN      0x4b564d04
 
 struct kvm_steal_time {
 	__u64 steal;
@@ -89,5 +91,12 @@ struct kvm_vcpu_pv_apf_data {
 	__u32 enabled;
 };
 
+#define KVM_PV_EOI_BIT 0
+#define KVM_PV_EOI_MASK (0x1 << KVM_PV_EOI_BIT)
+#define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
+#define KVM_PV_EOI_DISABLED 0x0
+
+#define KVM_PV_EVENT_PORT	(0x505UL)
+
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 5a9d4e3..4b9e575 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -617,6 +617,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SIGNAL_MSI 77
 #define KVM_CAP_PPC_GET_SMMU_INFO 78
 #define KVM_CAP_S390_COW 79
+#define KVM_CAP_PPC_ALLOC_HTAB 80
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -828,6 +829,8 @@ struct kvm_s390_ucas_mapping {
 #define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 /* Available with KVM_CAP_PPC_GET_SMMU_INFO */
 #define KVM_PPC_GET_SMMU_INFO	  _IOR(KVMIO,  0xa6, struct kvm_ppc_smmu_info)
+/* Available with KVM_CAP_PPC_ALLOC_HTAB */
+#define KVM_PPC_ALLOCATE_HTAB	  _IOWR(KVMIO, 0xa7, __u32)
 
 /*
  * ioctls for vcpu fds
diff --git a/linux-headers/linux/kvm_para.h b/linux-headers/linux/kvm_para.h
index 7bdcf93..f6be0bb 100644
--- a/linux-headers/linux/kvm_para.h
+++ b/linux-headers/linux/kvm_para.h
@@ -20,6 +20,12 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+/* The bit of supported pv event */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The pv event value */
+#define KVM_PV_EVENT_PANICKED	1
+
 /*
  * hypercalls use architecture specific
  */
-- 
1.7.1

[PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED

[Wen Congyang]

The guest will be in this state when it is panicked.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 qapi-schema.json |    6 +++++-
 qmp.c            |    3 ++-
 vl.c             |    7 ++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index cddf63a..3f699ff 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -119,11 +119,15 @@
 # @suspended: guest is suspended (ACPI S3)
 #
 # @watchdog: the watchdog action is configured to pause and has been triggered
+#
+# @guest-panicked: the panicked action is configured to pause and has been
+# triggered.
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
             'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
-            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
+            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
+            'guest-panicked' ] }
 
 ##
 # @StatusInfo:
diff --git a/qmp.c b/qmp.c
index a111dff..3b0c9bc 100644
--- a/qmp.c
+++ b/qmp.c
@@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
         error_set(errp, QERR_MIGRATION_EXPECTED);
         return;
     } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-               runstate_check(RUN_STATE_SHUTDOWN)) {
+               runstate_check(RUN_STATE_SHUTDOWN) ||
+               runstate_check(RUN_STATE_GUEST_PANICKED)) {
         error_set(errp, QERR_RESET_REQUIRED);
         return;
     } else if (runstate_check(RUN_STATE_SUSPENDED)) {
diff --git a/vl.c b/vl.c
index 856e089..55dcdf2 100644
--- a/vl.c
+++ b/vl.c
@@ -363,6 +363,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
     { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
     { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
+    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
@@ -377,6 +378,9 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
 
@@ -1530,7 +1534,8 @@ static bool main_loop_should_exit(void)
         qemu_system_reset(VMRESET_REPORT);
         resume_all_vcpus();
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-            runstate_check(RUN_STATE_SHUTDOWN)) {
+            runstate_check(RUN_STATE_SHUTDOWN) ||
+            runstate_check(RUN_STATE_GUEST_PANICKED)) {
             bdrv_iterate(iostatus_bdrv_it, NULL);
             vm_start();
         }
-- 
1.7.1

[PATCH v8 4/6] add a new qevent: QEVENT_GUEST_PANICKED

[Wen Congyang]

This event will be emited when the guest is panicked.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 monitor.c |    1 +
 monitor.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 49dccfe..c892b7e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -458,6 +458,7 @@ static const char *monitor_event_names[] = {
     [QEVENT_SUSPEND] = "SUSPEND",
     [QEVENT_WAKEUP] = "WAKEUP",
     [QEVENT_BALLOON_CHANGE] = "BALLOON_CHANGE",
+    [QEVENT_GUEST_PANICKED] = "GUEST_PANICKED",
 };
 QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
 
diff --git a/monitor.h b/monitor.h
index 5f4de1b..cb7de8c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -42,6 +42,7 @@ typedef enum MonitorEvent {
     QEVENT_SUSPEND,
     QEVENT_WAKEUP,
     QEVENT_BALLOON_CHANGE,
+    QEVENT_GUEST_PANICKED,
 
     /* Add to 'monitor_event_names' array in monitor.c when
      * defining new events here */
-- 
1.7.1

[PATCH v8 5/6] introduce a new qom device to deal with panicked event

[Wen Congyang]

If the target is x86/x86_64, the guest's kernel will write 0x01 to the
port KVM_PV_EVENT_PORT when it is panciked. This patch introduces a new
qom device kvm_pv_ioport to listen this I/O port, and deal with panicked
event according to panicked_action's value. The possible actions are:
1. emit QEVENT_GUEST_PANICKED only
2. emit QEVENT_GUEST_PANICKED and pause the guest
3. emit QEVENT_GUEST_PANICKED and poweroff the guest
4. emit QEVENT_GUEST_PANICKED and reset the guest

I/O ports does not work for some targets(for example: s390). And you
can implement another qom device, and include it's code into pv_event.c
for such target.

Note: if we emit QEVENT_GUEST_PANICKED only, and the management
application does not receive this event(the management may not
run when the event is emitted), the management won't know the
guest is panicked.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/kvm/Makefile.objs |    2 +-
 hw/kvm/pv_event.c    |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/kvm/pv_ioport.c   |   93 ++++++++++++++++++++++++++++++++++++++++++
 hw/pc_piix.c         |    9 ++++
 kvm.h                |    2 +
 5 files changed, 214 insertions(+), 1 deletions(-)
 create mode 100644 hw/kvm/pv_event.c
 create mode 100644 hw/kvm/pv_ioport.c

diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
index 226497a..23e3b30 100644
--- a/hw/kvm/Makefile.objs
+++ b/hw/kvm/Makefile.objs
@@ -1 +1 @@
-obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
+obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
new file mode 100644
index 0000000..8897237
--- /dev/null
+++ b/hw/kvm/pv_event.c
@@ -0,0 +1,109 @@
+/*
+ * QEMU KVM support, paravirtual event device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <linux/kvm_para.h>
+#include <asm/kvm_para.h>
+#include <qobject.h>
+#include <qjson.h>
+#include <monitor.h>
+#include <sysemu.h>
+#include <kvm.h>
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
+
+#define PV_EVENT_DRIVER     "kvm_pv_event"
+
+struct pv_event_action {
+    char *panicked_action;
+    int panicked_action_value;
+};
+
+#define DEFINE_PV_EVENT_PROPERTIES(_state, _conf)   \
+    DEFINE_PROP_STRING("panicked_action", _state, _conf.panicked_action)
+
+static void panicked_mon_event(const char *action)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'action': %s }", action);
+    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+    qobject_decref(data);
+}
+
+static void panicked_perform_action(uint32_t panicked_action)
+{
+    switch (panicked_action) {
+    case PANICKED_REPORT:
+        panicked_mon_event("report");
+        break;
+
+    case PANICKED_PAUSE:
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        break;
+
+    case PANICKED_POWEROFF:
+        panicked_mon_event("poweroff");
+        qemu_system_shutdown_request();
+        break;
+    case PANICKED_RESET:
+        panicked_mon_event("reset");
+        qemu_system_reset_request();
+        break;
+    }
+}
+
+static uint64_t supported_event(void)
+{
+    return 1 << KVM_PV_FEATURE_PANICKED;
+}
+
+static void handle_event(int event, struct pv_event_action *conf)
+{
+    if (event == KVM_PV_EVENT_PANICKED) {
+        panicked_perform_action(conf->panicked_action_value);
+    }
+}
+
+static int pv_event_init(struct pv_event_action *conf)
+{
+    if (!conf->panicked_action) {
+        conf->panicked_action_value = PANICKED_REPORT;
+    } else if (strcasecmp(conf->panicked_action, "none") == 0) {
+        conf->panicked_action_value = PANICKED_REPORT;
+    } else if (strcasecmp(conf->panicked_action, "pause") == 0) {
+        conf->panicked_action_value = PANICKED_PAUSE;
+    } else if (strcasecmp(conf->panicked_action, "poweroff") == 0) {
+        conf->panicked_action_value = PANICKED_POWEROFF;
+    } else if (strcasecmp(conf->panicked_action, "reset") == 0) {
+        conf->panicked_action_value = PANICKED_RESET;
+    } else {
+        return -1;
+    }
+
+    return 0;
+}
+
+#if defined(KVM_PV_EVENT_PORT)
+
+#include "pv_ioport.c"
+
+#else
+void kvm_pv_event_init(void *opaque)
+{
+}
+#endif
diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
new file mode 100644
index 0000000..c2ed6b5
--- /dev/null
+++ b/hw/kvm/pv_ioport.c
@@ -0,0 +1,93 @@
+/*
+ * QEMU KVM support, paravirtual I/O port device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/isa.h"
+
+typedef struct {
+    ISADevice dev;
+    struct pv_event_action conf;
+    MemoryRegion ioport;
+} PVIOPortState;
+
+static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
+{
+    return supported_event();
+}
+
+static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
+{
+    PVIOPortState *s = opaque;
+
+    handle_event(val, &s->conf);
+}
+
+static const MemoryRegionOps pv_io_ops = {
+    .read = pv_io_read,
+    .write = pv_io_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static int pv_ioport_initfn(ISADevice *dev)
+{
+    PVIOPortState *s = DO_UPCAST(PVIOPortState, dev, dev);
+
+    if (pv_event_init(&s->conf) < 0)
+        return -1;
+
+    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
+    isa_register_ioport(dev, &s->ioport, KVM_PV_EVENT_PORT);
+
+    return 0;
+}
+
+static Property pv_ioport_properties[] = {
+    DEFINE_PV_EVENT_PROPERTIES(PVIOPortState, conf),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pv_ioport_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+
+    ic->init = pv_ioport_initfn;
+    dc->no_user = 1;
+    dc->props = pv_ioport_properties;
+}
+
+static TypeInfo pv_ioport_info = {
+    .name          = PV_EVENT_DRIVER,
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(PVIOPortState),
+    .class_init    = pv_ioport_class_init,
+};
+
+static void pv_ioport_register_types(void)
+{
+    type_register_static(&pv_ioport_info);
+}
+
+type_init(pv_ioport_register_types)
+
+void kvm_pv_event_init(void *opaque)
+{
+    ISABus *bus = opaque;
+    ISADevice *dev;
+
+    dev = isa_create(bus, PV_EVENT_DRIVER);
+    qdev_init_nofail(&dev->qdev);
+}
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0c0096f..4af8403 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -46,6 +46,9 @@
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
+#ifdef CONFIG_KVM
+#   include <asm/kvm_para.h>
+#endif
 
 #define MAX_IDE_BUS 2
 
@@ -285,6 +288,12 @@ static void pc_init1(MemoryRegion *system_memory,
     if (pci_enabled) {
         pc_pci_device_init(pci_bus);
     }
+
+#ifdef KVM_PV_EVENT_PORT
+    if (kvm_enabled()) {
+        kvm_pv_event_init(isa_bus);
+    }
+#endif
 }
 
 static void pc_init_pci(ram_addr_t ram_size,
diff --git a/kvm.h b/kvm.h
index 2617dd5..598dcbe 100644
--- a/kvm.h
+++ b/kvm.h
@@ -222,4 +222,6 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
 int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
+
+void kvm_pv_event_init(void *opaque);
 #endif
-- 
1.7.1

[PATCH v8 6/6] allower the user to disable pv event support

[Wen Congyang]

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/pc_piix.c    |    6 +++++-
 qemu-config.c   |    4 ++++
 qemu-options.hx |    3 ++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 4af8403..9b877a7 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -151,6 +151,8 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
     void *fw_cfg = NULL;
+    QemuOptsList *list = qemu_find_opts("machine");
+    bool enable_pv_event;
 
     pc_cpus_init(cpu_model);
 
@@ -289,8 +291,10 @@ static void pc_init1(MemoryRegion *system_memory,
         pc_pci_device_init(pci_bus);
     }
 
+    enable_pv_event = qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
+                                        "enable_pv_event", false);
 #ifdef KVM_PV_EVENT_PORT
-    if (kvm_enabled()) {
+    if (kvm_enabled() && enable_pv_event) {
         kvm_pv_event_init(isa_bus);
     }
 #endif
diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..5ec5aa9 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -595,6 +595,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "dt_compatible",
             .type = QEMU_OPT_STRING,
             .help = "Overrides the \"compatible\" property of the dt root node",
+        }, {
+            .name = "enable_pv_event",
+            .type = QEMU_OPT_BOOL,
+            .help = "handle pv event"
         },
         { /* End of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 5e7d0dc..94d7865 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -37,7 +37,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                property accel=accel1[:accel2[:...]] selects accelerator\n"
     "                supported accelerators are kvm, xen, tcg (default: tcg)\n"
     "                kernel_irqchip=on|off controls accelerated irqchip support\n"
-    "                kvm_shadow_mem=size of KVM shadow MMU\n",
+    "                kvm_shadow_mem=size of KVM shadow MMU\n"
+    "                enable_pv_event=on|off controls pv event support\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
-- 
1.7.1

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Andrew Jones]

On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> diff --git a/Documentation/virtual/kvm/pv_event.txt b/Documentation/virtual/kvm/pv_event.txt
> new file mode 100644
> index 0000000..0ebc890
> --- /dev/null
> +++ b/Documentation/virtual/kvm/pv_event.txt
> @@ -0,0 +1,32 @@
> +The KVM paravirtual event interface
> +=================================
> +
> +Initializing the paravirtual event interface
> +======================
> +kvm_pv_event_init()
> +Argiments:
> +	None
> +
> +Return Value:
> +	0 : The guest kernel can't use paravirtual event interface.
> +	-1: The guest kernel can use paravirtual event interface.
> +

This documentation has the can and can't backwards.

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Wen Congyang]

At 08/08/2012 05:12 PM, Andrew Jones Wrote:
> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>> diff --git a/Documentation/virtual/kvm/pv_event.txt b/Documentation/virtual/kvm/pv_event.txt
>> new file mode 100644
>> index 0000000..0ebc890
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/pv_event.txt
>> @@ -0,0 +1,32 @@
>> +The KVM paravirtual event interface
>> +=================================
>> +
>> +Initializing the paravirtual event interface
>> +======================
>> +kvm_pv_event_init()
>> +Argiments:
>> +	None
>> +
>> +Return Value:
>> +	0 : The guest kernel can't use paravirtual event interface.
>> +	-1: The guest kernel can use paravirtual event interface.
>> +
> 
> This documentation has the can and can't backwards.
> 

Yes, I will fix it.

Thanks
Wen Congyang

Re: [Qemu-devel] [PATCH v8 5/6] introduce a new qom device to deal with panicked event

[Blue Swirl]

On Wed, Aug 8, 2012 at 2:47 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
> port KVM_PV_EVENT_PORT when it is panciked. This patch introduces a new
> qom device kvm_pv_ioport to listen this I/O port, and deal with panicked
> event according to panicked_action's value. The possible actions are:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause the guest
> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>
> I/O ports does not work for some targets(for example: s390). And you
> can implement another qom device, and include it's code into pv_event.c
> for such target.
>
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/kvm/Makefile.objs |    2 +-
>  hw/kvm/pv_event.c    |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/kvm/pv_ioport.c   |   93 ++++++++++++++++++++++++++++++++++++++++++
>  hw/pc_piix.c         |    9 ++++
>  kvm.h                |    2 +
>  5 files changed, 214 insertions(+), 1 deletions(-)
>  create mode 100644 hw/kvm/pv_event.c
>  create mode 100644 hw/kvm/pv_ioport.c
>
> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
> index 226497a..23e3b30 100644
> --- a/hw/kvm/Makefile.objs
> +++ b/hw/kvm/Makefile.objs
> @@ -1 +1 @@
> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
> new file mode 100644
> index 0000000..8897237
> --- /dev/null
> +++ b/hw/kvm/pv_event.c
> @@ -0,0 +1,109 @@
> +/*
> + * QEMU KVM support, paravirtual event device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <linux/kvm_para.h>
> +#include <asm/kvm_para.h>
> +#include <qobject.h>
> +#include <qjson.h>
> +#include <monitor.h>
> +#include <sysemu.h>
> +#include <kvm.h>
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
> +
> +#define PV_EVENT_DRIVER     "kvm_pv_event"
> +
> +struct pv_event_action {

PVEventAction

> +    char *panicked_action;
> +    int panicked_action_value;
> +};
> +
> +#define DEFINE_PV_EVENT_PROPERTIES(_state, _conf)   \
> +    DEFINE_PROP_STRING("panicked_action", _state, _conf.panicked_action)
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(uint32_t panicked_action)
> +{
> +    switch (panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_POWEROFF:
> +        panicked_mon_event("poweroff");
> +        qemu_system_shutdown_request();
> +        break;

Misses a line break unlike other cases.

> +    case PANICKED_RESET:
> +        panicked_mon_event("reset");
> +        qemu_system_reset_request();
> +        break;
> +    }
> +}
> +
> +static uint64_t supported_event(void)
> +{
> +    return 1 << KVM_PV_FEATURE_PANICKED;
> +}
> +
> +static void handle_event(int event, struct pv_event_action *conf)
> +{
> +    if (event == KVM_PV_EVENT_PANICKED) {
> +        panicked_perform_action(conf->panicked_action_value);
> +    }
> +}
> +
> +static int pv_event_init(struct pv_event_action *conf)
> +{
> +    if (!conf->panicked_action) {
> +        conf->panicked_action_value = PANICKED_REPORT;
> +    } else if (strcasecmp(conf->panicked_action, "none") == 0) {
> +        conf->panicked_action_value = PANICKED_REPORT;
> +    } else if (strcasecmp(conf->panicked_action, "pause") == 0) {
> +        conf->panicked_action_value = PANICKED_PAUSE;
> +    } else if (strcasecmp(conf->panicked_action, "poweroff") == 0) {
> +        conf->panicked_action_value = PANICKED_POWEROFF;
> +    } else if (strcasecmp(conf->panicked_action, "reset") == 0) {
> +        conf->panicked_action_value = PANICKED_RESET;
> +    } else {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +#if defined(KVM_PV_EVENT_PORT)
> +
> +#include "pv_ioport.c"

I'd rather not include any .c files but insert the contents here directly.

> +
> +#else
> +void kvm_pv_event_init(void *opaque)
> +{
> +}
> +#endif
> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
> new file mode 100644
> index 0000000..c2ed6b5
> --- /dev/null
> +++ b/hw/kvm/pv_ioport.c
> @@ -0,0 +1,93 @@
> +/*
> + * QEMU KVM support, paravirtual I/O port device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/isa.h"
> +
> +typedef struct {
> +    ISADevice dev;
> +    struct pv_event_action conf;
> +    MemoryRegion ioport;
> +} PVIOPortState;
> +
> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
> +{
> +    return supported_event();
> +}
> +
> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
> +                        unsigned size)
> +{
> +    PVIOPortState *s = opaque;
> +
> +    handle_event(val, &s->conf);
> +}
> +
> +static const MemoryRegionOps pv_io_ops = {
> +    .read = pv_io_read,
> +    .write = pv_io_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static int pv_ioport_initfn(ISADevice *dev)
> +{
> +    PVIOPortState *s = DO_UPCAST(PVIOPortState, dev, dev);
> +
> +    if (pv_event_init(&s->conf) < 0)
> +        return -1;

Mandatory braces missing.

> +
> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
> +    isa_register_ioport(dev, &s->ioport, KVM_PV_EVENT_PORT);
> +
> +    return 0;
> +}
> +
> +static Property pv_ioport_properties[] = {
> +    DEFINE_PV_EVENT_PROPERTIES(PVIOPortState, conf),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +
> +    ic->init = pv_ioport_initfn;
> +    dc->no_user = 1;
> +    dc->props = pv_ioport_properties;
> +}
> +
> +static TypeInfo pv_ioport_info = {
> +    .name          = PV_EVENT_DRIVER,
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(PVIOPortState),
> +    .class_init    = pv_ioport_class_init,
> +};
> +
> +static void pv_ioport_register_types(void)
> +{
> +    type_register_static(&pv_ioport_info);
> +}
> +
> +type_init(pv_ioport_register_types)
> +
> +void kvm_pv_event_init(void *opaque)
> +{
> +    ISABus *bus = opaque;
> +    ISADevice *dev;
> +
> +    dev = isa_create(bus, PV_EVENT_DRIVER);
> +    qdev_init_nofail(&dev->qdev);
> +}
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 0c0096f..4af8403 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -46,6 +46,9 @@
>  #ifdef CONFIG_XEN
>  #  include <xen/hvm/hvm_info_table.h>
>  #endif
> +#ifdef CONFIG_KVM
> +#   include <asm/kvm_para.h>
> +#endif

I'd remove this and the #ifdeffery below since a stub function is
provided. This is not performance critical.

>
>  #define MAX_IDE_BUS 2
>
> @@ -285,6 +288,12 @@ static void pc_init1(MemoryRegion *system_memory,
>      if (pci_enabled) {
>          pc_pci_device_init(pci_bus);
>      }
> +
> +#ifdef KVM_PV_EVENT_PORT
> +    if (kvm_enabled()) {
> +        kvm_pv_event_init(isa_bus);
> +    }
> +#endif
>  }
>
>  static void pc_init_pci(ram_addr_t ram_size,
> diff --git a/kvm.h b/kvm.h
> index 2617dd5..598dcbe 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -222,4 +222,6 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
>  int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
>  int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
> +
> +void kvm_pv_event_init(void *opaque);
>  #endif
> --
> 1.7.1
>
>

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> We can know the guest is panicked when the guest runs on xen.
> But we do not have such feature on kvm.
> 
> Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked.
> 
> We have three solutions to implement this feature:
> 1. use vmcall
> 2. use I/O port
> 3. use virtio-serial.
> 
> We have decided to avoid touching hypervisor. The reason why I choose
> choose the I/O port is:
> 1. it is easier to implememt
> 2. it does not depend any virtual device
> 3. it can work when starting the kernel

How about searching for the "Kernel panic - not syncing" string 
in the guests serial output? Say libvirtd could take an action upon
that?

Advantages:
- It works for all architectures.
- It does not depend on any virtual device.
- It works as early as serial console output does (panics before
that should be rare).
- It allows you to see why the guest panicked.

> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  Documentation/virtual/kvm/pv_event.txt |   32 ++++++++++++++++++++++++++++++++
>  arch/ia64/include/asm/kvm_para.h       |   14 ++++++++++++++
>  arch/powerpc/include/asm/kvm_para.h    |   14 ++++++++++++++
>  arch/s390/include/asm/kvm_para.h       |   14 ++++++++++++++
>  arch/x86/include/asm/kvm_para.h        |   27 +++++++++++++++++++++++++++
>  arch/x86/kernel/kvm.c                  |   25 +++++++++++++++++++++++++
>  include/linux/kvm_para.h               |   23 +++++++++++++++++++++++
>  7 files changed, 149 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/virtual/kvm/pv_event.txt
> 
> diff --git a/Documentation/virtual/kvm/pv_event.txt b/Documentation/virtual/kvm/pv_event.txt
> new file mode 100644
> index 0000000..0ebc890
> --- /dev/null
> +++ b/Documentation/virtual/kvm/pv_event.txt
> @@ -0,0 +1,32 @@
> +The KVM paravirtual event interface
> +=================================
> +
> +Initializing the paravirtual event interface
> +======================
> +kvm_pv_event_init()
> +Argiments:
> +	None
> +
> +Return Value:
> +	0 : The guest kernel can't use paravirtual event interface.
> +	-1: The guest kernel can use paravirtual event interface.
> +
> +Querying whether the event can be ejected
> +======================
> +kvm_pv_has_feature()
> +Arguments:
> +	feature: The bit value of this paravirtual event to query
> +
> +Return Value:
> +	0: The guest kernel can't eject this paravirtual event.
> +	1: The guest kernel can eject this paravirtual event.
> +
> +
> +Ejecting paravirtual event
> +======================
> +kvm_pv_eject_event()
> +Arguments:
> +	event: The event to be ejected.
> +
> +Return Value:
> +	None
> diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
> index 2019cb9..b5ec658 100644
> --- a/arch/ia64/include/asm/kvm_para.h
> +++ b/arch/ia64/include/asm/kvm_para.h
> @@ -31,6 +31,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
>  	return false;
>  }
>  
> +static inline int kvm_arch_pv_event_init(void)
> +{
> +	return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
>  #endif
>  
>  #endif
> diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
> index c18916b..01b98c7 100644
> --- a/arch/powerpc/include/asm/kvm_para.h
> +++ b/arch/powerpc/include/asm/kvm_para.h
> @@ -211,6 +211,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
>  	return false;
>  }
>  
> +static inline int kvm_arch_pv_event_init(void)
> +{
> +	return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* __POWERPC_KVM_PARA_H__ */
> diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
> index da44867..00ce058 100644
> --- a/arch/s390/include/asm/kvm_para.h
> +++ b/arch/s390/include/asm/kvm_para.h
> @@ -154,6 +154,20 @@ static inline bool kvm_check_and_clear_guest_paused(void)
>  	return false;
>  }
>  
> +static inline int kvm_arch_pv_event_init(void)
> +{
> +	return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
>  #endif
>  
>  #endif /* __S390_KVM_PARA_H */
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 2f7712e..7d297f0 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -96,8 +96,11 @@ struct kvm_vcpu_pv_apf_data {
>  #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
>  #define KVM_PV_EOI_DISABLED 0x0
>  
> +#define KVM_PV_EVENT_PORT	(0x505UL)
> +
>  #ifdef __KERNEL__
>  #include <asm/processor.h>
> +#include <linux/ioport.h>
>  
>  extern void kvmclock_init(void);
>  extern int kvm_register_clock(char *txt);
> @@ -228,6 +231,30 @@ static inline void kvm_disable_steal_time(void)
>  }
>  #endif
>  
> +static inline int kvm_arch_pv_event_init(void)
> +{
> +	if (!request_region(KVM_PV_EVENT_PORT, 1, "KVM_PV_EVENT"))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	unsigned int features = inl(KVM_PV_EVENT_PORT);
> +
> +	/* Reading from an invalid I/O port will return -1 */
> +	if (features == ~0)
> +		features = 0;
> +
> +	return features;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +	outl(event, KVM_PV_EVENT_PORT);
> +}
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* _ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index c1d61ee..6129459 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -368,6 +368,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
>  	.notifier_call = kvm_pv_reboot_notify,
>  };
>  
> +static int
> +kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
> +{
> +	kvm_pv_eject_event(KVM_PV_EVENT_PANICKED);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block kvm_pv_panic_nb = {
> +	.notifier_call = kvm_pv_panic_notify,
> +};
> +
>  static u64 kvm_steal_clock(int cpu)
>  {
>  	u64 steal;
> @@ -447,6 +458,20 @@ static void __init kvm_apf_trap_init(void)
>  	set_intr_gate(14, &async_page_fault);
>  }
>  
> +static void __init kvm_pv_panicked_event_init(void)
> +{
> +	if (!kvm_para_available())
> +		return;
> +
> +	if (kvm_pv_event_init())
> +		return;
> +
> +	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
> +		atomic_notifier_chain_register(&panic_notifier_list,
> +			&kvm_pv_panic_nb);
> +}
> +arch_initcall(kvm_pv_panicked_event_init);
> +
>  void __init kvm_guest_init(void)
>  {
>  	int i;
> diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
> index ff476dd..8e0fb81 100644
> --- a/include/linux/kvm_para.h
> +++ b/include/linux/kvm_para.h
> @@ -20,6 +20,12 @@
>  #define KVM_HC_FEATURES			3
>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>  
> +/* The bit of supported pv event */
> +#define KVM_PV_FEATURE_PANICKED	0
> +
> +/* The pv event value */
> +#define KVM_PV_EVENT_PANICKED	1
> +
>  /*
>   * hypercalls use architecture specific
>   */
> @@ -33,5 +39,22 @@ static inline int kvm_para_has_feature(unsigned int feature)
>  		return 1;
>  	return 0;
>  }
> +
> +static inline int kvm_pv_event_init(void)
> +{
> +	return kvm_arch_pv_event_init();
> +}
> +
> +static inline int kvm_pv_has_feature(unsigned int feature)
> +{
> +	if (kvm_arch_pv_features() & (1UL << feature))
> +		return 1;
> +	return 0;
> +}
> +
> +static inline void kvm_pv_eject_event(unsigned int event)
> +{
> +	kvm_arch_pv_eject_event(event);
> +}
>  #endif /* __KERNEL__ */
>  #endif /* __LINUX_KVM_PARA_H */
> -- 
> 1.7.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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Eric Blake]

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

On 08/13/2012 12:21 PM, Marcelo Tosatti wrote:
> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>> We can know the guest is panicked when the guest runs on xen.
>> But we do not have such feature on kvm.
>>
>> Another purpose of this feature is: management app(for example:
>> libvirt) can do auto dump when the guest is panicked. If management
>> app does not do auto dump, the guest's user can do dump by hand if
>> he sees the guest is panicked.
>>
>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
>>
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when starting the kernel
> 
> How about searching for the "Kernel panic - not syncing" string 
> in the guests serial output? Say libvirtd could take an action upon
> that?
> 
> Advantages:
> - It works for all architectures.
> - It does not depend on any virtual device.

But it _does_ depend on a serial console, and furthermore requires
libvirt to tee the serial console (right now, libvirt can treat the
console as an opaque pass-through to the end user, but if you expect
libvirt to parse the serial console for a particular string, you've lost
some efficiency).

> - It works as early as serial console output does (panics before
> that should be rare).
> - It allows you to see why the guest panicked.

I think your arguments for a serial console have already been made and
refuted in earlier versions of this patch series, which is WHY this
series is still applicable.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Mon, Aug 13, 2012 at 01:48:39PM -0600, Eric Blake wrote:
> On 08/13/2012 12:21 PM, Marcelo Tosatti wrote:
> > On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> >> We can know the guest is panicked when the guest runs on xen.
> >> But we do not have such feature on kvm.
> >>
> >> Another purpose of this feature is: management app(for example:
> >> libvirt) can do auto dump when the guest is panicked. If management
> >> app does not do auto dump, the guest's user can do dump by hand if
> >> he sees the guest is panicked.
> >>
> >> We have three solutions to implement this feature:
> >> 1. use vmcall
> >> 2. use I/O port
> >> 3. use virtio-serial.
> >>
> >> We have decided to avoid touching hypervisor. The reason why I choose
> >> choose the I/O port is:
> >> 1. it is easier to implememt
> >> 2. it does not depend any virtual device
> >> 3. it can work when starting the kernel
> > 
> > How about searching for the "Kernel panic - not syncing" string 
> > in the guests serial output? Say libvirtd could take an action upon
> > that?
> > 
> > Advantages:
> > - It works for all architectures.
> > - It does not depend on any virtual device.
> 
> But it _does_ depend on a serial console,

Which already exists and is supported.

>  and furthermore requires
> libvirt to tee the serial console (right now, libvirt can treat the
> console as an opaque pass-through to the end user, but if you expect
> libvirt to parse the serial console for a particular string, you've lost
> some efficiency).
> 
> > - It works as early as serial console output does (panics before
> > that should be rare).
> > - It allows you to see why the guest panicked.
> 
> I think your arguments for a serial console have already been made and
> refuted in earlier versions of this patch series, which is WHY this
> series is still applicable.

Refuted why, exactly? Efficiency to parse serial console output in
libvirt should not be a major issue surely?

Either way:

The device should be arch independent, as "panic" is not specific 
to a particular architecture. For example virtio which would also work
on S390.

Custom IO port == virtual device, so that depends on virtual
device already.

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Gleb Natapov]

On Mon, Aug 13, 2012 at 05:24:52PM -0300, Marcelo Tosatti wrote:
> On Mon, Aug 13, 2012 at 01:48:39PM -0600, Eric Blake wrote:
> > On 08/13/2012 12:21 PM, Marcelo Tosatti wrote:
> > > On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> > >> We can know the guest is panicked when the guest runs on xen.
> > >> But we do not have such feature on kvm.
> > >>
> > >> Another purpose of this feature is: management app(for example:
> > >> libvirt) can do auto dump when the guest is panicked. If management
> > >> app does not do auto dump, the guest's user can do dump by hand if
> > >> he sees the guest is panicked.
> > >>
> > >> We have three solutions to implement this feature:
> > >> 1. use vmcall
> > >> 2. use I/O port
> > >> 3. use virtio-serial.
> > >>
> > >> We have decided to avoid touching hypervisor. The reason why I choose
> > >> choose the I/O port is:
> > >> 1. it is easier to implememt
> > >> 2. it does not depend any virtual device
> > >> 3. it can work when starting the kernel
> > > 
> > > How about searching for the "Kernel panic - not syncing" string 
> > > in the guests serial output? Say libvirtd could take an action upon
> > > that?
> > > 
> > > Advantages:
> > > - It works for all architectures.
> > > - It does not depend on any virtual device.
> > 
> > But it _does_ depend on a serial console,
> 
> Which already exists and is supported.
> 
> >  and furthermore requires
> > libvirt to tee the serial console (right now, libvirt can treat the
> > console as an opaque pass-through to the end user, but if you expect
> > libvirt to parse the serial console for a particular string, you've lost
> > some efficiency).
> > 
> > > - It works as early as serial console output does (panics before
> > > that should be rare).
> > > - It allows you to see why the guest panicked.
> > 
> > I think your arguments for a serial console have already been made and
> > refuted in earlier versions of this patch series, which is WHY this
> > series is still applicable.
> 
> Refuted why, exactly? Efficiency to parse serial console output in
> libvirt should not be a major issue surely?
> 
It is not zero config (guests do not send console output to serial by
default). If vm users want to use serial for its working console panic
notification will trigger every time user examines dmesg with "Kernel
panic - not syncing" in it.

--
			Gleb.

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Daniel P. Berrange]

On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> > We can know the guest is panicked when the guest runs on xen.
> > But we do not have such feature on kvm.
> > 
> > Another purpose of this feature is: management app(for example:
> > libvirt) can do auto dump when the guest is panicked. If management
> > app does not do auto dump, the guest's user can do dump by hand if
> > he sees the guest is panicked.
> > 
> > We have three solutions to implement this feature:
> > 1. use vmcall
> > 2. use I/O port
> > 3. use virtio-serial.
> > 
> > We have decided to avoid touching hypervisor. The reason why I choose
> > choose the I/O port is:
> > 1. it is easier to implememt
> > 2. it does not depend any virtual device
> > 3. it can work when starting the kernel
> 
> How about searching for the "Kernel panic - not syncing" string 
> in the guests serial output? Say libvirtd could take an action upon
> that?

No, this is not satisfactory. It depends on the guest OS being
configured to use the serial port for console output which we
cannot mandate, since it may well be required for other purposes.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Jan Kiszka]

On 2012-08-14 10:56, Daniel P. Berrange wrote:
> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>>> We can know the guest is panicked when the guest runs on xen.
>>> But we do not have such feature on kvm.
>>>
>>> Another purpose of this feature is: management app(for example:
>>> libvirt) can do auto dump when the guest is panicked. If management
>>> app does not do auto dump, the guest's user can do dump by hand if
>>> he sees the guest is panicked.
>>>
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when starting the kernel
>>
>> How about searching for the "Kernel panic - not syncing" string 
>> in the guests serial output? Say libvirtd could take an action upon
>> that?
> 
> No, this is not satisfactory. It depends on the guest OS being
> configured to use the serial port for console output which we
> cannot mandate, since it may well be required for other purposes.

Well, we have more than a single serial port, even when leaving
virtio-serial aside...

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Yan Vugenfirer]

On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:

> On 2012-08-14 10:56, Daniel P. Berrange wrote:
>> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>>>> We can know the guest is panicked when the guest runs on xen.
>>>> But we do not have such feature on kvm.
>>>> 
>>>> Another purpose of this feature is: management app(for example:
>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>> he sees the guest is panicked.
>>>> 
>>>> We have three solutions to implement this feature:
>>>> 1. use vmcall
>>>> 2. use I/O port
>>>> 3. use virtio-serial.
>>>> 
>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>> choose the I/O port is:
>>>> 1. it is easier to implememt
>>>> 2. it does not depend any virtual device
>>>> 3. it can work when starting the kernel
>>> 
>>> How about searching for the "Kernel panic - not syncing" string 
>>> in the guests serial output? Say libvirtd could take an action upon
>>> that?
>> 
>> No, this is not satisfactory. It depends on the guest OS being
>> configured to use the serial port for console output which we
>> cannot mandate, since it may well be required for other purposes.
> 
Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)

What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.

Yan. 


> Well, we have more than a single serial port, even when leaving
> virtio-serial aside...
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
> --
> 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

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Jan Kiszka]

On 2012-08-14 16:55, Yan Vugenfirer wrote:
> 
> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
> 
>> On 2012-08-14 10:56, Daniel P. Berrange wrote:
>>> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>>>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>>>>> We can know the guest is panicked when the guest runs on xen.
>>>>> But we do not have such feature on kvm.
>>>>>
>>>>> Another purpose of this feature is: management app(for example:
>>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>>> he sees the guest is panicked.
>>>>>
>>>>> We have three solutions to implement this feature:
>>>>> 1. use vmcall
>>>>> 2. use I/O port
>>>>> 3. use virtio-serial.
>>>>>
>>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>>> choose the I/O port is:
>>>>> 1. it is easier to implememt
>>>>> 2. it does not depend any virtual device
>>>>> 3. it can work when starting the kernel
>>>>
>>>> How about searching for the "Kernel panic - not syncing" string 
>>>> in the guests serial output? Say libvirtd could take an action upon
>>>> that?
>>>
>>> No, this is not satisfactory. It depends on the guest OS being
>>> configured to use the serial port for console output which we
>>> cannot mandate, since it may well be required for other purposes.
>>
> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
> 
> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.

What prevents writing the magic words to a second serial port in the
same way via that callback?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Tue, Aug 14, 2012 at 10:47:48AM +0300, Gleb Natapov wrote:
> On Mon, Aug 13, 2012 at 05:24:52PM -0300, Marcelo Tosatti wrote:
> > On Mon, Aug 13, 2012 at 01:48:39PM -0600, Eric Blake wrote:
> > > On 08/13/2012 12:21 PM, Marcelo Tosatti wrote:
> > > > On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> > > >> We can know the guest is panicked when the guest runs on xen.
> > > >> But we do not have such feature on kvm.
> > > >>
> > > >> Another purpose of this feature is: management app(for example:
> > > >> libvirt) can do auto dump when the guest is panicked. If management
> > > >> app does not do auto dump, the guest's user can do dump by hand if
> > > >> he sees the guest is panicked.
> > > >>
> > > >> We have three solutions to implement this feature:
> > > >> 1. use vmcall
> > > >> 2. use I/O port
> > > >> 3. use virtio-serial.
> > > >>
> > > >> We have decided to avoid touching hypervisor. The reason why I choose
> > > >> choose the I/O port is:
> > > >> 1. it is easier to implememt
> > > >> 2. it does not depend any virtual device
> > > >> 3. it can work when starting the kernel
> > > > 
> > > > How about searching for the "Kernel panic - not syncing" string 
> > > > in the guests serial output? Say libvirtd could take an action upon
> > > > that?
> > > > 
> > > > Advantages:
> > > > - It works for all architectures.
> > > > - It does not depend on any virtual device.
> > > 
> > > But it _does_ depend on a serial console,
> > 
> > Which already exists and is supported.
> > 
> > >  and furthermore requires
> > > libvirt to tee the serial console (right now, libvirt can treat the
> > > console as an opaque pass-through to the end user, but if you expect
> > > libvirt to parse the serial console for a particular string, you've lost
> > > some efficiency).
> > > 
> > > > - It works as early as serial console output does (panics before
> > > > that should be rare).
> > > > - It allows you to see why the guest panicked.
> > > 
> > > I think your arguments for a serial console have already been made and
> > > refuted in earlier versions of this patch series, which is WHY this
> > > series is still applicable.
> > 
> > Refuted why, exactly? Efficiency to parse serial console output in
> > libvirt should not be a major issue surely?
> > 
> It is not zero config (guests do not send console output to serial by
> default). If vm users want to use serial for its working console panic
> notification will trigger every time user examines dmesg with "Kernel
> panic - not syncing" in it.

Ok, then it would have to be a dedicated serial console which starts
to become funny.

Use a simple virtio device, then, it starts early enough (or can be made
to) during kernel init for most relevant production panics, and works
for all architectures.

Re: [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
> 
> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
> 
> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> >>>> We can know the guest is panicked when the guest runs on xen.
> >>>> But we do not have such feature on kvm.
> >>>> 
> >>>> Another purpose of this feature is: management app(for example:
> >>>> libvirt) can do auto dump when the guest is panicked. If management
> >>>> app does not do auto dump, the guest's user can do dump by hand if
> >>>> he sees the guest is panicked.
> >>>> 
> >>>> We have three solutions to implement this feature:
> >>>> 1. use vmcall
> >>>> 2. use I/O port
> >>>> 3. use virtio-serial.
> >>>> 
> >>>> We have decided to avoid touching hypervisor. The reason why I choose
> >>>> choose the I/O port is:
> >>>> 1. it is easier to implememt
> >>>> 2. it does not depend any virtual device
> >>>> 3. it can work when starting the kernel
> >>> 
> >>> How about searching for the "Kernel panic - not syncing" string 
> >>> in the guests serial output? Say libvirtd could take an action upon
> >>> that?
> >> 
> >> No, this is not satisfactory. It depends on the guest OS being
> >> configured to use the serial port for console output which we
> >> cannot mandate, since it may well be required for other purposes.
> > 
> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
> 
> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
> 
> Yan. 

Considering whether a "panic-device" should cover other OSes is also 
something to consider. Even for Linux, is "panic" the only case which
should be reported via the mechanism? What about oopses without panic? 

Is the mechanism general enough for supporting new events, etc.

> 
> > Well, we have more than a single serial port, even when leaving
> > virtio-serial aside...
> > 
> > Jan
> > 
> > -- 
> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> > Corporate Competence Center Embedded Linux
> > --
> > 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Gleb Natapov]

On Tue, Aug 14, 2012 at 12:29:38PM -0300, Marcelo Tosatti wrote:
> On Tue, Aug 14, 2012 at 10:47:48AM +0300, Gleb Natapov wrote:
> > On Mon, Aug 13, 2012 at 05:24:52PM -0300, Marcelo Tosatti wrote:
> > > On Mon, Aug 13, 2012 at 01:48:39PM -0600, Eric Blake wrote:
> > > > On 08/13/2012 12:21 PM, Marcelo Tosatti wrote:
> > > > > On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> > > > >> We can know the guest is panicked when the guest runs on xen.
> > > > >> But we do not have such feature on kvm.
> > > > >>
> > > > >> Another purpose of this feature is: management app(for example:
> > > > >> libvirt) can do auto dump when the guest is panicked. If management
> > > > >> app does not do auto dump, the guest's user can do dump by hand if
> > > > >> he sees the guest is panicked.
> > > > >>
> > > > >> We have three solutions to implement this feature:
> > > > >> 1. use vmcall
> > > > >> 2. use I/O port
> > > > >> 3. use virtio-serial.
> > > > >>
> > > > >> We have decided to avoid touching hypervisor. The reason why I choose
> > > > >> choose the I/O port is:
> > > > >> 1. it is easier to implememt
> > > > >> 2. it does not depend any virtual device
> > > > >> 3. it can work when starting the kernel
> > > > > 
> > > > > How about searching for the "Kernel panic - not syncing" string 
> > > > > in the guests serial output? Say libvirtd could take an action upon
> > > > > that?
> > > > > 
> > > > > Advantages:
> > > > > - It works for all architectures.
> > > > > - It does not depend on any virtual device.
> > > > 
> > > > But it _does_ depend on a serial console,
> > > 
> > > Which already exists and is supported.
> > > 
> > > >  and furthermore requires
> > > > libvirt to tee the serial console (right now, libvirt can treat the
> > > > console as an opaque pass-through to the end user, but if you expect
> > > > libvirt to parse the serial console for a particular string, you've lost
> > > > some efficiency).
> > > > 
> > > > > - It works as early as serial console output does (panics before
> > > > > that should be rare).
> > > > > - It allows you to see why the guest panicked.
> > > > 
> > > > I think your arguments for a serial console have already been made and
> > > > refuted in earlier versions of this patch series, which is WHY this
> > > > series is still applicable.
> > > 
> > > Refuted why, exactly? Efficiency to parse serial console output in
> > > libvirt should not be a major issue surely?
> > > 
> > It is not zero config (guests do not send console output to serial by
> > default). If vm users want to use serial for its working console panic
> > notification will trigger every time user examines dmesg with "Kernel
> > panic - not syncing" in it.
> 
> Ok, then it would have to be a dedicated serial console which starts
> to become funny.
> 
We do have support for many virtio-serial channels.

> Use a simple virtio device, then, it starts early enough (or can be made
> to) during kernel init for most relevant production panics, and works
> for all architectures.
The only downside of using dedicated virtio-serial channel that I can
see is that to catch early panic all of the virtio should be compiled
in.

--
			Gleb.

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Anthony Liguori]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
>> 
>> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
>> 
>> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
>> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>> >>>> We can know the guest is panicked when the guest runs on xen.
>> >>>> But we do not have such feature on kvm.
>> >>>> 
>> >>>> Another purpose of this feature is: management app(for example:
>> >>>> libvirt) can do auto dump when the guest is panicked. If management
>> >>>> app does not do auto dump, the guest's user can do dump by hand if
>> >>>> he sees the guest is panicked.
>> >>>> 
>> >>>> We have three solutions to implement this feature:
>> >>>> 1. use vmcall
>> >>>> 2. use I/O port
>> >>>> 3. use virtio-serial.
>> >>>> 
>> >>>> We have decided to avoid touching hypervisor. The reason why I choose
>> >>>> choose the I/O port is:
>> >>>> 1. it is easier to implememt
>> >>>> 2. it does not depend any virtual device
>> >>>> 3. it can work when starting the kernel
>> >>> 
>> >>> How about searching for the "Kernel panic - not syncing" string 
>> >>> in the guests serial output? Say libvirtd could take an action upon
>> >>> that?
>> >> 
>> >> No, this is not satisfactory. It depends on the guest OS being
>> >> configured to use the serial port for console output which we
>> >> cannot mandate, since it may well be required for other purposes.
>> > 
>> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
>> 
>> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
>> 
>> Yan. 
>
> Considering whether a "panic-device" should cover other OSes is also 
> something to consider. Even for Linux, is "panic" the only case which
> should be reported via the mechanism? What about oopses without panic? 
>
> Is the mechanism general enough for supporting new events, etc.

Hi,

I think this discussion is gone of the deep end.

Forget about !x86 platforms.  They have their own way to do this sort of
thing.  Think of this feature like a status LED on a motherboard.  These
are very common and usually controlled by IO ports.

We're simply reserving a "status LED" for the guest to indicate that it
has paniced.  Let's not over engineer this.

Regards,

Anthony Liguori

>
>> 
>> > Well, we have more than a single serial port, even when leaving
>> > virtio-serial aside...
>> > 
>> > Jan
>> > 
>> > -- 
>> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
>> > Corporate Competence Center Embedded Linux
>> > --
>> > 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
> Marcelo Tosatti <mtosatti@redhat.com> writes:
> 
> > On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
> >> 
> >> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
> >> 
> >> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
> >> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
> >> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> >> >>>> We can know the guest is panicked when the guest runs on xen.
> >> >>>> But we do not have such feature on kvm.
> >> >>>> 
> >> >>>> Another purpose of this feature is: management app(for example:
> >> >>>> libvirt) can do auto dump when the guest is panicked. If management
> >> >>>> app does not do auto dump, the guest's user can do dump by hand if
> >> >>>> he sees the guest is panicked.
> >> >>>> 
> >> >>>> We have three solutions to implement this feature:
> >> >>>> 1. use vmcall
> >> >>>> 2. use I/O port
> >> >>>> 3. use virtio-serial.
> >> >>>> 
> >> >>>> We have decided to avoid touching hypervisor. The reason why I choose
> >> >>>> choose the I/O port is:
> >> >>>> 1. it is easier to implememt
> >> >>>> 2. it does not depend any virtual device
> >> >>>> 3. it can work when starting the kernel
> >> >>> 
> >> >>> How about searching for the "Kernel panic - not syncing" string 
> >> >>> in the guests serial output? Say libvirtd could take an action upon
> >> >>> that?
> >> >> 
> >> >> No, this is not satisfactory. It depends on the guest OS being
> >> >> configured to use the serial port for console output which we
> >> >> cannot mandate, since it may well be required for other purposes.
> >> > 
> >> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
> >> 
> >> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
> >> 
> >> Yan. 
> >
> > Considering whether a "panic-device" should cover other OSes is also \

> > something to consider. Even for Linux, is "panic" the only case which
> > should be reported via the mechanism? What about oopses without panic? 
> >
> > Is the mechanism general enough for supporting new events, etc.
> 
> Hi,
> 
> I think this discussion is gone of the deep end.
> 
> Forget about !x86 platforms.  They have their own way to do this sort of
> thing.  

The panic function in kernel/panic.c has the following options, which
appear to be arch independent, on panic:

- reboot 
- blink

None are paravirtual interfaces however.

> Think of this feature like a status LED on a motherboard.  These
> are very common and usually controlled by IO ports.
> 
> We're simply reserving a "status LED" for the guest to indicate that it
> has paniced.  Let's not over engineer this.

My concern is that you end up with state that is dependant on x86.

Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED

Having the ability to stop/restart the guest (and even introducing a 
new VM runstate) is more than a status LED analogy.

Can this new infrastructure be used by other architectures?

Do you consider allowing support for Windows as overengineering?

> Regards,
> 
> Anthony Liguori
> 
> >
> >> 
> >> > Well, we have more than a single serial port, even when leaving
> >> > virtio-serial aside...
> >> > 
> >> > Jan
> >> > 
> >> > -- 
> >> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> >> > Corporate Competence Center Embedded Linux
> >> > --
> >> > 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Anthony Liguori]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>> 
>> > On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
>> >> 
>> >> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
>> >> 
>> >> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
>> >> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>> >> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>> >> >>>> We can know the guest is panicked when the guest runs on xen.
>> >> >>>> But we do not have such feature on kvm.
>> >> >>>> 
>> >> >>>> Another purpose of this feature is: management app(for example:
>> >> >>>> libvirt) can do auto dump when the guest is panicked. If management
>> >> >>>> app does not do auto dump, the guest's user can do dump by hand if
>> >> >>>> he sees the guest is panicked.
>> >> >>>> 
>> >> >>>> We have three solutions to implement this feature:
>> >> >>>> 1. use vmcall
>> >> >>>> 2. use I/O port
>> >> >>>> 3. use virtio-serial.
>> >> >>>> 
>> >> >>>> We have decided to avoid touching hypervisor. The reason why I choose
>> >> >>>> choose the I/O port is:
>> >> >>>> 1. it is easier to implememt
>> >> >>>> 2. it does not depend any virtual device
>> >> >>>> 3. it can work when starting the kernel
>> >> >>> 
>> >> >>> How about searching for the "Kernel panic - not syncing" string 
>> >> >>> in the guests serial output? Say libvirtd could take an action upon
>> >> >>> that?
>> >> >> 
>> >> >> No, this is not satisfactory. It depends on the guest OS being
>> >> >> configured to use the serial port for console output which we
>> >> >> cannot mandate, since it may well be required for other purposes.
>> >> > 
>> >> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
>> >> 
>> >> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
>> >> 
>> >> Yan. 
>> >
>> > Considering whether a "panic-device" should cover other OSes is also \
>
>> > something to consider. Even for Linux, is "panic" the only case which
>> > should be reported via the mechanism? What about oopses without panic? 
>> >
>> > Is the mechanism general enough for supporting new events, etc.
>> 
>> Hi,
>> 
>> I think this discussion is gone of the deep end.
>> 
>> Forget about !x86 platforms.  They have their own way to do this sort of
>> thing.  
>
> The panic function in kernel/panic.c has the following options, which
> appear to be arch independent, on panic:
>
> - reboot 
> - blink

Not sure the semantics of blink but that might be a good place for a
pvops hook.

>
> None are paravirtual interfaces however.
>
>> Think of this feature like a status LED on a motherboard.  These
>> are very common and usually controlled by IO ports.
>> 
>> We're simply reserving a "status LED" for the guest to indicate that it
>> has paniced.  Let's not over engineer this.
>
> My concern is that you end up with state that is dependant on x86.
>
> Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
>
> Having the ability to stop/restart the guest (and even introducing a 
> new VM runstate) is more than a status LED analogy.

I must admit, I don't know why a new runstate is necessary/useful.  The
kernel shouldn't have to care about the difference between a halted guest
and a panicked guest.  That level of information belongs in userspace IMHO.

> Can this new infrastructure be used by other architectures?

I guess I don't understand why the kernel side of this isn't anything
more than a paravirt op hook that does a single outb() with the
remaining logic handled 100% in QEMU.

> Do you consider allowing support for Windows as overengineering?

I don't think there is a way to hook BSOD on Windows so attempting to
engineer something that works with Windows seems odd, no?

Regards,

Anthony Liguori

>
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> >> 
>> >> > Well, we have more than a single serial port, even when leaving
>> >> > virtio-serial aside...
>> >> > 
>> >> > Jan
>> >> > 
>> >> > -- 
>> >> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
>> >> > Corporate Competence Center Embedded Linux
>> >> > --
>> >> > 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Peter Maydell]

On 14 August 2012 19:53, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Forget about !x86 platforms.  They have their own way to do this sort of
> thing.  Think of this feature like a status LED on a motherboard.  These
> are very common and usually controlled by IO ports.

Please don't forget !x86 platforms, we are cute and loveable really :-)

> We're simply reserving a "status LED" for the guest to indicate that it
> has paniced.  Let's not over engineer this.

...not that QEMU actually has any kind of "front panel lights and switches"
interface at all, it might be nice to have one. I bet a lot of the embedded
boards have function DIP switches, heartbeat LEDs, etc etc...

-- PMM

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Marcelo Tosatti]

On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
> Marcelo Tosatti <mtosatti@redhat.com> writes:
> 
> > On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
> >> Marcelo Tosatti <mtosatti@redhat.com> writes:
> >> 
> >> > On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
> >> >> 
> >> >> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
> >> >> 
> >> >> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
> >> >> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
> >> >> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> >> >> >>>> We can know the guest is panicked when the guest runs on xen.
> >> >> >>>> But we do not have such feature on kvm.
> >> >> >>>> 
> >> >> >>>> Another purpose of this feature is: management app(for example:
> >> >> >>>> libvirt) can do auto dump when the guest is panicked. If management
> >> >> >>>> app does not do auto dump, the guest's user can do dump by hand if
> >> >> >>>> he sees the guest is panicked.
> >> >> >>>> 
> >> >> >>>> We have three solutions to implement this feature:
> >> >> >>>> 1. use vmcall
> >> >> >>>> 2. use I/O port
> >> >> >>>> 3. use virtio-serial.
> >> >> >>>> 
> >> >> >>>> We have decided to avoid touching hypervisor. The reason why I choose
> >> >> >>>> choose the I/O port is:
> >> >> >>>> 1. it is easier to implememt
> >> >> >>>> 2. it does not depend any virtual device
> >> >> >>>> 3. it can work when starting the kernel
> >> >> >>> 
> >> >> >>> How about searching for the "Kernel panic - not syncing" string 
> >> >> >>> in the guests serial output? Say libvirtd could take an action upon
> >> >> >>> that?
> >> >> >> 
> >> >> >> No, this is not satisfactory. It depends on the guest OS being
> >> >> >> configured to use the serial port for console output which we
> >> >> >> cannot mandate, since it may well be required for other purposes.
> >> >> > 
> >> >> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
> >> >> 
> >> >> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
> >> >> 
> >> >> Yan. 
> >> >
> >> > Considering whether a "panic-device" should cover other OSes is also \
> >
> >> > something to consider. Even for Linux, is "panic" the only case which
> >> > should be reported via the mechanism? What about oopses without panic? 
> >> >
> >> > Is the mechanism general enough for supporting new events, etc.
> >> 
> >> Hi,
> >> 
> >> I think this discussion is gone of the deep end.
> >> 
> >> Forget about !x86 platforms.  They have their own way to do this sort of
> >> thing.  
> >
> > The panic function in kernel/panic.c has the following options, which
> > appear to be arch independent, on panic:
> >
> > - reboot 
> > - blink
> 
> Not sure the semantics of blink but that might be a good place for a
> pvops hook.
> 
> >
> > None are paravirtual interfaces however.
> >
> >> Think of this feature like a status LED on a motherboard.  These
> >> are very common and usually controlled by IO ports.
> >> 
> >> We're simply reserving a "status LED" for the guest to indicate that it
> >> has paniced.  Let's not over engineer this.
> >
> > My concern is that you end up with state that is dependant on x86.
> >
> > Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
> >
> > Having the ability to stop/restart the guest (and even introducing a 
> > new VM runstate) is more than a status LED analogy.
> 
> I must admit, I don't know why a new runstate is necessary/useful.  The
> kernel shouldn't have to care about the difference between a halted guest
> and a panicked guest.  That level of information belongs in userspace IMHO.
> 
> > Can this new infrastructure be used by other architectures?
> 
> I guess I don't understand why the kernel side of this isn't anything
> more than a paravirt op hook that does a single outb() with the
> remaining logic handled 100% in QEMU.

>From the patch description:

"Another purpose of this feature is: management app(for example:
libvirt) can do auto dump when the guest is panicked. If management
app does not do auto dump, the guest's user can do dump by hand if
he sees the guest is panicked."

Wen, auto dump means dump of guest memory?

In that case, the notification should obviously stop the guest 
otherwise the guest might be reset by the time memdump from QEMU 
monitor runs.

But kexec supports dumping of memory already (i suppose it can 
do automatic dump+{reboot,shutdown}).

> > Do you consider allowing support for Windows as overengineering?
> 
> I don't think there is a way to hook BSOD on Windows so attempting to
> engineer something that works with Windows seems odd, no?

Unsure about hooking at BSOD time. But Windows has configurable 
memory dump/reset/reboot, so yes it should not necessary.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >> Regards,
> >> 
> >> Anthony Liguori
> >> 
> >> >
> >> >> 
> >> >> > Well, we have more than a single serial port, even when leaving
> >> >> > virtio-serial aside...
> >> >> > 
> >> >> > Jan
> >> >> > 
> >> >> > -- 
> >> >> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> >> >> > Corporate Competence Center Embedded Linux
> >> >> > --
> >> >> > 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
> --
> 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Anthony Liguori]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>> 
>> > On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
>> >> Marcelo Tosatti <mtosatti@redhat.com> writes:
>> >> 
>> >> > On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
>> >> >> 
>> >> >> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
>> >> >> 
>> >> >> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
>> >> >> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>> >> >> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>> >> >> >>>> We can know the guest is panicked when the guest runs on xen.
>> >> >> >>>> But we do not have such feature on kvm.
>> >> >> >>>> 
>> >> >> >>>> Another purpose of this feature is: management app(for example:
>> >> >> >>>> libvirt) can do auto dump when the guest is panicked. If management
>> >> >> >>>> app does not do auto dump, the guest's user can do dump by hand if
>> >> >> >>>> he sees the guest is panicked.
>> >> >> >>>> 
>> >> >> >>>> We have three solutions to implement this feature:
>> >> >> >>>> 1. use vmcall
>> >> >> >>>> 2. use I/O port
>> >> >> >>>> 3. use virtio-serial.
>> >> >> >>>> 
>> >> >> >>>> We have decided to avoid touching hypervisor. The reason why I choose
>> >> >> >>>> choose the I/O port is:
>> >> >> >>>> 1. it is easier to implememt
>> >> >> >>>> 2. it does not depend any virtual device
>> >> >> >>>> 3. it can work when starting the kernel
>> >> >> >>> 
>> >> >> >>> How about searching for the "Kernel panic - not syncing" string 
>> >> >> >>> in the guests serial output? Say libvirtd could take an action upon
>> >> >> >>> that?
>> >> >> >> 
>> >> >> >> No, this is not satisfactory. It depends on the guest OS being
>> >> >> >> configured to use the serial port for console output which we
>> >> >> >> cannot mandate, since it may well be required for other purposes.
>> >> >> > 
>> >> >> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
>> >> >> 
>> >> >> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
>> >> >> 
>> >> >> Yan. 
>> >> >
>> >> > Considering whether a "panic-device" should cover other OSes is also \
>> >
>> >> > something to consider. Even for Linux, is "panic" the only case which
>> >> > should be reported via the mechanism? What about oopses without panic? 
>> >> >
>> >> > Is the mechanism general enough for supporting new events, etc.
>> >> 
>> >> Hi,
>> >> 
>> >> I think this discussion is gone of the deep end.
>> >> 
>> >> Forget about !x86 platforms.  They have their own way to do this sort of
>> >> thing.  
>> >
>> > The panic function in kernel/panic.c has the following options, which
>> > appear to be arch independent, on panic:
>> >
>> > - reboot 
>> > - blink
>> 
>> Not sure the semantics of blink but that might be a good place for a
>> pvops hook.
>> 
>> >
>> > None are paravirtual interfaces however.
>> >
>> >> Think of this feature like a status LED on a motherboard.  These
>> >> are very common and usually controlled by IO ports.
>> >> 
>> >> We're simply reserving a "status LED" for the guest to indicate that it
>> >> has paniced.  Let's not over engineer this.
>> >
>> > My concern is that you end up with state that is dependant on x86.
>> >
>> > Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
>> >
>> > Having the ability to stop/restart the guest (and even introducing a 
>> > new VM runstate) is more than a status LED analogy.
>> 
>> I must admit, I don't know why a new runstate is necessary/useful.  The
>> kernel shouldn't have to care about the difference between a halted guest
>> and a panicked guest.  That level of information belongs in userspace IMHO.
>> 
>> > Can this new infrastructure be used by other architectures?
>> 
>> I guess I don't understand why the kernel side of this isn't anything
>> more than a paravirt op hook that does a single outb() with the
>> remaining logic handled 100% in QEMU.
>
> From the patch description:
>
> "Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked."

Why does this mandated another runstate?  QEMU can simply mark the VCPUs
as stopped and raise a QMP event.  The kernel doesn't care if the VCPUs
are stopped or panicked.

> Wen, auto dump means dump of guest memory?
>
> In that case, the notification should obviously stop the guest 
> otherwise the guest might be reset by the time memdump from QEMU 
> monitor runs.
>
> But kexec supports dumping of memory already (i suppose it can 
> do automatic dump+{reboot,shutdown}).
>
>> > Do you consider allowing support for Windows as overengineering?
>> 
>> I don't think there is a way to hook BSOD on Windows so attempting to
>> engineer something that works with Windows seems odd, no?
>
> Unsure about hooking at BSOD time. But Windows has configurable 
> memory dump/reset/reboot, so yes it should not necessary.

Do you mean it's not necessary to hook BSOD?

I've very often gotten asked: We know 1 person is experiencing this
crash condition, can we figure out from the host how many other VMs are
experiencing this crash too instead of waiting for a user to complain?

That's the primary use-case for this notification IMHO.  Just a simple
status LED from the guest to indicate that it's in a bad state.

Regards,

Anthony Liguori

>
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> >> Regards,
>> >> 
>> >> Anthony Liguori
>> >> 
>> >> >
>> >> >> 
>> >> >> > Well, we have more than a single serial port, even when leaving
>> >> >> > virtio-serial aside...
>> >> >> > 
>> >> >> > Jan
>> >> >> > 
>> >> >> > -- 
>> >> >> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
>> >> >> > Corporate Competence Center Embedded Linux
>> >> >> > --
>> >> >> > 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
>> --
>> 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked\

[Marcelo Tosatti]

On Tue, Aug 14, 2012 at 05:59:06PM -0500, Anthony Liguori wrote:
> Marcelo Tosatti <mtosatti@redhat.com> writes:
> 
> > On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
> >> Marcelo Tosatti <mtosatti@redhat.com> writes:
> >> 
> >> > On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
> >> >> Marcelo Tosatti <mtosatti@redhat.com> writes:
> >> >> 
> >> >> > On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
> >> >> >> 
> >> >> >> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
> >> >> >> 
> >> >> >> > On 2012-08-14 10:56, Daniel P. Berrange wrote:
> >> >> >> >> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
> >> >> >> >>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
> >> >> >> >>>> We can know the guest is panicked when the guest runs on xen.
> >> >> >> >>>> But we do not have such feature on kvm.
> >> >> >> >>>> 
> >> >> >> >>>> Another purpose of this feature is: management app(for example:
> >> >> >> >>>> libvirt) can do auto dump when the guest is panicked. If management
> >> >> >> >>>> app does not do auto dump, the guest's user can do dump by hand if
> >> >> >> >>>> he sees the guest is panicked.
> >> >> >> >>>> 
> >> >> >> >>>> We have three solutions to implement this feature:
> >> >> >> >>>> 1. use vmcall
> >> >> >> >>>> 2. use I/O port
> >> >> >> >>>> 3. use virtio-serial.
> >> >> >> >>>> 
> >> >> >> >>>> We have decided to avoid touching hypervisor. The reason why I choose
> >> >> >> >>>> choose the I/O port is:
> >> >> >> >>>> 1. it is easier to implememt
> >> >> >> >>>> 2. it does not depend any virtual device
> >> >> >> >>>> 3. it can work when starting the kernel
> >> >> >> >>> 
> >> >> >> >>> How about searching for the "Kernel panic - not syncing" string 
> >> >> >> >>> in the guests serial output? Say libvirtd could take an action upon
> >> >> >> >>> that?
> >> >> >> >> 
> >> >> >> >> No, this is not satisfactory. It depends on the guest OS being
> >> >> >> >> configured to use the serial port for console output which we
> >> >> >> >> cannot mandate, since it may well be required for other purposes.
> >> >> >> > 
> >> >> >> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
> >> >> >> 
> >> >> >> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
> >> >> >> 
> >> >> >> Yan. 
> >> >> >
> >> >> > Considering whether a "panic-device" should cover other OSes is also \
> >> >
> >> >> > something to consider. Even for Linux, is "panic" the only case which
> >> >> > should be reported via the mechanism? What about oopses without panic? 
> >> >> >
> >> >> > Is the mechanism general enough for supporting new events, etc.
> >> >> 
> >> >> Hi,
> >> >> 
> >> >> I think this discussion is gone of the deep end.
> >> >> 
> >> >> Forget about !x86 platforms.  They have their own way to do this sort of
> >> >> thing.  
> >> >
> >> > The panic function in kernel/panic.c has the following options, which
> >> > appear to be arch independent, on panic:
> >> >
> >> > - reboot 
> >> > - blink
> >> 
> >> Not sure the semantics of blink but that might be a good place for a
> >> pvops hook.
> >> 
> >> >
> >> > None are paravirtual interfaces however.
> >> >
> >> >> Think of this feature like a status LED on a motherboard.  These
> >> >> are very common and usually controlled by IO ports.
> >> >> 
> >> >> We're simply reserving a "status LED" for the guest to indicate that it
> >> >> has paniced.  Let's not over engineer this.
> >> >
> >> > My concern is that you end up with state that is dependant on x86.
> >> >
> >> > Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
> >> >
> >> > Having the ability to stop/restart the guest (and even introducing a 
> >> > new VM runstate) is more than a status LED analogy.
> >> 
> >> I must admit, I don't know why a new runstate is necessary/useful.  The
> >> kernel shouldn't have to care about the difference between a halted guest
> >> and a panicked guest.  That level of information belongs in userspace IMHO.
> >> 
> >> > Can this new infrastructure be used by other architectures?
> >> 
> >> I guess I don't understand why the kernel side of this isn't anything
> >> more than a paravirt op hook that does a single outb() with the
> >> remaining logic handled 100% in QEMU.
> >
> > From the patch description:
> >
> > "Another purpose of this feature is: management app(for example:
> > libvirt) can do auto dump when the guest is panicked. If management
> > app does not do auto dump, the guest's user can do dump by hand if
> > he sees the guest is panicked."
> 
> Why does this mandated another runstate?  

Good question.

> QEMU can simply mark the VCPUs as stopped and raise a QMP event.

Yes. As long as management app is able to find out for what the reason
the VM has been stopped (that is, its not an issue to lose the QMP
event).

> The kernel doesn't care if the VCPUs
> are stopped or panicked.
> > Wen, auto dump means dump of guest memory?
> >
> > In that case, the notification should obviously stop the guest 
> > otherwise the guest might be reset by the time memdump from QEMU 
> > monitor runs.
> >
> > But kexec supports dumping of memory already (i suppose it can 
> > do automatic dump+{reboot,shutdown}).
> >
> >> > Do you consider allowing support for Windows as overengineering?
> >> 
> >> I don't think there is a way to hook BSOD on Windows so attempting to
> >> engineer something that works with Windows seems odd, no?
> >
> > Unsure about hooking at BSOD time. But Windows has configurable 
> > memory dump/reset/reboot, so yes it should not necessary.
> 
> Do you mean it's not necessary to hook BSOD?

If all you need is dumping memory and rebooting the guest, then Windows
can do that automatically. Linux probably does, if not its possible 
to make it do so.

> I've very often gotten asked: We know 1 person is experiencing this
> crash condition, can we figure out from the host how many other VMs are
> experiencing this crash too instead of waiting for a user to complain?
> 
> That's the primary use-case for this notification IMHO.  Just a simple
> status LED from the guest to indicate that it's in a bad state.

That makes sense. But it appears to me that using an interface which is
not specific to x86 is interesting, so as to not require another
driver and matching QEMU code for other architectures. That is, 
for the "paravirtual status-LED-on-panic", there is no advantage 
in making every architecture different.

Also configuration of reboot-on-panic should override
panic-via-hypervisor (guest settings have priority over
panic-via-hypervisor).

For the usecase above (recording a critical event), it also makes sense
to support Windows.


> Regards,
> 
> Anthony Liguori
> 
> >
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >> 
> >> >
> >> >> Regards,
> >> >> 
> >> >> Anthony Liguori
> >> >> 
> >> >> >
> >> >> >> 
> >> >> >> > Well, we have more than a single serial port, even when leaving
> >> >> >> > virtio-serial aside...
> >> >> >> > 
> >> >> >> > Jan
> >> >> >> > 
> >> >> >> > -- 
> >> >> >> > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> >> >> >> > Corporate Competence Center Embedded Linux
> >> >> >> > --
> >> >> >> > 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
> >> --
> >> 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Gleb Natapov]

On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
> > Do you consider allowing support for Windows as overengineering?
> 
> I don't think there is a way to hook BSOD on Windows so attempting to
> engineer something that works with Windows seems odd, no?
> 
Yan says in other email that is is possible to register a bugcheck callback.

--
			Gleb.

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Yan Vugenfirer]

On Aug 14, 2012, at 10:35 PM, Anthony Liguori wrote:

> Marcelo Tosatti <mtosatti@redhat.com> writes:
> 
>> On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
>>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>>> 
>>>> On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
>>>>> 
>>>>> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
>>>>> 
>>>>>> On 2012-08-14 10:56, Daniel P. Berrange wrote:
>>>>>>> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>>>>>>>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>>>>>>>>> We can know the guest is panicked when the guest runs on xen.
>>>>>>>>> But we do not have such feature on kvm.
>>>>>>>>> 
>>>>>>>>> Another purpose of this feature is: management app(for example:
>>>>>>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>>>>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>>>>>>> he sees the guest is panicked.
>>>>>>>>> 
>>>>>>>>> We have three solutions to implement this feature:
>>>>>>>>> 1. use vmcall
>>>>>>>>> 2. use I/O port
>>>>>>>>> 3. use virtio-serial.
>>>>>>>>> 
>>>>>>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>>>>>>> choose the I/O port is:
>>>>>>>>> 1. it is easier to implememt
>>>>>>>>> 2. it does not depend any virtual device
>>>>>>>>> 3. it can work when starting the kernel
>>>>>>>> 
>>>>>>>> How about searching for the "Kernel panic - not syncing" string 
>>>>>>>> in the guests serial output? Say libvirtd could take an action upon
>>>>>>>> that?
>>>>>>> 
>>>>>>> No, this is not satisfactory. It depends on the guest OS being
>>>>>>> configured to use the serial port for console output which we
>>>>>>> cannot mandate, since it may well be required for other purposes.
>>>>>> 
>>>>> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
>>>>> 
>>>>> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
>>>>> 
>>>>> Yan. 
>>>> 
>>>> Considering whether a "panic-device" should cover other OSes is also \
>> 
>>>> something to consider. Even for Linux, is "panic" the only case which
>>>> should be reported via the mechanism? What about oopses without panic? 
>>>> 
>>>> Is the mechanism general enough for supporting new events, etc.
>>> 
>>> Hi,
>>> 
>>> I think this discussion is gone of the deep end.
>>> 
>>> Forget about !x86 platforms.  They have their own way to do this sort of
>>> thing.  
>> 
>> The panic function in kernel/panic.c has the following options, which
>> appear to be arch independent, on panic:
>> 
>> - reboot 
>> - blink
> 
> Not sure the semantics of blink but that might be a good place for a
> pvops hook.
> 
>> 
>> None are paravirtual interfaces however.
>> 
>>> Think of this feature like a status LED on a motherboard.  These
>>> are very common and usually controlled by IO ports.
>>> 
>>> We're simply reserving a "status LED" for the guest to indicate that it
>>> has paniced.  Let's not over engineer this.
>> 
>> My concern is that you end up with state that is dependant on x86.
>> 
>> Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
>> 
>> Having the ability to stop/restart the guest (and even introducing a 
>> new VM runstate) is more than a status LED analogy.
> 
> I must admit, I don't know why a new runstate is necessary/useful.  The
> kernel shouldn't have to care about the difference between a halted guest
> and a panicked guest.  That level of information belongs in userspace IMHO.
> 
>> Can this new infrastructure be used by other architectures?
> 
> I guess I don't understand why the kernel side of this isn't anything
> more than a paravirt op hook that does a single outb() with the
> remaining logic handled 100% in QEMU.
> 
>> Do you consider allowing support for Windows as overengineering?
> 
> I don't think there is a way to hook BSOD on Windows so attempting to
> engineer something that works with Windows seems odd, no?
> 

Actually there is a way (http://msdn.microsoft.com/en-us/library/windows/hardware/ff553105(v=vs.85).aspx). That's what I just mentioned already done in Windows virtio-net driver. 


Best regards,
Yan.

> Regards,
> 
> Anthony Liguori
> 
>> 
>>> Regards,
>>> 
>>> Anthony Liguori
>>> 
>>>> 
>>>>> 
>>>>>> Well, we have more than a single serial port, even when leaving
>>>>>> virtio-serial aside...
>>>>>> 
>>>>>> Jan
>>>>>> 
>>>>>> -- 
>>>>>> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
>>>>>> Corporate Competence Center Embedded Linux
>>>>>> --
>>>>>> 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
> --
> 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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Yan Vugenfirer]

On Aug 15, 2012, at 12:56 PM, Gleb Natapov wrote:

> On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
>>> Do you consider allowing support for Windows as overengineering?
>> 
>> I don't think there is a way to hook BSOD on Windows so attempting to
>> engineer something that works with Windows seems odd, no?
>> 
> Yan says in other email that is is possible to register a bugcheck callback.
> 

Here you go - http://msdn.microsoft.com/en-us/library/windows/hardware/ff553105(v=vs.85).aspx
Already done in virtio-net for two reasons: 1. we could configure virtio-net to notify QEMU in a hacky way (write 1 to VIRTIO_PCI_ISR register) that there was a bugckeck .It was very useful debugging complex WHQL issues that involved host networking. 2. Store additional information (for example time stamps of last receive packet, last interrupt and etc) in crash dump.

Yan.

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

Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked

[Wen Congyang]

At 08/15/2012 04:53 AM, Marcelo Tosatti Wrote:
> On Tue, Aug 14, 2012 at 02:35:34PM -0500, Anthony Liguori wrote:
>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>>
>>> On Tue, Aug 14, 2012 at 01:53:01PM -0500, Anthony Liguori wrote:
>>>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>>>>
>>>>> On Tue, Aug 14, 2012 at 05:55:54PM +0300, Yan Vugenfirer wrote:
>>>>>>
>>>>>> On Aug 14, 2012, at 1:42 PM, Jan Kiszka wrote:
>>>>>>
>>>>>>> On 2012-08-14 10:56, Daniel P. Berrange wrote:
>>>>>>>> On Mon, Aug 13, 2012 at 03:21:32PM -0300, Marcelo Tosatti wrote:
>>>>>>>>> On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote:
>>>>>>>>>> We can know the guest is panicked when the guest runs on xen.
>>>>>>>>>> But we do not have such feature on kvm.
>>>>>>>>>>
>>>>>>>>>> Another purpose of this feature is: management app(for example:
>>>>>>>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>>>>>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>>>>>>>> he sees the guest is panicked.
>>>>>>>>>>
>>>>>>>>>> We have three solutions to implement this feature:
>>>>>>>>>> 1. use vmcall
>>>>>>>>>> 2. use I/O port
>>>>>>>>>> 3. use virtio-serial.
>>>>>>>>>>
>>>>>>>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>>>>>>>> choose the I/O port is:
>>>>>>>>>> 1. it is easier to implememt
>>>>>>>>>> 2. it does not depend any virtual device
>>>>>>>>>> 3. it can work when starting the kernel
>>>>>>>>>
>>>>>>>>> How about searching for the "Kernel panic - not syncing" string 
>>>>>>>>> in the guests serial output? Say libvirtd could take an action upon
>>>>>>>>> that?
>>>>>>>>
>>>>>>>> No, this is not satisfactory. It depends on the guest OS being
>>>>>>>> configured to use the serial port for console output which we
>>>>>>>> cannot mandate, since it may well be required for other purposes.
>>>>>>>
>>>>>> Please don't forget Windows guests, there is no console and no "Kernel Panic" string ;)
>>>>>>
>>>>>> What I used for debugging purposes on Windows guest is to register a bugcheck callback in virtio-net driver and write 1 to VIRTIO_PCI_ISR register.
>>>>>>
>>>>>> Yan. 
>>>>>
>>>>> Considering whether a "panic-device" should cover other OSes is also \
>>>
>>>>> something to consider. Even for Linux, is "panic" the only case which
>>>>> should be reported via the mechanism? What about oopses without panic? 
>>>>>
>>>>> Is the mechanism general enough for supporting new events, etc.
>>>>
>>>> Hi,
>>>>
>>>> I think this discussion is gone of the deep end.
>>>>
>>>> Forget about !x86 platforms.  They have their own way to do this sort of
>>>> thing.  
>>>
>>> The panic function in kernel/panic.c has the following options, which
>>> appear to be arch independent, on panic:
>>>
>>> - reboot 
>>> - blink
>>
>> Not sure the semantics of blink but that might be a good place for a
>> pvops hook.
>>
>>>
>>> None are paravirtual interfaces however.
>>>
>>>> Think of this feature like a status LED on a motherboard.  These
>>>> are very common and usually controlled by IO ports.
>>>>
>>>> We're simply reserving a "status LED" for the guest to indicate that it
>>>> has paniced.  Let's not over engineer this.
>>>
>>> My concern is that you end up with state that is dependant on x86.
>>>
>>> Subject: [PATCH v8 3/6] add a new runstate: RUN_STATE_GUEST_PANICKED
>>>
>>> Having the ability to stop/restart the guest (and even introducing a 
>>> new VM runstate) is more than a status LED analogy.
>>
>> I must admit, I don't know why a new runstate is necessary/useful.  The
>> kernel shouldn't have to care about the difference between a halted guest
>> and a panicked guest.  That level of information belongs in userspace IMHO.
>>
>>> Can this new infrastructure be used by other architectures?
>>
>> I guess I don't understand why the kernel side of this isn't anything
>> more than a paravirt op hook that does a single outb() with the
>> remaining logic handled 100% in QEMU.
> 
>>From the patch description:
> 
> "Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked."
> 
> Wen, auto dump means dump of guest memory?

Yes.

> 
> In that case, the notification should obviously stop the guest 
> otherwise the guest might be reset by the time memdump from QEMU 
> monitor runs.

Yes, the guest is stopped while auto dumping.

> 
> But kexec supports dumping of memory already (i suppose it can 
> do automatic dump+{reboot,shutdown}).

It can be easily done in management app.

Thanks
Wen Congyang

> 
>>> Do you consider allowing support for Windows as overengineering?
>>
>> I don't think there is a way to hook BSOD on Windows so attempting to
>> engineer something that works with Windows seems odd, no?
> 
> Unsure about hooking at BSOD time. But Windows has configurable 
> memory dump/reset/reboot, so yes it should not necessary.
> 
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>>>>
>>>>>>
>>>>>>> Well, we have more than a single serial port, even when leaving
>>>>>>> virtio-serial aside...
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>> -- 
>>>>>>> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
>>>>>>> Corporate Competence Center Embedded Linux
>>>>>>> --
>>>>>>> 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
>> --
>> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [Qemu-devel] [PATCH v8 5/6] introduce a new qom device to deal with panicked event

[Wen Congyang]

At 08/09/2012 03:01 AM, Blue Swirl Wrote:
> On Wed, Aug 8, 2012 at 2:47 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>> port KVM_PV_EVENT_PORT when it is panciked. This patch introduces a new
>> qom device kvm_pv_ioport to listen this I/O port, and deal with panicked
>> event according to panicked_action's value. The possible actions are:
>> 1. emit QEVENT_GUEST_PANICKED only
>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>
>> I/O ports does not work for some targets(for example: s390). And you
>> can implement another qom device, and include it's code into pv_event.c
>> for such target.
>>
>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>> application does not receive this event(the management may not
>> run when the event is emitted), the management won't know the
>> guest is panicked.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  hw/kvm/Makefile.objs |    2 +-
>>  hw/kvm/pv_event.c    |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/kvm/pv_ioport.c   |   93 ++++++++++++++++++++++++++++++++++++++++++
>>  hw/pc_piix.c         |    9 ++++
>>  kvm.h                |    2 +
>>  5 files changed, 214 insertions(+), 1 deletions(-)
>>  create mode 100644 hw/kvm/pv_event.c
>>  create mode 100644 hw/kvm/pv_ioport.c
>>
>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>> index 226497a..23e3b30 100644
>> --- a/hw/kvm/Makefile.objs
>> +++ b/hw/kvm/Makefile.objs
>> @@ -1 +1 @@
>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>> new file mode 100644
>> index 0000000..8897237
>> --- /dev/null
>> +++ b/hw/kvm/pv_event.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * QEMU KVM support, paravirtual event device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include <linux/kvm_para.h>
>> +#include <asm/kvm_para.h>
>> +#include <qobject.h>
>> +#include <qjson.h>
>> +#include <monitor.h>
>> +#include <sysemu.h>
>> +#include <kvm.h>
>> +
>> +/* Possible values for action parameter. */
>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>> +
>> +#define PV_EVENT_DRIVER     "kvm_pv_event"
>> +
>> +struct pv_event_action {
> 
> PVEventAction
> 
>> +    char *panicked_action;
>> +    int panicked_action_value;
>> +};
>> +
>> +#define DEFINE_PV_EVENT_PROPERTIES(_state, _conf)   \
>> +    DEFINE_PROP_STRING("panicked_action", _state, _conf.panicked_action)
>> +
>> +static void panicked_mon_event(const char *action)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>> +    qobject_decref(data);
>> +}
>> +
>> +static void panicked_perform_action(uint32_t panicked_action)
>> +{
>> +    switch (panicked_action) {
>> +    case PANICKED_REPORT:
>> +        panicked_mon_event("report");
>> +        break;
>> +
>> +    case PANICKED_PAUSE:
>> +        panicked_mon_event("pause");
>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>> +        break;
>> +
>> +    case PANICKED_POWEROFF:
>> +        panicked_mon_event("poweroff");
>> +        qemu_system_shutdown_request();
>> +        break;
> 
> Misses a line break unlike other cases.
> 
>> +    case PANICKED_RESET:
>> +        panicked_mon_event("reset");
>> +        qemu_system_reset_request();
>> +        break;
>> +    }
>> +}
>> +
>> +static uint64_t supported_event(void)
>> +{
>> +    return 1 << KVM_PV_FEATURE_PANICKED;
>> +}
>> +
>> +static void handle_event(int event, struct pv_event_action *conf)
>> +{
>> +    if (event == KVM_PV_EVENT_PANICKED) {
>> +        panicked_perform_action(conf->panicked_action_value);
>> +    }
>> +}
>> +
>> +static int pv_event_init(struct pv_event_action *conf)
>> +{
>> +    if (!conf->panicked_action) {
>> +        conf->panicked_action_value = PANICKED_REPORT;
>> +    } else if (strcasecmp(conf->panicked_action, "none") == 0) {
>> +        conf->panicked_action_value = PANICKED_REPORT;
>> +    } else if (strcasecmp(conf->panicked_action, "pause") == 0) {
>> +        conf->panicked_action_value = PANICKED_PAUSE;
>> +    } else if (strcasecmp(conf->panicked_action, "poweroff") == 0) {
>> +        conf->panicked_action_value = PANICKED_POWEROFF;
>> +    } else if (strcasecmp(conf->panicked_action, "reset") == 0) {
>> +        conf->panicked_action_value = PANICKED_RESET;
>> +    } else {
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +#if defined(KVM_PV_EVENT_PORT)
>> +
>> +#include "pv_ioport.c"
> 
> I'd rather not include any .c files but insert the contents here directly.
> 
>> +
>> +#else
>> +void kvm_pv_event_init(void *opaque)
>> +{
>> +}
>> +#endif
>> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
>> new file mode 100644
>> index 0000000..c2ed6b5
>> --- /dev/null
>> +++ b/hw/kvm/pv_ioport.c
>> @@ -0,0 +1,93 @@
>> +/*
>> + * QEMU KVM support, paravirtual I/O port device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "hw/isa.h"
>> +
>> +typedef struct {
>> +    ISADevice dev;
>> +    struct pv_event_action conf;
>> +    MemoryRegion ioport;
>> +} PVIOPortState;
>> +
>> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
>> +{
>> +    return supported_event();
>> +}
>> +
>> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
>> +                        unsigned size)
>> +{
>> +    PVIOPortState *s = opaque;
>> +
>> +    handle_event(val, &s->conf);
>> +}
>> +
>> +static const MemoryRegionOps pv_io_ops = {
>> +    .read = pv_io_read,
>> +    .write = pv_io_write,
>> +    .impl = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    },
>> +};
>> +
>> +static int pv_ioport_initfn(ISADevice *dev)
>> +{
>> +    PVIOPortState *s = DO_UPCAST(PVIOPortState, dev, dev);
>> +
>> +    if (pv_event_init(&s->conf) < 0)
>> +        return -1;
> 
> Mandatory braces missing.
> 
>> +
>> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
>> +    isa_register_ioport(dev, &s->ioport, KVM_PV_EVENT_PORT);
>> +
>> +    return 0;
>> +}
>> +
>> +static Property pv_ioport_properties[] = {
>> +    DEFINE_PV_EVENT_PROPERTIES(PVIOPortState, conf),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
>> +
>> +    ic->init = pv_ioport_initfn;
>> +    dc->no_user = 1;
>> +    dc->props = pv_ioport_properties;
>> +}
>> +
>> +static TypeInfo pv_ioport_info = {
>> +    .name          = PV_EVENT_DRIVER,
>> +    .parent        = TYPE_ISA_DEVICE,
>> +    .instance_size = sizeof(PVIOPortState),
>> +    .class_init    = pv_ioport_class_init,
>> +};
>> +
>> +static void pv_ioport_register_types(void)
>> +{
>> +    type_register_static(&pv_ioport_info);
>> +}
>> +
>> +type_init(pv_ioport_register_types)
>> +
>> +void kvm_pv_event_init(void *opaque)
>> +{
>> +    ISABus *bus = opaque;
>> +    ISADevice *dev;
>> +
>> +    dev = isa_create(bus, PV_EVENT_DRIVER);
>> +    qdev_init_nofail(&dev->qdev);
>> +}
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index 0c0096f..4af8403 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -46,6 +46,9 @@
>>  #ifdef CONFIG_XEN
>>  #  include <xen/hvm/hvm_info_table.h>
>>  #endif
>> +#ifdef CONFIG_KVM
>> +#   include <asm/kvm_para.h>
>> +#endif
> 
> I'd remove this and the #ifdeffery below since a stub function is
> provided. This is not performance critical.

The stub function is in the file hw/kvm/pv_event.c, and this file
will be complied only when CONFIG_KVM is y.

Thanks
Wen Congyang

> 
>>
>>  #define MAX_IDE_BUS 2
>>
>> @@ -285,6 +288,12 @@ static void pc_init1(MemoryRegion *system_memory,
>>      if (pci_enabled) {
>>          pc_pci_device_init(pci_bus);
>>      }
>> +
>> +#ifdef KVM_PV_EVENT_PORT
>> +    if (kvm_enabled()) {
>> +        kvm_pv_event_init(isa_bus);
>> +    }
>> +#endif
>>  }
>>
>>  static void pc_init_pci(ram_addr_t ram_size,
>> diff --git a/kvm.h b/kvm.h
>> index 2617dd5..598dcbe 100644
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -222,4 +222,6 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
>>  int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
>>  int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
>> +
>> +void kvm_pv_event_init(void *opaque);
>>  #endif
>> --
>> 1.7.1
>>
>>
> 

Re: [Qemu-devel] [PATCH v8 5/6] introduce a new qom device to deal with panicked event

[Blue Swirl]

On Wed, Aug 22, 2012 at 7:30 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> At 08/09/2012 03:01 AM, Blue Swirl Wrote:
>> On Wed, Aug 8, 2012 at 2:47 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>>> port KVM_PV_EVENT_PORT when it is panciked. This patch introduces a new
>>> qom device kvm_pv_ioport to listen this I/O port, and deal with panicked
>>> event according to panicked_action's value. The possible actions are:
>>> 1. emit QEVENT_GUEST_PANICKED only
>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>>
>>> I/O ports does not work for some targets(for example: s390). And you
>>> can implement another qom device, and include it's code into pv_event.c
>>> for such target.
>>>
>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>>> application does not receive this event(the management may not
>>> run when the event is emitted), the management won't know the
>>> guest is panicked.
>>>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> ---
>>>  hw/kvm/Makefile.objs |    2 +-
>>>  hw/kvm/pv_event.c    |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/kvm/pv_ioport.c   |   93 ++++++++++++++++++++++++++++++++++++++++++
>>>  hw/pc_piix.c         |    9 ++++
>>>  kvm.h                |    2 +
>>>  5 files changed, 214 insertions(+), 1 deletions(-)
>>>  create mode 100644 hw/kvm/pv_event.c
>>>  create mode 100644 hw/kvm/pv_ioport.c
>>>
>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>>> index 226497a..23e3b30 100644
>>> --- a/hw/kvm/Makefile.objs
>>> +++ b/hw/kvm/Makefile.objs
>>> @@ -1 +1 @@
>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>>> new file mode 100644
>>> index 0000000..8897237
>>> --- /dev/null
>>> +++ b/hw/kvm/pv_event.c
>>> @@ -0,0 +1,109 @@
>>> +/*
>>> + * QEMU KVM support, paravirtual event device
>>> + *
>>> + * Copyright Fujitsu, Corp. 2012
>>> + *
>>> + * Authors:
>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include <linux/kvm_para.h>
>>> +#include <asm/kvm_para.h>
>>> +#include <qobject.h>
>>> +#include <qjson.h>
>>> +#include <monitor.h>
>>> +#include <sysemu.h>
>>> +#include <kvm.h>
>>> +
>>> +/* Possible values for action parameter. */
>>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>>> +
>>> +#define PV_EVENT_DRIVER     "kvm_pv_event"
>>> +
>>> +struct pv_event_action {
>>
>> PVEventAction
>>
>>> +    char *panicked_action;
>>> +    int panicked_action_value;
>>> +};
>>> +
>>> +#define DEFINE_PV_EVENT_PROPERTIES(_state, _conf)   \
>>> +    DEFINE_PROP_STRING("panicked_action", _state, _conf.panicked_action)
>>> +
>>> +static void panicked_mon_event(const char *action)
>>> +{
>>> +    QObject *data;
>>> +
>>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>>> +    qobject_decref(data);
>>> +}
>>> +
>>> +static void panicked_perform_action(uint32_t panicked_action)
>>> +{
>>> +    switch (panicked_action) {
>>> +    case PANICKED_REPORT:
>>> +        panicked_mon_event("report");
>>> +        break;
>>> +
>>> +    case PANICKED_PAUSE:
>>> +        panicked_mon_event("pause");
>>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>>> +        break;
>>> +
>>> +    case PANICKED_POWEROFF:
>>> +        panicked_mon_event("poweroff");
>>> +        qemu_system_shutdown_request();
>>> +        break;
>>
>> Misses a line break unlike other cases.
>>
>>> +    case PANICKED_RESET:
>>> +        panicked_mon_event("reset");
>>> +        qemu_system_reset_request();
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static uint64_t supported_event(void)
>>> +{
>>> +    return 1 << KVM_PV_FEATURE_PANICKED;
>>> +}
>>> +
>>> +static void handle_event(int event, struct pv_event_action *conf)
>>> +{
>>> +    if (event == KVM_PV_EVENT_PANICKED) {
>>> +        panicked_perform_action(conf->panicked_action_value);
>>> +    }
>>> +}
>>> +
>>> +static int pv_event_init(struct pv_event_action *conf)
>>> +{
>>> +    if (!conf->panicked_action) {
>>> +        conf->panicked_action_value = PANICKED_REPORT;
>>> +    } else if (strcasecmp(conf->panicked_action, "none") == 0) {
>>> +        conf->panicked_action_value = PANICKED_REPORT;
>>> +    } else if (strcasecmp(conf->panicked_action, "pause") == 0) {
>>> +        conf->panicked_action_value = PANICKED_PAUSE;
>>> +    } else if (strcasecmp(conf->panicked_action, "poweroff") == 0) {
>>> +        conf->panicked_action_value = PANICKED_POWEROFF;
>>> +    } else if (strcasecmp(conf->panicked_action, "reset") == 0) {
>>> +        conf->panicked_action_value = PANICKED_RESET;
>>> +    } else {
>>> +        return -1;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +#if defined(KVM_PV_EVENT_PORT)
>>> +
>>> +#include "pv_ioport.c"
>>
>> I'd rather not include any .c files but insert the contents here directly.
>>
>>> +
>>> +#else
>>> +void kvm_pv_event_init(void *opaque)
>>> +{
>>> +}
>>> +#endif
>>> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
>>> new file mode 100644
>>> index 0000000..c2ed6b5
>>> --- /dev/null
>>> +++ b/hw/kvm/pv_ioport.c
>>> @@ -0,0 +1,93 @@
>>> +/*
>>> + * QEMU KVM support, paravirtual I/O port device
>>> + *
>>> + * Copyright Fujitsu, Corp. 2012
>>> + *
>>> + * Authors:
>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include "hw/isa.h"
>>> +
>>> +typedef struct {
>>> +    ISADevice dev;
>>> +    struct pv_event_action conf;
>>> +    MemoryRegion ioport;
>>> +} PVIOPortState;
>>> +
>>> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
>>> +{
>>> +    return supported_event();
>>> +}
>>> +
>>> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
>>> +                        unsigned size)
>>> +{
>>> +    PVIOPortState *s = opaque;
>>> +
>>> +    handle_event(val, &s->conf);
>>> +}
>>> +
>>> +static const MemoryRegionOps pv_io_ops = {
>>> +    .read = pv_io_read,
>>> +    .write = pv_io_write,
>>> +    .impl = {
>>> +        .min_access_size = 4,
>>> +        .max_access_size = 4,
>>> +    },
>>> +};
>>> +
>>> +static int pv_ioport_initfn(ISADevice *dev)
>>> +{
>>> +    PVIOPortState *s = DO_UPCAST(PVIOPortState, dev, dev);
>>> +
>>> +    if (pv_event_init(&s->conf) < 0)
>>> +        return -1;
>>
>> Mandatory braces missing.
>>
>>> +
>>> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
>>> +    isa_register_ioport(dev, &s->ioport, KVM_PV_EVENT_PORT);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static Property pv_ioport_properties[] = {
>>> +    DEFINE_PV_EVENT_PROPERTIES(PVIOPortState, conf),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
>>> +
>>> +    ic->init = pv_ioport_initfn;
>>> +    dc->no_user = 1;
>>> +    dc->props = pv_ioport_properties;
>>> +}
>>> +
>>> +static TypeInfo pv_ioport_info = {
>>> +    .name          = PV_EVENT_DRIVER,
>>> +    .parent        = TYPE_ISA_DEVICE,
>>> +    .instance_size = sizeof(PVIOPortState),
>>> +    .class_init    = pv_ioport_class_init,
>>> +};
>>> +
>>> +static void pv_ioport_register_types(void)
>>> +{
>>> +    type_register_static(&pv_ioport_info);
>>> +}
>>> +
>>> +type_init(pv_ioport_register_types)
>>> +
>>> +void kvm_pv_event_init(void *opaque)
>>> +{
>>> +    ISABus *bus = opaque;
>>> +    ISADevice *dev;
>>> +
>>> +    dev = isa_create(bus, PV_EVENT_DRIVER);
>>> +    qdev_init_nofail(&dev->qdev);
>>> +}
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 0c0096f..4af8403 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -46,6 +46,9 @@
>>>  #ifdef CONFIG_XEN
>>>  #  include <xen/hvm/hvm_info_table.h>
>>>  #endif
>>> +#ifdef CONFIG_KVM
>>> +#   include <asm/kvm_para.h>
>>> +#endif
>>
>> I'd remove this and the #ifdeffery below since a stub function is
>> provided. This is not performance critical.
>
> The stub function is in the file hw/kvm/pv_event.c, and this file
> will be complied only when CONFIG_KVM is y.

Usually the stubs are always provided, compare to for example
kvm-stub.c. Wouldn't that simplify the code?

>
> Thanks
> Wen Congyang
>
>>
>>>
>>>  #define MAX_IDE_BUS 2
>>>
>>> @@ -285,6 +288,12 @@ static void pc_init1(MemoryRegion *system_memory,
>>>      if (pci_enabled) {
>>>          pc_pci_device_init(pci_bus);
>>>      }
>>> +
>>> +#ifdef KVM_PV_EVENT_PORT
>>> +    if (kvm_enabled()) {
>>> +        kvm_pv_event_init(isa_bus);
>>> +    }
>>> +#endif
>>>  }
>>>
>>>  static void pc_init_pci(ram_addr_t ram_size,
>>> diff --git a/kvm.h b/kvm.h
>>> index 2617dd5..598dcbe 100644
>>> --- a/kvm.h
>>> +++ b/kvm.h
>>> @@ -222,4 +222,6 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>>>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
>>>  int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
>>>  int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
>>> +
>>> +void kvm_pv_event_init(void *opaque);
>>>  #endif
>>> --
>>> 1.7.1
>>>
>>>
>>
>