[PATCH for-8.0 0/7] virtio, pci, xics: 3-phase reset conversions

[PATCH for-8.0 0/7] virtio, pci, xics: 3-phase reset conversions

[Peter Maydell]

This patchset converts a miscellaneous collection of classes
to 3-phase reset. The common link, as with other series I've
sent out recently, is converting child classes that currently
use device_class_set_parent_reset() so that we can remove
that function. To do this we first need to convert the parent
class, and then the subclass.

The first two patches handle TYPE_VIRTIO_VGA_BASE, and its parent
TYPE_VIRTIO_PCI.

The second two handle the parent TYPE_PCIE_ROOT_PORT and its two
child classes TYPE_CXL_ROOT_PORT and TYPE_PNV_PHB_ROOT_PORT.

The last three deal with TYPE_PHB3_MSI and the parenT TYPE_ICS.

This is all 8.0 material.

thanks
-- PMM

Peter Maydell (7):
  hw/virtio: Convert TYPE_VIRTIO_PCI to 3-phase reset
  hw/display/virtio-vga: Convert TYPE_VIRTIO_VGA_BASE to 3-phase reset
  pci: Convert TYPE_PCIE_ROOT_PORT to 3-phase reset
  pci: Convert child classes of TYPE_PCIE_ROOT_PORT to 3-phase reset
  hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()
  hw/intc/xics: Convert TYPE_ICS to 3-phase reset
  hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset

 hw/display/virtio-vga.h        |  2 +-
 include/hw/pci/pcie_port.h     |  2 +-
 include/hw/ppc/xics.h          |  2 +-
 hw/display/virtio-vga.c        | 15 +++++++++------
 hw/intc/xics.c                 | 11 ++++++-----
 hw/pci-bridge/cxl_root_port.c  | 14 +++++++++-----
 hw/pci-bridge/pcie_root_port.c |  8 +++++---
 hw/pci-host/pnv_phb.c          | 18 ++++++++++--------
 hw/pci-host/pnv_phb3_msi.c     | 22 +++++++++-------------
 hw/virtio/virtio-pci.c         |  8 +++++---
 10 files changed, 56 insertions(+), 46 deletions(-)

-- 
2.25.1

[PATCH for-8.0 1/7] hw/virtio: Convert TYPE_VIRTIO_PCI to 3-phase reset

[Peter Maydell]

Convert the TYPE_VIRTIO_PCI class to 3-phase reset.  This is
necessary so that we can convert the subclass TYPE_VIRTIO_VGA_BASE
also to 3-phase reset.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio-pci.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a1c9dfa7bb5..7873083b860 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2008,9 +2008,10 @@ static void virtio_pci_reset(DeviceState *qdev)
     }
 }
 
-static void virtio_pci_bus_reset(DeviceState *qdev)
+static void virtio_pci_bus_reset_hold(Object *obj)
 {
-    PCIDevice *dev = PCI_DEVICE(qdev);
+    PCIDevice *dev = PCI_DEVICE(obj);
+    DeviceState *qdev = DEVICE(obj);
 
     virtio_pci_reset(qdev);
 
@@ -2071,6 +2072,7 @@ static void virtio_pci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     device_class_set_props(dc, virtio_pci_properties);
     k->realize = virtio_pci_realize;
@@ -2080,7 +2082,7 @@ static void virtio_pci_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_OTHERS;
     device_class_set_parent_realize(dc, virtio_pci_dc_realize,
                                     &vpciklass->parent_dc_realize);
-    dc->reset = virtio_pci_bus_reset;
+    rc->phases.hold = virtio_pci_bus_reset_hold;
 }
 
 static const TypeInfo virtio_pci_info = {
-- 
2.25.1

Re: [PATCH for-8.0 1/7] hw/virtio: Convert TYPE_VIRTIO_PCI to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_VIRTIO_PCI class to 3-phase reset.  This is
> necessary so that we can convert the subclass TYPE_VIRTIO_VGA_BASE
> also to 3-phase reset.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/virtio/virtio-pci.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH for-8.0 2/7] hw/display/virtio-vga: Convert TYPE_VIRTIO_VGA_BASE to 3-phase reset

[Peter Maydell]

Convert the TYPE_VIRTIO_VGA_BASE class to 3-phase reset, so we
don't need to use device_class_set_parent_reset() any more.

