[PATCH] Add support for pvpanic pci device

[PATCH] Add support for pvpanic pci device

[Mihai Carabas]

This patchset adds support for pvpanic pci device. The patchset was assembled
from chuncks from some old patches [1] from 2018 which were left unmerged and
some additions from me.

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

After that you need to run a Linux kernel as guest, but you have to also apply
the patches I will send for adding pci support for the pvpanic driver.

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

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

Peng Hao (3):
  hw/misc/pvpanic: Add the PCI interface
  hw/arm/virt: Use the pvpanic pci device
  pvpanic : update pvpanic document

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

 default-configs/devices/arm-softmmu.mak |  1 +
 docs/specs/pvpanic.txt                  | 12 ++++-
 hw/misc/Kconfig                         |  2 +-
 hw/misc/meson.build                     |  2 +-
 hw/misc/pvpanic.c                       | 96 +++++++++++++++++++++++++++------
 include/hw/misc/pvpanic.h               |  3 +-
 include/hw/pci/pci.h                    |  1 +
 7 files changed, 97 insertions(+), 20 deletions(-)

-- 
1.8.3.1

[PATCH 1/6] 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/6] 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/6] hw/misc/pvpanic: Add the PCI interface

[Mihai Carabas]

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

Add pvpanic new type "TYPE_PVPANIC_PCI".

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 hw/misc/pvpanic.c         | 64 ++++++++++++++++++++++++++++++++++++++++++++---
 include/hw/misc/pvpanic.h |  1 +
 include/hw/pci/pci.h      |  1 +
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 2e085f4..b0bf7d4 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.
@@ -21,6 +23,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
+#include "hw/pci/pci.h"
 
 /* The bit of supported pv event, TODO: include uapi header and remove this */
 #define PVPANIC_F_PANICKED      0
@@ -34,6 +37,10 @@ typedef struct PVPanicISAState PVPanicISAState;
 DECLARE_INSTANCE_CHECKER(PVPanicISAState, PVPANIC_ISA_DEVICE,
                          TYPE_PVPANIC_ISA)
 
+typedef struct PVPanicPCIState PVPanicPCIState;
+DECLARE_INSTANCE_CHECKER(PVPanicPCIState, PVPANIC_PCI_DEVICE,
+                         TYPE_PVPANIC_PCI)
+
 static void handle_event(int event)
 {
     static bool logged;
@@ -67,21 +74,32 @@ struct PVPanicISAState {
     uint16_t ioport;
 };
 
+/* PVPanicPCIState for PCI device and
+ * use mmio.
+ */
+typedef struct PVPanicPCIState {
+    /*< private>*/
+    PCIDevice dev;
+
+    /*<public>*/
+    MemoryRegion mr;
+} PVPanicPCIState;
+
 /* 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 +154,47 @@ static TypeInfo pvpanic_isa_info = {
     .class_init    = pvpanic_isa_class_init,
 };
 
+/* pvpanic pci device*/
+
+static void pvpanic_pci_realizefn(PCIDevice *dev, Error **errp)
+{
+    PVPanicPCIState *s = DO_UPCAST(PVPanicPCIState, dev, dev);
+
+    memory_region_init_io(&s->mr, OBJECT(s), &pvpanic_ops, s,
+                          TYPE_PVPANIC_PCI, 2);
+    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mr);
+}
+
+static void pvpanic_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+
+    pc->realize = pvpanic_pci_realizefn;
+    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
+    pc->device_id = PCI_DEVICE_ID_REDHAT_PVPANIC;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_SYSTEM_OTHER;
+
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static TypeInfo pvpanic_pci_info = {
+    .name          = TYPE_PVPANIC_PCI,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PVPanicPCIState),
+    .class_init    = pvpanic_pci_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_PCIE_DEVICE },
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { }
+    }
+};
+
 static void pvpanic_register_types(void)
 {
     type_register_static(&pvpanic_isa_info);
+    type_register_static(&pvpanic_pci_info);
 }
 
 type_init(pvpanic_register_types)
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index 30e9f8f..6fa6a62 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -18,6 +18,7 @@
 #include "qom/object.h"
 
 #define TYPE_PVPANIC_ISA "pvpanic"
