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

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

[Wen Congyang]

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

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

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

We have decided to avoid touching hypervisor. The reason why I choose
choose the I/O port is:
1. it is easier to implememt
2. it does not depend any virtual device
3. it can work when 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

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

[Wen Congyang]

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

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

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

diff --git a/block.h b/block.h
index c89590d..4ed042d 100644
--- a/block.h
+++ b/block.h
@@ -337,6 +337,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 8904db1..c7cee31 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

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

[Wen Congyang]

Corresponding kvm.git hash: 37e41afa and apply my patch for kvm

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

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

[Wen Congyang]

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

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

diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..5889af7 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 c7cee31..5e33c97 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

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

[Wen Congyang]

This event will be emited when the guest is panicked.

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

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

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

[Wen Congyang]

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

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

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

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

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

[PATCH 6/6 v7] allow the user to disable pv event support

[Wen Congyang]

The qom device uses a fixed PIO port that might conflict
with (non-Linux) guest expectations and/or future device models.
So allow the user to disable it.

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

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

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

[Jan Kiszka]

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

On 2012-07-21 09:12, Wen Congyang wrote:
> We can know the guest is panicked when the guest runs on xen.
> But we do not have such feature on kvm.
> 
> Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked.
> 
> We have three solutions to implement this feature:
> 1. use vmcall
> 2. use I/O port
> 3. use virtio-serial.
> 
> We have decided to avoid touching hypervisor. The reason why I choose
> choose the I/O port is:
> 1. it is easier to implememt
> 2. it does not depend any virtual device
> 3. it can work when 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))

Reading from an invalid I/O port will return -1. So your test will
deliver a wrong result on a platform that doesn't support this PV channel.

Jan

> +		return 1;
> +	return 0;
> +}
>  #endif /* __KERNEL__ */
>  #endif /* __LINUX_KVM_PARA_H */
> 





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

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

[Wen Congyang]

At 07/21/2012 03:19 PM, Jan Kiszka Wrote:
> On 2012-07-21 09:12, Wen Congyang wrote:
>> We can know the guest is panicked when the guest runs on xen.
>> But we do not have such feature on kvm.
>>
>> Another purpose of this feature is: management app(for example:
>> libvirt) can do auto dump when the guest is panicked. If management
>> app does not do auto dump, the guest's user can do dump by hand if
>> he sees the guest is panicked.
>>
>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
>>
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when 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))
> 
> Reading from an invalid I/O port will return -1. So your test will
> deliver a wrong result on a platform that doesn't support this PV channel.

Yes, you are right. I will update it.

Thanks
Wen Congyang

> 
> Jan
> 
>> +		return 1;
>> +	return 0;
>> +}
>>  #endif /* __KERNEL__ */
>>  #endif /* __LINUX_KVM_PARA_H */
>>
> 
> 
> 
> 

[PATCH v7.5] kvm: notify host when the guest is panicked

[Wen Congyang]

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

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

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

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 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     |   13 +++++++++++++
 arch/x86/kernel/kvm.c               |   14 ++++++++++++++
 include/linux/kvm_para.h            |   13 +++++++++++++
 6 files changed, 55 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..647561b 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,17 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	unsigned int features = inl(KVM_PV_PORT);
+
+	/* Reading from an invalid I/O port will return -1 */
+	if (features == ~0)
+		features = 0;
+
+	return features;
+}
+
 #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

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

[Sasha Levin]

On 07/21/2012 09:12 AM, Wen Congyang wrote:
> +#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);
> +}
> +

Why is this safe?

I'm not sure you can just pick any ioport you'd like and use it.

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

[Sasha Levin]

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

Was the option of implementing a virtio-watchdog driver considered?

You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.

Why can't we re-use everything we have now, push a virtio watchdog driver into drivers/watchdog/, and gain a more complete solution to detecting hangs inside the guest.

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

[Anthony Liguori]

