All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6] kvm: notify host when the guest is panicked
@ 2012-07-06  9:36 ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:36 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] 44+ messages in thread

* [Qemu-devel] [PATCH v6] kvm: notify host when the guest is panicked
@ 2012-07-06  9:36 ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:36 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] 44+ messages in thread

* [PATCH 1/7 v6] start vm after reseting it
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:38   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:38 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.

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

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

diff --git a/block.h b/block.h
index 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..898987c 100644
--- a/vl.c
+++ b/vl.c
@@ -331,7 +331,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH },
 
-    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_RUNNING },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
@@ -364,7 +364,7 @@ static const RunStateTransition runstate_transitions_def[] = {
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
-    { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
@@ -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] 44+ messages in thread

* [Qemu-devel] [PATCH 1/7 v6] start vm after reseting it
@ 2012-07-06  9:38   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:38 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.

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

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

diff --git a/block.h b/block.h
index 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..898987c 100644
--- a/vl.c
+++ b/vl.c
@@ -331,7 +331,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH },
 
-    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_RUNNING },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
@@ -364,7 +364,7 @@ static const RunStateTransition runstate_transitions_def[] = {
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
-    { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
@@ -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] 44+ messages in thread

* [PATCH 2/7 v6] update linux headers
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:38   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:38 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] 44+ messages in thread

* [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-06  9:38   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:38 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] 44+ messages in thread

* [PATCH 3/7 v6] add a new runstate: RUN_STATE_GUEST_PANICKED
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:39   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:39 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

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

diff --git a/qapi-schema.json b/qapi-schema.json
index 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 898987c..ea5ef1c 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_RUNNING },
+    { 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] 44+ messages in thread

* [Qemu-devel] [PATCH 3/7 v6] add a new runstate: RUN_STATE_GUEST_PANICKED
@ 2012-07-06  9:39   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:39 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

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

diff --git a/qapi-schema.json b/qapi-schema.json
index 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 898987c..ea5ef1c 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_RUNNING },
+    { 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] 44+ messages in thread

* [PATCH 4/7 v6] add a new qevent: QEVENT_GUEST_PANICKED
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:40   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:40 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

This event will be emited when the guest is panicked.

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

diff --git a/monitor.c b/monitor.c
index 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] 44+ messages in thread

* [Qemu-devel] [PATCH 4/7 v6] add a new qevent: QEVENT_GUEST_PANICKED
@ 2012-07-06  9:40   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:40 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

This event will be emited when the guest is panicked.

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

diff --git a/monitor.c b/monitor.c
index 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] 44+ messages in thread