Note that this is an abstract class itself; none of the subclasses
override its reset method.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/virtio-vga.h |  2 +-
 hw/display/virtio-vga.c | 15 +++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/display/virtio-vga.h b/hw/display/virtio-vga.h
index 977ad5edc29..0bd9db1ceea 100644
--- a/hw/display/virtio-vga.h
+++ b/hw/display/virtio-vga.h
@@ -23,7 +23,7 @@ struct VirtIOVGABase {
 struct VirtIOVGABaseClass {
     VirtioPCIClass parent_class;
 
-    DeviceReset parent_reset;
+    ResettablePhases parent_phases;
 };
 
 #endif /* VIRTIO_VGA_H */
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 4dcb34c4a74..e6fb0aa876c 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -165,13 +165,15 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     }
 }
 
-static void virtio_vga_base_reset(DeviceState *dev)
+static void virtio_vga_base_reset_hold(Object *obj)
 {
-    VirtIOVGABaseClass *klass = VIRTIO_VGA_BASE_GET_CLASS(dev);
-    VirtIOVGABase *vvga = VIRTIO_VGA_BASE(dev);
+    VirtIOVGABaseClass *klass = VIRTIO_VGA_BASE_GET_CLASS(obj);
+    VirtIOVGABase *vvga = VIRTIO_VGA_BASE(obj);
 
     /* reset virtio-gpu */
-    klass->parent_reset(dev);
+    if (klass->parent_phases.hold) {
+        klass->parent_phases.hold(obj);
+    }
 
     /* reset vga */
     vga_common_reset(&vvga->vga);
@@ -203,13 +205,14 @@ static void virtio_vga_base_class_init(ObjectClass *klass, void *data)
     VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
     VirtIOVGABaseClass *v = VIRTIO_VGA_BASE_CLASS(klass);
     PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
     device_class_set_props(dc, virtio_vga_base_properties);
     dc->vmsd = &vmstate_virtio_vga_base;
     dc->hotpluggable = false;
-    device_class_set_parent_reset(dc, virtio_vga_base_reset,
-                                  &v->parent_reset);
+    resettable_class_set_parent_phases(rc, NULL, virtio_vga_base_reset_hold,
+                                       NULL, &v->parent_phases);
 
     k->realize = virtio_vga_base_realize;
     pcidev_k->romfile = "vgabios-virtio.bin";
-- 
2.25.1

Re: [PATCH for-8.0 2/7] hw/display/virtio-vga: Convert TYPE_VIRTIO_VGA_BASE to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_VIRTIO_VGA_BASE class to 3-phase reset, so we
> don't need to use device_class_set_parent_reset() any more.
> 
> Note that this is an abstract class itself; none of the subclasses
> override its reset method.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/display/virtio-vga.h |  2 +-
>   hw/display/virtio-vga.c | 15 +++++++++------
>   2 files changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH for-8.0 3/7] pci: Convert TYPE_PCIE_ROOT_PORT to 3-phase reset

[Peter Maydell]

Convert the TYPE_PCIE_ROOT_PORT device to 3-phase reset; this is a
necessary precursor to converting any of its child classes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/pci-bridge/pcie_root_port.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 460e48269d4..36bc0bafa7e 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -43,9 +43,10 @@ static void rp_write_config(PCIDevice *d, uint32_t address,
     pcie_aer_root_write_config(d, address, val, len, root_cmd);
 }
 
-static void rp_reset(DeviceState *qdev)
+static void rp_reset_hold(Object *obj)
 {
-    PCIDevice *d = PCI_DEVICE(qdev);
+    PCIDevice *d = PCI_DEVICE(obj);
+    DeviceState *qdev = DEVICE(obj);
 
     rp_aer_vector_update(d);
     pcie_cap_root_reset(d);
@@ -171,13 +172,14 @@ static void rp_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     k->is_bridge = true;
     k->config_write = rp_write_config;
     k->realize = rp_realize;
     k->exit = rp_exit;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    dc->reset = rp_reset;
+    rc->phases.hold = rp_reset_hold;
     device_class_set_props(dc, rp_props);
 }
 
-- 
2.25.1

Re: [PATCH for-8.0 3/7] pci: Convert TYPE_PCIE_ROOT_PORT to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_PCIE_ROOT_PORT device to 3-phase reset; this is a
> necessary precursor to converting any of its child classes.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/pci-bridge/pcie_root_port.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH for-8.0 4/7] pci: Convert child classes of TYPE_PCIE_ROOT_PORT to 3-phase reset