Sasha Levin <levinsasha928@gmail.com> writes:

> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>> We can know the guest is panicked when the guest runs on xen.
>> But we do not have such feature on kvm.
>> 
>> Another purpose of this feature is: management app(for example:
>> libvirt) can do auto dump when the guest is panicked. If management
>> app does not do auto dump, the guest's user can do dump by hand if
>> he sees the guest is panicked.
>> 
>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
>> 
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when starting the kernel
>
> Was the option of implementing a virtio-watchdog driver considered?
>
> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>
> Why can't we re-use everything we have now, push a virtio watchdog
> driver into drivers/watchdog/, and gain a more complete solution to
> detecting hangs inside the guest.

The purpose of virtio is not to reinvent every possible type of device.
There are plenty of hardware watchdogs that are very suitable to be used
for this purpose.  QEMU implements quite a few already.

Watchdogs are not performance sensitive so there's no point in using
virtio.

Regards,

Anthony Liguori

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

[Anthony Liguori]

Sasha Levin <levinsasha928@gmail.com> writes:

> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>> +#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);
>> +}
>> +
>
> Why is this safe?
>
> I'm not sure you can just pick any ioport you'd like and use it.

There are three ways I/O ports get used on a PC:

1) Platform devices
 - This is well defined since the vast majority of platform devices are
   implemented within a single chip.  If you're emulating an i440fx
   chipset, the PIIX4 spec has an exhaustive list.

2) PCI devices
 - Typically, PCI only allocates ports starting at 0x0d00 to avoid
   conflicts with ISA devices.

3) ISA devices
 - ISA uses subtractive decoding so any ISA device can access.  In
   theory, an ISA device could attempt to use port 0x0505 but it's
   unlikely.  In a modern guest, there aren't really any ISA devices being
   added either.

So yes, picking port 0x0505 is safe for something like this (as long as
you check to make sure that you really are under KVM).

Regards,

Anthony Liguori

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sasha Levin]

On 07/22/2012 09:14 PM, Anthony Liguori wrote:
> Sasha Levin <levinsasha928@gmail.com> writes:
> 
>> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>>> We can know the guest is panicked when the guest runs on xen.
>>> But we do not have such feature on kvm.
>>>
>>> Another purpose of this feature is: management app(for example:
>>> libvirt) can do auto dump when the guest is panicked. If management
>>> app does not do auto dump, the guest's user can do dump by hand if
>>> he sees the guest is panicked.
>>>
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when starting the kernel
>>
>> Was the option of implementing a virtio-watchdog driver considered?
>>
>> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>>
>> Why can't we re-use everything we have now, push a virtio watchdog
>> driver into drivers/watchdog/, and gain a more complete solution to
>> detecting hangs inside the guest.
> 
> The purpose of virtio is not to reinvent every possible type of device.
> There are plenty of hardware watchdogs that are very suitable to be used
> for this purpose.  QEMU implements quite a few already.
> 
> Watchdogs are not performance sensitive so there's no point in using
> virtio.

The issue here is not performance, but the adding of a brand new guest-host interface.

virtio-rng isn't performance sensitive either, yet it was implemented using virtio so there wouldn't be yet another interface to communicate between guest and host.

This patch goes ahead to add a "arch pv features" interface using ioports, without any idea what it might be used for beyond this watchdog.

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

[Sasha Levin]

On 07/22/2012 09:22 PM, Anthony Liguori wrote:
> Sasha Levin <levinsasha928@gmail.com> writes:
> 
>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>> +#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);
>>> +}
>>> +
>>
>> Why is this safe?
>>
>> I'm not sure you can just pick any ioport you'd like and use it.
> 
> There are three ways I/O ports get used on a PC:
> 
> 1) Platform devices
>  - This is well defined since the vast majority of platform devices are
>    implemented within a single chip.  If you're emulating an i440fx
>    chipset, the PIIX4 spec has an exhaustive list.
> 
> 2) PCI devices
>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>    conflicts with ISA devices.
> 
> 3) ISA devices
>  - ISA uses subtractive decoding so any ISA device can access.  In
>    theory, an ISA device could attempt to use port 0x0505 but it's
>    unlikely.  In a modern guest, there aren't really any ISA devices being
>    added either.
> 
> So yes, picking port 0x0505 is safe for something like this (as long as
> you check to make sure that you really are under KVM).