* [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:41   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:41 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

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

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/kvm/Makefile.objs |    2 +-
 hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
 hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c           |    9 +++
 kvm.h                |    3 +
 vl.c                 |    4 ++
 6 files changed, 223 insertions(+), 1 deletions(-)
 create mode 100644 hw/kvm/pv_event.c
 create mode 100644 hw/kvm/pv_ioport.c

diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
index 226497a..23e3b30 100644
--- a/hw/kvm/Makefile.objs
+++ b/hw/kvm/Makefile.objs
@@ -1 +1 @@
-obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
+obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
new file mode 100644
index 0000000..d7ded37
--- /dev/null
+++ b/hw/kvm/pv_event.c
@@ -0,0 +1,73 @@
+/*
+ * QEMU KVM support, paravirtual event device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <linux/kvm_para.h>
+#include <asm/kvm_para.h>
+#include <qobject.h>
+#include <qjson.h>
+#include <monitor.h>
+#include <sysemu.h>
+#include <kvm.h>
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void panicked_mon_event(const char *action)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'action': %s }", action);
+    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+    qobject_decref(data);
+}
+
+static void panicked_perform_action(uint32_t panicked_action)
+{
+    switch (panicked_action) {
+    case PANICKED_REPORT:
+        panicked_mon_event("report");
+        break;
+
+    case PANICKED_PAUSE:
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        break;
+
+    case PANICKED_POWEROFF:
+        panicked_mon_event("poweroff");
+        exit(0);
+        break;
+    case PANICKED_RESET:
+        panicked_mon_event("reset");
+        qemu_system_reset_request();
+        break;
+    }
+}
+
+#if defined(KVM_PV_PORT)
+#include "pv_ioport.c"
+
+void kvm_pv_event_init(void)
+{
+    pv_ioport_init(panicked_action);
+}
+#else
+void kvm_pv_event_init(void)
+{
+}
+#endif
diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
new file mode 100644
index 0000000..e93d819
--- /dev/null
+++ b/hw/kvm/pv_ioport.c
@@ -0,0 +1,133 @@
+/*
+ * QEMU KVM support, paravirtual I/O port device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/isa.h"
+
+typedef struct {
+    ISADevice dev;
+    MemoryRegion ioport;
+    uint32_t panicked_action;
+} PVState;
+
+static PVState *pv_state;
+
+static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
+{
+    return 1 << KVM_PV_FEATURE_PANICKED;
+}
+
+static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
+{
+    PVState *s = opaque;
+
+    if (val == KVM_PV_PANICKED) {
+        panicked_perform_action(s->panicked_action);
+    }
+}
+
+static const MemoryRegionOps pv_io_ops = {
+    .read = pv_io_read,
+    .write = pv_io_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static int pv_ioport_initfn(ISADevice *dev)
+{
+    PVState *s = DO_UPCAST(PVState, dev, dev);
+
+    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
+    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
+
+    pv_state = s;
+
+    return 0;
+}
+
+static const VMStateDescription pv_ioport_vmsd = {
+    .name = "pv_ioport",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(panicked_action, PVState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property pv_ioport_properties[] = {
+    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pv_ioport_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+
+    ic->init = pv_ioport_initfn;
+    dc->no_user = 1;
+    dc->vmsd = &pv_ioport_vmsd;
+    dc->props = pv_ioport_properties;
+}
+
+static TypeInfo pv_ioport_info = {
+    .name          = "kvm_pv_ioport",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(PVState),
+    .class_init    = pv_ioport_class_init,
+};
+
+static void pv_ioport_register_types(void)
+{
+    type_register_static(&pv_ioport_info);
+}
+
+type_init(pv_ioport_register_types)
+
+static int is_isa_bus(BusState *bus, void *opaque)
+{
+    const char *bus_type_name;
+    ISABus **isa_bus_p = opaque;
+
+    bus_type_name = object_class_get_name(bus->obj.class);
+    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
+        *isa_bus_p = ISA_BUS(&bus->obj);
+        return -1;
+    }
+
+    return 0;
+}
+
+static ISABus *get_isa_bus(void)
+{
+    ISABus *isa_bus = NULL;
+
+    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
+
+    return isa_bus;
+}
+
+static void pv_ioport_init(uint32_t panicked_action)
+{
+    ISADevice *dev;
+    ISABus *bus;
+
+    bus = get_isa_bus();
+    dev = isa_create(bus, "kvm_pv_ioport");
+    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);
+    qdev_init_nofail(&dev->qdev);
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index ec9a364..a28d078 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_event_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+    return 0;
+}
diff --git a/kvm.h b/kvm.h
index 9c7b0ea..1f7c72b 100644
--- a/kvm.h
+++ b/kvm.h
@@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
+
+void kvm_pv_event_init(void);
+int select_panicked_action(const char *p);
 #endif
diff --git a/vl.c b/vl.c
index ea5ef1c..f5cd28d 100644
--- a/vl.c
+++ b/vl.c
@@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (kvm_enabled()) {
+        kvm_pv_event_init();
+    }
+
     qdev_machine_creation_done();
 
     if (rom_load_all() != 0) {
-- 
1.7.1


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

* [Qemu-devel] [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-06  9:41   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:41 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

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

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/kvm/Makefile.objs |    2 +-
 hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
 hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c           |    9 +++
 kvm.h                |    3 +
 vl.c                 |    4 ++
 6 files changed, 223 insertions(+), 1 deletions(-)
 create mode 100644 hw/kvm/pv_event.c
 create mode 100644 hw/kvm/pv_ioport.c

diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
index 226497a..23e3b30 100644
--- a/hw/kvm/Makefile.objs
+++ b/hw/kvm/Makefile.objs
@@ -1 +1 @@
-obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
+obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
new file mode 100644
index 0000000..d7ded37
--- /dev/null
+++ b/hw/kvm/pv_event.c
@@ -0,0 +1,73 @@
+/*
+ * QEMU KVM support, paravirtual event device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <linux/kvm_para.h>
+#include <asm/kvm_para.h>
+#include <qobject.h>
+#include <qjson.h>
+#include <monitor.h>
+#include <sysemu.h>
+#include <kvm.h>
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void panicked_mon_event(const char *action)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'action': %s }", action);
+    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+    qobject_decref(data);
+}
+
+static void panicked_perform_action(uint32_t panicked_action)
+{
+    switch (panicked_action) {
+    case PANICKED_REPORT:
+        panicked_mon_event("report");
+        break;
+
+    case PANICKED_PAUSE:
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        break;
+
+    case PANICKED_POWEROFF:
+        panicked_mon_event("poweroff");
+        exit(0);
+        break;
+    case PANICKED_RESET:
+        panicked_mon_event("reset");
+        qemu_system_reset_request();
+        break;
+    }
+}
+
+#if defined(KVM_PV_PORT)
+#include "pv_ioport.c"
+
+void kvm_pv_event_init(void)
+{
+    pv_ioport_init(panicked_action);
+}
+#else
+void kvm_pv_event_init(void)
+{
+}
+#endif
diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
new file mode 100644
index 0000000..e93d819
--- /dev/null
+++ b/hw/kvm/pv_ioport.c
@@ -0,0 +1,133 @@
+/*
+ * QEMU KVM support, paravirtual I/O port device
+ *
+ * Copyright Fujitsu, Corp. 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/isa.h"
+
+typedef struct {
+    ISADevice dev;
+    MemoryRegion ioport;
+    uint32_t panicked_action;
+} PVState;
+
+static PVState *pv_state;
+
+static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
+{
+    return 1 << KVM_PV_FEATURE_PANICKED;
+}
+
+static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
+{
+    PVState *s = opaque;
+
+    if (val == KVM_PV_PANICKED) {
+        panicked_perform_action(s->panicked_action);
+    }
+}
+
+static const MemoryRegionOps pv_io_ops = {
+    .read = pv_io_read,
+    .write = pv_io_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static int pv_ioport_initfn(ISADevice *dev)
+{
+    PVState *s = DO_UPCAST(PVState, dev, dev);
+
+    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
+    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
+
+    pv_state = s;
+
+    return 0;
+}
+
+static const VMStateDescription pv_ioport_vmsd = {
+    .name = "pv_ioport",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(panicked_action, PVState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property pv_ioport_properties[] = {
+    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pv_ioport_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+
+    ic->init = pv_ioport_initfn;
+    dc->no_user = 1;
+    dc->vmsd = &pv_ioport_vmsd;
+    dc->props = pv_ioport_properties;
+}
+
+static TypeInfo pv_ioport_info = {
+    .name          = "kvm_pv_ioport",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(PVState),
+    .class_init    = pv_ioport_class_init,
+};
+
+static void pv_ioport_register_types(void)
+{
+    type_register_static(&pv_ioport_info);
+}
+
+type_init(pv_ioport_register_types)
+
+static int is_isa_bus(BusState *bus, void *opaque)
+{
+    const char *bus_type_name;
+    ISABus **isa_bus_p = opaque;
+
+    bus_type_name = object_class_get_name(bus->obj.class);
+    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
+        *isa_bus_p = ISA_BUS(&bus->obj);
+        return -1;
+    }
+
+    return 0;
+}
+
+static ISABus *get_isa_bus(void)
+{
+    ISABus *isa_bus = NULL;
+
+    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
+
+    return isa_bus;
+}
+
+static void pv_ioport_init(uint32_t panicked_action)
+{
+    ISADevice *dev;
+    ISABus *bus;
+
+    bus = get_isa_bus();
+    dev = isa_create(bus, "kvm_pv_ioport");
+    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);
+    qdev_init_nofail(&dev->qdev);
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index ec9a364..a28d078 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_event_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+    return 0;
+}
diff --git a/kvm.h b/kvm.h
index 9c7b0ea..1f7c72b 100644
--- a/kvm.h
+++ b/kvm.h
@@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
+
+void kvm_pv_event_init(void);
+int select_panicked_action(const char *p);
 #endif
diff --git a/vl.c b/vl.c
index ea5ef1c..f5cd28d 100644
--- a/vl.c
+++ b/vl.c
@@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (kvm_enabled()) {
+        kvm_pv_event_init();
+    }
+
     qdev_machine_creation_done();
 
     if (rom_load_all() != 0) {
-- 
1.7.1

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

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

The onpanic parameter can have the following value:
1. none
2. pause
3. poweroff
4. reset

The action for each value when the guest is panicked:
1. none: emit QEVENT_GUEST_PANICKED only
2. pause: emit QEVENT_GUEST_PANICKED and pause the guest
3. poweroff: emit QEVENT_GUEST_PANICKED and poweroff the guest
4. reset: emit QEVENT_GUEST_PANICKED and reset the guest

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/kvm/pv_event.c |   17 +++++++++++++++++
 qemu-options.hx   |   15 +++++++++++++++
 vl.c              |    6 ++++++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
index d7ded37..890abcd 100644
--- a/hw/kvm/pv_event.c
+++ b/hw/kvm/pv_event.c
@@ -59,6 +59,23 @@ static void panicked_perform_action(uint32_t panicked_action)
     }
 }
 
+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;
+}
+
 #if defined(KVM_PV_PORT)
 #include "pv_ioport.c"
 
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 f5cd28d..1a68257 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);
             }
-- 
1.7.1


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

* [Qemu-devel] [PATCH 6/7 v6] deal with guest panicked event accoring to -onpanic parameter
@ 2012-07-06  9:41   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:41 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

The onpanic parameter can have the following value:
1. none
2. pause
3. poweroff
4. reset

The action for each value when the guest is panicked:
1. none: emit QEVENT_GUEST_PANICKED only
2. pause: emit QEVENT_GUEST_PANICKED and pause the guest
3. poweroff: emit QEVENT_GUEST_PANICKED and poweroff the guest
4. reset: emit QEVENT_GUEST_PANICKED and reset the guest

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/kvm/pv_event.c |   17 +++++++++++++++++
 qemu-options.hx   |   15 +++++++++++++++
 vl.c              |    6 ++++++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
index d7ded37..890abcd 100644
--- a/hw/kvm/pv_event.c
+++ b/hw/kvm/pv_event.c
@@ -59,6 +59,23 @@ static void panicked_perform_action(uint32_t panicked_action)
     }
 }
 
+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;
+}
+
 #if defined(KVM_PV_PORT)
 #include "pv_ioport.c"
 
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 f5cd28d..1a68257 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);
             }
-- 
1.7.1

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

* [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
  2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
@ 2012-07-06  9:41   ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:41 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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 1a68257..091c43b 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] 44+ messages in thread

* [Qemu-devel] [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
@ 2012-07-06  9:41   ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06  9:41 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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 1a68257..091c43b 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] 44+ messages in thread

* Re: [PATCH 2/7 v6] update linux headers
  2012-07-06  9:38   ` [Qemu-devel] " Wen Congyang
@ 2012-07-06 10:25     ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 10:25 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-06 11:38, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Which kvm.git hash is this referring? Please state this to avoid that we
are merging support for kernel features that are still under review.

Jan

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

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



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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-06 10:25     ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 10:25 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-06 11:38, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Which kvm.git hash is this referring? Please state this to avoid that we
are merging support for kernel features that are still under review.

Jan

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

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

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

* Re: [PATCH 2/7 v6] update linux headers
  2012-07-06 10:25     ` [Qemu-devel] " Jan Kiszka
  (?)
@ 2012-07-06 10:50       ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06 10:50 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 07/06/2012 06:25 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:38, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> Which kvm.git hash is this referring? Please state this to avoid that we
> are merging support for kernel features that are still under review.

The following kvm.git:
http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary

I apply my kvm's patch, and use ./scripts/update-linux-headers.sh to
generate this patch.

Thanks
Wen Congyang

> 
> Jan
> 
>> ---
>>  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
>>   */
>>
> 


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

* Re: [PATCH 2/7 v6] update linux headers
@ 2012-07-06 10:50       ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06 10:50 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 07/06/2012 06:25 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:38, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> Which kvm.git hash is this referring? Please state this to avoid that we
> are merging support for kernel features that are still under review.

The following kvm.git:
http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary

I apply my kvm's patch, and use ./scripts/update-linux-headers.sh to
generate this patch.

Thanks
Wen Congyang

> 
> Jan
> 
>> ---
>>  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
>>   */
>>
> 

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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-06 10:50       ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-06 10:50 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 07/06/2012 06:25 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:38, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> Which kvm.git hash is this referring? Please state this to avoid that we
> are merging support for kernel features that are still under review.

The following kvm.git:
http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary

I apply my kvm's patch, and use ./scripts/update-linux-headers.sh to
generate this patch.

Thanks
Wen Congyang

> 
> Jan
> 
>> ---
>>  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
>>   */
>>
> 

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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
  2012-07-06 10:50       ` Wen Congyang
@ 2012-07-06 10:55         ` 陳韋任 (Wei-Ren Chen)
  -1 siblings, 0 replies; 44+ messages in thread
From: 陳韋任 (Wei-Ren Chen) @ 2012-07-06 10:55 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Jan Kiszka, Gleb Natapov, kvm list, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

> > Which kvm.git hash is this referring? Please state this to avoid that we
> > are merging support for kernel features that are still under review.
> 
> The following kvm.git:
> http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary

  What Jia ask for is git hash, should be something like,

    8aca521512a14c439624191bd0a891c52f91b401

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-06 10:55         ` 陳韋任 (Wei-Ren Chen)
  0 siblings, 0 replies; 44+ messages in thread
From: 陳韋任 (Wei-Ren Chen) @ 2012-07-06 10:55 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

> > Which kvm.git hash is this referring? Please state this to avoid that we
> > are merging support for kernel features that are still under review.
> 
> The following kvm.git:
> http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary

  What Jia ask for is git hash, should be something like,

    8aca521512a14c439624191bd0a891c52f91b401

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

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

* Re: [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
  2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-07-06 11:05     ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:05 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-06 11:41, Wen Congyang wrote:
> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
> device kvm_pv_ioport to listen this I/O port, and deal with panicked
> event according to panicked_action's value. The possible actions are:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause the guest
> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
> 4. emit QEVENT_GUEST_PANICKED and reset the guest
> 
> I/O ports does not work for some targets(for example: s390). And you
> can implement another qom device, and include it's code into pv_event.c
> for such target.
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/kvm/Makefile.objs |    2 +-
>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c           |    9 +++
>  kvm.h                |    3 +
>  vl.c                 |    4 ++
>  6 files changed, 223 insertions(+), 1 deletions(-)
>  create mode 100644 hw/kvm/pv_event.c
>  create mode 100644 hw/kvm/pv_ioport.c
> 
> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
> index 226497a..23e3b30 100644
> --- a/hw/kvm/Makefile.objs
> +++ b/hw/kvm/Makefile.objs
> @@ -1 +1 @@
> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
> new file mode 100644
> index 0000000..d7ded37
> --- /dev/null
> +++ b/hw/kvm/pv_event.c
> @@ -0,0 +1,73 @@
> +/*
> + * QEMU KVM support, paravirtual event device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <linux/kvm_para.h>
> +#include <asm/kvm_para.h>
> +#include <qobject.h>
> +#include <qjson.h>
> +#include <monitor.h>
> +#include <sysemu.h>
> +#include <kvm.h>
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
> +
> +static int panicked_action = PANICKED_REPORT;

Avoid global variables please when there are device states. This one is
unneeded anyway (and will generate warnings when build without KVM_PV_PORT).

> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(uint32_t panicked_action)
> +{
> +    switch (panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_POWEROFF:
> +        panicked_mon_event("poweroff");
> +        exit(0);

We have qemu_system_shutdown_request.

> +        break;
> +    case PANICKED_RESET:
> +        panicked_mon_event("reset");
> +        qemu_system_reset_request();
> +        break;
> +    }
> +}
> +
> +#if defined(KVM_PV_PORT)
> +#include "pv_ioport.c"
> +
> +void kvm_pv_event_init(void)
> +{
> +    pv_ioport_init(panicked_action);
> +}
> +#else
> +void kvm_pv_event_init(void)
> +{
> +}
> +#endif

Generally, the split-up of handling and transport layer is a good idea
to allow other arch to support this interface. However, its current form
is a bit unfortunate as it does not properly separate the logic of the
events (so far only panic action) from the transport mechanism (PIO) and
as it registers the transport as a configurable device, not the event
handler. Make sure that pv_ioport only deals with registering against
the right bus and forwarding of the PV gate accesses to the event
handling layer. Device name and properties should be defined by the
event layer as well (but then registered by the transport layer).

> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
> new file mode 100644
> index 0000000..e93d819
> --- /dev/null
> +++ b/hw/kvm/pv_ioport.c
> @@ -0,0 +1,133 @@
> +/*
> + * QEMU KVM support, paravirtual I/O port device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/isa.h"
> +
> +typedef struct {
> +    ISADevice dev;
> +    MemoryRegion ioport;
> +    uint32_t panicked_action;

As explained above, this layer should not know about things like
"panicked_action".

> +} PVState;
> +
> +static PVState *pv_state;
> +
> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
> +{
> +    return 1 << KVM_PV_FEATURE_PANICKED;
> +}
> +
> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
> +                        unsigned size)
> +{
> +    PVState *s = opaque;
> +
> +    if (val == KVM_PV_PANICKED) {
> +        panicked_perform_action(s->panicked_action);
> +    }
> +}
> +
> +static const MemoryRegionOps pv_io_ops = {
> +    .read = pv_io_read,
> +    .write = pv_io_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static int pv_ioport_initfn(ISADevice *dev)
> +{
> +    PVState *s = DO_UPCAST(PVState, dev, dev);
> +
> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
> +    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
> +
> +    pv_state = s;
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription pv_ioport_vmsd = {
> +    .name = "pv_ioport",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(panicked_action, PVState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

Unneeded as panicked_action is a host-side property, not a
guest-changeable state. Your device is stateless, thus has no vmstate.

> +
> +static Property pv_ioport_properties[] = {
> +    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +
> +    ic->init = pv_ioport_initfn;
> +    dc->no_user = 1;
> +    dc->vmsd = &pv_ioport_vmsd;
> +    dc->props = pv_ioport_properties;
> +}
> +
> +static TypeInfo pv_ioport_info = {
> +    .name          = "kvm_pv_ioport",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(PVState),
> +    .class_init    = pv_ioport_class_init,
> +};
> +
> +static void pv_ioport_register_types(void)
> +{
> +    type_register_static(&pv_ioport_info);
> +}
> +
> +type_init(pv_ioport_register_types)
> +
> +static int is_isa_bus(BusState *bus, void *opaque)
> +{
> +    const char *bus_type_name;
> +    ISABus **isa_bus_p = opaque;
> +
> +    bus_type_name = object_class_get_name(bus->obj.class);
> +    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
> +        *isa_bus_p = ISA_BUS(&bus->obj);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static ISABus *get_isa_bus(void)
> +{
> +    ISABus *isa_bus = NULL;
> +
> +    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
> +
> +    return isa_bus;
> +}

Unneeded if the bus is passed on creation from the pc board setup.
That's the official way.

> +
> +static void pv_ioport_init(uint32_t panicked_action)
> +{
> +    ISADevice *dev;
> +    ISABus *bus;
> +
> +    bus = get_isa_bus();
> +    dev = isa_create(bus, "kvm_pv_ioport");
> +    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);

Nope, configuration should works via "-global device.property=value".
You likely want to define a special property that translates action
names into enum values, see e.g. the lost tick policy.

> +    qdev_init_nofail(&dev->qdev);
> +}
> diff --git a/kvm-stub.c b/kvm-stub.c
> index ec9a364..a28d078 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_event_init(void)
> +{
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    return 0;
> +}

Both will be unneeded.

> diff --git a/kvm.h b/kvm.h
> index 9c7b0ea..1f7c72b 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
>  
>  int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
> +
> +void kvm_pv_event_init(void);
> +int select_panicked_action(const char *p);
>  #endif
> diff --git a/vl.c b/vl.c
> index ea5ef1c..f5cd28d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (kvm_enabled()) {
> +        kvm_pv_event_init();
> +    }

Initialization is better located in the setup code of the board that
supports this device (here the PC). Very similar to kvm clock.

> +
>      qdev_machine_creation_done();
>  
>      if (rom_load_all() != 0) {
> 

Jan

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



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

* Re: [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-06 11:05     ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:05 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

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

Avoid global variables please when there are device states. This one is
unneeded anyway (and will generate warnings when build without KVM_PV_PORT).

> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(uint32_t panicked_action)
> +{
> +    switch (panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_POWEROFF:
> +        panicked_mon_event("poweroff");
> +        exit(0);

We have qemu_system_shutdown_request.

> +        break;
> +    case PANICKED_RESET:
> +        panicked_mon_event("reset");
> +        qemu_system_reset_request();
> +        break;
> +    }
> +}
> +
> +#if defined(KVM_PV_PORT)
> +#include "pv_ioport.c"
> +
> +void kvm_pv_event_init(void)
> +{
> +    pv_ioport_init(panicked_action);
> +}
> +#else
> +void kvm_pv_event_init(void)
> +{
> +}
> +#endif

Generally, the split-up of handling and transport layer is a good idea
to allow other arch to support this interface. However, its current form
is a bit unfortunate as it does not properly separate the logic of the
events (so far only panic action) from the transport mechanism (PIO) and
as it registers the transport as a configurable device, not the event
handler. Make sure that pv_ioport only deals with registering against
the right bus and forwarding of the PV gate accesses to the event
handling layer. Device name and properties should be defined by the
event layer as well (but then registered by the transport layer).

> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
> new file mode 100644
> index 0000000..e93d819
> --- /dev/null
> +++ b/hw/kvm/pv_ioport.c
> @@ -0,0 +1,133 @@
> +/*
> + * QEMU KVM support, paravirtual I/O port device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/isa.h"
> +
> +typedef struct {
> +    ISADevice dev;
> +    MemoryRegion ioport;
> +    uint32_t panicked_action;

As explained above, this layer should not know about things like
"panicked_action".

> +} PVState;
> +
> +static PVState *pv_state;
> +
> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
> +{
> +    return 1 << KVM_PV_FEATURE_PANICKED;
> +}
> +
> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
> +                        unsigned size)
> +{
> +    PVState *s = opaque;
> +
> +    if (val == KVM_PV_PANICKED) {
> +        panicked_perform_action(s->panicked_action);
> +    }
> +}
> +
> +static const MemoryRegionOps pv_io_ops = {
> +    .read = pv_io_read,
> +    .write = pv_io_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static int pv_ioport_initfn(ISADevice *dev)
> +{
> +    PVState *s = DO_UPCAST(PVState, dev, dev);
> +
> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
> +    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
> +
> +    pv_state = s;
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription pv_ioport_vmsd = {
> +    .name = "pv_ioport",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(panicked_action, PVState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

Unneeded as panicked_action is a host-side property, not a
guest-changeable state. Your device is stateless, thus has no vmstate.

> +
> +static Property pv_ioport_properties[] = {
> +    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +
> +    ic->init = pv_ioport_initfn;
> +    dc->no_user = 1;
> +    dc->vmsd = &pv_ioport_vmsd;
> +    dc->props = pv_ioport_properties;
> +}
> +
> +static TypeInfo pv_ioport_info = {
> +    .name          = "kvm_pv_ioport",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(PVState),
> +    .class_init    = pv_ioport_class_init,
> +};
> +
> +static void pv_ioport_register_types(void)
> +{
> +    type_register_static(&pv_ioport_info);
> +}
> +
> +type_init(pv_ioport_register_types)
> +
> +static int is_isa_bus(BusState *bus, void *opaque)
> +{
> +    const char *bus_type_name;
> +    ISABus **isa_bus_p = opaque;
> +
> +    bus_type_name = object_class_get_name(bus->obj.class);
> +    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
> +        *isa_bus_p = ISA_BUS(&bus->obj);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static ISABus *get_isa_bus(void)
> +{
> +    ISABus *isa_bus = NULL;
> +
> +    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
> +
> +    return isa_bus;
> +}

Unneeded if the bus is passed on creation from the pc board setup.
That's the official way.

> +
> +static void pv_ioport_init(uint32_t panicked_action)
> +{
> +    ISADevice *dev;
> +    ISABus *bus;
> +
> +    bus = get_isa_bus();
> +    dev = isa_create(bus, "kvm_pv_ioport");
> +    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);

Nope, configuration should works via "-global device.property=value".
You likely want to define a special property that translates action
names into enum values, see e.g. the lost tick policy.

> +    qdev_init_nofail(&dev->qdev);
> +}
> diff --git a/kvm-stub.c b/kvm-stub.c
> index ec9a364..a28d078 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_event_init(void)
> +{
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    return 0;
> +}

Both will be unneeded.

> diff --git a/kvm.h b/kvm.h
> index 9c7b0ea..1f7c72b 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
>  
>  int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
> +
> +void kvm_pv_event_init(void);
> +int select_panicked_action(const char *p);
>  #endif
> diff --git a/vl.c b/vl.c
> index ea5ef1c..f5cd28d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (kvm_enabled()) {
> +        kvm_pv_event_init();
> +    }

Initialization is better located in the setup code of the board that
supports this device (here the PC). Very similar to kvm clock.

> +
>      qdev_machine_creation_done();
>  
>      if (rom_load_all() != 0) {
> 

Jan

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

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

* Re: [Qemu-devel] [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-06 11:05     ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:05 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

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

Avoid global variables please when there are device states. This one is
unneeded anyway (and will generate warnings when build without KVM_PV_PORT).

> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(uint32_t panicked_action)
> +{
> +    switch (panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_POWEROFF:
> +        panicked_mon_event("poweroff");
> +        exit(0);

We have qemu_system_shutdown_request.

> +        break;
> +    case PANICKED_RESET:
> +        panicked_mon_event("reset");
> +        qemu_system_reset_request();
> +        break;
> +    }
> +}
> +
> +#if defined(KVM_PV_PORT)
> +#include "pv_ioport.c"
> +
> +void kvm_pv_event_init(void)
> +{
> +    pv_ioport_init(panicked_action);
> +}
> +#else
> +void kvm_pv_event_init(void)
> +{
> +}
> +#endif

Generally, the split-up of handling and transport layer is a good idea
to allow other arch to support this interface. However, its current form
is a bit unfortunate as it does not properly separate the logic of the
events (so far only panic action) from the transport mechanism (PIO) and
as it registers the transport as a configurable device, not the event
handler. Make sure that pv_ioport only deals with registering against
the right bus and forwarding of the PV gate accesses to the event
handling layer. Device name and properties should be defined by the
event layer as well (but then registered by the transport layer).

> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
> new file mode 100644
> index 0000000..e93d819
> --- /dev/null
> +++ b/hw/kvm/pv_ioport.c
> @@ -0,0 +1,133 @@
> +/*
> + * QEMU KVM support, paravirtual I/O port device
> + *
> + * Copyright Fujitsu, Corp. 2012
> + *
> + * Authors:
> + *     Wen Congyang <wency@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/isa.h"
> +
> +typedef struct {
> +    ISADevice dev;
> +    MemoryRegion ioport;
> +    uint32_t panicked_action;

As explained above, this layer should not know about things like
"panicked_action".

> +} PVState;
> +
> +static PVState *pv_state;
> +
> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
> +{
> +    return 1 << KVM_PV_FEATURE_PANICKED;
> +}
> +
> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
> +                        unsigned size)
> +{
> +    PVState *s = opaque;
> +
> +    if (val == KVM_PV_PANICKED) {
> +        panicked_perform_action(s->panicked_action);
> +    }
> +}
> +
> +static const MemoryRegionOps pv_io_ops = {
> +    .read = pv_io_read,
> +    .write = pv_io_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static int pv_ioport_initfn(ISADevice *dev)
> +{
> +    PVState *s = DO_UPCAST(PVState, dev, dev);
> +
> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
> +    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
> +
> +    pv_state = s;
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription pv_ioport_vmsd = {
> +    .name = "pv_ioport",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(panicked_action, PVState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

Unneeded as panicked_action is a host-side property, not a
guest-changeable state. Your device is stateless, thus has no vmstate.

> +
> +static Property pv_ioport_properties[] = {
> +    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +
> +    ic->init = pv_ioport_initfn;
> +    dc->no_user = 1;
> +    dc->vmsd = &pv_ioport_vmsd;
> +    dc->props = pv_ioport_properties;
> +}
> +
> +static TypeInfo pv_ioport_info = {
> +    .name          = "kvm_pv_ioport",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(PVState),
> +    .class_init    = pv_ioport_class_init,
> +};
> +
> +static void pv_ioport_register_types(void)
> +{
> +    type_register_static(&pv_ioport_info);
> +}
> +
> +type_init(pv_ioport_register_types)
> +
> +static int is_isa_bus(BusState *bus, void *opaque)
> +{
> +    const char *bus_type_name;
> +    ISABus **isa_bus_p = opaque;
> +
> +    bus_type_name = object_class_get_name(bus->obj.class);
> +    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
> +        *isa_bus_p = ISA_BUS(&bus->obj);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static ISABus *get_isa_bus(void)
> +{
> +    ISABus *isa_bus = NULL;
> +
> +    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
> +
> +    return isa_bus;
> +}

Unneeded if the bus is passed on creation from the pc board setup.
That's the official way.

> +
> +static void pv_ioport_init(uint32_t panicked_action)
> +{
> +    ISADevice *dev;
> +    ISABus *bus;
> +
> +    bus = get_isa_bus();
> +    dev = isa_create(bus, "kvm_pv_ioport");
> +    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);

Nope, configuration should works via "-global device.property=value".
You likely want to define a special property that translates action
names into enum values, see e.g. the lost tick policy.

> +    qdev_init_nofail(&dev->qdev);
> +}
> diff --git a/kvm-stub.c b/kvm-stub.c
> index ec9a364..a28d078 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_event_init(void)
> +{
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    return 0;
> +}

Both will be unneeded.

> diff --git a/kvm.h b/kvm.h
> index 9c7b0ea..1f7c72b 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
>  
>  int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
> +
> +void kvm_pv_event_init(void);
> +int select_panicked_action(const char *p);
>  #endif
> diff --git a/vl.c b/vl.c
> index ea5ef1c..f5cd28d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (kvm_enabled()) {
> +        kvm_pv_event_init();
> +    }

Initialization is better located in the setup code of the board that
supports this device (here the PC). Very similar to kvm clock.

> +
>      qdev_machine_creation_done();
>  
>      if (rom_load_all() != 0) {
> 

Jan

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

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

* Re: [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
  2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-07-06 11:09     ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:09 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-06 11:41, Wen Congyang wrote:
> The action is the same as -onpanic parameter.

As explained in patch 5, now that we have a related device, this no
longer needs to be a machine property.

Would could be a machine property is enabling/disabling this device.
That's probably useful as it uses a fixed PIO port that might conflict
with (non-Linux) guest expectations and/or future device models.

Jan

> 
> 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 1a68257..091c43b 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 = "";
>      }
> 

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



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

* Re: [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
@ 2012-07-06 11:09     ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:09 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-06 11:41, Wen Congyang wrote:
> The action is the same as -onpanic parameter.

As explained in patch 5, now that we have a related device, this no
longer needs to be a machine property.

Would could be a machine property is enabling/disabling this device.
That's probably useful as it uses a fixed PIO port that might conflict
with (non-Linux) guest expectations and/or future device models.

Jan

> 
> 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 1a68257..091c43b 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 = "";
>      }
> 

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

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

* Re: [Qemu-devel] [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
@ 2012-07-06 11:09     ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:09 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-06 11:41, Wen Congyang wrote:
> The action is the same as -onpanic parameter.

As explained in patch 5, now that we have a related device, this no
longer needs to be a machine property.

Would could be a machine property is enabling/disabling this device.
That's probably useful as it uses a fixed PIO port that might conflict
with (non-Linux) guest expectations and/or future device models.

Jan

> 
> 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 1a68257..091c43b 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 = "";
>      }
> 

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

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

* Re: [PATCH 6/7 v6] deal with guest panicked event accoring to -onpanic parameter
  2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-07-06 11:12     ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:12 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-06 11:41, Wen Congyang wrote:
> The onpanic parameter can have the following value:
> 1. none
> 2. pause
> 3. poweroff
> 4. reset
> 
> The action for each value when the guest is panicked:
> 1. none: emit QEVENT_GUEST_PANICKED only
> 2. pause: emit QEVENT_GUEST_PANICKED and pause the guest
> 3. poweroff: emit QEVENT_GUEST_PANICKED and poweroff the guest
> 4. reset: emit QEVENT_GUEST_PANICKED and reset the guest

This is redundant to patch 7 and the preferred device property approach.

Jan

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/kvm/pv_event.c |   17 +++++++++++++++++
>  qemu-options.hx   |   15 +++++++++++++++
>  vl.c              |    6 ++++++
>  3 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
> index d7ded37..890abcd 100644
> --- a/hw/kvm/pv_event.c
> +++ b/hw/kvm/pv_event.c
> @@ -59,6 +59,23 @@ static void panicked_perform_action(uint32_t panicked_action)
>      }
>  }
>  
> +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;
> +}
> +
>  #if defined(KVM_PV_PORT)
>  #include "pv_ioport.c"
>  
> 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 f5cd28d..1a68257 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);
>              }
> 

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



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

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

On 2012-07-06 11:41, Wen Congyang wrote:
> The onpanic parameter can have the following value:
> 1. none
> 2. pause
> 3. poweroff
> 4. reset
> 
> The action for each value when the guest is panicked:
> 1. none: emit QEVENT_GUEST_PANICKED only
> 2. pause: emit QEVENT_GUEST_PANICKED and pause the guest
> 3. poweroff: emit QEVENT_GUEST_PANICKED and poweroff the guest
> 4. reset: emit QEVENT_GUEST_PANICKED and reset the guest

This is redundant to patch 7 and the preferred device property approach.

Jan

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/kvm/pv_event.c |   17 +++++++++++++++++
>  qemu-options.hx   |   15 +++++++++++++++
>  vl.c              |    6 ++++++
>  3 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
> index d7ded37..890abcd 100644
> --- a/hw/kvm/pv_event.c
> +++ b/hw/kvm/pv_event.c
> @@ -59,6 +59,23 @@ static void panicked_perform_action(uint32_t panicked_action)
>      }
>  }
>  
> +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;
> +}
> +
>  #if defined(KVM_PV_PORT)
>  #include "pv_ioport.c"
>  
> 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 f5cd28d..1a68257 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);
>              }
> 

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

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

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

On 2012-07-06 11:41, Wen Congyang wrote:
> The onpanic parameter can have the following value:
> 1. none
> 2. pause
> 3. poweroff
> 4. reset
> 
> The action for each value when the guest is panicked:
> 1. none: emit QEVENT_GUEST_PANICKED only
> 2. pause: emit QEVENT_GUEST_PANICKED and pause the guest
> 3. poweroff: emit QEVENT_GUEST_PANICKED and poweroff the guest
> 4. reset: emit QEVENT_GUEST_PANICKED and reset the guest

This is redundant to patch 7 and the preferred device property approach.

Jan

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/kvm/pv_event.c |   17 +++++++++++++++++
>  qemu-options.hx   |   15 +++++++++++++++
>  vl.c              |    6 ++++++
>  3 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
> index d7ded37..890abcd 100644
> --- a/hw/kvm/pv_event.c
> +++ b/hw/kvm/pv_event.c
> @@ -59,6 +59,23 @@ static void panicked_perform_action(uint32_t panicked_action)
>      }
>  }
>  
> +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;
> +}
> +
>  #if defined(KVM_PV_PORT)
>  #include "pv_ioport.c"
>  
> 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 f5cd28d..1a68257 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);
>              }
> 

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

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

* Re: [PATCH 2/7 v6] update linux headers
  2012-07-06 10:50       ` Wen Congyang
@ 2012-07-06 11:16         ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:16 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-06 12:50, Wen Congyang wrote:
> At 07/06/2012 06:25 PM, Jan Kiszka Wrote:
>> On 2012-07-06 11:38, Wen Congyang wrote:
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>
>> Which kvm.git hash is this referring? Please state this to avoid that we
>> are merging support for kernel features that are still under review.
> 
> The following kvm.git:
> http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary
> 
> I apply my kvm's patch, and use ./scripts/update-linux-headers.sh to
> generate this patch.

OK, means the feature is not yet upstream. So this series is for
reference and early review until the kernel interface is accepted. I
personally push my kernel bits first before suggesting the userspace
parts. Avoids duplicate work if the former changes again during review.

Jan

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



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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-06 11:16         ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-06 11:16 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-06 12:50, Wen Congyang wrote:
> At 07/06/2012 06:25 PM, Jan Kiszka Wrote:
>> On 2012-07-06 11:38, Wen Congyang wrote:
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>
>> Which kvm.git hash is this referring? Please state this to avoid that we
>> are merging support for kernel features that are still under review.
> 
> The following kvm.git:
> http://git.kernel.org/?p=virt/kvm/kvm.git;a=summary
> 
> I apply my kvm's patch, and use ./scripts/update-linux-headers.sh to
> generate this patch.

OK, means the feature is not yet upstream. So this series is for
reference and early review until the kernel interface is accepted. I
personally push my kernel bits first before suggesting the userspace
parts. Avoids duplicate work if the former changes again during review.

Jan

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

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

* Re: [PATCH 2/7 v6] update linux headers
  2012-07-06  9:38   ` [Qemu-devel] " Wen Congyang
@ 2012-07-07 14:35     ` Paul Gortmaker
  -1 siblings, 0 replies; 44+ messages in thread
From: Paul Gortmaker @ 2012-07-07 14:35 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 Fri, Jul 6, 2012 at 5:38 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Minor nit, but adding text "for kvm" to the end of your subject can be
useful for people who are browsing short log of commits without
the context of the full patch series.   Since there were questions
about the what/why of the patch, perhaps a non-null long log would
have been a good addition too.

Thanks,
Paul.
--

> ---
>  linux-headers/asm-x86/kvm_para.h |    2 ++
>  linux-headers/linux/kvm_para.h   |    6 ++++++
>  2 files changed, 8 insertions(+), 0 deletions(-)

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

* Re: [Qemu-devel] [PATCH 2/7 v6] update linux headers
@ 2012-07-07 14:35     ` Paul Gortmaker
  0 siblings, 0 replies; 44+ messages in thread
From: Paul Gortmaker @ 2012-07-07 14:35 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Fri, Jul 6, 2012 at 5:38 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Minor nit, but adding text "for kvm" to the end of your subject can be
useful for people who are browsing short log of commits without
the context of the full patch series.   Since there were questions
about the what/why of the patch, perhaps a non-null long log would
have been a good addition too.

Thanks,
Paul.
--

> ---
>  linux-headers/asm-x86/kvm_para.h |    2 ++
>  linux-headers/linux/kvm_para.h   |    6 ++++++
>  2 files changed, 8 insertions(+), 0 deletions(-)

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

* Re: [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
  2012-07-06 11:09     ` Jan Kiszka
@ 2012-07-09 10:44       ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-09 10:44 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 07/06/2012 07:09 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:41, Wen Congyang wrote:
>> The action is the same as -onpanic parameter.
> 
> As explained in patch 5, now that we have a related device, this no
> longer needs to be a machine property.
> 
> Would could be a machine property is enabling/disabling this device.
> That's probably useful as it uses a fixed PIO port that might conflict
> with (non-Linux) guest expectations and/or future device models.

Yes, it is very useful. I will do it.

Thanks
Wen Congyang

> 
> Jan
> 
>>
>> 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 1a68257..091c43b 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 = "";
>>      }
>>
> 


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