[Peter Maydell]

Convert the TYPE_CXL_ROOT_PORT and TYPE_PNV_PHB_ROOT_PORT classes to
3-phase reset, so they don't need to use the deprecated
device_class_set_parent_reset() function any more.

We have to do both in the same commit, because they keep the
parent_reset field in their common parent class's class struct.

Note that pnv_phb_root_port_class_init() was pointlessly setting
dc->reset twice, once by calling device_class_set_parent_reset()
and once directly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/pci/pcie_port.h    |  2 +-
 hw/pci-bridge/cxl_root_port.c | 14 +++++++++-----
 hw/pci-host/pnv_phb.c         | 18 ++++++++++--------
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 7b8193061ac..d9b5d075049 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -80,7 +80,7 @@ DECLARE_CLASS_CHECKERS(PCIERootPortClass, PCIE_ROOT_PORT,
 struct PCIERootPortClass {
     PCIDeviceClass parent_class;
     DeviceRealize parent_realize;
-    DeviceReset parent_reset;
+    ResettablePhases parent_phases;
 
     uint8_t (*aer_vector)(const PCIDevice *dev);
     int (*interrupts_init)(PCIDevice *dev, Error **errp);
diff --git a/hw/pci-bridge/cxl_root_port.c b/hw/pci-bridge/cxl_root_port.c
index fb213fa06ef..6664783974c 100644
--- a/hw/pci-bridge/cxl_root_port.c
+++ b/hw/pci-bridge/cxl_root_port.c
@@ -138,12 +138,14 @@ static void cxl_rp_realize(DeviceState *dev, Error **errp)
                      component_bar);
 }
 
-static void cxl_rp_reset(DeviceState *dev)
+static void cxl_rp_reset_hold(Object *obj)
 {
-    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
-    CXLRootPort *crp = CXL_ROOT_PORT(dev);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(obj);
+    CXLRootPort *crp = CXL_ROOT_PORT(obj);
 
-    rpc->parent_reset(dev);
+    if (rpc->parent_phases.hold) {
+        rpc->parent_phases.hold(obj);
+    }
 
     latch_registers(crp);
 }
@@ -199,6 +201,7 @@ static void cxl_root_port_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc        = DEVICE_CLASS(oc);
     PCIDeviceClass *k      = PCI_DEVICE_CLASS(oc);
+    ResettableClass *rc    = RESETTABLE_CLASS(oc);
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(oc);
 
     k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -209,7 +212,8 @@ static void cxl_root_port_class_init(ObjectClass *oc, void *data)
     k->config_write = cxl_rp_write_config;
 
     device_class_set_parent_realize(dc, cxl_rp_realize, &rpc->parent_realize);
-    device_class_set_parent_reset(dc, cxl_rp_reset, &rpc->parent_reset);
+    resettable_class_set_parent_phases(rc, NULL, cxl_rp_reset_hold, NULL,
+                                       &rpc->parent_phases);
 
     rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;
     rpc->acs_offset = GEN_PCIE_ROOT_PORT_ACS_OFFSET;
diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 0b26b43736f..c62b08538ac 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -199,14 +199,16 @@ static void pnv_phb_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = true;
 }
 
