[Qemu-devel] [PATCH v15 0/6] Add pvpanic device to deal with guest panic event

[Qemu-devel] [PATCH v15 0/6] Add pvpanic device to deal with guest panic event

[Hu Tao]

This series introduces a new simulated device, pvpanic, to notify
qemu when guest panic event happens.

Along with this series, there are two patches to add seabios ACPI
driver and kernel ACPI driver for the device, respectively.

Tested with combinations of qemu(kvm)/qemu(tcg), piix/q35, and
default ioport/custom ioport. The only fail case is q35 with custom
ioport. The reason is, in seabios we can't differentiate whether the
passed in DSDT is for q35 or not, and the DSDT patching must uses the
right offset depending this. Addressing this may conflicts with what
people doing to move ACPI tables from seabios to qemu, and, I prefixes
some patches with 'RFC'.


Changes from v14:

  - use QEMU0001 as HID(I mistakenly used MSFT0001 in previous version)
  - device name changed from pvevent to pvpanic
  - pass ioport to seabios through fw_cfg
  - document improvement

v14: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02293.html

Hu Tao (6):
  add a new runstate: RUN_STATE_GUEST_PANICKED
  add a new qevent: QEVENT_GUEST_PANICKED
  introduce a new qom device to deal with panicked event
  pvpanic: add document of pvpanic
  pc_piix, pc_q35: export fw_cfg
  pvpanic: pass configurable ioport to seabios

 QMP/qmp-events.txt        |  14 ++++++
 docs/specs/pvpanic.txt    |  14 ++++++
 hw/Makefile.objs          |   1 +
 hw/fw_cfg.h               |   2 +
 hw/i386/pc_piix.c         |   7 +--
 hw/i386/pc_q35.c          |   9 ++--
 hw/pc.h                   |   6 +++
 hw/pvpanic.c              | 126 ++++++++++++++++++++++++++++++++++++++++++++++
 include/monitor/monitor.h |   1 +
 include/sysemu/sysemu.h   |   1 +
 monitor.c                 |   1 +
 qapi-schema.json          |   5 +-
 qmp.c                     |   3 +-
 vl.c                      |  13 ++++-
 14 files changed, 192 insertions(+), 11 deletions(-)
 create mode 100644 docs/specs/pvpanic.txt
 create mode 100644 hw/pvpanic.c

-- 
1.8.1.4

[Qemu-devel] [PATCH v15 1/6] add a new runstate: RUN_STATE_GUEST_PANICKED

[Hu Tao]

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 include/sysemu/sysemu.h |  1 +
 qapi-schema.json        |  5 ++++-
 qmp.c                   |  3 +--
 vl.c                    | 13 +++++++++++--
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6578782..b6fffc8 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -22,6 +22,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid);
 bool runstate_check(RunState state);
 void runstate_set(RunState new_state);
 int runstate_is_running(void);
+bool runstate_needs_reset(void);
 typedef struct vm_change_state_entry VMChangeStateEntry;
 typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 4494e53..842ba36 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -174,11 +174,14 @@
 # @suspended: guest is suspended (ACPI S3)
 #
 # @watchdog: the watchdog action is configured to pause and has been triggered
+#
+# @guest-panicked: guest has been panicked as a result of guest OS panic
 ##
 { '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' ] }
 
 ##
 # @SnapshotInfo
diff --git a/qmp.c b/qmp.c
index 55b056b..a1ebb5d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -149,8 +149,7 @@ void qmp_cont(Error **errp)
 {
     Error *local_err = NULL;
 
-    if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-               runstate_check(RUN_STATE_SHUTDOWN)) {
+    if (runstate_needs_reset()) {
         error_set(errp, QERR_RESET_REQUIRED);
         return;
     } else if (runstate_check(RUN_STATE_SUSPENDED)) {
diff --git a/vl.c b/vl.c
index a621aec..289e7ef 100644
--- a/vl.c
+++ b/vl.c
@@ -592,6 +592,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 },
 
@@ -606,6 +607,8 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
 
@@ -647,6 +650,13 @@ int runstate_is_running(void)
     return runstate_check(RUN_STATE_RUNNING);
 }
 
+bool runstate_needs_reset(void)
+{
+    return runstate_check(RUN_STATE_INTERNAL_ERROR) ||
+        runstate_check(RUN_STATE_SHUTDOWN) ||
+        runstate_check(RUN_STATE_GUEST_PANICKED);
+}
+
 StatusInfo *qmp_query_status(Error **errp)
 {
     StatusInfo *info = g_malloc0(sizeof(*info));
@@ -1992,8 +2002,7 @@ static bool main_loop_should_exit(void)
         cpu_synchronize_all_states();
         qemu_system_reset(VMRESET_REPORT);
         resume_all_vcpus();
-        if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-            runstate_check(RUN_STATE_SHUTDOWN)) {
+        if (runstate_needs_reset()) {
             runstate_set(RUN_STATE_PAUSED);
         }
     }
-- 
1.8.1.4

[Qemu-devel] [PATCH v15 2/6] add a new qevent: QEVENT_GUEST_PANICKED

[Hu Tao]

This event will be emited when qemu detects guest panic.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 QMP/qmp-events.txt        | 14 ++++++++++++++
 include/monitor/monitor.h |  1 +
 monitor.c                 |  1 +
 3 files changed, 16 insertions(+)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index b2698e4..a725fab 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -428,3 +428,17 @@ Example:
 
 Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
 followed respectively by the RESET, SHUTDOWN, or STOP events.