* Re: [Qemu-devel] [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'
@ 2012-07-09 10:44       ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-09 10:44 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 07/06/2012 07:09 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:41, Wen Congyang wrote:
>> The action is the same as -onpanic parameter.
> 
> As explained in patch 5, now that we have a related device, this no
> longer needs to be a machine property.
> 
> Would could be a machine property is enabling/disabling this device.
> That's probably useful as it uses a fixed PIO port that might conflict
> with (non-Linux) guest expectations and/or future device models.

Yes, it is very useful. I will do it.

Thanks
Wen Congyang

> 
> Jan
> 
>>
>> 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 1a68257..091c43b 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 = "";
>>      }
>>
> 

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

* Re: [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
  2012-07-06 11:05     ` Jan Kiszka
@ 2012-07-18  1:54       ` Wen Congyang
  -1 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-18  1:54 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:41, Wen Congyang wrote:
>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>> event according to panicked_action's value. The possible actions are:
>> 1. emit QEVENT_GUEST_PANICKED only
>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>
>> I/O ports does not work for some targets(for example: s390). And you
>> can implement another qom device, and include it's code into pv_event.c
>> for such target.
>>
>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>> application does not receive this event(the management may not
>> run when the event is emitted), the management won't know the
>> guest is panicked.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  hw/kvm/Makefile.objs |    2 +-
>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kvm-stub.c           |    9 +++
>>  kvm.h                |    3 +
>>  vl.c                 |    4 ++
>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>  create mode 100644 hw/kvm/pv_event.c
>>  create mode 100644 hw/kvm/pv_ioport.c
>>
>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>> index 226497a..23e3b30 100644
>> --- a/hw/kvm/Makefile.objs
>> +++ b/hw/kvm/Makefile.objs
>> @@ -1 +1 @@
>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>> new file mode 100644
>> index 0000000..d7ded37
>> --- /dev/null
>> +++ b/hw/kvm/pv_event.c
>> @@ -0,0 +1,73 @@
>> +/*
>> + * QEMU KVM support, paravirtual event device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include <linux/kvm_para.h>
>> +#include <asm/kvm_para.h>
>> +#include <qobject.h>
>> +#include <qjson.h>
>> +#include <monitor.h>
>> +#include <sysemu.h>
>> +#include <kvm.h>
>> +
>> +/* Possible values for action parameter. */
>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>> +
>> +static int panicked_action = PANICKED_REPORT;
> 
> Avoid global variables please when there are device states. This one is
> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).