Is there anything that actually prevents me from using PCI ports lower than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is implemented by lkvm for example), so placing PCI below 0x0d00 might even make sense in that case.

Furthermore, I can place one of these brand new virtio-mmio devices which got introduced recently wherever I want right now - Having a device that uses 0x505 would cause a pretty non-obvious failure mode.

Either way, If we are going to grab an ioport, then:

 - It should be documented well somewhere in Documentation/virt/kvm
 - It should go through request_region() to actually claim those ioports.
 - It should fail gracefully if that port is taken for some reason, instead of not even checking it.

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

[Sasha Levin]

On 07/22/2012 10:19 PM, Sasha Levin wrote:
> On 07/22/2012 09:22 PM, Anthony Liguori wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>>
>>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>>> +#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);
>>>> +}
>>>> +
>>>
>>> Why is this safe?
>>>
>>> I'm not sure you can just pick any ioport you'd like and use it.
>>
>> There are three ways I/O ports get used on a PC:
>>
>> 1) Platform devices
>>  - This is well defined since the vast majority of platform devices are
>>    implemented within a single chip.  If you're emulating an i440fx
>>    chipset, the PIIX4 spec has an exhaustive list.
>>
>> 2) PCI devices
>>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>>    conflicts with ISA devices.
>>
>> 3) ISA devices
>>  - ISA uses subtractive decoding so any ISA device can access.  In
>>    theory, an ISA device could attempt to use port 0x0505 but it's
>>    unlikely.  In a modern guest, there aren't really any ISA devices being
>>    added either.
>>
>> So yes, picking port 0x0505 is safe for something like this (as long as
>> you check to make sure that you really are under KVM).
> 
> Is there anything that actually prevents me from using PCI ports lower than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is implemented by lkvm for example), so placing PCI below 0x0d00 might even make sense in that case.
> 
> Furthermore, I can place one of these brand new virtio-mmio devices which got introduced recently wherever I want right now - Having a device that uses 0x505 would cause a pretty non-obvious failure mode.
> 
> Either way, If we are going to grab an ioport, then:
> 
>  - It should be documented well somewhere in Documentation/virt/kvm
>  - It should go through request_region() to actually claim those ioports.
>  - It should fail gracefully if that port is taken for some reason, instead of not even checking it.
> 

Out of curiosity I tested that, and apparently lkvm has no problem allocating virtio-pci devices in that range:

sh-4.2# pwd
/sys/devices/pci0000:00/0000:00:01.0
sh-4.2# cat resource | head -n1
0x0000000000000500 0x00000000000005ff 0x0000000000040101

This was with the commit in question applied.

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

[Anthony Liguori]

Sasha Levin <levinsasha928@gmail.com> writes:

> On 07/22/2012 09:22 PM, Anthony Liguori wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>> 
>>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>>> +#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);
>>>> +}
>>>> +
>>>
>>> Why is this safe?
>>>
>>> I'm not sure you can just pick any ioport you'd like and use it.
>> 
>> There are three ways I/O ports get used on a PC:
>> 
>> 1) Platform devices
>>  - This is well defined since the vast majority of platform devices are
>>    implemented within a single chip.  If you're emulating an i440fx
>>    chipset, the PIIX4 spec has an exhaustive list.
>> 
>> 2) PCI devices
>>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>>    conflicts with ISA devices.
>> 
>> 3) ISA devices
>>  - ISA uses subtractive decoding so any ISA device can access.  In
>>    theory, an ISA device could attempt to use port 0x0505 but it's
>>    unlikely.  In a modern guest, there aren't really any ISA devices being
>>    added either.
>> 
>> So yes, picking port 0x0505 is safe for something like this (as long as
>> you check to make sure that you really are under KVM).
>
> Is there anything that actually prevents me from using PCI ports lower
> than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is
> implemented by lkvm for example), so placing PCI below 0x0d00 might
> even make sense in that case.