+
+GUEST_PANICKED
+--------------
+
+Emitted when guest OS panic is detected.
+
+Data:
+
+- "action": Action that has been taken (json-string, currently always "pause").
+
+Example:
+
+{ "event": "GUEST_PANICKED",
+     "data": { "action": "pause" } }
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 87fb49c..4006905 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -45,6 +45,7 @@ typedef enum MonitorEvent {
     QEVENT_WAKEUP,
     QEVENT_BALLOON_CHANGE,
     QEVENT_SPICE_MIGRATE_COMPLETED,
+    QEVENT_GUEST_PANICKED,
 
     /* Add to 'monitor_event_names' array in monitor.c when
      * defining new events here */
diff --git a/monitor.c b/monitor.c
index 1b5acf8..d8893a9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -464,6 +464,7 @@ static const char *monitor_event_names[] = {
     [QEVENT_WAKEUP] = "WAKEUP",
     [QEVENT_BALLOON_CHANGE] = "BALLOON_CHANGE",
     [QEVENT_SPICE_MIGRATE_COMPLETED] = "SPICE_MIGRATE_COMPLETED",
+    [QEVENT_GUEST_PANICKED] = "GUEST_PANICKED",
 };
 QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
 
-- 
1.8.1.4

[Qemu-devel] [PATCH v15 3/6] introduce a new qom device to deal with panicked event

[Hu Tao]

pvpanic device is used to send guest panic event from guest to qemu.

When guest panic happens, pvpanic device driver will write a event
number to IO port 0x505(which is the IO port occupied by pvpanic device,
by default). On receiving the event, pvpanic device will pause guest
cpu(s), and send a qmp event QEVENT_GUEST_PANICKED.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/Makefile.objs |   1 +
 hw/pvpanic.c     | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 hw/pvpanic.c

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index eb7eb31..4d09747 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -213,4 +213,5 @@ obj-$(CONFIG_KVM) += ivshmem.o
 obj-$(CONFIG_LINUX) += vfio_pci.o
 endif
 
+common-obj-y += pvpanic.o
 endif
diff --git a/hw/pvpanic.c b/hw/pvpanic.c
new file mode 100644
index 0000000..ff0a116
--- /dev/null
+++ b/hw/pvpanic.c
@@ -0,0 +1,116 @@
+/*
+ * QEMU simulated pvpanic device.
+ *
+ * Copyright Fujitsu, Corp. 2013
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *     Hu Tao <hutao@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 <qapi/qmp/qobject.h>
+#include <qapi/qmp/qjson.h>
+#include <monitor/monitor.h>
+#include <sysemu/sysemu.h>
+#include <sysemu/kvm.h>
+
+/* The bit of supported pv event */
+#define PVPANIC_F_PANICKED      0
+
+/* The pv event value */
+#define PVPANIC_PANICKED        (1 << PVPANIC_F_PANICKED)
+
+#define TYPE_ISA_PVPANIC_DEVICE    "pvpanic"
+#define ISA_PVPANIC_DEVICE(obj)    \
+    OBJECT_CHECK(PVPanicState, (obj), TYPE_ISA_PVPANIC_DEVICE)
+
+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 handle_event(int event)
+{
+    if (event == PVPANIC_PANICKED) {
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        return;
+    }
+}
+
+#include "hw/isa.h"
+
+typedef struct PVPanicState {
+    ISADevice parent_obj;
+
+    MemoryRegion io;
+    uint16_t ioport;
+} PVPanicState;
+
+/* return supported events on read */
+static uint64_t pvpanic_ioport_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return PVPANIC_PANICKED;
+}
+
+static void pvpanic_ioport_write(void *opaque, hwaddr addr, uint64_t val,
+                                 unsigned size)
+{
+    handle_event(val);
+}
+
+static const MemoryRegionOps pvpanic_ops = {
+    .read = pvpanic_ioport_read,
+    .write = pvpanic_ioport_write,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static int pvpanic_isa_initfn(ISADevice *dev)
+{
+    PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
+
+    memory_region_init_io(&s->io, &pvpanic_ops, s, "pvpanic", 1);
+    isa_register_ioport(dev, &s->io, s->ioport);
+
+    return 0;
+}
+
+static Property pvpanic_isa_properties[] = {
+    DEFINE_PROP_UINT16("ioport", PVPanicState, ioport, 0x505),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pvpanic_isa_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+
+    ic->init = pvpanic_isa_initfn;
+    dc->no_user = 1;
+    dc->props = pvpanic_isa_properties;
+}
+
+static TypeInfo pvpanic_isa_info = {
+    .name          = TYPE_ISA_PVPANIC_DEVICE,
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(PVPanicState),
+    .class_init    = pvpanic_isa_class_init,
+};
+
+static void pvpanic_register_types(void)
+{
+    type_register_static(&pvpanic_isa_info);
+}
+
+type_init(pvpanic_register_types)
-- 
1.8.1.4

[Qemu-devel] [PATCH v15 4/6] pvpanic: add document of pvpanic

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 docs/specs/pvpanic.txt | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 docs/specs/pvpanic.txt

diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
new file mode 100644
index 0000000..761d20c
--- /dev/null
+++ b/docs/specs/pvpanic.txt
@@ -0,0 +1,25 @@
+PVPANIC DEVICE
+==============
+
+pvpanic device is a simulated ISA device, through which guest panic
+event is sent to qemu, and a QMP event is generated. This allows
+management apps(e.g. libvirt) to be notified and respond to the
+event.
+
+pvpanic uses port 0x505 by default to receive panic event from guest.
+The port is configurable by specifying ioport property.
+
+pvpanic device is defined with ACPI ID "QEMU0001". To send panic
+event, guest evaluates method WRPT, specifying a byte, bit 0 set,
+as argument. Other bits are reserved.
+
+To use it, one will have to:
+
+1. add the device by specifying `-device pvpanic' in the qemu command
+   line.
+
+2. load pvpanic device driver in guest OS.
+
+The management app has the options of waiting for GUEST_PANICKED events,
+and/or polling for guest-panicked RunState, to learn when the pvpanic
+device has fired a panic event.
-- 
1.8.1.4

[Qemu-devel] [RFC][PATCH v15 5/6] pc_piix, pc_q35: export fw_cfg

[Hu Tao]

This lets other devices pass informations to seabios.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/pc_piix.c | 7 ++++---
 hw/i386/pc_q35.c  | 9 ++++++---
 hw/pc.h           | 6 ++++++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0ee3b3b..2a57b50 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -54,6 +54,8 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
+void *fw_cfg_piix;
+
 /* PC hardware initialisation */
 static void pc_init1(MemoryRegion *system_memory,
                      MemoryRegion *system_io,
@@ -84,7 +86,6 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    void *fw_cfg = NULL;
 
     pc_cpus_init(cpu_model);
     pc_acpi_init("acpi-dsdt.aml");
@@ -112,7 +113,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        fw_cfg = pc_memory_init(system_memory,
+        fw_cfg_piix = pc_memory_init(system_memory,
                        kernel_filename, kernel_cmdline, initrd_filename,
                        below_4g_mem_size, above_4g_mem_size,
                        rom_memory, &ram_memory);
@@ -209,7 +210,7 @@ static void pc_init1(MemoryRegion *system_memory,
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
                               gsi[9], *smi_irq,
-                              kvm_enabled(), fw_cfg);
+                              kvm_enabled(), fw_cfg_piix);
         smbus_eeprom_init(smbus, 8, NULL, 0);
     }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4f5f347..ce7798c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,6 +45,8 @@
 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
 
+void *fw_cfg_q35;
+
 /* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
  *    BIOS will read it and start S3 resume at POST Entry */
 static void pc_cmos_set_s3_resume(void *opaque, int irq, int level)
@@ -111,9 +113,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(get_system_memory(), kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size,
-                       rom_memory, &ram_memory);
+        fw_cfg_q35 = pc_memory_init(get_system_memory(), kernel_filename,
+                                    kernel_cmdline, initrd_filename,
+                                    below_4g_mem_size, above_4g_mem_size,
+                                    rom_memory, &ram_memory);
     }
 
     /* irq lines */
diff --git a/hw/pc.h b/hw/pc.h
index dbbd8cd..8ca73f1 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -145,6 +145,12 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
 extern PCIDevice *piix4_dev;
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
 
+/* pc_piix.c */
+extern void *fw_cfg_piix;
+
+/* pc_q35.c */
+extern void *fw_cfg_q35;
+
 /* vga.c */
 enum vga_retrace_method {
     VGA_RETRACE_DUMB,
-- 
1.8.1.4

[Qemu-devel] [RFC][PATCH v15 6/6] pvpanic: pass configurable ioport to seabios

[Hu Tao]

This lets seabios patch the corresponding SSDT entry.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/fw_cfg.h  |  2 ++
 hw/pvpanic.c | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 05c8df1..07cc941 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -1,6 +1,8 @@
 #ifndef FW_CFG_H
 #define FW_CFG_H
 
+#include "exec/hwaddr.h"
+
 #define FW_CFG_SIGNATURE        0x00
 #define FW_CFG_ID               0x01
 #define FW_CFG_UUID             0x02
diff --git a/hw/pvpanic.c b/hw/pvpanic.c
index ff0a116..94c3b25 100644
--- a/hw/pvpanic.c
+++ b/hw/pvpanic.c
@@ -17,6 +17,8 @@
 #include <monitor/monitor.h>
 #include <sysemu/sysemu.h>
 #include <sysemu/kvm.h>
+#include "hw/fw_cfg.h"
+#include "hw/pc.h"
 
 /* The bit of supported pv event */
 #define PVPANIC_F_PANICKED      0
@@ -83,6 +85,18 @@ static int pvpanic_isa_initfn(ISADevice *dev)
     memory_region_init_io(&s->io, &pvpanic_ops, s, "pvpanic", 1);
     isa_register_ioport(dev, &s->io, s->ioport);
 
+    if (fw_cfg_piix) {
+        fw_cfg_add_file(fw_cfg_piix, "etc/pvpanic-port",
+                        g_memdup(&s->ioport, sizeof(s->ioport)),
+                        sizeof(s->ioport));
+    }
+
+    if (fw_cfg_q35) {
+        fw_cfg_add_file(fw_cfg_q35, "etc/pvpanic-port",
+                        g_memdup(&s->ioport, sizeof(s->ioport)),
+                        sizeof(s->ioport));
+    }
+
     return 0;
 }

-- 
1.8.1.4

[Qemu-devel] [PATCH v15 1/2] add pvpanic device driver

[Hu Tao]

pvpanic device is used to notify host(qemu) when guest panic happens.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---

ref: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg03630.html
The version number is 15 to consist with qemu patches.

 src/acpi-dsdt-isa.dsl | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
index 23761db..87a31b9 100644
--- a/src/acpi-dsdt-isa.dsl
+++ b/src/acpi-dsdt-isa.dsl
@@ -99,4 +99,34 @@ Scope(\_SB.PCI0.ISA) {
             IRQNoFlags() { 3 }
         })
     }
+
+    Device(PEVT) {
+        Name(_HID, "QEMU0001")
+        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
+        Field(PEOR, ByteAcc, NoLock, Preserve) {
+            PEPT,   8,
+        }
+
+        Method(_STA, 0, NotSerialized) {
+            Store(PEPT, Local0)
+            If (LEqual(Local0, Zero)) {
+                Return (0x00)
+            } Else {
+                Return (0x0F)
+            }
+        }
+
+        Method(RDPT, 0, NotSerialized) {
+            Store(PEPT, Local0)
+            Return (Local0)
+        }
+
+        Method(WRPT, 1, NotSerialized) {
+            Store(Arg0, PEPT)
+        }
+
+        Name(_CRS, ResourceTemplate() {
+            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
+        })
+    }
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
 src/acpi.c            |  9 +++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
index 87a31b9..43fe719 100644
--- a/src/acpi-dsdt-isa.dsl
+++ b/src/acpi-dsdt-isa.dsl
@@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
 
     Device(PEVT) {
         Name(_HID, "QEMU0001")
-        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
+        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
+        Name(PEST, 0x505)
+        OperationRegion(PEOR, SystemIO, PEST, 0x01)
         Field(PEOR, ByteAcc, NoLock, Preserve) {
             PEPT,   8,
         }
@@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
         }
 
         Name(_CRS, ResourceTemplate() {
-            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
+            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
         })
+
+        CreateWordField(_CRS, IO._MIN, IOMN)
+        CreateWordField(_CRS, IO._MAX, IOMX)
+
+        Method(_INI, 0, NotSerialized) {
+            Store(PEST, IOMN)
+            Store(PEST, IOMX)
+        }
     }
 }
diff --git a/src/acpi.c b/src/acpi.c
index 119d1c1..42fa06e 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
     PCI_DEVICE_END
 };
 
+static void patch_dsdt(void *dsdt)
+{
+    u8 *dsdt_ptr = dsdt;
+    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);
+
+    *(u16 *)(dsdt_ptr + *dsdt_isa_pest) = pvpanic_port;
+}
+
 static void fill_dsdt(struct fadt_descriptor_rev1 *fadt, void *dsdt)
 {
     if (fadt->dsdt) {
         free((void *)le32_to_cpu(fadt->dsdt));
     }
+    patch_dsdt(dsdt);
     fadt->dsdt = cpu_to_le32((u32)dsdt);
     fadt->checksum -= checksum(fadt, sizeof(*fadt));
     dprintf(1, "ACPI DSDT=%p\n", dsdt);
-- 
1.8.1.4

Re: [Qemu-devel] [RFC][PATCH v15 6/6] pvpanic: pass configurable ioport to seabios

[Paolo Bonzini]

Il 21/03/2013 09:35, Hu Tao ha scritto:
> This lets seabios patch the corresponding SSDT entry.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/fw_cfg.h  |  2 ++
>  hw/pvpanic.c | 14 ++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 05c8df1..07cc941 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -1,6 +1,8 @@
>  #ifndef FW_CFG_H
>  #define FW_CFG_H
>  
> +#include "exec/hwaddr.h"
> +
>  #define FW_CFG_SIGNATURE        0x00
>  #define FW_CFG_ID               0x01
>  #define FW_CFG_UUID             0x02
> diff --git a/hw/pvpanic.c b/hw/pvpanic.c
> index ff0a116..94c3b25 100644
> --- a/hw/pvpanic.c
> +++ b/hw/pvpanic.c
> @@ -17,6 +17,8 @@
>  #include <monitor/monitor.h>
>  #include <sysemu/sysemu.h>
>  #include <sysemu/kvm.h>
> +#include "hw/fw_cfg.h"
> +#include "hw/pc.h"
>  
>  /* The bit of supported pv event */
>  #define PVPANIC_F_PANICKED      0
> @@ -83,6 +85,18 @@ static int pvpanic_isa_initfn(ISADevice *dev)
>      memory_region_init_io(&s->io, &pvpanic_ops, s, "pvpanic", 1);
>      isa_register_ioport(dev, &s->io, s->ioport);
>  
> +    if (fw_cfg_piix) {
> +        fw_cfg_add_file(fw_cfg_piix, "etc/pvpanic-port",
> +                        g_memdup(&s->ioport, sizeof(s->ioport)),
> +                        sizeof(s->ioport));
> +    }
> +
> +    if (fw_cfg_q35) {
> +        fw_cfg_add_file(fw_cfg_q35, "etc/pvpanic-port",
> +                        g_memdup(&s->ioport, sizeof(s->ioport)),
> +                        sizeof(s->ioport));
> +    }
> +
>      return 0;
>  }
> 

You can get fw_cfg with this patch (untested):

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 63a1998..ac6f1a1 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -511,6 +511,8 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     s->machine_ready.notify = fw_cfg_machine_ready;
     qemu_add_machine_init_done_notifier(&s->machine_ready);
 
+    object_property_add_child(qdev_get_machine(), "fw_cfg", OBJECT(s), NULL);
+
     return s;
 }
 
and then

    void *fw_cfg = object_resolve_path("/machine/fw_cfg");
    if (fw_cfg) {
        ...
    }

Note that this must be done only once if you create more than 1 pvpanic device.

Also, I would like machine types for 1.5 and newer to create pvpanic by default.

Paolo

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Paolo Bonzini]

Il 21/03/2013 10:08, Hu Tao ha scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
>  src/acpi.c            |  9 +++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
> index 87a31b9..43fe719 100644
> --- a/src/acpi-dsdt-isa.dsl
> +++ b/src/acpi-dsdt-isa.dsl
> @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
>  
>      Device(PEVT) {
>          Name(_HID, "QEMU0001")
> -        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
> +        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
> +        Name(PEST, 0x505)
> +        OperationRegion(PEOR, SystemIO, PEST, 0x01)
>          Field(PEOR, ByteAcc, NoLock, Preserve) {
>              PEPT,   8,
>          }
> @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
>          }
>  
>          Name(_CRS, ResourceTemplate() {
> -            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
> +            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
>          })
> +
> +        CreateWordField(_CRS, IO._MIN, IOMN)
> +        CreateWordField(_CRS, IO._MAX, IOMX)
> +
> +        Method(_INI, 0, NotSerialized) {
> +            Store(PEST, IOMN)
> +            Store(PEST, IOMX)
> +        }
>      }
>  }
> diff --git a/src/acpi.c b/src/acpi.c
> index 119d1c1..42fa06e 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
>      PCI_DEVICE_END
>  };
>  
> +static void patch_dsdt(void *dsdt)
> +{
> +    u8 *dsdt_ptr = dsdt;
> +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);

The default must be 0.

Also, here:


+
+        Method(_STA, 0, NotSerialized) {
+            Store(PEPT, Local0)
+            If (LEqual(Local0, Zero)) {
+                Return (0x00)
+            } Else {
+                Return (0x0F)
+            }
+        }
+

You must change it to look at PEST instead of PEPT (i.e. do not probe,
just see if you have a meaningful address).

Just squash the patches, it's simpler that way probably.

Paolo

> +    *(u16 *)(dsdt_ptr + *dsdt_isa_pest) = pvpanic_port;
> +}
> +
>  static void fill_dsdt(struct fadt_descriptor_rev1 *fadt, void *dsdt)
>  {
>      if (fadt->dsdt) {
>          free((void *)le32_to_cpu(fadt->dsdt));
>      }
> +    patch_dsdt(dsdt);
>      fadt->dsdt = cpu_to_le32((u32)dsdt);
>      fadt->checksum -= checksum(fadt, sizeof(*fadt));
>      dprintf(1, "ACPI DSDT=%p\n", dsdt);
> 

[PATCH v15] pvpanic: pvpanic device driver

[Hu Tao]

pvpanic device is a qemu simulated device through which guest panic
event is sent to host.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---

ref: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg03630.html
The version number is 15 to consist with qemu patches.

 drivers/platform/x86/Kconfig   |   7 +++
 drivers/platform/x86/Makefile  |   2 +
 drivers/platform/x86/pvpanic.c | 115 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 drivers/platform/x86/pvpanic.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7ab0b2f..c72598e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -768,4 +768,11 @@ config APPLE_GMUX
 	  graphics as well as the backlight. Currently only backlight
 	  control is supported by the driver.
 
+config PVPANIC
+	tristate "pvpanic device support"
+	depends on ACPI
+	---help---
+	  This driver provides support for pvpanic device, which is a qemu
+	  simulated device through which guest panic event is sent to host.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index bf7e4f9..b52f1c4 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -50,3 +50,5 @@ obj-$(CONFIG_INTEL_MID_POWER_BUTTON)	+= intel_mid_powerbtn.o
 obj-$(CONFIG_INTEL_OAKTRAIL)	+= intel_oaktrail.o
 obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
 obj-$(CONFIG_APPLE_GMUX)	+= apple-gmux.o
+
+obj-$(CONFIG_PVPANIC)           += pvpanic.o
diff --git a/drivers/platform/x86/pvpanic.c b/drivers/platform/x86/pvpanic.c
new file mode 100644
index 0000000..e6ea595
--- /dev/null
+++ b/drivers/platform/x86/pvpanic.c
@@ -0,0 +1,115 @@
+/*
+ *  pvpanic.c - pvpanic Device Support
+ *
+ *  Copyright (C) 2013 Fujitsu.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+
+MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
+MODULE_DESCRIPTION("pvpanic device driver");
+MODULE_LICENSE("GPL");
+
+static int pvpanic_add(struct acpi_device *device);
+static int pvpanic_remove(struct acpi_device *device);
+
+static const struct acpi_device_id pvpanic_device_ids[] = {
+	{ "QEMU0001", 0},
+	{ "", 0},
+};
+MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
+
+#define PVPANIC_PANICKED	(1 << 0)
+
+static acpi_handle handle;
+
+static struct acpi_driver pvpanic_driver = {
+	.name =		"pvpanic",
+	.class =	"QEMU",
+	.ids =		pvpanic_device_ids,
+	.ops =		{
+				.add =		pvpanic_add,
+				.remove =	pvpanic_remove,
+			},
+	.owner =	THIS_MODULE,
+};
+
+static void
+pvpanic_send_event(unsigned int event)
+{
+	union acpi_object arg;
+	struct acpi_object_list arg_list;
+
+	if (!handle)
+		return;
+
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = event;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+
+	acpi_evaluate_object(handle, "WRPT", &arg_list, NULL);
+}
+
+static int
+pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
+		     void *unused)
+{
+	pvpanic_send_event(PVPANIC_PANICKED);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block pvpanic_panic_nb = {
+	.notifier_call = pvpanic_panic_notify,
+};
+
+static int pvpanic_add(struct acpi_device *device)
+{
+	acpi_status status;
+	u64 ret;
+
+	status = acpi_evaluate_integer(device->handle, "_STA", NULL,
+				       &ret);
+
+	if (ACPI_FAILURE(status) || !ret)
+		return -ENODEV;
+
+	handle = device->handle;
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &pvpanic_panic_nb);
+
+	return 0;
+}
+
+static int pvpanic_remove(struct acpi_device *device)
+{
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &pvpanic_panic_nb);
+	handle = NULL;
+	return 0;
+}
+
+module_acpi_driver(pvpanic_driver);
-- 
1.8.1.4

[Qemu-devel] [PATCH v15] pvpanic: pvpanic device driver

[Hu Tao]

pvpanic device is a qemu simulated device through which guest panic
event is sent to host.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---

ref: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg03630.html
The version number is 15 to consist with qemu patches.

 drivers/platform/x86/Kconfig   |   7 +++
 drivers/platform/x86/Makefile  |   2 +
 drivers/platform/x86/pvpanic.c | 115 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 drivers/platform/x86/pvpanic.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7ab0b2f..c72598e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -768,4 +768,11 @@ config APPLE_GMUX
 	  graphics as well as the backlight. Currently only backlight
 	  control is supported by the driver.
 
+config PVPANIC
+	tristate "pvpanic device support"
+	depends on ACPI
+	---help---
+	  This driver provides support for pvpanic device, which is a qemu
+	  simulated device through which guest panic event is sent to host.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index bf7e4f9..b52f1c4 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -50,3 +50,5 @@ obj-$(CONFIG_INTEL_MID_POWER_BUTTON)	+= intel_mid_powerbtn.o
 obj-$(CONFIG_INTEL_OAKTRAIL)	+= intel_oaktrail.o
 obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
 obj-$(CONFIG_APPLE_GMUX)	+= apple-gmux.o
+
+obj-$(CONFIG_PVPANIC)           += pvpanic.o
diff --git a/drivers/platform/x86/pvpanic.c b/drivers/platform/x86/pvpanic.c
new file mode 100644
index 0000000..e6ea595
--- /dev/null
+++ b/drivers/platform/x86/pvpanic.c
@@ -0,0 +1,115 @@
+/*
+ *  pvpanic.c - pvpanic Device Support
+ *
+ *  Copyright (C) 2013 Fujitsu.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+
+MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
+MODULE_DESCRIPTION("pvpanic device driver");
+MODULE_LICENSE("GPL");
+
+static int pvpanic_add(struct acpi_device *device);
+static int pvpanic_remove(struct acpi_device *device);
+
+static const struct acpi_device_id pvpanic_device_ids[] = {
+	{ "QEMU0001", 0},
+	{ "", 0},
+};
+MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
+
+#define PVPANIC_PANICKED	(1 << 0)
+
+static acpi_handle handle;
+
+static struct acpi_driver pvpanic_driver = {
+	.name =		"pvpanic",
+	.class =	"QEMU",
+	.ids =		pvpanic_device_ids,
+	.ops =		{
+				.add =		pvpanic_add,
+				.remove =	pvpanic_remove,
+			},
+	.owner =	THIS_MODULE,
+};
+
+static void
+pvpanic_send_event(unsigned int event)
+{
+	union acpi_object arg;
+	struct acpi_object_list arg_list;
+
+	if (!handle)
+		return;
+
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = event;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+
+	acpi_evaluate_object(handle, "WRPT", &arg_list, NULL);
+}
+
+static int
+pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
+		     void *unused)
+{
+	pvpanic_send_event(PVPANIC_PANICKED);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block pvpanic_panic_nb = {
+	.notifier_call = pvpanic_panic_notify,
+};
+
+static int pvpanic_add(struct acpi_device *device)
+{
+	acpi_status status;
+	u64 ret;
+
+	status = acpi_evaluate_integer(device->handle, "_STA", NULL,
+				       &ret);
+
+	if (ACPI_FAILURE(status) || !ret)
+		return -ENODEV;
+
+	handle = device->handle;
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &pvpanic_panic_nb);
+
+	return 0;
+}
+
+static int pvpanic_remove(struct acpi_device *device)
+{
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &pvpanic_panic_nb);
+	handle = NULL;
+	return 0;
+}
+
+module_acpi_driver(pvpanic_driver);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v15 4/6] pvpanic: add document of pvpanic

[Paolo Bonzini]

Il 21/03/2013 09:35, Hu Tao ha scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  docs/specs/pvpanic.txt | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 docs/specs/pvpanic.txt
> 
> diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
> new file mode 100644
> index 0000000..761d20c
> --- /dev/null
> +++ b/docs/specs/pvpanic.txt
> @@ -0,0 +1,25 @@
> +PVPANIC DEVICE
> +==============
> +
> +pvpanic device is a simulated ISA device, through which guest panic
> +event is sent to qemu, and a QMP event is generated. This allows
> +management apps(e.g. libvirt) to be notified and respond to the
> +event.
> +
> +pvpanic uses port 0x505 by default to receive panic event from guest.
> +The port is configurable by specifying ioport property.

Please document the ISA interface too.

> +pvpanic device is defined with ACPI ID "QEMU0001". To send panic
> +event, guest evaluates method WRPT, specifying a byte, bit 0 set,
> +as argument. Other bits are reserved.

Please document RDPT too, and mention that the ACPI device will
automatically refer to the right port in case it is modified.

> +To use it, one will have to:
> +
> +1. add the device by specifying `-device pvpanic' in the qemu command
> +   line.

As mentioned earlier, I'd like this to be the default in 1.5 and newer
machine types.

Paolo

> +2. load pvpanic device driver in guest OS.
> +
> +The management app has the options of waiting for GUEST_PANICKED events,
> +and/or polling for guest-panicked RunState, to learn when the pvpanic
> +device has fired a panic event.
> 

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

On Thu, Mar 21, 2013 at 10:11:54AM +0100, Paolo Bonzini wrote:
> Il 21/03/2013 10:08, Hu Tao ha scritto:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
> >  src/acpi.c            |  9 +++++++++
> >  2 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
> > index 87a31b9..43fe719 100644
> > --- a/src/acpi-dsdt-isa.dsl
> > +++ b/src/acpi-dsdt-isa.dsl
> > @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
> >  
> >      Device(PEVT) {
> >          Name(_HID, "QEMU0001")
> > -        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
> > +        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
> > +        Name(PEST, 0x505)
> > +        OperationRegion(PEOR, SystemIO, PEST, 0x01)
> >          Field(PEOR, ByteAcc, NoLock, Preserve) {
> >              PEPT,   8,
> >          }
> > @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
> >          }
> >  
> >          Name(_CRS, ResourceTemplate() {
> > -            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
> > +            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
> >          })
> > +
> > +        CreateWordField(_CRS, IO._MIN, IOMN)
> > +        CreateWordField(_CRS, IO._MAX, IOMX)
> > +
> > +        Method(_INI, 0, NotSerialized) {
> > +            Store(PEST, IOMN)
> > +            Store(PEST, IOMX)
> > +        }
> >      }
> >  }
> > diff --git a/src/acpi.c b/src/acpi.c
> > index 119d1c1..42fa06e 100644
> > --- a/src/acpi.c
> > +++ b/src/acpi.c
> > @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
> >      PCI_DEVICE_END
> >  };
> >  
> > +static void patch_dsdt(void *dsdt)
> > +{
> > +    u8 *dsdt_ptr = dsdt;
> > +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);
> 
> The default must be 0.
> 
> Also, here:
> 
> 
> +
> +        Method(_STA, 0, NotSerialized) {
> +            Store(PEPT, Local0)
> +            If (LEqual(Local0, Zero)) {
> +                Return (0x00)
> +            } Else {
> +                Return (0x0F)
> +            }
> +        }
> +
> 
> You must change it to look at PEST instead of PEPT (i.e. do not probe,
> just see if you have a meaningful address).
> 
> Just squash the patches, it's simpler that way probably.