Hmm, do you mean introduce another qom device to store event action?

Thanks
Wen Congyang

> 
>> +
>> +static void panicked_mon_event(const char *action)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>> +    qobject_decref(data);
>> +}
>> +
>> +static void panicked_perform_action(uint32_t panicked_action)
>> +{
>> +    switch (panicked_action) {
>> +    case PANICKED_REPORT:
>> +        panicked_mon_event("report");
>> +        break;
>> +
>> +    case PANICKED_PAUSE:
>> +        panicked_mon_event("pause");
>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>> +        break;
>> +
>> +    case PANICKED_POWEROFF:
>> +        panicked_mon_event("poweroff");
>> +        exit(0);
> 
> We have qemu_system_shutdown_request.
> 
>> +        break;
>> +    case PANICKED_RESET:
>> +        panicked_mon_event("reset");
>> +        qemu_system_reset_request();
>> +        break;
>> +    }
>> +}
>> +
>> +#if defined(KVM_PV_PORT)
>> +#include "pv_ioport.c"
>> +
>> +void kvm_pv_event_init(void)
>> +{
>> +    pv_ioport_init(panicked_action);
>> +}
>> +#else
>> +void kvm_pv_event_init(void)
>> +{
>> +}
>> +#endif
> 
> Generally, the split-up of handling and transport layer is a good idea
> to allow other arch to support this interface. However, its current form
> is a bit unfortunate as it does not properly separate the logic of the
> events (so far only panic action) from the transport mechanism (PIO) and
> as it registers the transport as a configurable device, not the event
> handler. Make sure that pv_ioport only deals with registering against
> the right bus and forwarding of the PV gate accesses to the event
> handling layer. Device name and properties should be defined by the
> event layer as well (but then registered by the transport layer).
> 
>> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
>> new file mode 100644
>> index 0000000..e93d819
>> --- /dev/null
>> +++ b/hw/kvm/pv_ioport.c
>> @@ -0,0 +1,133 @@
>> +/*
>> + * QEMU KVM support, paravirtual I/O port device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "hw/isa.h"
>> +
>> +typedef struct {
>> +    ISADevice dev;
>> +    MemoryRegion ioport;
>> +    uint32_t panicked_action;
> 
> As explained above, this layer should not know about things like
> "panicked_action".
> 
>> +} PVState;
>> +
>> +static PVState *pv_state;
>> +
>> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
>> +{
>> +    return 1 << KVM_PV_FEATURE_PANICKED;
>> +}
>> +
>> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
>> +                        unsigned size)
>> +{
>> +    PVState *s = opaque;
>> +
>> +    if (val == KVM_PV_PANICKED) {
>> +        panicked_perform_action(s->panicked_action);
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps pv_io_ops = {
>> +    .read = pv_io_read,
>> +    .write = pv_io_write,
>> +    .impl = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    },
>> +};
>> +
>> +static int pv_ioport_initfn(ISADevice *dev)
>> +{
>> +    PVState *s = DO_UPCAST(PVState, dev, dev);
>> +
>> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
>> +    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
>> +
>> +    pv_state = s;
>> +
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription pv_ioport_vmsd = {
>> +    .name = "pv_ioport",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(panicked_action, PVState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
> 
> Unneeded as panicked_action is a host-side property, not a
> guest-changeable state. Your device is stateless, thus has no vmstate.
> 
>> +
>> +static Property pv_ioport_properties[] = {
>> +    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
>> +
>> +    ic->init = pv_ioport_initfn;
>> +    dc->no_user = 1;
>> +    dc->vmsd = &pv_ioport_vmsd;
>> +    dc->props = pv_ioport_properties;
>> +}
>> +
>> +static TypeInfo pv_ioport_info = {
>> +    .name          = "kvm_pv_ioport",
>> +    .parent        = TYPE_ISA_DEVICE,
>> +    .instance_size = sizeof(PVState),
>> +    .class_init    = pv_ioport_class_init,
>> +};
>> +
>> +static void pv_ioport_register_types(void)
>> +{
>> +    type_register_static(&pv_ioport_info);
>> +}
>> +
>> +type_init(pv_ioport_register_types)
>> +
>> +static int is_isa_bus(BusState *bus, void *opaque)
>> +{
>> +    const char *bus_type_name;
>> +    ISABus **isa_bus_p = opaque;
>> +
>> +    bus_type_name = object_class_get_name(bus->obj.class);
>> +    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
>> +        *isa_bus_p = ISA_BUS(&bus->obj);
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static ISABus *get_isa_bus(void)
>> +{
>> +    ISABus *isa_bus = NULL;
>> +
>> +    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
>> +
>> +    return isa_bus;
>> +}
> 
> Unneeded if the bus is passed on creation from the pc board setup.
> That's the official way.
> 
>> +
>> +static void pv_ioport_init(uint32_t panicked_action)
>> +{
>> +    ISADevice *dev;
>> +    ISABus *bus;
>> +
>> +    bus = get_isa_bus();
>> +    dev = isa_create(bus, "kvm_pv_ioport");
>> +    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);
> 
> Nope, configuration should works via "-global device.property=value".
> You likely want to define a special property that translates action
> names into enum values, see e.g. the lost tick policy.
> 
>> +    qdev_init_nofail(&dev->qdev);
>> +}
>> diff --git a/kvm-stub.c b/kvm-stub.c
>> index ec9a364..a28d078 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_event_init(void)
>> +{
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    return 0;
>> +}
> 
> Both will be unneeded.
> 
>> diff --git a/kvm.h b/kvm.h
>> index 9c7b0ea..1f7c72b 100644
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
>>  
>>  int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
>> +
>> +void kvm_pv_event_init(void);
>> +int select_panicked_action(const char *p);
>>  #endif
>> diff --git a/vl.c b/vl.c
>> index ea5ef1c..f5cd28d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
>>          exit(1);
>>      }
>>  
>> +    if (kvm_enabled()) {
>> +        kvm_pv_event_init();
>> +    }
> 
> Initialization is better located in the setup code of the board that
> supports this device (here the PC). Very similar to kvm clock.
> 
>> +
>>      qdev_machine_creation_done();
>>  
>>      if (rom_load_all() != 0) {
>>
> 
> Jan
> 


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

* Re: [Qemu-devel] [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-18  1:54       ` Wen Congyang
  0 siblings, 0 replies; 44+ messages in thread
From: Wen Congyang @ 2012-07-18  1:54 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:41, Wen Congyang wrote:
>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>> event according to panicked_action's value. The possible actions are:
>> 1. emit QEVENT_GUEST_PANICKED only
>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>
>> I/O ports does not work for some targets(for example: s390). And you
>> can implement another qom device, and include it's code into pv_event.c
>> for such target.
>>
>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>> application does not receive this event(the management may not
>> run when the event is emitted), the management won't know the
>> guest is panicked.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  hw/kvm/Makefile.objs |    2 +-
>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kvm-stub.c           |    9 +++
>>  kvm.h                |    3 +
>>  vl.c                 |    4 ++
>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>  create mode 100644 hw/kvm/pv_event.c
>>  create mode 100644 hw/kvm/pv_ioport.c
>>
>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>> index 226497a..23e3b30 100644
>> --- a/hw/kvm/Makefile.objs
>> +++ b/hw/kvm/Makefile.objs
>> @@ -1 +1 @@
>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>> new file mode 100644
>> index 0000000..d7ded37
>> --- /dev/null
>> +++ b/hw/kvm/pv_event.c
>> @@ -0,0 +1,73 @@
>> +/*
>> + * QEMU KVM support, paravirtual event device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include <linux/kvm_para.h>
>> +#include <asm/kvm_para.h>
>> +#include <qobject.h>
>> +#include <qjson.h>
>> +#include <monitor.h>
>> +#include <sysemu.h>
>> +#include <kvm.h>
>> +
>> +/* Possible values for action parameter. */
>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>> +
>> +static int panicked_action = PANICKED_REPORT;
> 
> Avoid global variables please when there are device states. This one is
> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).