On modern systems, the OS goes by whatever is in the ACPI table
describing the PCI bus.  In QEMU, we have:

    WordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
          0x0000,             // Address Space Granularity
          0x0D00,             // Address Range Minimum
          0xFFFF,             // Address Range Maximum
          0x0000,             // Address Translation Offset
          0xF300,             // Address Length
          ,, , TypeStatic)

So Linux will always use 0x0D00 -> 0xFFFF for the valid
range. Practically speaking, you can't use anything below 0x0D00 because
the PCI bus configuration registers live at 0xCF8-0xCFF.  If you tried
to create the region starting at 0x0500 you'd have to limit it to 0xCF8
to avoid conflicting with the PCI host controller.

That's not a useful amount of space for I/O ports so that would be a
pretty dumb thing to do.

> Furthermore, I can place one of these brand new virtio-mmio devices
> which got introduced recently wherever I want right now - Having a
> device that uses 0x505 would cause a pretty non-obvious failure mode.

I think you're confusing PIO with MMIO.  They are separate address
spaces.

You could certainly argue that relying on PIO is way too architecture
specific since that's only available on x86.  That's a good argument but
the counter is that other architectures have their own interfaces for
this sort of thing.

> Either way, If we are going to grab an ioport, then:
>
>  - It should be documented well somewhere in Documentation/virt/kvm
>  - It should go through request_region() to actually claim those ioports.
>  - It should fail gracefully if that port is taken for some reason,
>  instead of not even checking it.

I agree with the above.

Regards,

Anthony Liguori

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

[Anthony Liguori]

Sasha Levin <levinsasha928@gmail.com> writes:

> On 07/22/2012 10:19 PM, Sasha Levin wrote:
>> On 07/22/2012 09:22 PM, Anthony Liguori wrote:
>>> Sasha Levin <levinsasha928@gmail.com> writes:
>>>
>>>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>>>> +#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);
>>>>> +}
>>>>> +
>>>>
>>>> Why is this safe?
>>>>
>>>> I'm not sure you can just pick any ioport you'd like and use it.
>>>
>>> There are three ways I/O ports get used on a PC:
>>>
>>> 1) Platform devices
>>>  - This is well defined since the vast majority of platform devices are
>>>    implemented within a single chip.  If you're emulating an i440fx
>>>    chipset, the PIIX4 spec has an exhaustive list.
>>>
>>> 2) PCI devices
>>>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>>>    conflicts with ISA devices.
>>>
>>> 3) ISA devices
>>>  - ISA uses subtractive decoding so any ISA device can access.  In
>>>    theory, an ISA device could attempt to use port 0x0505 but it's
>>>    unlikely.  In a modern guest, there aren't really any ISA devices being
>>>    added either.
>>>
>>> So yes, picking port 0x0505 is safe for something like this (as long as
>>> you check to make sure that you really are under KVM).
>> 
>> Is there anything that actually prevents me from using PCI ports lower than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is implemented by lkvm for example), so placing PCI below 0x0d00 might even make sense in that case.
>> 
>> Furthermore, I can place one of these brand new virtio-mmio devices which got introduced recently wherever I want right now - Having a device that uses 0x505 would cause a pretty non-obvious failure mode.
>> 
>> Either way, If we are going to grab an ioport, then:
>> 
>>  - It should be documented well somewhere in Documentation/virt/kvm
>>  - It should go through request_region() to actually claim those ioports.
>>  - It should fail gracefully if that port is taken for some reason, instead of not even checking it.
>> 
>
> Out of curiosity I tested that, and apparently lkvm has no problem allocating virtio-pci devices in that range:
>
> sh-4.2# pwd
> /sys/devices/pci0000:00/0000:00:01.0
> sh-4.2# cat resource | head -n1
> 0x0000000000000500 0x00000000000005ff 0x0000000000040101
>
> This was with the commit in question applied.

