All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] kvm: notify host when the guest is panicked
@ 2012-06-27  6:55 Wen Congyang
  2012-06-27  6:57 ` [PATCH 1/6 v5] start vm after reseting it Wen Congyang
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  6:55 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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 startint the kernel

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 arch/ia64/include/asm/kvm_para.h    |    5 +++++
 arch/powerpc/include/asm/kvm_para.h |    5 +++++
 arch/s390/include/asm/kvm_para.h    |    5 +++++
 arch/x86/include/asm/kvm_para.h     |    7 +++++++
 arch/x86/kernel/kvm.c               |   14 ++++++++++++++
 include/linux/kvm_para.h            |   13 +++++++++++++
 6 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 2019cb9..187c0e2 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -31,6 +31,11 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index c18916b..be81aac 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -211,6 +211,11 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #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 a988329..3d993b7 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -154,6 +154,11 @@ static inline bool kvm_check_and_clear_guest_paused(void)
 	return false;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #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 63ab166..c8ad86e 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -89,6 +89,8 @@ struct kvm_vcpu_pv_apf_data {
 	__u32 enabled;
 };
 
+#define KVM_PV_PORT	(0x505UL)
+
 #ifdef __KERNEL__
 #include <asm/processor.h>
 
@@ -221,6 +223,11 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return inl(KVM_PV_PORT);
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index e554e5a..9a97f7e 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -328,6 +328,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)
+{
+	outl(KVM_PV_PANICKED, KVM_PV_PORT);
+	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;
@@ -414,6 +425,9 @@ void __init kvm_guest_init(void)
 
 	paravirt_ops_setup();
 	register_reboot_notifier(&kvm_pv_reboot_nb);
+	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
+		atomic_notifier_chain_register(&panic_notifier_list,
+			&kvm_pv_panic_nb);
 	for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
 		spin_lock_init(&async_pf_sleepers[i].lock);
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index ff476dd..e73efcf 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 the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
@@ -33,5 +39,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
 		return 1;
 	return 0;
 }
+
+static inline int kvm_pv_has_feature(unsigned int feature)
+{
+	if (kvm_arch_pv_features() & (1UL << feature))
+		return 1;
+	return 0;
+}
 #endif /* __KERNEL__ */
 #endif /* __LINUX_KVM_PARA_H */
-- 
1.7.1


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

* [PATCH 1/6 v5] start vm after reseting it
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
@ 2012-06-27  6:57 ` Wen Congyang
  2012-06-27  6:58 ` [PATCH 2/6 v5] update linux headers Wen Congyang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  6:57 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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.

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

diff --git a/block.h b/block.h
index d135652..d9570dd 100644
--- a/block.h
+++ b/block.h
@@ -365,6 +365,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 1329c30..b38aa5f 100644
--- a/vl.c
+++ b/vl.c
@@ -1532,7 +1532,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


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

* [PATCH 2/6 v5] update linux headers
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
  2012-06-27  6:57 ` [PATCH 1/6 v5] start vm after reseting it Wen Congyang
@ 2012-06-27  6:58 ` Wen Congyang
  2012-06-27  7:00 ` [PATCH 3/6 v5] add a new runstate: RUN_STATE_GUEST_PANICKED Wen Congyang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  6:58 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 linux-headers/asm-x86/kvm_para.h |    2 ++
 linux-headers/linux/kvm_para.h   |    6 ++++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/linux-headers/asm-x86/kvm_para.h b/linux-headers/asm-x86/kvm_para.h
index f2ac46a..f9d858f 100644
--- a/linux-headers/asm-x86/kvm_para.h
+++ b/linux-headers/asm-x86/kvm_para.h
@@ -89,5 +89,7 @@ struct kvm_vcpu_pv_apf_data {
 	__u32 enabled;
 };
 
+#define KVM_PV_PORT	(0x505UL)
+
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/linux-headers/linux/kvm_para.h b/linux-headers/linux/kvm_para.h
index 7bdcf93..4fbd625 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 the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
-- 
1.7.1


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

* [PATCH 3/6 v5] add a new runstate: RUN_STATE_GUEST_PANICKED
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
  2012-06-27  6:57 ` [PATCH 1/6 v5] start vm after reseting it Wen Congyang
  2012-06-27  6:58 ` [PATCH 2/6 v5] update linux headers Wen Congyang
@ 2012-06-27  7:00 ` Wen Congyang
  2012-06-27  7:01 ` [PATCH 4/6 v5] add a new qevent: QEVENT_GUEST_PANICKED Wen Congyang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  7:00 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Add a new runstate RUN_STATE_GUEST_PANICKED. The guest can be in this
state if it is paused due to panicked event.

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 3b6e346..009f653 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 b38aa5f..c0e5d3c 100644
--- a/vl.c
+++ b/vl.c
@@ -361,6 +361,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 },
 
@@ -375,6 +376,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_PAUSED },
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
 
@@ -1531,7 +1535,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


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

* [PATCH 4/6 v5] add a new qevent: QEVENT_GUEST_PANICKED
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
                   ` (2 preceding siblings ...)
  2012-06-27  7:00 ` [PATCH 3/6 v5] add a new runstate: RUN_STATE_GUEST_PANICKED Wen Congyang