Hmm, do you mean introduce another qom device to store event action?

Thanks
Wen Congyang

> 
>> +
>> +static void panicked_mon_event(const char *action)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>> +    qobject_decref(data);
>> +}
>> +
>> +static void panicked_perform_action(uint32_t panicked_action)
>> +{
>> +    switch (panicked_action) {
>> +    case PANICKED_REPORT:
>> +        panicked_mon_event("report");
>> +        break;
>> +
>> +    case PANICKED_PAUSE:
>> +        panicked_mon_event("pause");
>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>> +        break;
>> +
>> +    case PANICKED_POWEROFF:
>> +        panicked_mon_event("poweroff");
>> +        exit(0);
> 
> We have qemu_system_shutdown_request.
> 
>> +        break;
>> +    case PANICKED_RESET:
>> +        panicked_mon_event("reset");
>> +        qemu_system_reset_request();
>> +        break;
>> +    }
>> +}
>> +
>> +#if defined(KVM_PV_PORT)
>> +#include "pv_ioport.c"
>> +
>> +void kvm_pv_event_init(void)
>> +{
>> +    pv_ioport_init(panicked_action);
>> +}
>> +#else
>> +void kvm_pv_event_init(void)
>> +{
>> +}
>> +#endif
> 
> Generally, the split-up of handling and transport layer is a good idea
> to allow other arch to support this interface. However, its current form
> is a bit unfortunate as it does not properly separate the logic of the
> events (so far only panic action) from the transport mechanism (PIO) and
> as it registers the transport as a configurable device, not the event
> handler. Make sure that pv_ioport only deals with registering against
> the right bus and forwarding of the PV gate accesses to the event
> handling layer. Device name and properties should be defined by the
> event layer as well (but then registered by the transport layer).
> 
>> diff --git a/hw/kvm/pv_ioport.c b/hw/kvm/pv_ioport.c
>> new file mode 100644
>> index 0000000..e93d819
>> --- /dev/null
>> +++ b/hw/kvm/pv_ioport.c
>> @@ -0,0 +1,133 @@
>> +/*
>> + * QEMU KVM support, paravirtual I/O port device
>> + *
>> + * Copyright Fujitsu, Corp. 2012
>> + *
>> + * Authors:
>> + *     Wen Congyang <wency@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "hw/isa.h"
>> +
>> +typedef struct {
>> +    ISADevice dev;
>> +    MemoryRegion ioport;
>> +    uint32_t panicked_action;
> 
> As explained above, this layer should not know about things like
> "panicked_action".
> 
>> +} PVState;
>> +
>> +static PVState *pv_state;
>> +
>> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
>> +{
>> +    return 1 << KVM_PV_FEATURE_PANICKED;
>> +}
>> +
>> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
>> +                        unsigned size)
>> +{
>> +    PVState *s = opaque;
>> +
>> +    if (val == KVM_PV_PANICKED) {
>> +        panicked_perform_action(s->panicked_action);
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps pv_io_ops = {
>> +    .read = pv_io_read,
>> +    .write = pv_io_write,
>> +    .impl = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    },
>> +};
>> +
>> +static int pv_ioport_initfn(ISADevice *dev)
>> +{
>> +    PVState *s = DO_UPCAST(PVState, dev, dev);
>> +
>> +    memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1);
>> +    isa_register_ioport(dev, &s->ioport, KVM_PV_PORT);
>> +
>> +    pv_state = s;
>> +
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription pv_ioport_vmsd = {
>> +    .name = "pv_ioport",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(panicked_action, PVState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
> 
> Unneeded as panicked_action is a host-side property, not a
> guest-changeable state. Your device is stateless, thus has no vmstate.
> 
>> +
>> +static Property pv_ioport_properties[] = {
>> +    DEFINE_PROP_UINT32("panicked_action", PVState, panicked_action, PANICKED_REPORT),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void pv_ioport_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
>> +
>> +    ic->init = pv_ioport_initfn;
>> +    dc->no_user = 1;
>> +    dc->vmsd = &pv_ioport_vmsd;
>> +    dc->props = pv_ioport_properties;
>> +}
>> +
>> +static TypeInfo pv_ioport_info = {
>> +    .name          = "kvm_pv_ioport",
>> +    .parent        = TYPE_ISA_DEVICE,
>> +    .instance_size = sizeof(PVState),
>> +    .class_init    = pv_ioport_class_init,
>> +};
>> +
>> +static void pv_ioport_register_types(void)
>> +{
>> +    type_register_static(&pv_ioport_info);
>> +}
>> +
>> +type_init(pv_ioport_register_types)
>> +
>> +static int is_isa_bus(BusState *bus, void *opaque)
>> +{
>> +    const char *bus_type_name;
>> +    ISABus **isa_bus_p = opaque;
>> +
>> +    bus_type_name = object_class_get_name(bus->obj.class);
>> +    if (!strcmp(bus_type_name, TYPE_ISA_BUS)) {
>> +        *isa_bus_p = ISA_BUS(&bus->obj);
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static ISABus *get_isa_bus(void)
>> +{
>> +    ISABus *isa_bus = NULL;
>> +
>> +    qbus_walk_children(sysbus_get_default(), NULL, is_isa_bus, &isa_bus);
>> +
>> +    return isa_bus;
>> +}
> 
> Unneeded if the bus is passed on creation from the pc board setup.
> That's the official way.
> 
>> +
>> +static void pv_ioport_init(uint32_t panicked_action)
>> +{
>> +    ISADevice *dev;
>> +    ISABus *bus;
>> +
>> +    bus = get_isa_bus();
>> +    dev = isa_create(bus, "kvm_pv_ioport");
>> +    qdev_prop_set_uint32(&dev->qdev, "panicked_action", panicked_action);
> 
> Nope, configuration should works via "-global device.property=value".
> You likely want to define a special property that translates action
> names into enum values, see e.g. the lost tick policy.
> 
>> +    qdev_init_nofail(&dev->qdev);
>> +}
>> diff --git a/kvm-stub.c b/kvm-stub.c
>> index ec9a364..a28d078 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_event_init(void)
>> +{
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    return 0;
>> +}
> 
> Both will be unneeded.
> 
>> diff --git a/kvm.h b/kvm.h
>> index 9c7b0ea..1f7c72b 100644
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -218,4 +218,7 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
>>  
>>  int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
>>  int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
>> +
>> +void kvm_pv_event_init(void);
>> +int select_panicked_action(const char *p);
>>  #endif
>> diff --git a/vl.c b/vl.c
>> index ea5ef1c..f5cd28d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3622,6 +3622,10 @@ int main(int argc, char **argv, char **envp)
>>          exit(1);
>>      }
>>  
>> +    if (kvm_enabled()) {
>> +        kvm_pv_event_init();
>> +    }
> 
> Initialization is better located in the setup code of the board that
> supports this device (here the PC). Very similar to kvm clock.
> 
>> +
>>      qdev_machine_creation_done();
>>  
>>      if (rom_load_all() != 0) {
>>
> 
> Jan
> 

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

* Re: [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
  2012-07-18  1:54       ` [Qemu-devel] " Wen Congyang
@ 2012-07-18  9:19         ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-18  9:19 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-18 03:54, Wen Congyang wrote:
> At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
>> On 2012-07-06 11:41, Wen Congyang wrote:
>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>>> event according to panicked_action's value. The possible actions are:
>>> 1. emit QEVENT_GUEST_PANICKED only
>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>>
>>> I/O ports does not work for some targets(for example: s390). And you
>>> can implement another qom device, and include it's code into pv_event.c
>>> for such target.
>>>
>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>>> application does not receive this event(the management may not
>>> run when the event is emitted), the management won't know the
>>> guest is panicked.
>>>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> ---
>>>  hw/kvm/Makefile.objs |    2 +-
>>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  kvm-stub.c           |    9 +++
>>>  kvm.h                |    3 +
>>>  vl.c                 |    4 ++
>>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>>  create mode 100644 hw/kvm/pv_event.c
>>>  create mode 100644 hw/kvm/pv_ioport.c
>>>
>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>>> index 226497a..23e3b30 100644
>>> --- a/hw/kvm/Makefile.objs
>>> +++ b/hw/kvm/Makefile.objs
>>> @@ -1 +1 @@
>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>>> new file mode 100644
>>> index 0000000..d7ded37
>>> --- /dev/null
>>> +++ b/hw/kvm/pv_event.c
>>> @@ -0,0 +1,73 @@
>>> +/*
>>> + * QEMU KVM support, paravirtual event device
>>> + *
>>> + * Copyright Fujitsu, Corp. 2012
>>> + *
>>> + * Authors:
>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include <linux/kvm_para.h>
>>> +#include <asm/kvm_para.h>
>>> +#include <qobject.h>
>>> +#include <qjson.h>
>>> +#include <monitor.h>
>>> +#include <sysemu.h>
>>> +#include <kvm.h>
>>> +
>>> +/* Possible values for action parameter. */
>>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>>> +
>>> +static int panicked_action = PANICKED_REPORT;
>>
>> Avoid global variables please when there are device states. This one is
>> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).
> 
> Hmm, do you mean introduce another qom device to store event action?