With all due respect, lkvm has a half-baked implementation of PCI.  This
is why you have to pass kernel parameters to disable ACPI and disable
PCI BIOS probing.

So yeah, you can do funky things in lkvm but that doesn't mean a system
that emulated actual hardware would ever do that.

Regards,

Anthony Liguori

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

[Anthony Liguori]

Sasha Levin <levinsasha928@gmail.com> writes:

> On 07/22/2012 09:14 PM, Anthony Liguori wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>> 
>>> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>>>> We can know the guest is panicked when the guest runs on xen.
>>>> But we do not have such feature on kvm.
>>>>
>>>> Another purpose of this feature is: management app(for example:
>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>> he sees the guest is panicked.
>>>>
>>>> We have three solutions to implement this feature:
>>>> 1. use vmcall
>>>> 2. use I/O port
>>>> 3. use virtio-serial.
>>>>
>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>> choose the I/O port is:
>>>> 1. it is easier to implememt
>>>> 2. it does not depend any virtual device
>>>> 3. it can work when starting the kernel
>>>
>>> Was the option of implementing a virtio-watchdog driver considered?
>>>
>>> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>>>
>>> Why can't we re-use everything we have now, push a virtio watchdog
>>> driver into drivers/watchdog/, and gain a more complete solution to
>>> detecting hangs inside the guest.
>> 
>> The purpose of virtio is not to reinvent every possible type of device.
>> There are plenty of hardware watchdogs that are very suitable to be used
>> for this purpose.  QEMU implements quite a few already.
>> 
>> Watchdogs are not performance sensitive so there's no point in using
>> virtio.
>
> The issue here is not performance, but the adding of a brand new
> guest-host interface.

We have:

1) Virtio--this is our preferred PV interface.  It needs PCI to be fully
initialized and probably will live as a module.

2) Hypercalls--this a secondary PV interface but is available very
early.  It's terminated in kvm.ko which means it can only operate on
things that are logically part of the CPU and/or APIC complex.

This patch introduces a third interface which is available early like
hypercalls but not necessarily terminated in kvm.ko.  That means it can
have a broader scope in functionality than (2).

We could just as well use a hypercall and have multiple commands issued
to that hypercall as a convention and add a new exit type to KVM that
sent that specific hypercall to userspace for processing.

But a PIO operation already has this behavior and requires no changes to kvm.ko.

> virtio-rng isn't performance sensitive either, yet it was implemented
> using virtio so there wouldn't be yet another interface to communicate
> between guest and host.

There isn't really an obvious discrete RNG that is widely supported.

> This patch goes ahead to add a "arch pv features" interface using
> ioports, without any idea what it might be used for beyond this
> watchdog.

It's not a watchdog--it's the opposite of a watchdog.

You know such a thing already exists in the kernel, right?  S390 has had
a hypercall like this for years.

Regards,

Anthony Liguori

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

[Sasha Levin]