+#define TYPE_PVPANIC_PCI "pvpanic-pci"
 
 #define PVPANIC_IOPORT_PROP "ioport"
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 0a59a06..ce68c90 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -107,6 +107,7 @@ extern bool pci_available;
 #define PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE 0x000e
 #define PCI_DEVICE_ID_REDHAT_MDPY        0x000f
 #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
+#define PCI_DEVICE_ID_REDHAT_PVPANIC     0x0101
 
 #define FMT_PCIBUS                      PRIx64
 
-- 
1.8.3.1

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

[Mihai Carabas]

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

Add pvpanic device in arm virt machine config file.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 default-configs/devices/arm-softmmu.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
index 9a94ebd..4efcca2 100644
--- a/default-configs/devices/arm-softmmu.mak
+++ b/default-configs/devices/arm-softmmu.mak
@@ -44,3 +44,4 @@ CONFIG_FSL_IMX6UL=y
 CONFIG_SEMIHOSTING=y
 CONFIG_ALLWINNER_H3=y
 CONFIG_ACPI_APEI=y
+CONFIG_PVPANIC=y
-- 
1.8.3.1

[PATCH 5/6] pvpanic : update pvpanic document

[Mihai Carabas]

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

Add mmio mode as a pci device support info in docs/specs/pvpanic.txt.

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..4258922 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 PCI device.
+
 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.
 
+PCI Interface
+-------------
+
+The PCI interface is similar to the ISA interface except that it uses an MMIO
+address space provided by its BAR0. For example, the arm virt machine may
+enable a pvpanic device by adding '-device pvpanic-pci' to the command line.
+
 ACPI Interface
 --------------
 
-- 
1.8.3.1

[PATCH 6/6] pvpanic: break dependency on ISA_BUS adding PCI

[Mihai Carabas]

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

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

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 3185456..b650982 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 || PCI
 
 config AUX
     bool
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index b0bf7d4..f80cf6c 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -106,6 +106,7 @@ static const MemoryRegionOps pvpanic_ops = {
     },
 };
 