-static void pnv_phb_root_port_reset(DeviceState *dev)
+static void pnv_phb_root_port_reset_hold(Object *obj)
 {
-    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
-    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
-    PCIDevice *d = PCI_DEVICE(dev);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(obj);
+    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(obj);
+    PCIDevice *d = PCI_DEVICE(obj);
     uint8_t *conf = d->config;
 
-    rpc->parent_reset(dev);
+    if (rpc->parent_phases.hold) {
+        rpc->parent_phases.hold(obj);
+    }
 
     if (phb_rp->version == 3) {
         return;
@@ -300,6 +302,7 @@ static Property pnv_phb_root_port_properties[] = {
 static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
 
@@ -308,9 +311,8 @@ static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data)
     device_class_set_props(dc, pnv_phb_root_port_properties);
     device_class_set_parent_realize(dc, pnv_phb_root_port_realize,
                                     &rpc->parent_realize);
-    device_class_set_parent_reset(dc, pnv_phb_root_port_reset,
-                                  &rpc->parent_reset);
-    dc->reset = &pnv_phb_root_port_reset;
+    resettable_class_set_parent_phases(rc, NULL, pnv_phb_root_port_reset_hold,
+                                       NULL, &rpc->parent_phases);
     dc->user_creatable = true;
 
     k->vendor_id = PCI_VENDOR_ID_IBM;
-- 
2.25.1

Re: [PATCH for-8.0 4/7] pci: Convert child classes of TYPE_PCIE_ROOT_PORT to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_CXL_ROOT_PORT and TYPE_PNV_PHB_ROOT_PORT classes to
> 3-phase reset, so they don't need to use the deprecated
> device_class_set_parent_reset() function any more.
> 
> We have to do both in the same commit, because they keep the
> parent_reset field in their common parent class's class struct.
> 
> Note that pnv_phb_root_port_class_init() was pointlessly setting
> dc->reset twice, once by calling device_class_set_parent_reset()
> and once directly.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/pci/pcie_port.h    |  2 +-
>   hw/pci-bridge/cxl_root_port.c | 14 +++++++++-----
>   hw/pci-host/pnv_phb.c         | 18 ++++++++++--------
>   3 files changed, 20 insertions(+), 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH for-8.0 5/7] hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()

[Peter Maydell]

The realize method for the TYPE_ICS class uses qemu_register_reset()
to register a reset handler, as a workaround for the fact that
currently objects which directly inherit from TYPE_DEVICE don't get
automatically reset.  However, the reset function directly calls
ics_reset(), which is the function that implements the legacy reset
method.  This means that only the parent class's data gets reset, and
a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has
to register its own reset function.

Make the TYPE_ICS reset function call device_cold_reset() instead:
this will handle reset for both the parent class and the subclass,
and will work whether the classes are using legacy reset or 3-phase
reset. This allows us to remove the reset function that the subclass
currently has to set up.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/xics.c             | 2 +-
 hw/pci-host/pnv_phb3_msi.c | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index dcd021af668..dd130467ccc 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev)
 
 static void ics_reset_handler(void *dev)
 {
-    ics_reset(dev);
+    device_cold_reset(dev);
 }
 
 static void ics_realize(DeviceState *dev, Error **errp)
diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
index 2f4112907b8..ae908fd9e41 100644
--- a/hw/pci-host/pnv_phb3_msi.c
+++ b/hw/pci-host/pnv_phb3_msi.c
@@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev)
     msi->rba_sum = 0;
 }
 
-static void phb3_msi_reset_handler(void *dev)
-{
-    phb3_msi_reset(dev);
-}
-
 void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base,
                                 uint32_t count)
 {
@@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp)
     }
 
     msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs);
-
-    qemu_register_reset(phb3_msi_reset_handler, dev);
 }
 
 static void phb3_msi_instance_init(Object *obj)
-- 
2.25.1

Re: [PATCH for-8.0 5/7] hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()

[Cédric Le Goater]

On 11/25/22 12:52, Peter Maydell wrote:
> The realize method for the TYPE_ICS class uses qemu_register_reset()
> to register a reset handler, as a workaround for the fact that
> currently objects which directly inherit from TYPE_DEVICE don't get
> automatically reset.  However, the reset function directly calls
> ics_reset(), which is the function that implements the legacy reset
> method.  This means that only the parent class's data gets reset, and
> a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has
> to register its own reset function.
> 
> Make the TYPE_ICS reset function call device_cold_reset() instead:
> this will handle reset for both the parent class and the subclass,
> and will work whether the classes are using legacy reset or 3-phase
> reset. This allows us to remove the reset function that the subclass
> currently has to set up.

Nice !

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/intc/xics.c             | 2 +-
>   hw/pci-host/pnv_phb3_msi.c | 7 -------
>   2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index dcd021af668..dd130467ccc 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev)
>   
>   static void ics_reset_handler(void *dev)
>   {
> -    ics_reset(dev);
> +    device_cold_reset(dev);
>   }
>   
>   static void ics_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
> index 2f4112907b8..ae908fd9e41 100644
> --- a/hw/pci-host/pnv_phb3_msi.c
> +++ b/hw/pci-host/pnv_phb3_msi.c
> @@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev)
>       msi->rba_sum = 0;
>   }
>   
> -static void phb3_msi_reset_handler(void *dev)
> -{
> -    phb3_msi_reset(dev);
> -}
> -
>   void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base,
>                                   uint32_t count)
>   {
> @@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp)
>       }
>   
>       msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs);
> -
> -    qemu_register_reset(phb3_msi_reset_handler, dev);
>   }
>   
>   static void phb3_msi_instance_init(Object *obj)