On 07/23/2012 12:29 AM, Anthony Liguori wrote:
> Sasha Levin <levinsasha928@gmail.com> writes:
> 
>> On 07/22/2012 10:19 PM, Sasha Levin wrote:
>>> On 07/22/2012 09:22 PM, Anthony Liguori wrote:
>>>> Sasha Levin <levinsasha928@gmail.com> writes:
>>>>
>>>>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>>>>> +#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);
>>>>>> +}
>>>>>> +
>>>>>
>>>>> Why is this safe?
>>>>>
>>>>> I'm not sure you can just pick any ioport you'd like and use it.
>>>>
>>>> There are three ways I/O ports get used on a PC:
>>>>
>>>> 1) Platform devices
>>>>  - This is well defined since the vast majority of platform devices are
>>>>    implemented within a single chip.  If you're emulating an i440fx
>>>>    chipset, the PIIX4 spec has an exhaustive list.
>>>>
>>>> 2) PCI devices
>>>>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>>>>    conflicts with ISA devices.
>>>>
>>>> 3) ISA devices
>>>>  - ISA uses subtractive decoding so any ISA device can access.  In
>>>>    theory, an ISA device could attempt to use port 0x0505 but it's
>>>>    unlikely.  In a modern guest, there aren't really any ISA devices being
>>>>    added either.
>>>>
>>>> So yes, picking port 0x0505 is safe for something like this (as long as
>>>> you check to make sure that you really are under KVM).
>>>
>>> Is there anything that actually prevents me from using PCI ports lower than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is implemented by lkvm for example), so placing PCI below 0x0d00 might even make sense in that case.
>>>
>>> Furthermore, I can place one of these brand new virtio-mmio devices which got introduced recently wherever I want right now - Having a device that uses 0x505 would cause a pretty non-obvious failure mode.
>>>
>>> Either way, If we are going to grab an ioport, then:
>>>
>>>  - It should be documented well somewhere in Documentation/virt/kvm
>>>  - It should go through request_region() to actually claim those ioports.
>>>  - It should fail gracefully if that port is taken for some reason, instead of not even checking it.
>>>
>>
>> Out of curiosity I tested that, and apparently lkvm has no problem allocating virtio-pci devices in that range:
>>
>> sh-4.2# pwd
>> /sys/devices/pci0000:00/0000:00:01.0
>> sh-4.2# cat resource | head -n1
>> 0x0000000000000500 0x00000000000005ff 0x0000000000040101
>>
>> This was with the commit in question applied.
> 
> With all due respect, lkvm has a half-baked implementation of PCI.  This
> is why you have to pass kernel parameters to disable ACPI and disable
> PCI BIOS probing.
> 
> So yeah, you can do funky things in lkvm but that doesn't mean a system
> that emulated actual hardware would ever do that.

We disable ACPI simply because we don't support it. MPtable is a perfectly valid mechanism to do everything we need so far, so implementing ACPI didn't interest either of us too much. What's more - why implement a "complete design disaster in every way" ;)

Regarding PCI probing, while we do force the use of direct memory probing this is because we lack anything which reassembles a BIOS. Like the above, this wasn't too interesting in a virtualized environment, and the kernel is pretty happy running without it. PCI probing does happen in a standard way.

I think that the interesting part in that test was not that you could actually put a PCI device in the 0x500 range, but that nothing failed and no one yelled at me (with the panic commit applied).

I'm not worried about port 0x505 being taken, I'm worried that it'll silently break a (although not very common/reasonable/typical) perfectly valid use case.

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

[Sasha Levin]

