diff mbox series

[14/42] hw/isa/piix3: Modernize reset handling

Message ID 20220901162613.6939-15-shentey@gmail.com (mailing list archive)
State New, archived
Headers show
Series Consolidate PIIX south bridges | expand

Commit Message

Bernhard Beschow Sept. 1, 2022, 4:25 p.m. UTC 
Rather than registering the reset handler via a function which
appends the handler to a global list, prefer to implement it as
a virtual method - PIIX4 does the same already.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé Sept. 1, 2022, 8:33 p.m. UTC | #1 
On 1/9/22 18:25, Bernhard Beschow wrote:
> Rather than registering the reset handler via a function which
> appends the handler to a global list, prefer to implement it as
> a virtual method - PIIX4 does the same already.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index c8c2dd6048..0350f70706 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -31,7 +31,6 @@
>   #include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
>   #include "hw/xen/xen.h"
> -#include "sysemu/reset.h"
>   #include "sysemu/runstate.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/acpi_aml_interface.h"
> @@ -156,9 +155,9 @@ static void piix3_write_config_xen(PCIDevice *dev,
>       piix3_write_config(dev, address, val, len);
>   }
>   
> -static void piix3_reset(void *opaque)
> +static void piix3_reset(DeviceState *dev)
>   {
> -    PIIX3State *d = opaque;
> +    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>       uint8_t *pci_conf = d->dev.config;
>   
>       pci_conf[0x04] = 0x07; /* master, memory and I/O */
> @@ -321,8 +320,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>       memory_region_add_subregion_overlap(pci_address_space_io(dev),
>                                           PIIX_RCR_IOPORT, &d->rcr_mem, 1);
>   
> -    qemu_register_reset(piix3_reset, d);
> -
>       i8257_dma_init(isa_bus, 0);
>   
>       /* RTC */
> @@ -397,6 +394,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>       AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
>   
> +    dc->reset       = piix3_reset;
>       dc->desc        = "ISA bridge";
>       dc->vmsd        = &vmstate_piix3;
>       dc->hotpluggable   = false;

Yay 4 years later...
https://lore.kernel.org/qemu-devel/20180108024558.17983-28-f4bug@amsat.org/

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Mark Cave-Ayland Sept. 18, 2022, 7:44 p.m. UTC | #2 
On 01/09/2022 17:25, Bernhard Beschow wrote:

> Rather than registering the reset handler via a function which
> appends the handler to a global list, prefer to implement it as
> a virtual method - PIIX4 does the same already.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index c8c2dd6048..0350f70706 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -31,7 +31,6 @@
>   #include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
>   #include "hw/xen/xen.h"
> -#include "sysemu/reset.h"
>   #include "sysemu/runstate.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/acpi_aml_interface.h"
> @@ -156,9 +155,9 @@ static void piix3_write_config_xen(PCIDevice *dev,
>       piix3_write_config(dev, address, val, len);
>   }
>   
> -static void piix3_reset(void *opaque)
> +static void piix3_reset(DeviceState *dev)
>   {
> -    PIIX3State *d = opaque;
> +    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>       uint8_t *pci_conf = d->dev.config;
>   
>       pci_conf[0x04] = 0x07; /* master, memory and I/O */
> @@ -321,8 +320,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>       memory_region_add_subregion_overlap(pci_address_space_io(dev),
>                                           PIIX_RCR_IOPORT, &d->rcr_mem, 1);
>   
> -    qemu_register_reset(piix3_reset, d);
> -
>       i8257_dma_init(isa_bus, 0);
>   
>       /* RTC */
> @@ -397,6 +394,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>       AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
>   
> +    dc->reset       = piix3_reset;
>       dc->desc        = "ISA bridge";
>       dc->vmsd        = &vmstate_piix3;
>       dc->hotpluggable   = false;

One minor point to be aware of here is that qdev reset is a PCI bus level reset 
compared with the existing qemu_register_reset() which is a machine level reset. What 
this means is that dc->reset can also be called writing to the relevant configuration 
space register on a PCI bridge - it may not be an issue here, but worth a mention.


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c8c2dd6048..0350f70706 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -31,7 +31,6 @@ 
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
-#include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
@@ -156,9 +155,9 @@  static void piix3_write_config_xen(PCIDevice *dev,
     piix3_write_config(dev, address, val, len);
 }
 
-static void piix3_reset(void *opaque)
+static void piix3_reset(DeviceState *dev)
 {
-    PIIX3State *d = opaque;
+    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -321,8 +320,6 @@  static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
-    qemu_register_reset(piix3_reset, d);
-
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
@@ -397,6 +394,7 @@  static void pci_piix3_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+    dc->reset       = piix3_reset;
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;