Re: [PATCH for-8.0 5/7] hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()

[Greg Kurz]

On Fri, 25 Nov 2022 13:24:00 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 11/25/22 12:52, Peter Maydell wrote:
> > The realize method for the TYPE_ICS class uses qemu_register_reset()
> > to register a reset handler, as a workaround for the fact that
> > currently objects which directly inherit from TYPE_DEVICE don't get
> > automatically reset.  However, the reset function directly calls
> > ics_reset(), which is the function that implements the legacy reset
> > method.  This means that only the parent class's data gets reset, and
> > a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has
> > to register its own reset function.
> > 
> > Make the TYPE_ICS reset function call device_cold_reset() instead:
> > this will handle reset for both the parent class and the subclass,
> > and will work whether the classes are using legacy reset or 3-phase
> > reset. This allows us to remove the reset function that the subclass
> > currently has to set up.
> 
> Nice !
> 

Seconded.

Reviewed-by: Greg Kurz <groug@kaod.org>

> > 
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks,
> 
> C.
> 
> > ---
> >   hw/intc/xics.c             | 2 +-
> >   hw/pci-host/pnv_phb3_msi.c | 7 -------
> >   2 files changed, 1 insertion(+), 8 deletions(-)
> > 
> > diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> > index dcd021af668..dd130467ccc 100644
> > --- a/hw/intc/xics.c
> > +++ b/hw/intc/xics.c
> > @@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev)
> >   
> >   static void ics_reset_handler(void *dev)
> >   {
> > -    ics_reset(dev);
> > +    device_cold_reset(dev);
> >   }
> >   
> >   static void ics_realize(DeviceState *dev, Error **errp)
> > diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
> > index 2f4112907b8..ae908fd9e41 100644
> > --- a/hw/pci-host/pnv_phb3_msi.c
> > +++ b/hw/pci-host/pnv_phb3_msi.c
> > @@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev)
> >       msi->rba_sum = 0;
> >   }
> >   
> > -static void phb3_msi_reset_handler(void *dev)
> > -{
> > -    phb3_msi_reset(dev);
> > -}
> > -
> >   void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base,
> >                                   uint32_t count)
> >   {
> > @@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp)
> >       }
> >   
> >       msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs);
> > -
> > -    qemu_register_reset(phb3_msi_reset_handler, dev);
> >   }
> >   
> >   static void phb3_msi_instance_init(Object *obj)
> 

Re: [PATCH for-8.0 5/7] hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()

[Philippe Mathieu-Daudé]

On 25/11/22 14:45, Greg Kurz wrote:
> On Fri, 25 Nov 2022 13:24:00 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> On 11/25/22 12:52, Peter Maydell wrote:
>>> The realize method for the TYPE_ICS class uses qemu_register_reset()
>>> to register a reset handler, as a workaround for the fact that
>>> currently objects which directly inherit from TYPE_DEVICE don't get
>>> automatically reset.  However, the reset function directly calls
>>> ics_reset(), which is the function that implements the legacy reset
>>> method.  This means that only the parent class's data gets reset, and
>>> a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has
>>> to register its own reset function.
>>>
>>> Make the TYPE_ICS reset function call device_cold_reset() instead:
>>> this will handle reset for both the parent class and the subclass,
>>> and will work whether the classes are using legacy reset or 3-phase
>>> reset. This allows us to remove the reset function that the subclass
>>> currently has to set up.
>>
>> Nice !
>>
> 
> Seconded.
> 

Thirded :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>

[PATCH for-8.0 6/7] hw/intc/xics: Convert TYPE_ICS to 3-phase reset

[Peter Maydell]

Convert the TYPE_ICS class to 3-phase reset; this will allow us
to convert the TYPE_PHB3_MSI class which inherits from it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/xics.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index dd130467ccc..c7f8abd71e4 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -564,9 +564,9 @@ static void ics_reset_irq(ICSIRQState *irq)
     irq->saved_priority = 0xff;
 }
 