On 07/23/2012 12:36 AM, Anthony Liguori wrote:
> Sasha Levin <levinsasha928@gmail.com> writes:
> 
>> On 07/22/2012 09:14 PM, Anthony Liguori wrote:
>>> Sasha Levin <levinsasha928@gmail.com> writes:
>>>
>>>> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>>>>> We can know the guest is panicked when the guest runs on xen.
>>>>> But we do not have such feature on kvm.
>>>>>
>>>>> Another purpose of this feature is: management app(for example:
>>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>>> he sees the guest is panicked.
>>>>>
>>>>> We have three solutions to implement this feature:
>>>>> 1. use vmcall
>>>>> 2. use I/O port
>>>>> 3. use virtio-serial.
>>>>>
>>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>>> choose the I/O port is:
>>>>> 1. it is easier to implememt
>>>>> 2. it does not depend any virtual device
>>>>> 3. it can work when starting the kernel
>>>>
>>>> Was the option of implementing a virtio-watchdog driver considered?
>>>>
>>>> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>>>>
>>>> Why can't we re-use everything we have now, push a virtio watchdog
>>>> driver into drivers/watchdog/, and gain a more complete solution to
>>>> detecting hangs inside the guest.
>>>
>>> The purpose of virtio is not to reinvent every possible type of device.
>>> There are plenty of hardware watchdogs that are very suitable to be used
>>> for this purpose.  QEMU implements quite a few already.
>>>
>>> Watchdogs are not performance sensitive so there's no point in using
>>> virtio.
>>
>> The issue here is not performance, but the adding of a brand new
>> guest-host interface.
> 
> We have:
> 
> 1) Virtio--this is our preferred PV interface.  It needs PCI to be fully
> initialized and probably will live as a module.
> 
> 2) Hypercalls--this a secondary PV interface but is available very
> early.  It's terminated in kvm.ko which means it can only operate on
> things that are logically part of the CPU and/or APIC complex.
> 
> This patch introduces a third interface which is available early like
> hypercalls but not necessarily terminated in kvm.ko.  That means it can
> have a broader scope in functionality than (2).
> 
> We could just as well use a hypercall and have multiple commands issued
> to that hypercall as a convention and add a new exit type to KVM that
> sent that specific hypercall to userspace for processing.
> 
> But a PIO operation already has this behavior and requires no changes to kvm.ko.

I don't dispute that there may be a need for another guest-host interface, but this patch can basically be called "kvm: notify host when the guest is panicked, oh, btw, and add a brand new undocumented interface"

The new interface should at least come in it's own patch, with documentation.

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

[Wen Congyang]

At 07/22/2012 07:39 PM, Sasha Levin Wrote:
> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>> We can know the guest is panicked when the guest runs on xen.
>> But we do not have such feature on kvm.
>>
>> Another purpose of this feature is: management app(for example:
>> libvirt) can do auto dump when the guest is panicked. If management
>> app does not do auto dump, the guest's user can do dump by hand if
>> he sees the guest is panicked.
>>
>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
>>
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when starting the kernel
> 
> Was the option of implementing a virtio-watchdog driver considered?

virtio-watchdog? What is this? I don't find it in qemu. Do I miss something?

Another reason why we don't use this:
If the watchdog timeouts, we cannot say the kernel is panicked. For
example, the kernel is hung, or the kernel is deadlock, or ...
the watchdog daemon can not have chance to touch watchdog device.

Thanks
Wen Congyang

> 
> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
> 
> Why can't we re-use everything we have now, push a virtio watchdog driver into drivers/watchdog/, and gain a more complete solution to detecting hangs inside the guest.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

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

[Wen Congyang]

