[PATCH 0/8] Add support for pvpanic mmio device

[PATCH 0/8] Add support for pvpanic mmio device

[Mihai Carabas]

The patchset was assembled from chuncks from some old patches from 2018 [1]
which were left unmerged and some additions from me. Surprisingly their Linux
kernel counterpart were merged (so the pvpanic driver from the kernel supports
mmio).

I have seen the discussions about moving the pvpanic to PCI [1]. Those patches
were sent but nothing happened. Also they are not trivial and require major
modifications at the driver level also. Given the fact that we already have
mmio driver support for pvpanic in the Linux kernel, I have sent these patches
to ask again the maintainers if this can be merged.

How to test this:
/usr/bin/qemu-system-aarch64 \
        -machine virt,gic-version=3,pvpanic=on

There is a new property for machine virt called pvpanic which is by default
turned off.

[1] http://patchwork.ozlabs.org/project/qemu-devel/cover/1543865209-50994-1-git-send-email-peng.hao2@zte.com.cn/

Mihai Carabas (1):
  pvpanic: break dependency on ISA_BUS

Peng Hao (5):
  hw/misc/pvpanic: Add the MMIO interface
  hw/arm/virt: Use the pvpanic device
  hw/arm/virt: add pvpanic device in virt acpi table
  hw/arm/virt: add configure interface for pvpanic-mmio
  pvpanic : update pvpanic document

Philippe Mathieu-Daudé (2):
  hw/misc/pvpanic: Build the pvpanic device for any machine
  hw/misc/pvpanic: Cosmetic renaming

 docs/specs/pvpanic.txt    | 12 ++++++-
 hw/arm/Kconfig            |  1 +
 hw/arm/virt-acpi-build.c  | 17 ++++++++++
 hw/arm/virt.c             | 47 +++++++++++++++++++++++++++
 hw/misc/Kconfig           |  2 +-
 hw/misc/meson.build       |  2 +-
 hw/misc/pvpanic.c         | 83 ++++++++++++++++++++++++++++++++++++++---------
 include/hw/arm/virt.h     |  2 ++
 include/hw/misc/pvpanic.h |  5 +--
 9 files changed, 150 insertions(+), 21 deletions(-)

-- 
1.8.3.1

[PATCH 1/8] hw/misc/pvpanic: Build the pvpanic device for any machine

[Mihai Carabas]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The 'pvpanic' ISA device can be use by any machine with an ISA bus.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/misc/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 793d45b..cb250dd 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -11,6 +11,7 @@ softmmu_ss.add(when: 'CONFIG_TMP105', if_true: files('tmp105.c'))
 softmmu_ss.add(when: 'CONFIG_TMP421', if_true: files('tmp421.c'))
 softmmu_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
 softmmu_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
+softmmu_ss.add(when: 'CONFIG_PVPANIC', if_true: files('pvpanic.c'))
 
 # ARM devices
 softmmu_ss.add(when: 'CONFIG_PL310', if_true: files('arm_l2x0.c'))