I think you should be fine with one device per bus binding, but those
will consist of a common event layer and just different I/O layers (for
bus registration and access).

Jan

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



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

* Re: [Qemu-devel] [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-18  9:19         ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-18  9:19 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-18 03:54, Wen Congyang wrote:
> At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
>> On 2012-07-06 11:41, Wen Congyang wrote:
>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>>> event according to panicked_action's value. The possible actions are:
>>> 1. emit QEVENT_GUEST_PANICKED only
>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>>
>>> I/O ports does not work for some targets(for example: s390). And you
>>> can implement another qom device, and include it's code into pv_event.c
>>> for such target.
>>>
>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>>> application does not receive this event(the management may not
>>> run when the event is emitted), the management won't know the
>>> guest is panicked.
>>>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> ---
>>>  hw/kvm/Makefile.objs |    2 +-
>>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  kvm-stub.c           |    9 +++
>>>  kvm.h                |    3 +
>>>  vl.c                 |    4 ++
>>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>>  create mode 100644 hw/kvm/pv_event.c
>>>  create mode 100644 hw/kvm/pv_ioport.c
>>>
>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>>> index 226497a..23e3b30 100644
>>> --- a/hw/kvm/Makefile.objs
>>> +++ b/hw/kvm/Makefile.objs
>>> @@ -1 +1 @@
>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>>> new file mode 100644
>>> index 0000000..d7ded37
>>> --- /dev/null
>>> +++ b/hw/kvm/pv_event.c
>>> @@ -0,0 +1,73 @@
>>> +/*
>>> + * QEMU KVM support, paravirtual event device
>>> + *
>>> + * Copyright Fujitsu, Corp. 2012
>>> + *
>>> + * Authors:
>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include <linux/kvm_para.h>
>>> +#include <asm/kvm_para.h>
>>> +#include <qobject.h>
>>> +#include <qjson.h>
>>> +#include <monitor.h>
>>> +#include <sysemu.h>
>>> +#include <kvm.h>
>>> +
>>> +/* Possible values for action parameter. */
>>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>>> +
>>> +static int panicked_action = PANICKED_REPORT;
>>
>> Avoid global variables please when there are device states. This one is
>> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).
> 
> Hmm, do you mean introduce another qom device to store event action?