At 07/23/2012 07:50 AM, Sasha Levin Wrote:
> On 07/23/2012 12:36 AM, Anthony Liguori wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>>
>>> On 07/22/2012 09:14 PM, Anthony Liguori wrote:
>>>> Sasha Levin <levinsasha928@gmail.com> writes:
>>>>
>>>>> On 07/21/2012 10:44 AM, Wen Congyang wrote:
>>>>>> We can know the guest is panicked when the guest runs on xen.
>>>>>> But we do not have such feature on kvm.
>>>>>>
>>>>>> Another purpose of this feature is: management app(for example:
>>>>>> libvirt) can do auto dump when the guest is panicked. If management
>>>>>> app does not do auto dump, the guest's user can do dump by hand if
>>>>>> he sees the guest is panicked.
>>>>>>
>>>>>> We have three solutions to implement this feature:
>>>>>> 1. use vmcall
>>>>>> 2. use I/O port
>>>>>> 3. use virtio-serial.
>>>>>>
>>>>>> We have decided to avoid touching hypervisor. The reason why I choose
>>>>>> choose the I/O port is:
>>>>>> 1. it is easier to implememt
>>>>>> 2. it does not depend any virtual device
>>>>>> 3. it can work when starting the kernel
>>>>>
>>>>> Was the option of implementing a virtio-watchdog driver considered?
>>>>>
>>>>> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>>>>>
>>>>> Why can't we re-use everything we have now, push a virtio watchdog
>>>>> driver into drivers/watchdog/, and gain a more complete solution to
>>>>> detecting hangs inside the guest.
>>>>
>>>> The purpose of virtio is not to reinvent every possible type of device.
>>>> There are plenty of hardware watchdogs that are very suitable to be used
>>>> for this purpose.  QEMU implements quite a few already.
>>>>
>>>> Watchdogs are not performance sensitive so there's no point in using
>>>> virtio.
>>>
>>> The issue here is not performance, but the adding of a brand new
>>> guest-host interface.
>>
>> We have:
>>
>> 1) Virtio--this is our preferred PV interface.  It needs PCI to be fully
>> initialized and probably will live as a module.
>>
>> 2) Hypercalls--this a secondary PV interface but is available very
>> early.  It's terminated in kvm.ko which means it can only operate on
>> things that are logically part of the CPU and/or APIC complex.
>>
>> This patch introduces a third interface which is available early like
>> hypercalls but not necessarily terminated in kvm.ko.  That means it can
>> have a broader scope in functionality than (2).
>>
>> We could just as well use a hypercall and have multiple commands issued
>> to that hypercall as a convention and add a new exit type to KVM that
>> sent that specific hypercall to userspace for processing.
>>
>> But a PIO operation already has this behavior and requires no changes to kvm.ko.
> 
> I don't dispute that there may be a need for another guest-host interface, but this patch can basically be called "kvm: notify host when the guest is panicked, oh, btw, and add a brand new undocumented interface"

I forgot to document this interface. I will add it.

Thanks
Wen Congyang

> 
> The new interface should at least come in it's own patch, with documentation.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

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

[Wen Congyang]

At 07/23/2012 04:19 AM, Sasha Levin Wrote:
> On 07/22/2012 09:22 PM, Anthony Liguori wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>>
>>> On 07/21/2012 09:12 AM, Wen Congyang wrote:
>>>> +#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);
>>>> +}
>>>> +
>>>
>>> Why is this safe?
>>>
>>> I'm not sure you can just pick any ioport you'd like and use it.
>>
>> There are three ways I/O ports get used on a PC:
>>
>> 1) Platform devices
>>  - This is well defined since the vast majority of platform devices are
>>    implemented within a single chip.  If you're emulating an i440fx
>>    chipset, the PIIX4 spec has an exhaustive list.
>>
>> 2) PCI devices
>>  - Typically, PCI only allocates ports starting at 0x0d00 to avoid
>>    conflicts with ISA devices.
>>
>> 3) ISA devices
>>  - ISA uses subtractive decoding so any ISA device can access.  In
>>    theory, an ISA device could attempt to use port 0x0505 but it's
>>    unlikely.  In a modern guest, there aren't really any ISA devices being
>>    added either.
>>
>> So yes, picking port 0x0505 is safe for something like this (as long as
>> you check to make sure that you really are under KVM).
> 
> Is there anything that actually prevents me from using PCI ports lower than 0x0d00? As you said in (3), ISA isn't really used anymore (nor is implemented by lkvm for example), so placing PCI below 0x0d00 might even make sense in that case.
> 
> Furthermore, I can place one of these brand new virtio-mmio devices which got introduced recently wherever I want right now - Having a device that uses 0x505 would cause a pretty non-obvious failure mode.
> 
> Either way, If we are going to grab an ioport, then:
> 
>  - It should be documented well somewhere in Documentation/virt/kvm
>  - It should go through request_region() to actually claim those ioports.

Good idea.

>  - It should fail gracefully if that port is taken for some reason, instead of not even checking it.

Yes, I agree it.

I will update it.

Thanks
Wen Congyang

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>