@ 2012-06-27  7:01 ` Wen Congyang
  2012-06-27  7:02 ` [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
  2012-06-27  7:04 ` [PATCH 6/6 v5] deal with panicked event accoring to '-machine panic_action=action' Wen Congyang
  5 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  7:01 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Add a new qevent QEVENT_GUEST_PANICKED. QEMU will emit this
event if 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 f6107ba..28f7482 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


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

* [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
                   ` (3 preceding siblings ...)
  2012-06-27  7:01 ` [PATCH 4/6 v5] add a new qevent: QEVENT_GUEST_PANICKED Wen Congyang
@ 2012-06-27  7:02 ` Wen Congyang
  2012-06-27 14:39   ` Jan Kiszka
  2012-06-27 14:52   ` Cornelia Huck
  2012-06-27  7:04 ` [PATCH 6/6 v5] deal with panicked event accoring to '-machine panic_action=action' Wen Congyang
  5 siblings, 2 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  7:02 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
So if qemu reads 0x1 from this port, we can do the folloing three
things according to the parameter -onpanic:
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

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>
---
 kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c      |    9 +++++
 kvm.h           |    3 ++
 qemu-options.hx |   15 ++++++++
 vl.c            |   10 +++++
 5 files changed, 138 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index f8e4328..9494dd2 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -19,6 +19,8 @@
 #include <stdarg.h>
 
 #include <linux/kvm.h>
+#include <linux/kvm_para.h>
+#include <asm/kvm_para.h>
 
 #include "qemu-common.h"
 #include "qemu-barrier.h"
@@ -32,6 +34,9 @@
 #include "bswap.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "iorange.h"
+#include "qemu-objects.h"
+#include "monitor.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return kvm_arch_on_sigbus(code, addr);
 }
+
+/* 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 */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
+                             uint64_t *data)
+{
+    *data = (1 << KVM_PV_FEATURE_PANICKED);
+}
+
+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(void)
+{
+    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");
+        exit(0);
+        break;
+    case PANICKED_RESET:
+        panicked_mon_event("reset");
+        qemu_system_reset_request();
+        break;
+    }
+}
+
+static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
+                              uint64_t data)
+{
+    if (data == KVM_PV_PANICKED) {
+        panicked_perform_action();
+    }
+}
+
+static void kvm_pv_port_destructor(IORange *iorange)
+{
+    g_free(iorange);
+}
+
+static IORangeOps pv_io_range_ops = {
+    .read = kvm_pv_port_read,
+    .write = kvm_pv_port_write,
+    .destructor = kvm_pv_port_destructor,
+};
+
+#if defined(KVM_PV_PORT)
+void kvm_pv_port_init(void)
+{
+    IORange *pv_io_range = g_malloc(sizeof(IORange));
+
+    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
+    ioport_register(pv_io_range);
+}
+#else
+void kvm_pv_port_init(void)
+{
+}
+#endif
+
+int select_panicked_action(const char *p)
+{
+    if (strcasecmp(p, "none") == 0) {
+        panicked_action = PANICKED_REPORT;
+    } else if (strcasecmp(p, "pause") == 0) {
+        panicked_action = PANICKED_PAUSE;
+    } else if (strcasecmp(p, "poweroff") == 0) {
+        panicked_action = PANICKED_POWEROFF;
+    } else if (strcasecmp(p, "reset") == 0) {
+        panicked_action = PANICKED_RESET;
+    } else {
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index ec9a364..318e967 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -151,3 +151,12 @@ int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq)
 {
     return -ENOSYS;
 }
+
+void kvm_pv_port_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+    return -1;
+}
diff --git a/kvm.h b/kvm.h
index 9c7b0ea..d174d5a 100644
--- a/kvm.h
+++ b/kvm.h
@@ -64,6 +64,9 @@ int kvm_has_gsi_routing(void);
 
 int kvm_allows_irq0_override(void);
 
+void kvm_pv_port_init(void);
+int select_panicked_action(const char *p);
+
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUArchState *env);
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 8b66264..4a061bf 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2743,6 +2743,21 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
+    "-onpanic none|pause|poweroff|reset\n" \
+    "                action when the guest is panicked [default=none]",
+    QEMU_ARCH_ALL)
+STEXI
+@item -onpanic @var{action}
+
+The @var{action} controls what QEmu will do when the guest is panicked.
+The default is @code{none} (emit QEVENT_GUEST_PANICKED only).
+Other possible actions are:
+@code{pause} (emit QEVENT_GUEST_PANICKED and pause the guest),
+@code{poweroff} (emit QEVENT_GUEST_PANICKED and forcefully poweroff the guest),
+@code{reset} (emit QEVENT_GUEST_PANICKED and forcefully reset the guest).
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/vl.c b/vl.c
index c0e5d3c..9164d29 100644
--- a/vl.c
+++ b/vl.c
@@ -3205,6 +3205,12 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+            case QEMU_OPTION_onpanic:
+                if (select_panicked_action(optarg) == -1) {
+                    fprintf(stderr, "Unknown -onpanic parameter\n");
+                    exit(1);
+                }
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3641,6 +3647,10 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    if (kvm_enabled()) {
+        kvm_pv_port_init();
+    }
+
     if (incoming) {
         Error *errp = NULL;
         int ret = qemu_start_incoming_migration(incoming, &errp);
-- 
1.7.1


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

* [PATCH 6/6 v5] deal with panicked event accoring to '-machine panic_action=action'
  2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
                   ` (4 preceding siblings ...)
  2012-06-27  7:02 ` [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
@ 2012-06-27  7:04 ` Wen Congyang
  5 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-06-27  7:04 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov
  Cc: Paolo Bonzini

The action is the same as -onpanic parameter.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 qemu-config.c   |    4 ++++
 qemu-options.hx |    4 +++-
 vl.c            |    7 +++++++
 3 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..805e7c4 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 = "panic_action",
+            .type = QEMU_OPT_STRING,
+            .help = "The action what QEMU will do when the guest is panicked",
         },
         { /* End of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 4a061bf..083a21d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -33,7 +33,9 @@ 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"
+    "                panic_action=none|pause|poweroff|reset controls what QEmu\n"
+    "                will do when the guest is panicked",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
diff --git a/vl.c b/vl.c
index 9164d29..7663699 100644
--- a/vl.c
+++ b/vl.c
@@ -2301,6 +2301,7 @@ int main(int argc, char **argv, char **envp)
     };
     const char *trace_events = NULL;
     const char *trace_file = NULL;
+    const char *panic_action = NULL;
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -3372,10 +3373,16 @@ int main(int argc, char **argv, char **envp)
         kernel_filename = qemu_opt_get(machine_opts, "kernel");
         initrd_filename = qemu_opt_get(machine_opts, "initrd");
         kernel_cmdline = qemu_opt_get(machine_opts, "append");
+        panic_action = qemu_opt_get(machine_opts, "panic_action");
     } else {
         kernel_filename = initrd_filename = kernel_cmdline = NULL;
     }
 
+    if (panic_action && select_panicked_action(panic_action) == -1) {
+        fprintf(stderr, "Unknown -panic_action parameter\n");
+        exit(1);
+    }
+
     if (!kernel_cmdline) {
         kernel_cmdline = "";
     }
-- 
1.7.1


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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-27  7:02 ` [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
@ 2012-06-27 14:39   ` Jan Kiszka
  2012-06-28  1:15     ` Wen Congyang
  2012-06-27 14:52   ` Cornelia Huck
  1 sibling, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2012-06-27 14:39 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

On 2012-06-27 09:02, Wen Congyang wrote:
> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
> So if qemu reads 0x1 from this port, we can do the folloing three
> things according to the parameter -onpanic:
> 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
> 
> 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>
> ---
>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c      |    9 +++++
>  kvm.h           |    3 ++
>  qemu-options.hx |   15 ++++++++
>  vl.c            |   10 +++++
>  5 files changed, 138 insertions(+), 0 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index f8e4328..9494dd2 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,8 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
> +#include <asm/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -32,6 +34,9 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
> +#include "monitor.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* 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 */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +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(void)
> +{
> +    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");
> +        exit(0);
> +        break;
> +    case PANICKED_RESET:
> +        panicked_mon_event("reset");
> +        qemu_system_reset_request();
> +        break;
> +    }
> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED) {
> +        panicked_perform_action();
> +    }
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +#if defined(KVM_PV_PORT)
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
> +    ioport_register(pv_io_range);