I think you should be fine with one device per bus binding, but those
will consist of a common event layer and just different I/O layers (for
bus registration and access).

Jan

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

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

* Re: [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
  2012-07-18  9:19         ` [Qemu-devel] " Jan Kiszka
@ 2012-07-18  9:25           ` Jan Kiszka
  -1 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-18  9:25 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-18 11:19, Jan Kiszka wrote:
> On 2012-07-18 03:54, Wen Congyang wrote:
>> At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
>>> On 2012-07-06 11:41, Wen Congyang wrote:
>>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>>>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>>>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>>>> event according to panicked_action's value. The possible actions are:
>>>> 1. emit QEVENT_GUEST_PANICKED only
>>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>>>
>>>> I/O ports does not work for some targets(for example: s390). And you
>>>> can implement another qom device, and include it's code into pv_event.c
>>>> for such target.
>>>>
>>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>>>> application does not receive this event(the management may not
>>>> run when the event is emitted), the management won't know the
>>>> guest is panicked.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> ---
>>>>  hw/kvm/Makefile.objs |    2 +-
>>>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  kvm-stub.c           |    9 +++
>>>>  kvm.h                |    3 +
>>>>  vl.c                 |    4 ++
>>>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>>>  create mode 100644 hw/kvm/pv_event.c
>>>>  create mode 100644 hw/kvm/pv_ioport.c
>>>>
>>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>>>> index 226497a..23e3b30 100644
>>>> --- a/hw/kvm/Makefile.objs
>>>> +++ b/hw/kvm/Makefile.objs
>>>> @@ -1 +1 @@
>>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>>>> new file mode 100644
>>>> index 0000000..d7ded37
>>>> --- /dev/null
>>>> +++ b/hw/kvm/pv_event.c
>>>> @@ -0,0 +1,73 @@
>>>> +/*
>>>> + * QEMU KVM support, paravirtual event device
>>>> + *
>>>> + * Copyright Fujitsu, Corp. 2012
>>>> + *
>>>> + * Authors:
>>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> + * See the COPYING file in the top-level directory.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/kvm_para.h>
>>>> +#include <asm/kvm_para.h>
>>>> +#include <qobject.h>
>>>> +#include <qjson.h>
>>>> +#include <monitor.h>
>>>> +#include <sysemu.h>
>>>> +#include <kvm.h>
>>>> +
>>>> +/* Possible values for action parameter. */
>>>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>>>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>>>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>>>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>>>> +
>>>> +static int panicked_action = PANICKED_REPORT;
>>>
>>> Avoid global variables please when there are device states. This one is
>>> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).
>>
>> Hmm, do you mean introduce another qom device to store event action?
> 
> I think you should be fine with one device per bus binding, but those
> will consist of a common event layer and just different I/O layers (for
> bus registration and access).