@@ -90,7 +91,6 @@ softmmu_ss.add(when: 'CONFIG_IOTKIT_SYSINFO', if_true: files('iotkit-sysinfo.c')
 softmmu_ss.add(when: 'CONFIG_ARMSSE_CPUID', if_true: files('armsse-cpuid.c'))
 softmmu_ss.add(when: 'CONFIG_ARMSSE_MHU', if_true: files('armsse-mhu.c'))
 
-softmmu_ss.add(when: 'CONFIG_PVPANIC', if_true: files('pvpanic.c'))
 softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c'))
 softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_scu.c', 'aspeed_sdmc.c', 'aspeed_xdma.c'))
 softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
-- 
1.8.3.1

[PATCH 2/8] hw/misc/pvpanic: Cosmetic renaming

[Mihai Carabas]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

To ease the MMIO device addition in the next patch, rename:
- ISA_PVPANIC_DEVICE -> PVPANIC_ISA_DEVICE.
- PVPanicState -> PVPanicISAState.
- TYPE_PVPANIC -> TYPE_PVPANIC_ISA.
- MemoryRegion io -> mr.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/misc/pvpanic.c         | 28 ++++++++++++++++------------
 include/hw/misc/pvpanic.h |  2 +-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 598d547..2e085f4 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -30,9 +30,9 @@
 #define PVPANIC_PANICKED        (1 << PVPANIC_F_PANICKED)
 #define PVPANIC_CRASHLOADED     (1 << PVPANIC_F_CRASHLOADED)
 
-typedef struct PVPanicState PVPanicState;
-DECLARE_INSTANCE_CHECKER(PVPanicState, ISA_PVPANIC_DEVICE,
-                         TYPE_PVPANIC)
+typedef struct PVPanicISAState PVPanicISAState;
+DECLARE_INSTANCE_CHECKER(PVPanicISAState, PVPANIC_ISA_DEVICE,
+                         TYPE_PVPANIC_ISA)
 
 static void handle_event(int event)
 {
@@ -56,10 +56,14 @@ static void handle_event(int event)
 
 #include "hw/isa/isa.h"
 
-struct PVPanicState {
+/*
+ * PVPanicISAState for ISA device and
+ * use ioport.
+ */
+struct PVPanicISAState {
     ISADevice parent_obj;
 
-    MemoryRegion io;
+    MemoryRegion mr;
     uint16_t ioport;
 };
 
@@ -86,15 +90,15 @@ static const MemoryRegionOps pvpanic_ops = {
 
 static void pvpanic_isa_initfn(Object *obj)
 {
-    PVPanicState *s = ISA_PVPANIC_DEVICE(obj);
+    PVPanicISAState *s = PVPANIC_ISA_DEVICE(obj);
 
-    memory_region_init_io(&s->io, OBJECT(s), &pvpanic_ops, s, "pvpanic", 1);
+    memory_region_init_io(&s->mr, OBJECT(s), &pvpanic_ops, s, "pvpanic", 1);
 }
 
 static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
 {
     ISADevice *d = ISA_DEVICE(dev);
-    PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
+    PVPanicISAState *s = PVPANIC_ISA_DEVICE(dev);
     FWCfgState *fw_cfg = fw_cfg_find();
     uint16_t *pvpanic_port;
 
@@ -107,11 +111,11 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
     fw_cfg_add_file(fw_cfg, "etc/pvpanic-port", pvpanic_port,
                     sizeof(*pvpanic_port));
 
-    isa_register_ioport(d, &s->io, s->ioport);
+    isa_register_ioport(d, &s->mr, s->ioport);
 }
 
 static Property pvpanic_isa_properties[] = {
-    DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicState, ioport, 0x505),
+    DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -125,9 +129,9 @@ static void pvpanic_isa_class_init(ObjectClass *klass, void *data)
 }
 
 static TypeInfo pvpanic_isa_info = {
-    .name          = TYPE_PVPANIC,
+    .name          = TYPE_PVPANIC_ISA,
     .parent        = TYPE_ISA_DEVICE,
-    .instance_size = sizeof(PVPanicState),
+    .instance_size = sizeof(PVPanicISAState),
     .instance_init = pvpanic_isa_initfn,
     .class_init    = pvpanic_isa_class_init,
 };
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index ae0c818..30e9f8f 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -17,7 +17,7 @@
 
 #include "qom/object.h"
 
-#define TYPE_PVPANIC "pvpanic"
+#define TYPE_PVPANIC_ISA "pvpanic"
 
 #define PVPANIC_IOPORT_PROP "ioport"
 
-- 
1.8.3.1

[PATCH 3/8] hw/misc/pvpanic: Add the MMIO interface

[Mihai Carabas]

From: Peng Hao <peng.hao2@zte.com.cn>

Add new pvpanic type: "TYPE_PVPANIC_MMIO".

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/misc/pvpanic.c         | 51 +++++++++++++++++++++++++++++++++++++++++++----
 include/hw/misc/pvpanic.h |  3 ++-
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 2e085f4..136f1d6 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -2,10 +2,12 @@
  * QEMU simulated pvpanic device.
  *
  * Copyright Fujitsu, Corp. 2013
+ * Copyright (c) 2018 ZTE Ltd.
  *
  * Authors:
  *     Wen Congyang <wency@cn.fujitsu.com>
  *     Hu Tao <hutao@cn.fujitsu.com>
+ *     Peng Hao <peng.hao2@zte.com.cn>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -34,6 +36,10 @@ typedef struct PVPanicISAState PVPanicISAState;
 DECLARE_INSTANCE_CHECKER(PVPanicISAState, PVPANIC_ISA_DEVICE,
                          TYPE_PVPANIC_ISA)
 
+typedef struct PVPanicMMIOState PVPanicMMIOState;
+DECLARE_INSTANCE_CHECKER(PVPanicMMIOState, PVPANIC_MMIO_DEVICE,
+                         TYPE_PVPANIC_MMIO)
+
 static void handle_event(int event)
 {
     static bool logged;
@@ -67,21 +73,32 @@ struct PVPanicISAState {
     uint16_t ioport;
 };
 
+/* PVPanicMMIOState for sysbus device and
+ * use mmio.
+ */
+typedef struct PVPanicMMIOState {
+    SysBusDevice parent_obj;
+    /*<private>*/
+
+    /* public */
+    MemoryRegion mr;
+} PVPanicMMIOState;
+
 /* return supported events on read */
-static uint64_t pvpanic_ioport_read(void *opaque, hwaddr addr, unsigned size)
+static uint64_t pvpanic_read(void *opaque, hwaddr addr, unsigned size)
 {
     return PVPANIC_PANICKED;
 }
 
-static void pvpanic_ioport_write(void *opaque, hwaddr addr, uint64_t val,
+static void pvpanic_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,
+    .read = pvpanic_read,
+    .write = pvpanic_write,
     .impl = {
         .min_access_size = 1,
         .max_access_size = 1,
@@ -136,9 +153,35 @@ static TypeInfo pvpanic_isa_info = {
     .class_init    = pvpanic_isa_class_init,
 };
 
+static void pvpanic_mmio_initfn(Object *obj)
+{
+    PVPanicMMIOState *s = PVPANIC_MMIO_DEVICE(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    memory_region_init_io(&s->mr, OBJECT(s), &pvpanic_ops, s,
+                          TYPE_PVPANIC_MMIO, 2);
+    sysbus_init_mmio(sbd, &s->mr);
+}
+
+static void pvpanic_mmio_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static TypeInfo pvpanic_mmio_info = {
+    .name          = TYPE_PVPANIC_MMIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PVPanicMMIOState),
+    .instance_init = pvpanic_mmio_initfn,
+    .class_init    = pvpanic_mmio_class_init,
+};
+
 static void pvpanic_register_types(void)
 {
     type_register_static(&pvpanic_isa_info);
+    type_register_static(&pvpanic_mmio_info);
 }
 
 type_init(pvpanic_register_types)
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index 30e9f8f..ee645dc 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -18,12 +18,13 @@
 #include "qom/object.h"
 
 #define TYPE_PVPANIC_ISA "pvpanic"
+#define TYPE_PVPANIC_MMIO "pvpanic-mmio"
 
 #define PVPANIC_IOPORT_PROP "ioport"
 
 static inline uint16_t pvpanic_port(void)
 {
-    Object *o = object_resolve_path_type("", TYPE_PVPANIC, NULL);
+    Object *o = object_resolve_path_type("", TYPE_PVPANIC_ISA, NULL);
     if (!o) {
         return 0;
     }
-- 
1.8.3.1

[PATCH 4/8] hw/arm/virt: Use the pvpanic device

[Mihai Carabas]

From: Peng Hao <peng.hao2@zte.com.cn>

Add pvpanic device in arm virt machine.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/arm/virt.c         | 25 +++++++++++++++++++++++++
 include/hw/arm/virt.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e465a98..63f09a6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -78,6 +78,7 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
+#include "hw/misc/pvpanic.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -152,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_ACPI_GED] =           { 0x09080000, ACPI_GED_EVT_SEL_LEN },
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
     [VIRT_PVTIME] =             { 0x090a0000, 0x00010000 },
+    [VIRT_PVPANIC] =            { 0x090b0000, 0x00000002 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -226,6 +228,27 @@ static void create_kaslr_seed(VirtMachineState *vms, const char *node)
     qemu_fdt_setprop_u64(vms->fdt, node, "kaslr-seed", seed);
 }
 
+static void create_pvpanic_device(const VirtMachineState *vms)
+{
+    char *nodename;
+    hwaddr base = vms->memmap[VIRT_PVPANIC].base;
+    hwaddr size = vms->memmap[VIRT_PVPANIC].size;
+
+    if (!vms->pvpanic) {
+        return;
+    }
+    sysbus_create_simple(TYPE_PVPANIC_MMIO, base, NULL);
+
+    nodename = g_strdup_printf("/pvpanic-mmio@%" PRIx64, base);
+    qemu_fdt_add_subnode(vms->fdt, nodename);
+    qemu_fdt_setprop_string(vms->fdt, nodename,
+                            "compatible", "qemu,pvpanic-mmio");
+    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
+                                 2, base, 2, size);
+
+    g_free(nodename);
+}
+
 static void create_fdt(VirtMachineState *vms)
 {
     MachineState *ms = MACHINE(vms);
@@ -1957,6 +1980,8 @@ static void machvirt_init(MachineState *machine)
 
     virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
 
+    create_pvpanic_device(vms);
+
     create_gic(vms);
 
     virt_cpu_post_init(vms, possible_cpus->len, sysmem);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index aad6d69..e6410c3 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -69,6 +69,7 @@ enum {
     VIRT_GIC_ITS,
     VIRT_GIC_REDIST,
     VIRT_SMMU,
+    VIRT_PVPANIC,
     VIRT_UART,
     VIRT_MMIO,
     VIRT_RTC,
@@ -139,6 +140,7 @@ struct VirtMachineState {
     bool highmem;
     bool highmem_ecam;
     bool its;
+    bool pvpanic;
     bool virt;
     bool ras;
     bool mte;
-- 
1.8.3.1

[PATCH 5/8] hw/arm/virt: add pvpanic device in virt acpi table

[Mihai Carabas]

From: Peng Hao <peng.hao2@zte.com.cn>

Add pvpanic device in virt acpi table, so when kernel command line
uses acpi=force, kernel can get info from acpi table.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/arm/virt-acpi-build.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9747a64..f64bf89 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -87,6 +87,20 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
     aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
+{
+    Aml *dev = aml_device("PEVT");
+    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
+               pvpanic_memmap->size, AML_READ_WRITE));
+
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
 static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
 {
     Aml *dev = aml_device("FWCF");
@@ -605,6 +619,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     if (vmc->acpi_expose_flash) {
         acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
     }
+    if (vms->pvpanic) {
+        acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
+    }
     acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
-- 
1.8.3.1

[PATCH 6/8] hw/arm/virt: add configure interface for pvpanic-mmio

[Mihai Carabas]

From: Peng Hao <peng.hao2@zte.com.cn>

Add the option to turn on/off the pvpanic-mmio device in virt machine.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/arm/virt.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 63f09a6..7548977 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2161,6 +2161,20 @@ static void virt_set_mte(Object *obj, bool value, Error **errp)
     vms->mte = value;
 }
 
+static bool virt_get_pvpanic(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->pvpanic;
+}
+
+static void virt_set_pvpanic(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->pvpanic = value;
+}
+
 static char *virt_get_gic_version(Object *obj, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -2594,6 +2608,14 @@ static void virt_instance_init(Object *obj)
                                     "guest CPU which implements the ARM "
                                     "Memory Tagging Extension");
 
+    /* Default disallows pvpanic-mmio instantiation */
+    vms->pvpanic = false;
+    object_property_add_bool(obj, "pvpanic", virt_get_pvpanic,
+                             virt_set_pvpanic);
+    object_property_set_description(obj, "pvpanic",
+                                    "Set on/off to enable/disable "
+                                    "PVPANIC MMIO device");
+
     vms->irqmap = a15irqmap;
 
     virt_flash_create(vms);
-- 
1.8.3.1

[PATCH 7/8] pvpanic : update pvpanic document

[Mihai Carabas]

From: Peng Hao <peng.hao2@zte.com.cn>

Add mmio support info in docs/specs/pvpanic.txt.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 docs/specs/pvpanic.txt | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt
index a90fbca..b50140b 100644
--- a/docs/specs/pvpanic.txt
+++ b/docs/specs/pvpanic.txt
@@ -1,7 +1,7 @@
 PVPANIC DEVICE
 ==============
 
-pvpanic device is a simulated ISA device, through which a guest panic
+pvpanic device is a simulated 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.
 
@@ -9,6 +9,9 @@ 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.
 
+The pvpanic device can be implemented as an ISA device (using IOPORT),
+or, since qemu 4.0, as a SYSBUS device (using MMIO).
+
 ISA Interface
 -------------
 
@@ -24,6 +27,13 @@ bit 1: a guest panic has happened and will be handled by the guest;
        the host should record it or report it, but should not affect
        the execution of the guest.
 
+SYSBUS Interface
+----------------
+
+The SYSBUS interface is similar to the ISA interface except that it uses
+MMIO. For example, the arm virt machine could put the pvpanic device at
+[0x90b0000, 0x90b0001], where currently only the first byte is used.
+
 ACPI Interface
 --------------
 
-- 
1.8.3.1

[PATCH 8/8] pvpanic: break dependency on ISA_BUS

[Mihai Carabas]

pvpanic is supported on ARM VIRT MACHINE as an MMIO device, no need for an ISA
bus.

Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/arm/Kconfig    | 1 +
 hw/misc/Kconfig   | 2 +-
 hw/misc/pvpanic.c | 4 ++++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index f303c6b..0dd570b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -26,6 +26,7 @@ config ARM_VIRT
     select ACPI_MEMORY_HOTPLUG
     select ACPI_HW_REDUCED
     select ACPI_NVDIMM
+    select PVPANIC
 
 config CHEETAH
     bool
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 3185456..5924e70 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -117,7 +117,7 @@ config IOTKIT_SYSINFO
 
 config PVPANIC
     bool
-    depends on ISA_BUS
+    depends on ISA_BUS || ARM_VIRT
 
 config AUX
     bool
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 136f1d6..a70adb8 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -105,6 +105,7 @@ static const MemoryRegionOps pvpanic_ops = {
     },
 };
 
+#ifdef CONFIG_ISA_BUS
 static void pvpanic_isa_initfn(Object *obj)
 {
     PVPanicISAState *s = PVPANIC_ISA_DEVICE(obj);
@@ -152,6 +153,7 @@ static TypeInfo pvpanic_isa_info = {
     .instance_init = pvpanic_isa_initfn,
     .class_init    = pvpanic_isa_class_init,
 };
+#endif
 
 static void pvpanic_mmio_initfn(Object *obj)
 {
@@ -180,7 +182,9 @@ static TypeInfo pvpanic_mmio_info = {
 
 static void pvpanic_register_types(void)
 {
+#ifdef CONFIG_ISA_BUS
     type_register_static(&pvpanic_isa_info);
+#endif
     type_register_static(&pvpanic_mmio_info);
 }
 
-- 
1.8.3.1

Re: [PATCH 0/8] Add support for pvpanic mmio device

[Peter Maydell]

On Thu, 22 Oct 2020 at 09:25, Mihai Carabas <mihai.carabas@oracle.com> wrote:
> The patchset was assembled from chuncks from some old patches from 2018 [1]
> which were left unmerged and some additions from me. Surprisingly their Linux
> kernel counterpart were merged (so the pvpanic driver from the kernel supports
> mmio).
>
> I have seen the discussions about moving the pvpanic to PCI [1]. Those patches
> were sent but nothing happened. Also they are not trivial and require major
> modifications at the driver level also. Given the fact that we already have
> mmio driver support for pvpanic in the Linux kernel, I have sent these patches
> to ask again the maintainers if this can be merged.

I'm afraid the answer is still the same. You need to provide
a convincing argument for why this needs to be an MMIO
device rather than a PCI device. I really don't want to
add MMIO devices to the virt board if I can avoid it,
because they're all extra code and potential extra
security boundary attack surface. PCI devices are guest
probeable and user-pluggable so they're almost always
nicer to use than MMIO.

thanks
-- PMM

Re: [PATCH 0/8] Add support for pvpanic mmio device

[Mihai Carabas]

On 10/22/2020 1:17 PM, Peter Maydell wrote:
> On Thu, 22 Oct 2020 at 09:25, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>> The patchset was assembled from chuncks from some old patches from 2018 [1]
>> which were left unmerged and some additions from me. Surprisingly their Linux
>> kernel counterpart were merged (so the pvpanic driver from the kernel supports
>> mmio).
>>
>> I have seen the discussions about moving the pvpanic to PCI [1]. Those patches
>> were sent but nothing happened. Also they are not trivial and require major
>> modifications at the driver level also. Given the fact that we already have
>> mmio driver support for pvpanic in the Linux kernel, I have sent these patches
>> to ask again the maintainers if this can be merged.
> 
> I'm afraid the answer is still the same. You need to provide
> a convincing argument for why this needs to be an MMIO
> device rather than a PCI device. I really don't want to
> add MMIO devices to the virt board if I can avoid it,
> because they're all extra code and potential extra
> security boundary attack surface. PCI devices are guest
> probeable and user-pluggable so they're almost always
> nicer to use than MMIO.
> 

Thank you for your input.

The reason why pvpanic should be MMIO is that is a special device which 
is not used commonly by the user (aka VM), it is not need to be 
hot-plugable and it does not have a hardware correspondent to be a PCI 
device. Another reason is that MMIO support was accepted in the kernel 
driver and it is pretty useless there without a device.

I know it seems that I want to get this on the short-path, but at this 
point having a kernel driver in the upstream and no device to test it 
against it is pretty weird.

Thank you,
Mihai

Re: [PATCH 0/8] Add support for pvpanic mmio device

[Peter Maydell]

On Mon, 26 Oct 2020 at 13:51, Mihai Carabas <mihai.carabas@oracle.com> wrote:
> On 10/22/2020 1:17 PM, Peter Maydell wrote:
> > I'm afraid the answer is still the same. You need to provide
> > a convincing argument for why this needs to be an MMIO
> > device rather than a PCI device. I really don't want to
> > add MMIO devices to the virt board if I can avoid it,
> > because they're all extra code and potential extra
> > security boundary attack surface. PCI devices are guest
> > probeable and user-pluggable so they're almost always
> > nicer to use than MMIO.
> >
>
> Thank you for your input.
>
> The reason why pvpanic should be MMIO is that is a special device which
> is not used commonly by the user (aka VM), it is not need to be
> hot-plugable and it does not have a hardware correspondent to be a PCI
> device.

"Not used commonly by the user" is a good argument for
"just use PCI" -- users who want the functionality can add
it to their QEMU command line or VM config, and the bulk of
users who don't don't have to worry about it.

PCI devices don't have to support hotplug; I agree you don't
need hotplug for this use case.

Using a PCI device would match up with the way the x86 pvpanic
device is an ISA bus device.

> Another reason is that MMIO support was accepted in the kernel
> driver and it is pretty useless there without a device.

> I know it seems that I want to get this on the short-path, but at this
> point having a kernel driver in the upstream and no device to test it
> against it is pretty weird.

That's the kernel folks' problem to deal with, because they accepted
the kernel driver without having confirmed that QEMU was going to
implement it. If I were them I'd deprecate and delete that driver code.
(They should certainly fix the bit of the documentation that claims
that QEMU implements it.)

thanks
-- PMM