This modeling is still not ok. We don't open-code ports anymore, we
introduce devices. And this doesn't belong inro generic code as it is
x86-only. Will avoid that #ifdef as well.

Thanks
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-27  7:02 ` [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
  2012-06-27 14:39   ` Jan Kiszka
@ 2012-06-27 14:52   ` Cornelia Huck
  2012-06-27 14:57     ` Daniel P. Berrange
  1 sibling, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2012-06-27 14:52 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Wed, 27 Jun 2012 15:02:23 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
> So if qemu reads 0x1 from this port, we can do the folloing three
> things according to the parameter -onpanic:
> 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

Would it be useful to add some "dump the guest" actions here?

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

> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}

> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED) {
> +        panicked_perform_action();
> +    }
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +#if defined(KVM_PV_PORT)
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
> +    ioport_register(pv_io_range);
> +}
> +#else
> +void kvm_pv_port_init(void)
> +{
> +}
> +#endif

> @@ -3641,6 +3647,10 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
> 
> +    if (kvm_enabled()) {
> +        kvm_pv_port_init();
> +    }
> +
>      if (incoming) {
>          Error *errp = NULL;
>          int ret = qemu_start_incoming_migration(incoming, &errp);

I/O ports won't work for s390, yet we'll likely want panic
notifications there as well. This will need some more abstraction.

Cornelia


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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-27 14:52   ` Cornelia Huck
@ 2012-06-27 14:57     ` Daniel P. Berrange
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrange @ 2012-06-27 14:57 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Wen Congyang, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Wed, Jun 27, 2012 at 04:52:32PM +0200, Cornelia Huck wrote:
> On Wed, 27 Jun 2012 15:02:23 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
> > When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
> > So if qemu reads 0x1 from this port, we can do the folloing three
> > things according to the parameter -onpanic:
> > 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
> 
> Would it be useful to add some "dump the guest" actions here?

Better off leaving that to the mgmt layer using QEMU. If you
tried to directly handle "dump the guest" in the context of
the panic notifier then you add all sorts of extra complexity
to this otherwise simple feature. For a start the you need to
tell it what filename to use, which is not something you can
necessarily decide at the time QEMU starts - you might want
a separate filename each time a panic ocurrs. THe mgmt app
might not even want QEMU to dump to a file - it might want
to use a socket, or pass in a file descriptor at time of
dump. All in all, it is better to keep the panic notifier
simple, and let the mgmt app then decide whether to take
a dump separately, using existing QEMU monitor commands
and features.

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 :|

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-27 14:39   ` Jan Kiszka
@ 2012-06-28  1:15     ` Wen Congyang
  2012-06-28  8:26         ` Jan Kiszka
  0 siblings, 1 reply; 22+ messages in thread
From: Wen Congyang @ 2012-06-28  1:15 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
> On 2012-06-27 09:02, Wen Congyang wrote:
>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>> So if qemu reads 0x1 from this port, we can do the folloing three
>> things according to the parameter -onpanic:
>> 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
>>
>> 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>
>> ---
>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kvm-stub.c      |    9 +++++
>>  kvm.h           |    3 ++
>>  qemu-options.hx |   15 ++++++++
>>  vl.c            |   10 +++++
>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index f8e4328..9494dd2 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -19,6 +19,8 @@
>>  #include <stdarg.h>
>>  
>>  #include <linux/kvm.h>
>> +#include <linux/kvm_para.h>
>> +#include <asm/kvm_para.h>
>>  
>>  #include "qemu-common.h"
>>  #include "qemu-barrier.h"
>> @@ -32,6 +34,9 @@
>>  #include "bswap.h"
>>  #include "memory.h"
>>  #include "exec-memory.h"
>> +#include "iorange.h"
>> +#include "qemu-objects.h"
>> +#include "monitor.h"
>>  
>>  /* This check must be after config-host.h is included */
>>  #ifdef CONFIG_EVENTFD
>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>  {
>>      return kvm_arch_on_sigbus(code, addr);
>>  }
>> +
>> +/* 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 */
>> +
>> +static int panicked_action = PANICKED_REPORT;
>> +
>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>> +                             uint64_t *data)
>> +{
>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>> +}
>> +
>> +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(void)
>> +{
>> +    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");
>> +        exit(0);
>> +        break;
>> +    case PANICKED_RESET:
>> +        panicked_mon_event("reset");
>> +        qemu_system_reset_request();
>> +        break;
>> +    }
>> +}
>> +
>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>> +                              uint64_t data)
>> +{
>> +    if (data == KVM_PV_PANICKED) {
>> +        panicked_perform_action();
>> +    }
>> +}
>> +
>> +static void kvm_pv_port_destructor(IORange *iorange)
>> +{
>> +    g_free(iorange);
>> +}
>> +
>> +static IORangeOps pv_io_range_ops = {
>> +    .read = kvm_pv_port_read,
>> +    .write = kvm_pv_port_write,
>> +    .destructor = kvm_pv_port_destructor,
>> +};
>> +
>> +#if defined(KVM_PV_PORT)
>> +void kvm_pv_port_init(void)
>> +{
>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>> +
>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>> +    ioport_register(pv_io_range);
> 
> This modeling is still not ok. We don't open-code ports anymore, we
> introduce devices. And this doesn't belong inro generic code as it is

Do you mean introducing a new device instead of I/O port?

Thanks
Wen Congyang

> x86-only. Will avoid that #ifdef as well.
> 
> Thanks
> Jan
> 


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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-28  1:15     ` Wen Congyang
@ 2012-06-28  8:26         ` Jan Kiszka
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-06-28  8:26 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

On 2012-06-28 03:15, Wen Congyang wrote:
> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>> On 2012-06-27 09:02, Wen Congyang wrote:
>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>> things according to the parameter -onpanic:
>>> 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
>>>
>>> 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>
>>> ---
>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  kvm-stub.c      |    9 +++++
>>>  kvm.h           |    3 ++
>>>  qemu-options.hx |   15 ++++++++
>>>  vl.c            |   10 +++++
>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index f8e4328..9494dd2 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -19,6 +19,8 @@
>>>  #include <stdarg.h>
>>>  
>>>  #include <linux/kvm.h>
>>> +#include <linux/kvm_para.h>
>>> +#include <asm/kvm_para.h>
>>>  
>>>  #include "qemu-common.h"
>>>  #include "qemu-barrier.h"
>>> @@ -32,6 +34,9 @@
>>>  #include "bswap.h"
>>>  #include "memory.h"
>>>  #include "exec-memory.h"
>>> +#include "iorange.h"
>>> +#include "qemu-objects.h"
>>> +#include "monitor.h"
>>>  
>>>  /* This check must be after config-host.h is included */
>>>  #ifdef CONFIG_EVENTFD
>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>  {
>>>      return kvm_arch_on_sigbus(code, addr);
>>>  }
>>> +
>>> +/* 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 */
>>> +
>>> +static int panicked_action = PANICKED_REPORT;
>>> +
>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>> +                             uint64_t *data)
>>> +{
>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>> +}
>>> +
>>> +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(void)
>>> +{
>>> +    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");
>>> +        exit(0);
>>> +        break;
>>> +    case PANICKED_RESET:
>>> +        panicked_mon_event("reset");
>>> +        qemu_system_reset_request();
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>> +                              uint64_t data)
>>> +{
>>> +    if (data == KVM_PV_PANICKED) {
>>> +        panicked_perform_action();
>>> +    }
>>> +}
>>> +
>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>> +{
>>> +    g_free(iorange);
>>> +}
>>> +
>>> +static IORangeOps pv_io_range_ops = {
>>> +    .read = kvm_pv_port_read,
>>> +    .write = kvm_pv_port_write,
>>> +    .destructor = kvm_pv_port_destructor,
>>> +};
>>> +
>>> +#if defined(KVM_PV_PORT)
>>> +void kvm_pv_port_init(void)
>>> +{
>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>> +
>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>> +    ioport_register(pv_io_range);
>>
>> This modeling is still not ok. We don't open-code ports anymore, we
>> introduce devices. And this doesn't belong inro generic code as it is
> 
> Do you mean introducing a new device instead of I/O port?

I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
and building that device only for target archs that supports it. Already
pointed you to examples in hw/kvm/.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-06-28  8:26         ` Jan Kiszka
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-06-28  8:26 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-06-28 03:15, Wen Congyang wrote:
> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>> On 2012-06-27 09:02, Wen Congyang wrote:
>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>> things according to the parameter -onpanic:
>>> 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
>>>
>>> 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>
>>> ---
>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  kvm-stub.c      |    9 +++++
>>>  kvm.h           |    3 ++
>>>  qemu-options.hx |   15 ++++++++
>>>  vl.c            |   10 +++++
>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index f8e4328..9494dd2 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -19,6 +19,8 @@
>>>  #include <stdarg.h>
>>>  
>>>  #include <linux/kvm.h>
>>> +#include <linux/kvm_para.h>
>>> +#include <asm/kvm_para.h>
>>>  
>>>  #include "qemu-common.h"
>>>  #include "qemu-barrier.h"
>>> @@ -32,6 +34,9 @@
>>>  #include "bswap.h"
>>>  #include "memory.h"
>>>  #include "exec-memory.h"
>>> +#include "iorange.h"
>>> +#include "qemu-objects.h"
>>> +#include "monitor.h"
>>>  
>>>  /* This check must be after config-host.h is included */
>>>  #ifdef CONFIG_EVENTFD
>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>  {
>>>      return kvm_arch_on_sigbus(code, addr);
>>>  }
>>> +
>>> +/* 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 */
>>> +
>>> +static int panicked_action = PANICKED_REPORT;
>>> +
>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>> +                             uint64_t *data)
>>> +{
>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>> +}
>>> +
>>> +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(void)
>>> +{
>>> +    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");
>>> +        exit(0);
>>> +        break;
>>> +    case PANICKED_RESET:
>>> +        panicked_mon_event("reset");
>>> +        qemu_system_reset_request();
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>> +                              uint64_t data)
>>> +{
>>> +    if (data == KVM_PV_PANICKED) {
>>> +        panicked_perform_action();
>>> +    }
>>> +}
>>> +
>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>> +{
>>> +    g_free(iorange);
>>> +}
>>> +
>>> +static IORangeOps pv_io_range_ops = {
>>> +    .read = kvm_pv_port_read,
>>> +    .write = kvm_pv_port_write,
>>> +    .destructor = kvm_pv_port_destructor,
>>> +};
>>> +
>>> +#if defined(KVM_PV_PORT)
>>> +void kvm_pv_port_init(void)
>>> +{
>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>> +
>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>> +    ioport_register(pv_io_range);
>>
>> This modeling is still not ok. We don't open-code ports anymore, we
>> introduce devices. And this doesn't belong inro generic code as it is
> 
> Do you mean introducing a new device instead of I/O port?

I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
and building that device only for target archs that supports it. Already
pointed you to examples in hw/kvm/.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-06-28  8:26         ` Jan Kiszka
@ 2012-07-03  6:07           ` Wen Congyang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-07-03  6:07 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
> On 2012-06-28 03:15, Wen Congyang wrote:
>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>> things according to the parameter -onpanic:
>>>> 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
>>>>
>>>> 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>
>>>> ---
>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  kvm-stub.c      |    9 +++++
>>>>  kvm.h           |    3 ++
>>>>  qemu-options.hx |   15 ++++++++
>>>>  vl.c            |   10 +++++
>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index f8e4328..9494dd2 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -19,6 +19,8 @@
>>>>  #include <stdarg.h>
>>>>  
>>>>  #include <linux/kvm.h>
>>>> +#include <linux/kvm_para.h>
>>>> +#include <asm/kvm_para.h>
>>>>  
>>>>  #include "qemu-common.h"
>>>>  #include "qemu-barrier.h"
>>>> @@ -32,6 +34,9 @@
>>>>  #include "bswap.h"
>>>>  #include "memory.h"
>>>>  #include "exec-memory.h"
>>>> +#include "iorange.h"
>>>> +#include "qemu-objects.h"
>>>> +#include "monitor.h"
>>>>  
>>>>  /* This check must be after config-host.h is included */
>>>>  #ifdef CONFIG_EVENTFD
>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>  {
>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>  }
>>>> +
>>>> +/* 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 */
>>>> +
>>>> +static int panicked_action = PANICKED_REPORT;
>>>> +
>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>> +                             uint64_t *data)
>>>> +{
>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>> +}
>>>> +
>>>> +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(void)
>>>> +{
>>>> +    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");
>>>> +        exit(0);
>>>> +        break;
>>>> +    case PANICKED_RESET:
>>>> +        panicked_mon_event("reset");
>>>> +        qemu_system_reset_request();
>>>> +        break;
>>>> +    }
>>>> +}
>>>> +
>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>> +                              uint64_t data)
>>>> +{
>>>> +    if (data == KVM_PV_PANICKED) {
>>>> +        panicked_perform_action();
>>>> +    }
>>>> +}
>>>> +
>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>> +{
>>>> +    g_free(iorange);
>>>> +}
>>>> +
>>>> +static IORangeOps pv_io_range_ops = {
>>>> +    .read = kvm_pv_port_read,
>>>> +    .write = kvm_pv_port_write,
>>>> +    .destructor = kvm_pv_port_destructor,
>>>> +};
>>>> +
>>>> +#if defined(KVM_PV_PORT)
>>>> +void kvm_pv_port_init(void)
>>>> +{
>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>> +
>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>> +    ioport_register(pv_io_range);
>>>
>>> This modeling is still not ok. We don't open-code ports anymore, we
>>> introduce devices. And this doesn't belong inro generic code as it is
>>
>> Do you mean introducing a new device instead of I/O port?
> 
> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device

A QOM device? Do you mean introduce a new device? If so, the guest should
have a driver to know such device. Another problem is: we cannot use
such device when the kernel is starting(the device's driver is not ready).
If we use a new device, I think virtio-serial is enough. The reason why
I do not use virtio-serial is: I want the feature can also work when the
kernel is starting.

Thanks
Wen Congyang
> and building that device only for target archs that supports it. Already
> pointed you to examples in hw/kvm/.
> 
> Jan
> 


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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-03  6:07           ` Wen Congyang
  0 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-07-03  6:07 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
> On 2012-06-28 03:15, Wen Congyang wrote:
>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>> things according to the parameter -onpanic:
>>>> 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
>>>>
>>>> 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>
>>>> ---
>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  kvm-stub.c      |    9 +++++
>>>>  kvm.h           |    3 ++
>>>>  qemu-options.hx |   15 ++++++++
>>>>  vl.c            |   10 +++++
>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index f8e4328..9494dd2 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -19,6 +19,8 @@
>>>>  #include <stdarg.h>
>>>>  
>>>>  #include <linux/kvm.h>
>>>> +#include <linux/kvm_para.h>
>>>> +#include <asm/kvm_para.h>
>>>>  
>>>>  #include "qemu-common.h"
>>>>  #include "qemu-barrier.h"
>>>> @@ -32,6 +34,9 @@
>>>>  #include "bswap.h"
>>>>  #include "memory.h"
>>>>  #include "exec-memory.h"
>>>> +#include "iorange.h"
>>>> +#include "qemu-objects.h"
>>>> +#include "monitor.h"
>>>>  
>>>>  /* This check must be after config-host.h is included */
>>>>  #ifdef CONFIG_EVENTFD
>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>  {
>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>  }
>>>> +
>>>> +/* 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 */
>>>> +
>>>> +static int panicked_action = PANICKED_REPORT;
>>>> +
>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>> +                             uint64_t *data)
>>>> +{
>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>> +}
>>>> +
>>>> +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(void)
>>>> +{
>>>> +    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");
>>>> +        exit(0);
>>>> +        break;
>>>> +    case PANICKED_RESET:
>>>> +        panicked_mon_event("reset");
>>>> +        qemu_system_reset_request();
>>>> +        break;
>>>> +    }
>>>> +}
>>>> +
>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>> +                              uint64_t data)
>>>> +{
>>>> +    if (data == KVM_PV_PANICKED) {
>>>> +        panicked_perform_action();
>>>> +    }
>>>> +}
>>>> +
>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>> +{
>>>> +    g_free(iorange);
>>>> +}
>>>> +
>>>> +static IORangeOps pv_io_range_ops = {
>>>> +    .read = kvm_pv_port_read,
>>>> +    .write = kvm_pv_port_write,
>>>> +    .destructor = kvm_pv_port_destructor,
>>>> +};
>>>> +
>>>> +#if defined(KVM_PV_PORT)
>>>> +void kvm_pv_port_init(void)
>>>> +{
>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>> +
>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>> +    ioport_register(pv_io_range);
>>>
>>> This modeling is still not ok. We don't open-code ports anymore, we
>>> introduce devices. And this doesn't belong inro generic code as it is
>>
>> Do you mean introducing a new device instead of I/O port?
> 
> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device

A QOM device? Do you mean introduce a new device? If so, the guest should
have a driver to know such device. Another problem is: we cannot use
such device when the kernel is starting(the device's driver is not ready).
If we use a new device, I think virtio-serial is enough. The reason why
I do not use virtio-serial is: I want the feature can also work when the
kernel is starting.

Thanks
Wen Congyang
> and building that device only for target archs that supports it. Already
> pointed you to examples in hw/kvm/.
> 
> Jan
> 

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-07-03  6:07           ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-07-03  6:36             ` Jan Kiszka
  -1 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-07-03  6:36 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

On 2012-07-03 08:07, Wen Congyang wrote:
> At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
>> On 2012-06-28 03:15, Wen Congyang wrote:
>>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>>> things according to the parameter -onpanic:
>>>>> 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
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  kvm-stub.c      |    9 +++++
>>>>>  kvm.h           |    3 ++
>>>>>  qemu-options.hx |   15 ++++++++
>>>>>  vl.c            |   10 +++++
>>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>> index f8e4328..9494dd2 100644
>>>>> --- a/kvm-all.c
>>>>> +++ b/kvm-all.c
>>>>> @@ -19,6 +19,8 @@
>>>>>  #include <stdarg.h>
>>>>>  
>>>>>  #include <linux/kvm.h>
>>>>> +#include <linux/kvm_para.h>
>>>>> +#include <asm/kvm_para.h>
>>>>>  
>>>>>  #include "qemu-common.h"
>>>>>  #include "qemu-barrier.h"
>>>>> @@ -32,6 +34,9 @@
>>>>>  #include "bswap.h"
>>>>>  #include "memory.h"
>>>>>  #include "exec-memory.h"
>>>>> +#include "iorange.h"
>>>>> +#include "qemu-objects.h"
>>>>> +#include "monitor.h"
>>>>>  
>>>>>  /* This check must be after config-host.h is included */
>>>>>  #ifdef CONFIG_EVENTFD
>>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>>  {
>>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>>  }
>>>>> +
>>>>> +/* 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 */
>>>>> +
>>>>> +static int panicked_action = PANICKED_REPORT;
>>>>> +
>>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                             uint64_t *data)
>>>>> +{
>>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>>> +}
>>>>> +
>>>>> +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(void)
>>>>> +{
>>>>> +    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");
>>>>> +        exit(0);
>>>>> +        break;
>>>>> +    case PANICKED_RESET:
>>>>> +        panicked_mon_event("reset");
>>>>> +        qemu_system_reset_request();
>>>>> +        break;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                              uint64_t data)
>>>>> +{
>>>>> +    if (data == KVM_PV_PANICKED) {
>>>>> +        panicked_perform_action();
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>>> +{
>>>>> +    g_free(iorange);
>>>>> +}
>>>>> +
>>>>> +static IORangeOps pv_io_range_ops = {
>>>>> +    .read = kvm_pv_port_read,
>>>>> +    .write = kvm_pv_port_write,
>>>>> +    .destructor = kvm_pv_port_destructor,
>>>>> +};
>>>>> +
>>>>> +#if defined(KVM_PV_PORT)
>>>>> +void kvm_pv_port_init(void)
>>>>> +{
>>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>>> +
>>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>>> +    ioport_register(pv_io_range);
>>>>
>>>> This modeling is still not ok. We don't open-code ports anymore, we
>>>> introduce devices. And this doesn't belong inro generic code as it is
>>>
>>> Do you mean introducing a new device instead of I/O port?
>>
>> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
> 
> A QOM device? Do you mean introduce a new device? If so, the guest should
> have a driver to know such device. Another problem is: we cannot use
> such device when the kernel is starting(the device's driver is not ready).
> If we use a new device, I think virtio-serial is enough. The reason why
> I do not use virtio-serial is: I want the feature can also work when the
> kernel is starting.

I'm not talking about changing the interface to the guest, I'm talking
about how to model it in QEMU. And that difference would be transparent
to the guest. I pointed you to examples like hw/kvm/clock.c.

Jan

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

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-03  6:36             ` Jan Kiszka
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-07-03  6:36 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-03 08:07, Wen Congyang wrote:
> At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
>> On 2012-06-28 03:15, Wen Congyang wrote:
>>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>>> things according to the parameter -onpanic:
>>>>> 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
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  kvm-stub.c      |    9 +++++
>>>>>  kvm.h           |    3 ++
>>>>>  qemu-options.hx |   15 ++++++++
>>>>>  vl.c            |   10 +++++
>>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>> index f8e4328..9494dd2 100644
>>>>> --- a/kvm-all.c
>>>>> +++ b/kvm-all.c
>>>>> @@ -19,6 +19,8 @@
>>>>>  #include <stdarg.h>
>>>>>  
>>>>>  #include <linux/kvm.h>
>>>>> +#include <linux/kvm_para.h>
>>>>> +#include <asm/kvm_para.h>
>>>>>  
>>>>>  #include "qemu-common.h"
>>>>>  #include "qemu-barrier.h"
>>>>> @@ -32,6 +34,9 @@
>>>>>  #include "bswap.h"
>>>>>  #include "memory.h"
>>>>>  #include "exec-memory.h"
>>>>> +#include "iorange.h"
>>>>> +#include "qemu-objects.h"
>>>>> +#include "monitor.h"
>>>>>  
>>>>>  /* This check must be after config-host.h is included */
>>>>>  #ifdef CONFIG_EVENTFD
>>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>>  {
>>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>>  }
>>>>> +
>>>>> +/* 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 */
>>>>> +
>>>>> +static int panicked_action = PANICKED_REPORT;
>>>>> +
>>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                             uint64_t *data)
>>>>> +{
>>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>>> +}
>>>>> +
>>>>> +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(void)
>>>>> +{
>>>>> +    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");
>>>>> +        exit(0);
>>>>> +        break;
>>>>> +    case PANICKED_RESET:
>>>>> +        panicked_mon_event("reset");
>>>>> +        qemu_system_reset_request();
>>>>> +        break;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                              uint64_t data)
>>>>> +{
>>>>> +    if (data == KVM_PV_PANICKED) {
>>>>> +        panicked_perform_action();
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>>> +{
>>>>> +    g_free(iorange);
>>>>> +}
>>>>> +
>>>>> +static IORangeOps pv_io_range_ops = {
>>>>> +    .read = kvm_pv_port_read,
>>>>> +    .write = kvm_pv_port_write,
>>>>> +    .destructor = kvm_pv_port_destructor,
>>>>> +};
>>>>> +
>>>>> +#if defined(KVM_PV_PORT)
>>>>> +void kvm_pv_port_init(void)
>>>>> +{
>>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>>> +
>>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>>> +    ioport_register(pv_io_range);
>>>>
>>>> This modeling is still not ok. We don't open-code ports anymore, we
>>>> introduce devices. And this doesn't belong inro generic code as it is
>>>
>>> Do you mean introducing a new device instead of I/O port?
>>
>> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
> 
> A QOM device? Do you mean introduce a new device? If so, the guest should
> have a driver to know such device. Another problem is: we cannot use
> such device when the kernel is starting(the device's driver is not ready).
> If we use a new device, I think virtio-serial is enough. The reason why
> I do not use virtio-serial is: I want the feature can also work when the
> kernel is starting.

I'm not talking about changing the interface to the guest, I'm talking
about how to model it in QEMU. And that difference would be transparent
to the guest. I pointed you to examples like hw/kvm/clock.c.

Jan

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

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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-03  6:36             ` Jan Kiszka
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-07-03  6:36 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-03 08:07, Wen Congyang wrote:
> At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
>> On 2012-06-28 03:15, Wen Congyang wrote:
>>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>>> things according to the parameter -onpanic:
>>>>> 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
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  kvm-stub.c      |    9 +++++
>>>>>  kvm.h           |    3 ++
>>>>>  qemu-options.hx |   15 ++++++++
>>>>>  vl.c            |   10 +++++
>>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>> index f8e4328..9494dd2 100644
>>>>> --- a/kvm-all.c
>>>>> +++ b/kvm-all.c
>>>>> @@ -19,6 +19,8 @@
>>>>>  #include <stdarg.h>
>>>>>  
>>>>>  #include <linux/kvm.h>
>>>>> +#include <linux/kvm_para.h>
>>>>> +#include <asm/kvm_para.h>
>>>>>  
>>>>>  #include "qemu-common.h"
>>>>>  #include "qemu-barrier.h"
>>>>> @@ -32,6 +34,9 @@
>>>>>  #include "bswap.h"
>>>>>  #include "memory.h"
>>>>>  #include "exec-memory.h"
>>>>> +#include "iorange.h"
>>>>> +#include "qemu-objects.h"
>>>>> +#include "monitor.h"
>>>>>  
>>>>>  /* This check must be after config-host.h is included */
>>>>>  #ifdef CONFIG_EVENTFD
>>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>>  {
>>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>>  }
>>>>> +
>>>>> +/* 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 */
>>>>> +
>>>>> +static int panicked_action = PANICKED_REPORT;
>>>>> +
>>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                             uint64_t *data)
>>>>> +{
>>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>>> +}
>>>>> +
>>>>> +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(void)
>>>>> +{
>>>>> +    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");
>>>>> +        exit(0);
>>>>> +        break;
>>>>> +    case PANICKED_RESET:
>>>>> +        panicked_mon_event("reset");
>>>>> +        qemu_system_reset_request();
>>>>> +        break;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>>> +                              uint64_t data)
>>>>> +{
>>>>> +    if (data == KVM_PV_PANICKED) {
>>>>> +        panicked_perform_action();
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>>> +{
>>>>> +    g_free(iorange);
>>>>> +}
>>>>> +
>>>>> +static IORangeOps pv_io_range_ops = {
>>>>> +    .read = kvm_pv_port_read,
>>>>> +    .write = kvm_pv_port_write,
>>>>> +    .destructor = kvm_pv_port_destructor,
>>>>> +};
>>>>> +
>>>>> +#if defined(KVM_PV_PORT)
>>>>> +void kvm_pv_port_init(void)
>>>>> +{
>>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>>> +
>>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>>> +    ioport_register(pv_io_range);
>>>>
>>>> This modeling is still not ok. We don't open-code ports anymore, we
>>>> introduce devices. And this doesn't belong inro generic code as it is
>>>
>>> Do you mean introducing a new device instead of I/O port?
>>
>> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
> 
> A QOM device? Do you mean introduce a new device? If so, the guest should
> have a driver to know such device. Another problem is: we cannot use
> such device when the kernel is starting(the device's driver is not ready).
> If we use a new device, I think virtio-serial is enough. The reason why
> I do not use virtio-serial is: I want the feature can also work when the
> kernel is starting.

I'm not talking about changing the interface to the guest, I'm talking
about how to model it in QEMU. And that difference would be transparent
to the guest. I pointed you to examples like hw/kvm/clock.c.

Jan

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

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-07-03  6:36             ` Jan Kiszka
@ 2012-07-03  6:43               ` Wen Congyang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-07-03  6:43 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 07/03/2012 02:36 PM, Jan Kiszka Wrote:
> On 2012-07-03 08:07, Wen Congyang wrote:
>> At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
>>> On 2012-06-28 03:15, Wen Congyang wrote:
>>>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>>>> things according to the parameter -onpanic:
>>>>>> 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
>>>>>>
>>>>>> 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>
>>>>>> ---
>>>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  kvm-stub.c      |    9 +++++
>>>>>>  kvm.h           |    3 ++
>>>>>>  qemu-options.hx |   15 ++++++++
>>>>>>  vl.c            |   10 +++++
>>>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>>> index f8e4328..9494dd2 100644
>>>>>> --- a/kvm-all.c
>>>>>> +++ b/kvm-all.c
>>>>>> @@ -19,6 +19,8 @@
>>>>>>  #include <stdarg.h>
>>>>>>  
>>>>>>  #include <linux/kvm.h>
>>>>>> +#include <linux/kvm_para.h>
>>>>>> +#include <asm/kvm_para.h>
>>>>>>  
>>>>>>  #include "qemu-common.h"
>>>>>>  #include "qemu-barrier.h"
>>>>>> @@ -32,6 +34,9 @@
>>>>>>  #include "bswap.h"
>>>>>>  #include "memory.h"
>>>>>>  #include "exec-memory.h"
>>>>>> +#include "iorange.h"
>>>>>> +#include "qemu-objects.h"
>>>>>> +#include "monitor.h"
>>>>>>  
>>>>>>  /* This check must be after config-host.h is included */
>>>>>>  #ifdef CONFIG_EVENTFD
>>>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>>>  {
>>>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>>>  }
>>>>>> +
>>>>>> +/* 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 */
>>>>>> +
>>>>>> +static int panicked_action = PANICKED_REPORT;
>>>>>> +
>>>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>>>> +                             uint64_t *data)
>>>>>> +{
>>>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>>>> +}
>>>>>> +
>>>>>> +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(void)
>>>>>> +{
>>>>>> +    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");
>>>>>> +        exit(0);
>>>>>> +        break;
>>>>>> +    case PANICKED_RESET:
>>>>>> +        panicked_mon_event("reset");
>>>>>> +        qemu_system_reset_request();
>>>>>> +        break;
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>>>> +                              uint64_t data)
>>>>>> +{
>>>>>> +    if (data == KVM_PV_PANICKED) {
>>>>>> +        panicked_perform_action();
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>>>> +{
>>>>>> +    g_free(iorange);
>>>>>> +}
>>>>>> +
>>>>>> +static IORangeOps pv_io_range_ops = {
>>>>>> +    .read = kvm_pv_port_read,
>>>>>> +    .write = kvm_pv_port_write,
>>>>>> +    .destructor = kvm_pv_port_destructor,
>>>>>> +};
>>>>>> +
>>>>>> +#if defined(KVM_PV_PORT)
>>>>>> +void kvm_pv_port_init(void)
>>>>>> +{
>>>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>>>> +
>>>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>>>> +    ioport_register(pv_io_range);
>>>>>
>>>>> This modeling is still not ok. We don't open-code ports anymore, we
>>>>> introduce devices. And this doesn't belong inro generic code as it is
>>>>
>>>> Do you mean introducing a new device instead of I/O port?
>>>
>>> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
>>
>> A QOM device? Do you mean introduce a new device? If so, the guest should
>> have a driver to know such device. Another problem is: we cannot use
>> such device when the kernel is starting(the device's driver is not ready).
>> If we use a new device, I think virtio-serial is enough. The reason why
>> I do not use virtio-serial is: I want the feature can also work when the
>> kernel is starting.
> 
> I'm not talking about changing the interface to the guest, I'm talking
> about how to model it in QEMU. And that difference would be transparent
> to the guest. I pointed you to examples like hw/kvm/clock.c.

OK, I will read the code in hw/kvm/clock.c

Thanks for your help.

Wen Congyang

> 
> Jan
> 


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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-03  6:43               ` Wen Congyang
  0 siblings, 0 replies; 22+ messages in thread
From: Wen Congyang @ 2012-07-03  6:43 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 07/03/2012 02:36 PM, Jan Kiszka Wrote:
> On 2012-07-03 08:07, Wen Congyang wrote:
>> At 06/28/2012 04:26 PM, Jan Kiszka Wrote:
>>> On 2012-06-28 03:15, Wen Congyang wrote:
>>>> At 06/27/2012 10:39 PM, Jan Kiszka Wrote:
>>>>> On 2012-06-27 09:02, Wen Congyang wrote:
>>>>>> When the guest is panicked, it will write 0x1 to the port KVM_PV_PORT.
>>>>>> So if qemu reads 0x1 from this port, we can do the folloing three
>>>>>> things according to the parameter -onpanic:
>>>>>> 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
>>>>>>
>>>>>> 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>
>>>>>> ---
>>>>>>  kvm-all.c       |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  kvm-stub.c      |    9 +++++
>>>>>>  kvm.h           |    3 ++
>>>>>>  qemu-options.hx |   15 ++++++++
>>>>>>  vl.c            |   10 +++++
>>>>>>  5 files changed, 138 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>>> index f8e4328..9494dd2 100644
>>>>>> --- a/kvm-all.c
>>>>>> +++ b/kvm-all.c
>>>>>> @@ -19,6 +19,8 @@
>>>>>>  #include <stdarg.h>
>>>>>>  
>>>>>>  #include <linux/kvm.h>
>>>>>> +#include <linux/kvm_para.h>
>>>>>> +#include <asm/kvm_para.h>
>>>>>>  
>>>>>>  #include "qemu-common.h"
>>>>>>  #include "qemu-barrier.h"
>>>>>> @@ -32,6 +34,9 @@
>>>>>>  #include "bswap.h"
>>>>>>  #include "memory.h"
>>>>>>  #include "exec-memory.h"
>>>>>> +#include "iorange.h"
>>>>>> +#include "qemu-objects.h"
>>>>>> +#include "monitor.h"
>>>>>>  
>>>>>>  /* This check must be after config-host.h is included */
>>>>>>  #ifdef CONFIG_EVENTFD
>>>>>> @@ -1931,3 +1936,99 @@ int kvm_on_sigbus(int code, void *addr)
>>>>>>  {
>>>>>>      return kvm_arch_on_sigbus(code, addr);
>>>>>>  }
>>>>>> +
>>>>>> +/* 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 */
>>>>>> +
>>>>>> +static int panicked_action = PANICKED_REPORT;
>>>>>> +
>>>>>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>>>>>> +                             uint64_t *data)
>>>>>> +{
>>>>>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>>>>>> +}
>>>>>> +
>>>>>> +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(void)
>>>>>> +{
>>>>>> +    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");
>>>>>> +        exit(0);
>>>>>> +        break;
>>>>>> +    case PANICKED_RESET:
>>>>>> +        panicked_mon_event("reset");
>>>>>> +        qemu_system_reset_request();
>>>>>> +        break;
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>>>>>> +                              uint64_t data)
>>>>>> +{
>>>>>> +    if (data == KVM_PV_PANICKED) {
>>>>>> +        panicked_perform_action();
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void kvm_pv_port_destructor(IORange *iorange)
>>>>>> +{
>>>>>> +    g_free(iorange);
>>>>>> +}
>>>>>> +
>>>>>> +static IORangeOps pv_io_range_ops = {
>>>>>> +    .read = kvm_pv_port_read,
>>>>>> +    .write = kvm_pv_port_write,
>>>>>> +    .destructor = kvm_pv_port_destructor,
>>>>>> +};
>>>>>> +
>>>>>> +#if defined(KVM_PV_PORT)
>>>>>> +void kvm_pv_port_init(void)
>>>>>> +{
>>>>>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>>>>>> +
>>>>>> +    iorange_init(pv_io_range, &pv_io_range_ops, KVM_PV_PORT, 1);
>>>>>> +    ioport_register(pv_io_range);
>>>>>
>>>>> This modeling is still not ok. We don't open-code ports anymore, we
>>>>> introduce devices. And this doesn't belong inro generic code as it is
>>>>
>>>> Do you mean introducing a new device instead of I/O port?
>>>
>>> I mean encapsulating the I/O registration (PIO or MMIO) in a QOM device
>>
>> A QOM device? Do you mean introduce a new device? If so, the guest should
>> have a driver to know such device. Another problem is: we cannot use
>> such device when the kernel is starting(the device's driver is not ready).
>> If we use a new device, I think virtio-serial is enough. The reason why
>> I do not use virtio-serial is: I want the feature can also work when the
>> kernel is starting.
> 
> I'm not talking about changing the interface to the guest, I'm talking
> about how to model it in QEMU. And that difference would be transparent
> to the guest. I pointed you to examples like hw/kvm/clock.c.

OK, I will read the code in hw/kvm/clock.c

Thanks for your help.

Wen Congyang

> 
> Jan
> 

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

* Re: [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
  2012-07-03  6:43               ` [Qemu-devel] " Wen Congyang
@ 2012-07-03  6:45                 ` Jan Kiszka
  -1 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-07-03  6:45 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

On 2012-07-03 08:43, Wen Congyang wrote:
>> I'm not talking about changing the interface to the guest, I'm talking
>> about how to model it in QEMU. And that difference would be transparent
>> to the guest. I pointed you to examples like hw/kvm/clock.c.
> 
> OK, I will read the code in hw/kvm/clock.c

Just to avoid confusion: That example is just good for a trivial
framework. It does vmstate saving, something you don't need as your
"device" is stateless. If you want to find out how to register PIO
ranges, also check e.g. hw/pcspk.c.

Jan

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



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

* Re: [Qemu-devel] [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-03  6:45                 ` Jan Kiszka
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2012-07-03  6:45 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-03 08:43, Wen Congyang wrote:
>> I'm not talking about changing the interface to the guest, I'm talking
>> about how to model it in QEMU. And that difference would be transparent
>> to the guest. I pointed you to examples like hw/kvm/clock.c.
> 
> OK, I will read the code in hw/kvm/clock.c

Just to avoid confusion: That example is just good for a trivial
framework. It does vmstate saving, something you don't need as your
"device" is stateless. If you want to find out how to register PIO
ranges, also check e.g. hw/pcspk.c.

Jan

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

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

end of thread, other threads:[~2012-07-03  6:45 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-27  6:55 [PATCH v5] kvm: notify host when the guest is panicked Wen Congyang
2012-06-27  6:57 ` [PATCH 1/6 v5] start vm after reseting it Wen Congyang
2012-06-27  6:58 ` [PATCH 2/6 v5] update linux headers Wen Congyang
2012-06-27  7:00 ` [PATCH 3/6 v5] add a new runstate: RUN_STATE_GUEST_PANICKED Wen Congyang
2012-06-27  7:01 ` [PATCH 4/6 v5] add a new qevent: QEVENT_GUEST_PANICKED Wen Congyang
2012-06-27  7:02 ` [PATCH 5/6 v5] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
2012-06-27 14:39   ` Jan Kiszka
2012-06-28  1:15     ` Wen Congyang
2012-06-28  8:26       ` Jan Kiszka
2012-06-28  8:26         ` Jan Kiszka
2012-07-03  6:07         ` Wen Congyang
2012-07-03  6:07           ` [Qemu-devel] " Wen Congyang
2012-07-03  6:36           ` Jan Kiszka
2012-07-03  6:36             ` [Qemu-devel] " Jan Kiszka
2012-07-03  6:36             ` Jan Kiszka
2012-07-03  6:43             ` Wen Congyang
2012-07-03  6:43               ` [Qemu-devel] " Wen Congyang
2012-07-03  6:45               ` Jan Kiszka
2012-07-03  6:45                 ` [Qemu-devel] " Jan Kiszka
2012-06-27 14:52   ` Cornelia Huck
2012-06-27 14:57     ` Daniel P. Berrange
2012-06-27  7:04 ` [PATCH 6/6 v5] deal with panicked event accoring to '-machine panic_action=action' Wen Congyang

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