To make this clearer: the I/O layer should embed a common state
structure of the event layer in its device state so that the event layer
can keep things like the action mode there.

Jan

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



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

* Re: [Qemu-devel] [PATCH 5/7 v6] introduce a new qom device to deal with panicked event
@ 2012-07-18  9:25           ` Jan Kiszka
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kiszka @ 2012-07-18  9:25 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-07-18 11:19, Jan Kiszka wrote:
> On 2012-07-18 03:54, Wen Congyang wrote:
>> At 07/06/2012 07:05 PM, Jan Kiszka Wrote:
>>> On 2012-07-06 11:41, Wen Congyang wrote:
>>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the
>>>> port KVM_PV_PORT when it is panciked. This patch introduces a new qom
>>>> device kvm_pv_ioport to listen this I/O port, and deal with panicked
>>>> event according to panicked_action's value. The possible actions are:
>>>> 1. emit QEVENT_GUEST_PANICKED only
>>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest
>>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest
>>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest
>>>>
>>>> I/O ports does not work for some targets(for example: s390). And you
>>>> can implement another qom device, and include it's code into pv_event.c
>>>> for such target.
>>>>
>>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>>>> application does not receive this event(the management may not
>>>> run when the event is emitted), the management won't know the
>>>> guest is panicked.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> ---
>>>>  hw/kvm/Makefile.objs |    2 +-
>>>>  hw/kvm/pv_event.c    |   73 +++++++++++++++++++++++++++
>>>>  hw/kvm/pv_ioport.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  kvm-stub.c           |    9 +++
>>>>  kvm.h                |    3 +
>>>>  vl.c                 |    4 ++
>>>>  6 files changed, 223 insertions(+), 1 deletions(-)
>>>>  create mode 100644 hw/kvm/pv_event.c
>>>>  create mode 100644 hw/kvm/pv_ioport.c
>>>>
>>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
>>>> index 226497a..23e3b30 100644
>>>> --- a/hw/kvm/Makefile.objs
>>>> +++ b/hw/kvm/Makefile.objs
>>>> @@ -1 +1 @@
>>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
>>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o
>>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c
>>>> new file mode 100644
>>>> index 0000000..d7ded37
>>>> --- /dev/null
>>>> +++ b/hw/kvm/pv_event.c
>>>> @@ -0,0 +1,73 @@
>>>> +/*
>>>> + * QEMU KVM support, paravirtual event device
>>>> + *
>>>> + * Copyright Fujitsu, Corp. 2012
>>>> + *
>>>> + * Authors:
>>>> + *     Wen Congyang <wency@cn.fujitsu.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> + * See the COPYING file in the top-level directory.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/kvm_para.h>
>>>> +#include <asm/kvm_para.h>
>>>> +#include <qobject.h>
>>>> +#include <qjson.h>
>>>> +#include <monitor.h>
>>>> +#include <sysemu.h>
>>>> +#include <kvm.h>
>>>> +
>>>> +/* Possible values for action parameter. */
>>>> +#define PANICKED_REPORT     1   /* emit QEVENT_GUEST_PANICKED only */
>>>> +#define PANICKED_PAUSE      2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>>>> +#define PANICKED_POWEROFF   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>>>> +#define PANICKED_RESET      4   /* emit QEVENT_GUEST_PANICKED and reset VM */
>>>> +
>>>> +static int panicked_action = PANICKED_REPORT;
>>>
>>> Avoid global variables please when there are device states. This one is
>>> unneeded anyway (and will generate warnings when build without KVM_PV_PORT).
>>
>> Hmm, do you mean introduce another qom device to store event action?
> 
> I think you should be fine with one device per bus binding, but those
> will consist of a common event layer and just different I/O layers (for
> bus registration and access).

To make this clearer: the I/O layer should embed a common state
structure of the event layer in its device state so that the event layer
can keep things like the action mode there.

Jan

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

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

end of thread, other threads:[~2012-07-18  9:26 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-06  9:36 [PATCH v6] kvm: notify host when the guest is panicked Wen Congyang
2012-07-06  9:36 ` [Qemu-devel] " Wen Congyang
2012-07-06  9:38 ` [PATCH 1/7 v6] start vm after reseting it Wen Congyang
2012-07-06  9:38   ` [Qemu-devel] " Wen Congyang
2012-07-06  9:38 ` [PATCH 2/7 v6] update linux headers Wen Congyang
2012-07-06  9:38   ` [Qemu-devel] " Wen Congyang
2012-07-06 10:25   ` Jan Kiszka
2012-07-06 10:25     ` [Qemu-devel] " Jan Kiszka
2012-07-06 10:50     ` Wen Congyang
2012-07-06 10:50       ` [Qemu-devel] " Wen Congyang
2012-07-06 10:50       ` Wen Congyang
2012-07-06 10:55       ` [Qemu-devel] " 陳韋任 (Wei-Ren Chen)
2012-07-06 10:55         ` 陳韋任 (Wei-Ren Chen)
2012-07-06 11:16       ` Jan Kiszka
2012-07-06 11:16         ` [Qemu-devel] " Jan Kiszka
2012-07-07 14:35   ` Paul Gortmaker
2012-07-07 14:35     ` [Qemu-devel] " Paul Gortmaker
2012-07-06  9:39 ` [PATCH 3/7 v6] add a new runstate: RUN_STATE_GUEST_PANICKED Wen Congyang
2012-07-06  9:39   ` [Qemu-devel] " Wen Congyang
2012-07-06  9:40 ` [PATCH 4/7 v6] add a new qevent: QEVENT_GUEST_PANICKED Wen Congyang
2012-07-06  9:40   ` [Qemu-devel] " Wen Congyang
2012-07-06  9:41 ` [PATCH 5/7 v6] introduce a new qom device to deal with panicked event Wen Congyang
2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
2012-07-06 11:05   ` Jan Kiszka
2012-07-06 11:05     ` [Qemu-devel] " Jan Kiszka
2012-07-06 11:05     ` Jan Kiszka
2012-07-18  1:54     ` Wen Congyang
2012-07-18  1:54       ` [Qemu-devel] " Wen Congyang
2012-07-18  9:19       ` Jan Kiszka
2012-07-18  9:19         ` [Qemu-devel] " Jan Kiszka
2012-07-18  9:25         ` Jan Kiszka
2012-07-18  9:25           ` [Qemu-devel] " Jan Kiszka
2012-07-06  9:41 ` [PATCH 6/7 v6] deal with guest panicked event accoring to -onpanic parameter Wen Congyang
2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
2012-07-06 11:12   ` Jan Kiszka
2012-07-06 11:12     ` [Qemu-devel] " Jan Kiszka
2012-07-06 11:12     ` Jan Kiszka
2012-07-06  9:41 ` [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action' Wen Congyang
2012-07-06  9:41   ` [Qemu-devel] " Wen Congyang
2012-07-06 11:09   ` Jan Kiszka
2012-07-06 11:09     ` [Qemu-devel] " Jan Kiszka
2012-07-06 11:09     ` Jan Kiszka
2012-07-09 10:44     ` Wen Congyang
2012-07-09 10:44       ` [Qemu-devel] " 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.