-static void ics_reset(DeviceState *dev)
+static void ics_reset_hold(Object *obj)
 {
-    ICSState *ics = ICS(dev);
+    ICSState *ics = ICS(obj);
     g_autofree uint8_t *flags = g_malloc(ics->nr_irqs);
     int i;
 
@@ -584,7 +584,7 @@ static void ics_reset(DeviceState *dev)
     if (kvm_irqchip_in_kernel()) {
         Error *local_err = NULL;
 
-        ics_set_kvm_state(ICS(dev), &local_err);
+        ics_set_kvm_state(ics, &local_err);
         if (local_err) {
             error_report_err(local_err);
         }
@@ -688,16 +688,17 @@ static Property ics_properties[] = {
 static void ics_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     dc->realize = ics_realize;
     device_class_set_props(dc, ics_properties);
-    dc->reset = ics_reset;
     dc->vmsd = &vmstate_ics;
     /*
      * Reason: part of XICS interrupt controller, needs to be wired up,
      * e.g. by spapr_irq_init().
      */
     dc->user_creatable = false;
+    rc->phases.hold = ics_reset_hold;
 }
 
 static const TypeInfo ics_info = {
-- 
2.25.1

Re: [PATCH for-8.0 6/7] hw/intc/xics: Convert TYPE_ICS to 3-phase reset

[Cédric Le Goater]

On 11/25/22 12:52, Peter Maydell wrote:
> Convert the TYPE_ICS class to 3-phase reset; this will allow us
> to convert the TYPE_PHB3_MSI class which inherits from it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/intc/xics.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index dd130467ccc..c7f8abd71e4 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -564,9 +564,9 @@ static void ics_reset_irq(ICSIRQState *irq)
>       irq->saved_priority = 0xff;
>   }
>   
> -static void ics_reset(DeviceState *dev)
> +static void ics_reset_hold(Object *obj)
>   {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS(obj);
>       g_autofree uint8_t *flags = g_malloc(ics->nr_irqs);
>       int i;
>   
> @@ -584,7 +584,7 @@ static void ics_reset(DeviceState *dev)
>       if (kvm_irqchip_in_kernel()) {
>           Error *local_err = NULL;
>   
> -        ics_set_kvm_state(ICS(dev), &local_err);
> +        ics_set_kvm_state(ics, &local_err);
>           if (local_err) {
>               error_report_err(local_err);
>           }
> @@ -688,16 +688,17 @@ static Property ics_properties[] = {
>   static void ics_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>   
>       dc->realize = ics_realize;
>       device_class_set_props(dc, ics_properties);
> -    dc->reset = ics_reset;
>       dc->vmsd = &vmstate_ics;
>       /*
>        * Reason: part of XICS interrupt controller, needs to be wired up,
>        * e.g. by spapr_irq_init().
>        */
>       dc->user_creatable = false;
> +    rc->phases.hold = ics_reset_hold;
>   }
>   
>   static const TypeInfo ics_info = {

Re: [PATCH for-8.0 6/7] hw/intc/xics: Convert TYPE_ICS to 3-phase reset

[Greg Kurz]

On Fri, 25 Nov 2022 11:52:39 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> Convert the TYPE_ICS class to 3-phase reset; this will allow us
> to convert the TYPE_PHB3_MSI class which inherits from it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/intc/xics.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index dd130467ccc..c7f8abd71e4 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -564,9 +564,9 @@ static void ics_reset_irq(ICSIRQState *irq)
>      irq->saved_priority = 0xff;
>  }
>  
> -static void ics_reset(DeviceState *dev)
> +static void ics_reset_hold(Object *obj)
>  {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS(obj);
>      g_autofree uint8_t *flags = g_malloc(ics->nr_irqs);
>      int i;
>  
> @@ -584,7 +584,7 @@ static void ics_reset(DeviceState *dev)
>      if (kvm_irqchip_in_kernel()) {
>          Error *local_err = NULL;
>  
> -        ics_set_kvm_state(ICS(dev), &local_err);
> +        ics_set_kvm_state(ics, &local_err);


>          if (local_err) {
>              error_report_err(local_err);
>          }
> @@ -688,16 +688,17 @@ static Property ics_properties[] = {
>  static void ics_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>  
>      dc->realize = ics_realize;
>      device_class_set_props(dc, ics_properties);
> -    dc->reset = ics_reset;
>      dc->vmsd = &vmstate_ics;
>      /*
>       * Reason: part of XICS interrupt controller, needs to be wired up,
>       * e.g. by spapr_irq_init().
>       */
>      dc->user_creatable = false;
> +    rc->phases.hold = ics_reset_hold;
>  }
>  
>  static const TypeInfo ics_info = {

Re: [PATCH for-8.0 6/7] hw/intc/xics: Convert TYPE_ICS to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_ICS class to 3-phase reset; this will allow us
> to convert the TYPE_PHB3_MSI class which inherits from it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/xics.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH for-8.0 7/7] hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset

[Peter Maydell]

Convert the TYPE_PHB3_MSI class to 3-phase reset, so we can
avoid using the device_class_set_parent_reset() function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/ppc/xics.h      |  2 +-
 hw/pci-host/pnv_phb3_msi.c | 15 +++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 00b80b08c27..95ead0dd7c9 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -95,7 +95,7 @@ struct ICSStateClass {
     DeviceClass parent_class;
 
     DeviceRealize parent_realize;
-    DeviceReset parent_reset;
+    ResettablePhases parent_phases;
 
     void (*reject)(ICSState *s, uint32_t irq);
     void (*resend)(ICSState *s);
diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
index ae908fd9e41..41e63b066f9 100644
--- a/hw/pci-host/pnv_phb3_msi.c
+++ b/hw/pci-host/pnv_phb3_msi.c
@@ -228,12 +228,14 @@ static void phb3_msi_resend(ICSState *ics)
     }
 }
 
-static void phb3_msi_reset(DeviceState *dev)
+static void phb3_msi_reset_hold(Object *obj)
 {
-    Phb3MsiState *msi = PHB3_MSI(dev);
-    ICSStateClass *icsc = ICS_GET_CLASS(dev);
+    Phb3MsiState *msi = PHB3_MSI(obj);
+    ICSStateClass *icsc = ICS_GET_CLASS(obj);
 
-    icsc->parent_reset(dev);
+    if (icsc->parent_phases.hold) {
+        icsc->parent_phases.hold(obj);
+    }
 
     memset(msi->rba, 0, sizeof(msi->rba));
     msi->rba_sum = 0;
@@ -287,11 +289,12 @@ static void phb3_msi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     ICSStateClass *isc = ICS_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     device_class_set_parent_realize(dc, phb3_msi_realize,
                                     &isc->parent_realize);
-    device_class_set_parent_reset(dc, phb3_msi_reset,
-                                  &isc->parent_reset);
+    resettable_class_set_parent_phases(rc, NULL, phb3_msi_reset_hold, NULL,
+                                       &isc->parent_phases);
 
     isc->reject = phb3_msi_reject;
     isc->resend = phb3_msi_resend;
-- 
2.25.1

Re: [PATCH for-8.0 7/7] hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset

[Cédric Le Goater]

On 11/25/22 12:52, Peter Maydell wrote:
> Convert the TYPE_PHB3_MSI class to 3-phase reset, so we can
> avoid using the device_class_set_parent_reset() function.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   include/hw/ppc/xics.h      |  2 +-
>   hw/pci-host/pnv_phb3_msi.c | 15 +++++++++------
>   2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 00b80b08c27..95ead0dd7c9 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -95,7 +95,7 @@ struct ICSStateClass {
>       DeviceClass parent_class;
>   
>       DeviceRealize parent_realize;
> -    DeviceReset parent_reset;
> +    ResettablePhases parent_phases;
>   
>       void (*reject)(ICSState *s, uint32_t irq);
>       void (*resend)(ICSState *s);
> diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
> index ae908fd9e41..41e63b066f9 100644
> --- a/hw/pci-host/pnv_phb3_msi.c
> +++ b/hw/pci-host/pnv_phb3_msi.c
> @@ -228,12 +228,14 @@ static void phb3_msi_resend(ICSState *ics)
>       }
>   }
>   
> -static void phb3_msi_reset(DeviceState *dev)
> +static void phb3_msi_reset_hold(Object *obj)
>   {
> -    Phb3MsiState *msi = PHB3_MSI(dev);
> -    ICSStateClass *icsc = ICS_GET_CLASS(dev);
> +    Phb3MsiState *msi = PHB3_MSI(obj);
> +    ICSStateClass *icsc = ICS_GET_CLASS(obj);
>   
> -    icsc->parent_reset(dev);
> +    if (icsc->parent_phases.hold) {
> +        icsc->parent_phases.hold(obj);
> +    }
>   
>       memset(msi->rba, 0, sizeof(msi->rba));
>       msi->rba_sum = 0;
> @@ -287,11 +289,12 @@ static void phb3_msi_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       ICSStateClass *isc = ICS_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>   
>       device_class_set_parent_realize(dc, phb3_msi_realize,
>                                       &isc->parent_realize);
> -    device_class_set_parent_reset(dc, phb3_msi_reset,
> -                                  &isc->parent_reset);
> +    resettable_class_set_parent_phases(rc, NULL, phb3_msi_reset_hold, NULL,
> +                                       &isc->parent_phases);
>   
>       isc->reject = phb3_msi_reject;
>       isc->resend = phb3_msi_resend;

Re: [PATCH for-8.0 7/7] hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset

[Philippe Mathieu-Daudé]

On 25/11/22 12:52, Peter Maydell wrote:
> Convert the TYPE_PHB3_MSI class to 3-phase reset, so we can
> avoid using the device_class_set_parent_reset() function.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/ppc/xics.h      |  2 +-
>   hw/pci-host/pnv_phb3_msi.c | 15 +++++++++------
>   2 files changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH for-8.0 0/7] virtio, pci, xics: 3-phase reset conversions

[Daniel Henrique Barboza]

On 11/25/22 08:52, Peter Maydell wrote:
> This patchset converts a miscellaneous collection of classes
> to 3-phase reset. The common link, as with other series I've
> sent out recently, is converting child classes that currently
> use device_class_set_parent_reset() so that we can remove
> that function. To do this we first need to convert the parent
> class, and then the subclass.
> 
> The first two patches handle TYPE_VIRTIO_VGA_BASE, and its parent
> TYPE_VIRTIO_PCI.
> 
> The second two handle the parent TYPE_PCIE_ROOT_PORT and its two
> child classes TYPE_CXL_ROOT_PORT and TYPE_PNV_PHB_ROOT_PORT.
> 
> The last three deal with TYPE_PHB3_MSI and the parenT TYPE_ICS.
> 
> This is all 8.0 material.

Tested with the powern8 machine model that uses xics. Series:


Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>

> 
> thanks
> -- PMM
> 
> Peter Maydell (7):
>    hw/virtio: Convert TYPE_VIRTIO_PCI to 3-phase reset
>    hw/display/virtio-vga: Convert TYPE_VIRTIO_VGA_BASE to 3-phase reset
>    pci: Convert TYPE_PCIE_ROOT_PORT to 3-phase reset
>    pci: Convert child classes of TYPE_PCIE_ROOT_PORT to 3-phase reset
>    hw/intc/xics: Reset TYPE_ICS objects with device_cold_reset()
>    hw/intc/xics: Convert TYPE_ICS to 3-phase reset
>    hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset
> 
>   hw/display/virtio-vga.h        |  2 +-
>   include/hw/pci/pcie_port.h     |  2 +-
>   include/hw/ppc/xics.h          |  2 +-
>   hw/display/virtio-vga.c        | 15 +++++++++------
>   hw/intc/xics.c                 | 11 ++++++-----
>   hw/pci-bridge/cxl_root_port.c  | 14 +++++++++-----
>   hw/pci-bridge/pcie_root_port.c |  8 +++++---
>   hw/pci-host/pnv_phb.c          | 18 ++++++++++--------
>   hw/pci-host/pnv_phb3_msi.c     | 22 +++++++++-------------
>   hw/virtio/virtio-pci.c         |  8 +++++---
>   10 files changed, 56 insertions(+), 46 deletions(-)
> 

Re: [PATCH for-8.0 0/7] virtio, pci, xics: 3-phase reset conversions

[Peter Maydell]

On Fri, 25 Nov 2022 at 11:52, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset converts a miscellaneous collection of classes
> to 3-phase reset. The common link, as with other series I've
> sent out recently, is converting child classes that currently
> use device_class_set_parent_reset() so that we can remove
> that function. To do this we first need to convert the parent
> class, and then the subclass.
>
> The first two patches handle TYPE_VIRTIO_VGA_BASE, and its parent
> TYPE_VIRTIO_PCI.
>
> The second two handle the parent TYPE_PCIE_ROOT_PORT and its two
> child classes TYPE_CXL_ROOT_PORT and TYPE_PNV_PHB_ROOT_PORT.
>
> The last three deal with TYPE_PHB3_MSI and the parenT TYPE_ICS.


I plan to pick these up and send them in in a pullreq with
various other reset-related patches of mine.

thanks
-- PMM