+#ifdef CONFIG_ISA_BUS
 static void pvpanic_isa_initfn(Object *obj)
 {
     PVPanicISAState *s = PVPANIC_ISA_DEVICE(obj);
@@ -153,6 +154,7 @@ static TypeInfo pvpanic_isa_info = {
     .instance_init = pvpanic_isa_initfn,
     .class_init    = pvpanic_isa_class_init,
 };
+#endif
 
 /* pvpanic pci device*/
 
@@ -193,7 +195,9 @@ static TypeInfo pvpanic_pci_info = {
 
 static void pvpanic_register_types(void)
 {
+#ifdef CONFIG_ISA_BUS
     type_register_static(&pvpanic_isa_info);
+#endif
     type_register_static(&pvpanic_pci_info);
 }
 
-- 
1.8.3.1

Re: [PATCH] Add support for pvpanic pci device

[Mihai Carabas]

La 28.10.2020 15:32, Mihai Carabas a scris:
> This patchset adds support for pvpanic pci device. The patchset was assembled
> from chuncks from some old patches [1] from 2018 which were left unmerged and
> some additions from me.
> 
> How to test this:
> /usr/bin/qemu-system-aarch64 \
>          -machine virt,gic-version=3 -device pvpanic-pci
> 
> After that you need to run a Linux kernel as guest, but you have to also apply
> the patches I will send for adding pci support for the pvpanic driver.

Here are the patches for Linux: https://lkml.org/lkml/2020/10/29/645
> 
> [1] http://patchwork.ozlabs.org/project/qemu-devel/cover/1544095560-70807-1-git-send-email-peng.hao2@zte.com.cn/
> 
> Mihai Carabas (1):
>    pvpanic: break dependency on ISA_BUS adding PCI
> 
> Peng Hao (3):
>    hw/misc/pvpanic: Add the PCI interface
>    hw/arm/virt: Use the pvpanic pci device
>    pvpanic : update pvpanic document
> 
> Philippe Mathieu-Daudé (2):
>    hw/misc/pvpanic: Build the pvpanic device for any machine
>    hw/misc/pvpanic: Cosmetic renaming
> 
>   default-configs/devices/arm-softmmu.mak |  1 +
>   docs/specs/pvpanic.txt                  | 12 ++++-
>   hw/misc/Kconfig                         |  2 +-
>   hw/misc/meson.build                     |  2 +-
>   hw/misc/pvpanic.c                       | 96 +++++++++++++++++++++++++++------
>   include/hw/misc/pvpanic.h               |  3 +-
>   include/hw/pci/pci.h                    |  1 +
>   7 files changed, 97 insertions(+), 20 deletions(-)
> 

Re: [PATCH] Add support for pvpanic pci device

[Mihai Carabas]

La 29.10.2020 14:30, Mihai Carabas a scris:
> La 28.10.2020 15:32, Mihai Carabas a scris:
>> This patchset adds support for pvpanic pci device. The patchset was 
>> assembled
>> from chuncks from some old patches [1] from 2018 which were left 
>> unmerged and
>> some additions from me.
>>
>> How to test this:
>> /usr/bin/qemu-system-aarch64 \
>>          -machine virt,gic-version=3 -device pvpanic-pci
>>
>> After that you need to run a Linux kernel as guest, but you have to 
>> also apply
>> the patches I will send for adding pci support for the pvpanic driver.
> 
> Here are the patches for Linux: https://lkml.org/lkml/2020/10/29/645
>>
>> [1] 
>> http://patchwork.ozlabs.org/project/qemu-devel/cover/1544095560-70807-1-git-send-email-peng.hao2@zte.com.cn/ 


Any feedback on this patchset?

Thank you,
Mihai Carabas

>>
>>
>> Mihai Carabas (1):
>>    pvpanic: break dependency on ISA_BUS adding PCI
>>
>> Peng Hao (3):
>>    hw/misc/pvpanic: Add the PCI interface
>>    hw/arm/virt: Use the pvpanic pci device
>>    pvpanic : update pvpanic document
>>
>> Philippe Mathieu-Daudé (2):
>>    hw/misc/pvpanic: Build the pvpanic device for any machine
>>    hw/misc/pvpanic: Cosmetic renaming
>>
>>   default-configs/devices/arm-softmmu.mak |  1 +
>>   docs/specs/pvpanic.txt                  | 12 ++++-
>>   hw/misc/Kconfig                         |  2 +-
>>   hw/misc/meson.build                     |  2 +-
>>   hw/misc/pvpanic.c                       | 96 
>> +++++++++++++++++++++++++++------
>>   include/hw/misc/pvpanic.h               |  3 +-
>>   include/hw/pci/pci.h                    |  1 +
>>   7 files changed, 97 insertions(+), 20 deletions(-)
>>
> 

Re: [PATCH 2/6] hw/misc/pvpanic: Cosmetic renaming

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:20, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> To ease the MMIO device addition in the next patch, rename:

I guess this should say "PCI device" now...

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

thanks
-- PMM

Re: [PATCH 4/6] hw/arm/virt: Use the pvpanic pci device

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:23, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Peng Hao <peng.hao2@zte.com.cn>
>
> Add pvpanic device in arm virt machine config file.
>
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> ---
>  default-configs/devices/arm-softmmu.mak | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
> index 9a94ebd..4efcca2 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -44,3 +44,4 @@ CONFIG_FSL_IMX6UL=y
>  CONFIG_SEMIHOSTING=y
>  CONFIG_ALLWINNER_H3=y
>  CONFIG_ACPI_APEI=y
> +CONFIG_PVPANIC=y

This should be done in a Kconfig file, I think, not in
default-configs/. More specifically, if you get the Kconfig
right for the pvpanic pci device when you add it (by giving
it a Kconfig stanza including "default y if PCI_DEVICES")
then all machines which have PCI, including Arm ones, will
cause it to be compiled; so this patch can go away entirely.

thanks
-- PMM

Re: [PATCH 5/6] pvpanic : update pvpanic document

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:26, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Peng Hao <peng.hao2@zte.com.cn>
>
> Add mmio mode as a pci device support info in docs/specs/pvpanic.txt.
>
> 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..4258922 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 PCI device.

Version number is outdated here. I would simply say "or as a PCI device".

> +
>  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.
>
> +PCI Interface
> +-------------
> +
> +The PCI interface is similar to the ISA interface except that it uses an MMIO
> +address space provided by its BAR0.

You should be specific about the access width(s) permitted here --
is the access to the register 8-bit only, 32-bit-only, or what ?

> For example, the arm virt machine may
> +enable a pvpanic device by adding '-device pvpanic-pci' to the command line.

No need to be specific to Arm virt here.
"Any machine with a PCI device can enable a pvpanic device by adding
'-device pvpanic-pci' to the command line."


> +
>  ACPI Interface
>  --------------

thanks
-- PMM

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

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:20, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> 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'))

This patch doesn't seem to actually change anything -- it's just
moving a line of code around in the file ?

thanks
-- PMM

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

[Peter Maydell]

On Mon, 14 Dec 2020 at 10:47, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 28 Oct 2020 at 14:20, Mihai Carabas <mihai.carabas@oracle.com> wrote:
> >
> > 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'))
>
> This patch doesn't seem to actually change anything -- it's just
> moving a line of code around in the file ?

Also, this is not the patch which Philippe wrote and I reviewed,
which was this one:
https://patchew.org/QEMU/1544095560-70807-1-git-send-email-peng.hao2@zte.com.cn/1544095560-70807-2-git-send-email-peng.hao2@zte.com.cn/

That patch (for the old build system) does what the commit message
says it does; this one (for the new build system) doesn't. Please
don't retain authorship and reviewed-by tags if the patch is changed
significantly like this.

Have a look at, for example, how the ne2000 ISA and PCI devices
in hw/net are structured and built (source file setup, meson.build,
Kconfig stanzas) to see how to do this kind of "common core, multiple
buses" setup.

thanks
-- PMM

Re: [PATCH 3/6] hw/misc/pvpanic: Add the PCI interface

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:20, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Peng Hao <peng.hao2@zte.com.cn>
>
> Add pvpanic new type "TYPE_PVPANIC_PCI".
>
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>

> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 0a59a06..ce68c90 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -107,6 +107,7 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE 0x000e
>  #define PCI_DEVICE_ID_REDHAT_MDPY        0x000f
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
> +#define PCI_DEVICE_ID_REDHAT_PVPANIC     0x0101

So the comment above this set of ID values says:
/* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */

and docs/specs/pci-ids.txt says:
# Contact Gerd Hoffmann <kraxel@redhat.com> to get a device ID
# assigned for your devices.

Have you done that ? (If so, then you also need to update
pci-ids.txt to document the new allocation.)

thanks
-- PMM

Re: [PATCH] Add support for pvpanic pci device

[Peter Maydell]

On Wed, 28 Oct 2020 at 14:17, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> This patchset adds support for pvpanic pci device. The patchset was assembled
> from chuncks from some old patches [1] from 2018 which were left unmerged and
> some additions from me.
>
> How to test this:
> /usr/bin/qemu-system-aarch64 \
>         -machine virt,gic-version=3 -device pvpanic-pci
>
> After that you need to run a Linux kernel as guest, but you have to also apply
> the patches I will send for adding pci support for the pvpanic driver.

Hi; I've reviewed most of the patches. I'm in principle in favour
of this, but the structuring of how the ISA and PCI versions of
the device are configured and built needs a little work. You
also need to sort out allocation of a PCI ID for it.

thanks
-- PMM

Re: [PATCH] Add support for pvpanic pci device

[Mihai Carabas]

La 14.12.2020 13:03, Peter Maydell a scris:
> On Wed, 28 Oct 2020 at 14:17, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>>
>> This patchset adds support for pvpanic pci device. The patchset was assembled
>> from chuncks from some old patches [1] from 2018 which were left unmerged and
>> some additions from me.
>>
>> How to test this:
>> /usr/bin/qemu-system-aarch64 \
>>          -machine virt,gic-version=3 -device pvpanic-pci
>>
>> After that you need to run a Linux kernel as guest, but you have to also apply
>> the patches I will send for adding pci support for the pvpanic driver.
> 
> Hi; I've reviewed most of the patches. I'm in principle in favour
> of this, but the structuring of how the ISA and PCI versions of
> the device are configured and built needs a little work. You
> also need to sort out allocation of a PCI ID for it.

Thank you for your feedback. I will apply it in the next days and send a 
new revision.

Thank you,
Mihai

> 
> thanks
> -- PMM
>