I forgot to add RFC to the title. This patch doesn't work for q35 with
custom ioport.

> 
> Paolo
> 
> > +    *(u16 *)(dsdt_ptr + *dsdt_isa_pest) = pvpanic_port;
> > +}
> > +
> >  static void fill_dsdt(struct fadt_descriptor_rev1 *fadt, void *dsdt)
> >  {
> >      if (fadt->dsdt) {
> >          free((void *)le32_to_cpu(fadt->dsdt));
> >      }
> > +    patch_dsdt(dsdt);
> >      fadt->dsdt = cpu_to_le32((u32)dsdt);
> >      fadt->checksum -= checksum(fadt, sizeof(*fadt));
> >      dprintf(1, "ACPI DSDT=%p\n", dsdt);
> > 

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Michael S. Tsirkin]

On Thu, Mar 21, 2013 at 05:08:34PM +0800, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

OK now you need to ACK GPLv2+ relicensing too :)

Could you please review that message "make acpi bits GPLv2 compatible"
and respond?

> ---
>  src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
>  src/acpi.c            |  9 +++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
> index 87a31b9..43fe719 100644
> --- a/src/acpi-dsdt-isa.dsl
> +++ b/src/acpi-dsdt-isa.dsl
> @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
>  
>      Device(PEVT) {
>          Name(_HID, "QEMU0001")
> -        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
> +        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
> +        Name(PEST, 0x505)
> +        OperationRegion(PEOR, SystemIO, PEST, 0x01)
>          Field(PEOR, ByteAcc, NoLock, Preserve) {
>              PEPT,   8,
>          }
> @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
>          }
>  
>          Name(_CRS, ResourceTemplate() {
> -            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
> +            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
>          })
> +
> +        CreateWordField(_CRS, IO._MIN, IOMN)
> +        CreateWordField(_CRS, IO._MAX, IOMX)
> +
> +        Method(_INI, 0, NotSerialized) {
> +            Store(PEST, IOMN)
> +            Store(PEST, IOMX)
> +        }
>      }
>  }
> diff --git a/src/acpi.c b/src/acpi.c
> index 119d1c1..42fa06e 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
>      PCI_DEVICE_END
>  };
>  
> +static void patch_dsdt(void *dsdt)
> +{
> +    u8 *dsdt_ptr = dsdt;
> +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);
> +
> +    *(u16 *)(dsdt_ptr + *dsdt_isa_pest) = pvpanic_port;
> +}
> +
>  static void fill_dsdt(struct fadt_descriptor_rev1 *fadt, void *dsdt)
>  {
>      if (fadt->dsdt) {
>          free((void *)le32_to_cpu(fadt->dsdt));
>      }
> +    patch_dsdt(dsdt);
>      fadt->dsdt = cpu_to_le32((u32)dsdt);
>      fadt->checksum -= checksum(fadt, sizeof(*fadt));
>      dprintf(1, "ACPI DSDT=%p\n", dsdt);
> -- 
> 1.8.1.4

Re: [Qemu-devel] [SeaBIOS] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Kevin O'Connor]

On Thu, Mar 21, 2013 at 05:08:34PM +0800, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

I don't think it is a good idea to dynamically modify the DSDT.  We've
been using the SSDT for that.

In any case, I think this would be a good candidate for merging after
the ACPI stuff is moved into QEMU.

-Kevin

Re: [Qemu-devel] [PATCH v15 4/6] pvpanic: add document of pvpanic

[Eric Blake]

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

On 03/21/2013 02:35 AM, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  docs/specs/pvpanic.txt | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 docs/specs/pvpanic.txt

Grammar nits:

> 
> diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
> new file mode 100644
> index 0000000..761d20c
> --- /dev/null
> +++ b/docs/specs/pvpanic.txt
> @@ -0,0 +1,25 @@
> +PVPANIC DEVICE
> +==============
> +
> +pvpanic device is a simulated ISA device, through which guest panic

s/which/which a/

> +event is sent to qemu, and a QMP event is generated. This allows
> +management apps(e.g. libvirt) to be notified and respond to the

space before ( in English text.

> +event.
> +
> +pvpanic uses port 0x505 by default to receive panic event from guest.

s/receive/receive a/
s/from guest/from the guest/

> +The port is configurable by specifying ioport property.
> +
> +pvpanic device is defined with ACPI ID "QEMU0001". To send panic

s/send/send a/

> +event, guest evaluates method WRPT, specifying a byte, bit 0 set,
s/guest/the guest/
/byte, bit 0/byte with bit 0/

> +as argument. Other bits are reserved.
> +
> +To use it, one will have to:
> +
> +1. add the device by specifying `-device pvpanic' in the qemu command
> +   line.
> +
> +2. load pvpanic device driver in guest OS.

s/in/in the/

> +
> +The management app has the options of waiting for GUEST_PANICKED events,

s/options/option/

> +and/or polling for guest-panicked RunState, to learn when the pvpanic
> +device has fired a panic event.
> 

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


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

Re: [Qemu-devel] [PATCH v15 4/6] pvpanic: add document of pvpanic

[Hu Tao]

On Thu, Mar 21, 2013 at 10:13:50AM +0100, Paolo Bonzini wrote:
> Il 21/03/2013 09:35, Hu Tao ha scritto:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  docs/specs/pvpanic.txt | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >  create mode 100644 docs/specs/pvpanic.txt
> > 
> > diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
> > new file mode 100644
> > index 0000000..761d20c
> > --- /dev/null
> > +++ b/docs/specs/pvpanic.txt
> > @@ -0,0 +1,25 @@
> > +PVPANIC DEVICE
> > +==============
> > +
> > +pvpanic device is a simulated ISA device, through which guest panic
> > +event is sent to qemu, and a QMP event is generated. This allows
> > +management apps(e.g. libvirt) to be notified and respond to the
> > +event.
> > +
> > +pvpanic uses port 0x505 by default to receive panic event from guest.
> > +The port is configurable by specifying ioport property.
> 
> Please document the ISA interface too.
> 
> > +pvpanic device is defined with ACPI ID "QEMU0001". To send panic
> > +event, guest evaluates method WRPT, specifying a byte, bit 0 set,
> > +as argument. Other bits are reserved.
> 
> Please document RDPT too, and mention that the ACPI device will
> automatically refer to the right port in case it is modified.
> 
> > +To use it, one will have to:
> > +
> > +1. add the device by specifying `-device pvpanic' in the qemu command
> > +   line.
> 
> As mentioned earlier, I'd like this to be the default in 1.5 and newer
> machine types.

I must have missed it. Thanks for review!

> 
> Paolo
> 
> > +2. load pvpanic device driver in guest OS.
> > +
> > +The management app has the options of waiting for GUEST_PANICKED events,
> > +and/or polling for guest-panicked RunState, to learn when the pvpanic
> > +device has fired a panic event.
> > 

Re: [Qemu-devel] [PATCH v15 4/6] pvpanic: add document of pvpanic

[Hu Tao]

On Thu, Mar 21, 2013 at 07:59:12PM -0600, Eric Blake wrote:
> On 03/21/2013 02:35 AM, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  docs/specs/pvpanic.txt | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >  create mode 100644 docs/specs/pvpanic.txt
> 
> Grammar nits:
> 
> > 
> > diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
> > new file mode 100644
> > index 0000000..761d20c
> > --- /dev/null
> > +++ b/docs/specs/pvpanic.txt
> > @@ -0,0 +1,25 @@
> > +PVPANIC DEVICE
> > +==============
> > +
> > +pvpanic device is a simulated ISA device, through which guest panic
> 
> s/which/which a/
> 
> > +event is sent to qemu, and a QMP event is generated. This allows
> > +management apps(e.g. libvirt) to be notified and respond to the
> 
> space before ( in English text.
> 
> > +event.
> > +
> > +pvpanic uses port 0x505 by default to receive panic event from guest.
> 
> s/receive/receive a/
> s/from guest/from the guest/
> 
> > +The port is configurable by specifying ioport property.
> > +
> > +pvpanic device is defined with ACPI ID "QEMU0001". To send panic
> 
> s/send/send a/
> 
> > +event, guest evaluates method WRPT, specifying a byte, bit 0 set,
> s/guest/the guest/
> /byte, bit 0/byte with bit 0/
> 
> > +as argument. Other bits are reserved.
> > +
> > +To use it, one will have to:
> > +
> > +1. add the device by specifying `-device pvpanic' in the qemu command
> > +   line.
> > +
> > +2. load pvpanic device driver in guest OS.
> 
> s/in/in the/
> 
> > +
> > +The management app has the options of waiting for GUEST_PANICKED events,
> 
> s/options/option/

Thanks for review!

> 
> > +and/or polling for guest-panicked RunState, to learn when the pvpanic
> > +device has fired a panic event.
> > 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [SeaBIOS] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

On Thu, Mar 21, 2013 at 06:54:59PM -0400, Kevin O'Connor wrote:
> On Thu, Mar 21, 2013 at 05:08:34PM +0800, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> 
> I don't think it is a good idea to dynamically modify the DSDT.  We've
> been using the SSDT for that.
> 
> In any case, I think this would be a good candidate for merging after
> the ACPI stuff is moved into QEMU.

Agreed. What do you think about patch 1? (of course with Paolo's
suggestion applied)

> 
> -Kevin

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

On Thu, Mar 21, 2013 at 11:54:25AM +0200, Michael S. Tsirkin wrote:
> On Thu, Mar 21, 2013 at 05:08:34PM +0800, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> 
> OK now you need to ACK GPLv2+ relicensing too :)
> 
> Could you please review that message "make acpi bits GPLv2 compatible"
> and respond?

Done. Thanks for reminding!

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Gleb Natapov]

On Thu, Mar 21, 2013 at 05:17:11PM +0800, Hu Tao wrote:
> On Thu, Mar 21, 2013 at 10:11:54AM +0100, Paolo Bonzini wrote:
> > Il 21/03/2013 10:08, Hu Tao ha scritto:
> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > > ---
> > >  src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
> > >  src/acpi.c            |  9 +++++++++
> > >  2 files changed, 21 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
> > > index 87a31b9..43fe719 100644
> > > --- a/src/acpi-dsdt-isa.dsl
> > > +++ b/src/acpi-dsdt-isa.dsl
> > > @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
> > >  
> > >      Device(PEVT) {
> > >          Name(_HID, "QEMU0001")
> > > -        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
> > > +        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
> > > +        Name(PEST, 0x505)
> > > +        OperationRegion(PEOR, SystemIO, PEST, 0x01)
> > >          Field(PEOR, ByteAcc, NoLock, Preserve) {
> > >              PEPT,   8,
> > >          }
> > > @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
> > >          }
> > >  
> > >          Name(_CRS, ResourceTemplate() {
> > > -            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
> > > +            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
> > >          })
> > > +
> > > +        CreateWordField(_CRS, IO._MIN, IOMN)
> > > +        CreateWordField(_CRS, IO._MAX, IOMX)
> > > +
> > > +        Method(_INI, 0, NotSerialized) {
> > > +            Store(PEST, IOMN)
> > > +            Store(PEST, IOMX)
> > > +        }
> > >      }
> > >  }
> > > diff --git a/src/acpi.c b/src/acpi.c
> > > index 119d1c1..42fa06e 100644
> > > --- a/src/acpi.c
> > > +++ b/src/acpi.c
> > > @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
> > >      PCI_DEVICE_END
> > >  };
> > >  
> > > +static void patch_dsdt(void *dsdt)
> > > +{
> > > +    u8 *dsdt_ptr = dsdt;
> > > +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);
> > 
> > The default must be 0.
> > 
> > Also, here:
> > 
> > 
> > +
> > +        Method(_STA, 0, NotSerialized) {
> > +            Store(PEPT, Local0)
> > +            If (LEqual(Local0, Zero)) {
> > +                Return (0x00)
> > +            } Else {
> > +                Return (0x0F)
> > +            }
> > +        }
> > +
> > 
> > You must change it to look at PEST instead of PEPT (i.e. do not probe,
> > just see if you have a meaningful address).
> > 
> > Just squash the patches, it's simpler that way probably.
> 
> I forgot to add RFC to the title. This patch doesn't work for q35 with
> custom ioport.
> 
Why doesn't it work with q35?

--
			Gleb.

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

On Fri, Mar 22, 2013 at 09:48:18AM +0200, Gleb Natapov wrote:
> On Thu, Mar 21, 2013 at 05:17:11PM +0800, Hu Tao wrote:
> > On Thu, Mar 21, 2013 at 10:11:54AM +0100, Paolo Bonzini wrote:
> > > Il 21/03/2013 10:08, Hu Tao ha scritto:
> > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > > > ---
> > > >  src/acpi-dsdt-isa.dsl | 14 ++++++++++++--
> > > >  src/acpi.c            |  9 +++++++++
> > > >  2 files changed, 21 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl
> > > > index 87a31b9..43fe719 100644
> > > > --- a/src/acpi-dsdt-isa.dsl
> > > > +++ b/src/acpi-dsdt-isa.dsl
> > > > @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) {
> > > >  
> > > >      Device(PEVT) {
> > > >          Name(_HID, "QEMU0001")
> > > > -        OperationRegion(PEOR, SystemIO, 0x0505, 0x01)
> > > > +        ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest
> > > > +        Name(PEST, 0x505)
> > > > +        OperationRegion(PEOR, SystemIO, PEST, 0x01)
> > > >          Field(PEOR, ByteAcc, NoLock, Preserve) {
> > > >              PEPT,   8,
> > > >          }
> > > > @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) {
> > > >          }
> > > >  
> > > >          Name(_CRS, ResourceTemplate() {
> > > > -            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01)
> > > > +            IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO)
> > > >          })
> > > > +
> > > > +        CreateWordField(_CRS, IO._MIN, IOMN)
> > > > +        CreateWordField(_CRS, IO._MAX, IOMX)
> > > > +
> > > > +        Method(_INI, 0, NotSerialized) {
> > > > +            Store(PEST, IOMN)
> > > > +            Store(PEST, IOMX)
> > > > +        }
> > > >      }
> > > >  }
> > > > diff --git a/src/acpi.c b/src/acpi.c
> > > > index 119d1c1..42fa06e 100644
> > > > --- a/src/acpi.c
> > > > +++ b/src/acpi.c
> > > > @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = {
> > > >      PCI_DEVICE_END
> > > >  };
> > > >  
> > > > +static void patch_dsdt(void *dsdt)
> > > > +{
> > > > +    u8 *dsdt_ptr = dsdt;
> > > > +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x505);
> > > 
> > > The default must be 0.
> > > 
> > > Also, here:
> > > 
> > > 
> > > +
> > > +        Method(_STA, 0, NotSerialized) {
> > > +            Store(PEPT, Local0)
> > > +            If (LEqual(Local0, Zero)) {
> > > +                Return (0x00)
> > > +            } Else {
> > > +                Return (0x0F)
> > > +            }
> > > +        }
> > > +
> > > 
> > > You must change it to look at PEST instead of PEPT (i.e. do not probe,
> > > just see if you have a meaningful address).
> > > 
> > > Just squash the patches, it's simpler that way probably.
> > 
> > I forgot to add RFC to the title. This patch doesn't work for q35 with
> > custom ioport.
> > 
> Why doesn't it work with q35?

ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest generates two dsdt_isa_pest,
one in out/acpi-dsdt.hex the other in out/q35-acpi-dsdt.hex. We should
use the one in q35-acpi-dsdt.hex to patch DSDT for q35. But including
q35-acpi-dsdt.hex, along with acpi-dsdt.hex, will result in redefinition
of dsdt_isa_pest. I didn't see a simple way to solve this.


-- 
Regards,
Hu Tao

[Qemu-devel] [PATCH v15.1 4/6] pvpanic: add document of pvpanic

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 docs/specs/pvpanic.txt | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 docs/specs/pvpanic.txt

diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
new file mode 100644
index 0000000..1f14063
--- /dev/null
+++ b/docs/specs/pvpanic.txt
@@ -0,0 +1,35 @@
+PVPANIC DEVICE
+==============
+
+pvpanic device is a simulated ISA device, through which a guest panic
+event is sent to qemu, and a QMP event is generated. This allows
+management apps (e.g. libvirt) to be notified and respond to the event.
+
+The management app has the option of waiting for GUEST_PANICKED events,
+and/or polling for guest-panicked RunState, to learn when the pvpanic
+device has fired a panic event.
+
+ISA Interface
+-------------
+
+pvpanic uses port 0x505 to receive a panic event from the guest. On
+written, bit 0 is set to indicate guest panic has happened. On read, bit
+0 is set to indicate guest panic notification is supported.
+
+ACPI Interface
+--------------
+
+pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
+
+RDPT:       To determine whether guest panic notification is supported.
+Rrguments:  None
+Return:     Returns a byte, bit 0 set to indicate guest panic
+            notification is supported.
+
+WRPT:       To send a guest panic event
+Arguments:  Arg0 is a byte, with bit 0 set to indicate guest panic has
+            happened.
+Return:     None
+
+The ACPI device will automatically refer to the right port in case it
+is modified.
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Paolo Bonzini]

Il 26/03/2013 02:59, Hu Tao ha scritto:
>>> > > 
>> > Why doesn't it work with q35?
> ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest generates two dsdt_isa_pest,
> one in out/acpi-dsdt.hex the other in out/q35-acpi-dsdt.hex. We should
> use the one in q35-acpi-dsdt.hex to patch DSDT for q35. But including
> q35-acpi-dsdt.hex, along with acpi-dsdt.hex, will result in redefinition
> of dsdt_isa_pest. I didn't see a simple way to solve this.

Compile it in a separate file and include it in the SSDT (build_ssdt in
src/acpi.c).

Paolo

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Hu Tao]

On Tue, Mar 26, 2013 at 08:03:09AM +0100, Paolo Bonzini wrote:
> Il 26/03/2013 02:59, Hu Tao ha scritto:
> >>> > > 
> >> > Why doesn't it work with q35?
> > ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest generates two dsdt_isa_pest,
> > one in out/acpi-dsdt.hex the other in out/q35-acpi-dsdt.hex. We should
> > use the one in q35-acpi-dsdt.hex to patch DSDT for q35. But including
> > q35-acpi-dsdt.hex, along with acpi-dsdt.hex, will result in redefinition
> > of dsdt_isa_pest. I didn't see a simple way to solve this.
> 
> Compile it in a separate file and include it in the SSDT (build_ssdt in
> src/acpi.c).

Doesn't work. iasl gives an error:

Data Table Compiler is not available yet

Even though it compiles, I think the generated offset (*dsdt_isa_pest)
won't be correct because it depends on acpi-dsdt.dsl/q35-acpi-dsdt.dsl.

-- 
Regards,
Hu Tao

Re: [Qemu-devel] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport

[Paolo Bonzini]

Il 26/03/2013 08:24, Hu Tao ha scritto:
>>> > > ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest generates two dsdt_isa_pest,
>>> > > one in out/acpi-dsdt.hex the other in out/q35-acpi-dsdt.hex. We should
>>> > > use the one in q35-acpi-dsdt.hex to patch DSDT for q35. But including
>>> > > q35-acpi-dsdt.hex, along with acpi-dsdt.hex, will result in redefinition
>>> > > of dsdt_isa_pest. I didn't see a simple way to solve this.
>> > 
>> > Compile it in a separate file and include it in the SSDT (build_ssdt in
>> > src/acpi.c).
> Doesn't work. iasl gives an error:
> 
> Data Table Compiler is not available yet
> 
> Even though it compiles, I think the generated offset (*dsdt_isa_pest)
> won't be correct because it depends on acpi-dsdt.dsl/q35-acpi-dsdt.dsl.

See how it is done for ssdt-misc.aml (in fact, you can simply extend
ssdt-misc.aml).

Paolo

[Qemu-devel] [PATCH v15.1] Add pvpanic device driver

[Hu Tao]

pvpanic device is used to notify host(qemu) when guest panic happens.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---

Paolo,

Can I add your Signed-off-by? Since the original code is given by you.

Thank you, Paolo :)

 src/acpi.c        |  3 +++
 src/ssdt-misc.dsl | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/src/acpi.c b/src/acpi.c
index bc4d8ea..fe504f0 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -534,6 +534,9 @@ build_ssdt(void)
         ssdt_ptr[acpi_pci64_valid[0]] = 0;
     }
 
+    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x0);
+    *(u16 *)(ssdt_ptr + *ssdt_isa_pest) = pvpanic_port;
+
     ssdt_ptr += sizeof(ssdp_misc_aml);
 
     // build Scope(_SB_) header
diff --git a/src/ssdt-misc.dsl b/src/ssdt-misc.dsl
index 679422b..acc850e 100644
--- a/src/ssdt-misc.dsl
+++ b/src/ssdt-misc.dsl
@@ -55,4 +55,50 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
             Zero   /* reserved */
         })
     }
+
+    External(\_SB.PCI0, DeviceObj)
+    External(\_SB.PCI0.ISA, DeviceObj)
+
+    Scope(\_SB.PCI0.ISA) {
+        Device(PEVT) {
+            Name(_HID, "QEMU0001")
+            /* PEST will be patched to be Zero if no such device */
+            ACPI_EXTRACT_NAME_WORD_CONST ssdt_isa_pest
+            Name(PEST, 0xFFFF)
+            OperationRegion(PEOR, SystemIO, PEST, 0x01)
+            Field(PEOR, ByteAcc, NoLock, Preserve) {
+                PEPT,   8,
+            }
+
+            Method(_STA, 0, NotSerialized) {
+                Store(PEST, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(RDPT, 0, NotSerialized) {
+                Store(PEPT, Local0)
+                Return (Local0)
+            }
+
+            Method(WRPT, 1, NotSerialized) {
+                Store(Arg0, PEPT)
+            }
+
+            Name(_CRS, ResourceTemplate() {
+                IO(Decode16, 0x00, 0x00, 0x01, 0x01, IO)
+            })
+
+            CreateWordField(_CRS, IO._MIN, IOMN)
+            CreateWordField(_CRS, IO._MAX, IOMX)
+
+            Method(_INI, 0, NotSerialized) {
+                Store(PEST, IOMN)
+                Store(PEST, IOMX)
+            }
+        }
+    }
 }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v15.1] Add pvpanic device driver

[Paolo Bonzini]

Il 26/03/2013 09:52, Hu Tao ha scritto:
> pvpanic device is used to notify host(qemu) when guest panic happens.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> 
> Paolo,
> 
> Can I add your Signed-off-by? Since the original code is given by you.

Sure, as you prefer.

Paolo

>  src/acpi.c        |  3 +++
>  src/ssdt-misc.dsl | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/src/acpi.c b/src/acpi.c
> index bc4d8ea..fe504f0 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -534,6 +534,9 @@ build_ssdt(void)
>          ssdt_ptr[acpi_pci64_valid[0]] = 0;
>      }
>  
> +    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x0);
> +    *(u16 *)(ssdt_ptr + *ssdt_isa_pest) = pvpanic_port;
> +
>      ssdt_ptr += sizeof(ssdp_misc_aml);
>  
>      // build Scope(_SB_) header
> diff --git a/src/ssdt-misc.dsl b/src/ssdt-misc.dsl
> index 679422b..acc850e 100644
> --- a/src/ssdt-misc.dsl
> +++ b/src/ssdt-misc.dsl
> @@ -55,4 +55,50 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
>              Zero   /* reserved */
>          })
>      }
> +
> +    External(\_SB.PCI0, DeviceObj)
> +    External(\_SB.PCI0.ISA, DeviceObj)
> +
> +    Scope(\_SB.PCI0.ISA) {
> +        Device(PEVT) {
> +            Name(_HID, "QEMU0001")
> +            /* PEST will be patched to be Zero if no such device */
> +            ACPI_EXTRACT_NAME_WORD_CONST ssdt_isa_pest
> +            Name(PEST, 0xFFFF)
> +            OperationRegion(PEOR, SystemIO, PEST, 0x01)
> +            Field(PEOR, ByteAcc, NoLock, Preserve) {
> +                PEPT,   8,
> +            }
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(PEST, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(RDPT, 0, NotSerialized) {
> +                Store(PEPT, Local0)
> +                Return (Local0)
> +            }
> +
> +            Method(WRPT, 1, NotSerialized) {
> +                Store(Arg0, PEPT)
> +            }
> +
> +            Name(_CRS, ResourceTemplate() {
> +                IO(Decode16, 0x00, 0x00, 0x01, 0x01, IO)
> +            })
> +
> +            CreateWordField(_CRS, IO._MIN, IOMN)
> +            CreateWordField(_CRS, IO._MAX, IOMX)
> +
> +            Method(_INI, 0, NotSerialized) {
> +                Store(PEST, IOMN)
> +                Store(PEST, IOMX)
> +            }
> +        }
> +    }
>  }
> 

[Qemu-devel] [PATCH v15.1 resend] Add pvpanic device driver

[Hu Tao]

pvpanic device is used to notify host(qemu) when guest panic happens.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 src/acpi.c        |  3 +++
 src/ssdt-misc.dsl | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/src/acpi.c b/src/acpi.c
index bc4d8ea..fe504f0 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -534,6 +534,9 @@ build_ssdt(void)
         ssdt_ptr[acpi_pci64_valid[0]] = 0;
     }
 
+    int pvpanic_port = romfile_loadint("etc/pvpanic-port", 0x0);
+    *(u16 *)(ssdt_ptr + *ssdt_isa_pest) = pvpanic_port;
+
     ssdt_ptr += sizeof(ssdp_misc_aml);
 
     // build Scope(_SB_) header
diff --git a/src/ssdt-misc.dsl b/src/ssdt-misc.dsl
index 679422b..acc850e 100644
--- a/src/ssdt-misc.dsl
+++ b/src/ssdt-misc.dsl
@@ -55,4 +55,50 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
             Zero   /* reserved */
         })
     }
+
+    External(\_SB.PCI0, DeviceObj)
+    External(\_SB.PCI0.ISA, DeviceObj)
+
+    Scope(\_SB.PCI0.ISA) {
+        Device(PEVT) {
+            Name(_HID, "QEMU0001")
+            /* PEST will be patched to be Zero if no such device */
+            ACPI_EXTRACT_NAME_WORD_CONST ssdt_isa_pest
+            Name(PEST, 0xFFFF)
+            OperationRegion(PEOR, SystemIO, PEST, 0x01)
+            Field(PEOR, ByteAcc, NoLock, Preserve) {
+                PEPT,   8,
+            }
+
+            Method(_STA, 0, NotSerialized) {
+                Store(PEST, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(RDPT, 0, NotSerialized) {
+                Store(PEPT, Local0)
+                Return (Local0)
+            }
+
+            Method(WRPT, 1, NotSerialized) {
+                Store(Arg0, PEPT)
+            }
+
+            Name(_CRS, ResourceTemplate() {
+                IO(Decode16, 0x00, 0x00, 0x01, 0x01, IO)
+            })
+
+            CreateWordField(_CRS, IO._MIN, IOMN)
+            CreateWordField(_CRS, IO._MAX, IOMX)
+
+            Method(_INI, 0, NotSerialized) {
+                Store(PEST, IOMN)
+                Store(PEST, IOMX)
+            }
+        }
+    }
 }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v15.1 4/6] pvpanic: add document of pvpanic

[Paolo Bonzini]

Il 26/03/2013 04:03, Hu Tao ha scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  docs/specs/pvpanic.txt | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 docs/specs/pvpanic.txt
> 
> diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
> new file mode 100644
> index 0000000..1f14063
> --- /dev/null
> +++ b/docs/specs/pvpanic.txt
> @@ -0,0 +1,35 @@
> +PVPANIC DEVICE
> +==============
> +
> +pvpanic device is a simulated ISA device, through which a guest panic
> +event is sent to qemu, and a QMP event is generated. This allows
> +management apps (e.g. libvirt) to be notified and respond to the event.
> +
> +The management app has the option of waiting for GUEST_PANICKED events,
> +and/or polling for guest-panicked RunState, to learn when the pvpanic
> +device has fired a panic event.
> +
> +ISA Interface
> +-------------
> +
> +pvpanic uses port 0x505 to receive a panic event from the guest. On
> +written, bit 0 is set to indicate guest panic has happened. On read, bit
> +0 is set to indicate guest panic notification is supported.
> +
> +ACPI Interface
> +--------------
> +
> +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> +
> +RDPT:       To determine whether guest panic notification is supported.
> +Rrguments:  None
> +Return:     Returns a byte, bit 0 set to indicate guest panic
> +            notification is supported.
> +
> +WRPT:       To send a guest panic event
> +Arguments:  Arg0 is a byte, with bit 0 set to indicate guest panic has
> +            happened.
> +Return:     None
> +
> +The ACPI device will automatically refer to the right port in case it
> +is modified.
> 

Looks good.

Paolo

Re: [Qemu-devel] [PATCH v15.1 4/6] pvpanic: add document of pvpanic

[Eric Blake]

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

On 03/25/2013 09:03 PM, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  docs/specs/pvpanic.txt | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 docs/specs/pvpanic.txt

> +ISA Interface
> +-------------
> +
> +pvpanic uses port 0x505 to receive a panic event from the guest. On
> +written, bit 0 is set to indicate guest panic has happened. On read, bit

s/written/write/

Also, mention that the remaining bits are reserved, and should be
written as 0, and ignored on read (that way, the spec can be updated
when we define a later bit, and older hosts won't be trying to set or
act on bits they don't understand).


> +ACPI Interface
> +--------------
> +
> +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> +
> +RDPT:       To determine whether guest panic notification is supported.
> +Rrguments:  None
> +Return:     Returns a byte, bit 0 set to indicate guest panic
> +            notification is supported.
> +
> +WRPT:       To send a guest panic event
> +Arguments:  Arg0 is a byte, with bit 0 set to indicate guest panic has
> +            happened.

Again, documenting that the remaining bits are reserved (write 0, ignore
on read) is a wise addition.

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


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

[Qemu-devel] [PATCH v15.2 4/6] pvpanic: add document of pvpanic

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 docs/specs/pvpanic.txt | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 docs/specs/pvpanic.txt

diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
new file mode 100644
index 0000000..6fe8bb6
--- /dev/null
+++ b/docs/specs/pvpanic.txt
@@ -0,0 +1,37 @@
+PVPANIC DEVICE
+==============
+
+pvpanic device is a simulated ISA device, through which a guest panic
+event is sent to qemu, and a QMP event is generated. This allows
+management apps (e.g. libvirt) to be notified and respond to the event.
+
+The management app has the option of waiting for GUEST_PANICKED events,
+and/or polling for guest-panicked RunState, to learn when the pvpanic
+device has fired a panic event.
+
+ISA Interface
+-------------
+
+pvpanic uses port 0x505 to receive a panic event from the guest. On
+write, bit 0 is set to indicate guest panic has happened. On read, bit
+0 is set to indicate guest panic notification is supported. Remaining
+bits are reserved, and should be written as 0, and ignored on read.
+
+ACPI Interface
+--------------
+
+pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
+
+RDPT:       To determine whether guest panic notification is supported.
+Rrguments:  None
+Return:     Returns a byte, bit 0 set to indicate guest panic
+            notification is supported. Other bits are reserved and
+            should be ignored.
+
+WRPT:       To send a guest panic event
+Arguments:  Arg0 is a byte, with bit 0 set to indicate guest panic has
+            happened. Other bits are reserved and should be cleared.
+Return:     None
+
+The ACPI device will automatically refer to the right port in case it
+is modified.
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v15.2 4/6] pvpanic: add document of pvpanic

[Eric Blake]

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

On 03/28/2013 12:16 AM, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  docs/specs/pvpanic.txt | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 docs/specs/pvpanic.txt

One typo remaining:

> +ACPI Interface
> +--------------
> +
> +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> +
> +RDPT:       To determine whether guest panic notification is supported.
> +Rrguments:  None

s/Rrguments/Arguments/

I think that can be fixed by whoever merges and sends the pull request,
without needing you to do a respin.  With that fixed, you have:

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

Re: [Qemu-devel] [PATCH v15.2 4/6] pvpanic: add document of pvpanic

[Paolo Bonzini]

----- Messaggio originale -----
> Da: "Eric Blake" <eblake@redhat.com>
> A: "Hu Tao" <hutao@cn.fujitsu.com>
> Cc: "qemu-devel" <qemu-devel@nongnu.org>, "Paolo Bonzini" <pbonzini@redhat.com>
> Inviato: Giovedì, 28 marzo 2013 13:16:32
> Oggetto: Re: [PATCH v15.2 4/6] pvpanic: add document of pvpanic
> 
> On 03/28/2013 12:16 AM, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  docs/specs/pvpanic.txt | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 docs/specs/pvpanic.txt
> 
> One typo remaining:
> 
> > +ACPI Interface
> > +--------------
> > +
> > +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> > +
> > +RDPT:       To determine whether guest panic notification is
> > supported.
> > +Rrguments:  None
> 
> s/Rrguments/Arguments/
> 
> I think that can be fixed by whoever merges and sends the pull
> request,
> without needing you to do a respin.  With that fixed, you have:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Hu, please repost the whole series as v16.  Simpler. :)

Paolo

Re: [Qemu-devel] [PATCH v15.2 4/6] pvpanic: add document of pvpanic

[Hu Tao]

On Thu, Mar 28, 2013 at 06:16:32AM -0600, Eric Blake wrote:
> On 03/28/2013 12:16 AM, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  docs/specs/pvpanic.txt | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 docs/specs/pvpanic.txt
> 
> One typo remaining:
> 
> > +ACPI Interface
> > +--------------
> > +
> > +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> > +
> > +RDPT:       To determine whether guest panic notification is supported.
> > +Rrguments:  None
> 
> s/Rrguments/Arguments/
> 
> I think that can be fixed by whoever merges and sends the pull request,
> without needing you to do a respin.  With that fixed, you have:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks for review!

-- 
Regards,
Hu Tao

Re: [Qemu-devel] [PATCH v15.2 4/6] pvpanic: add document of pvpanic

[Hu Tao]

On Thu, Mar 28, 2013 at 10:57:57AM -0400, Paolo Bonzini wrote:
> 
> 
> ----- Messaggio originale -----
> > Da: "Eric Blake" <eblake@redhat.com>
> > A: "Hu Tao" <hutao@cn.fujitsu.com>
> > Cc: "qemu-devel" <qemu-devel@nongnu.org>, "Paolo Bonzini" <pbonzini@redhat.com>
> > Inviato: Giovedì, 28 marzo 2013 13:16:32
> > Oggetto: Re: [PATCH v15.2 4/6] pvpanic: add document of pvpanic
> > 
> > On 03/28/2013 12:16 AM, Hu Tao wrote:
> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > > ---
> > >  docs/specs/pvpanic.txt | 37 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 37 insertions(+)
> > >  create mode 100644 docs/specs/pvpanic.txt
> > 
> > One typo remaining:
> > 
> > > +ACPI Interface
> > > +--------------
> > > +
> > > +pvpanic device is defined with ACPI ID "QEMU0001". Custom methods:
> > > +
> > > +RDPT:       To determine whether guest panic notification is
> > > supported.
> > > +Rrguments:  None
> > 
> > s/Rrguments/Arguments/
> > 
> > I think that can be fixed by whoever merges and sends the pull
> > request,
> > without needing you to do a respin.  With that fixed, you have:
> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Hu, please repost the whole series as v16.  Simpler. :)

No problem :)

-- 
Regards,
Hu Tao