[PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Philippe Mathieu-Daudé]

Since v1:
- Removed the pci-host devices
- Removed the vmcoreinfo conversion (elmarco) but add a comment.
- Added Igor's R-b tag.

Following the thread discussion between Peter/Markus/Damien about
reset handlers:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
I started to remove qemu_register_reset() calls from few qdevified
devices (the trivial ones).

Regards,

Phil.

v1: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html

Philippe Mathieu-Daudé (8):
  hw/acpi/piix4: Convert reset handler to DeviceReset
  hw/isa/piix4: Convert reset handler to DeviceReset
  hw/ide/piix: Convert reset handler to DeviceReset
  hw/ide/sii3112: Convert reset handler to DeviceReset
  hw/ide/via82c: Convert reset handler to DeviceReset
  hw/isa/vt82c686: Convert reset handler to DeviceReset
  hw/input/lm832x: Convert reset handler to DeviceReset
  hw/misc/vmcoreinfo: Document its reset handler

 hw/acpi/piix4.c      |  7 +++----
 hw/ide/piix.c        |  8 +++-----
 hw/ide/sii3112.c     |  7 +++----
 hw/ide/via.c         | 10 ++++------
 hw/input/lm832x.c    | 12 +++++-------
 hw/isa/piix4.c       |  7 +++----
 hw/isa/vt82c686.c    | 11 ++++-------
 hw/misc/vmcoreinfo.c |  1 +
 8 files changed, 26 insertions(+), 37 deletions(-)

-- 
2.21.0

[PATCH v2 1/8] hw/acpi/piix4: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX4/PM is a PCI device within the PIIX4 chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/acpi/piix4.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 5742c3df87..4e079b39bd 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -27,7 +27,6 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/acpi/acpi.h"
-#include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
@@ -344,9 +343,9 @@ static const VMStateDescription vmstate_acpi = {
     }
 };
 
-static void piix4_reset(void *opaque)
+static void piix4_pm_reset(DeviceState *dev)
 {
-    PIIX4PMState *s = opaque;
+    PIIX4PMState *s = PIIX4_PM(dev);
     PCIDevice *d = PCI_DEVICE(s);
     uint8_t *pci_conf = d->config;
 
@@ -542,7 +541,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 
     s->machine_ready.notify = piix4_pm_machine_ready;
     qemu_add_machine_init_done_notifier(&s->machine_ready);
-    qemu_register_reset(piix4_reset, s);
 
     piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
                                    pci_get_bus(dev), s);
@@ -692,6 +690,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
     k->revision = 0x03;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
+    dc->reset = piix4_pm_reset;
     dc->desc = "PM";
     dc->vmsd = &vmstate_acpi;
     dc->props = piix4_pm_properties;
-- 
2.21.0

[PATCH v2 2/8] hw/isa/piix4: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX4/ISA is a PCI device within the PIIX4 chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/isa/piix4.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 3294056cd5..890d999abf 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -28,7 +28,6 @@
 #include "hw/isa/isa.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
-#include "sysemu/reset.h"
 
 PCIDevice *piix4_dev;
 
@@ -40,9 +39,9 @@ typedef struct PIIX4State {
 #define PIIX4_PCI_DEVICE(obj) \
     OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE)
 
-static void piix4_reset(void *opaque)
+static void piix4_isa_reset(DeviceState *dev)
 {
-    PIIX4State *d = opaque;
+    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; // master, memory and I/O
@@ -97,7 +96,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
     piix4_dev = &d->dev;
-    qemu_register_reset(piix4_reset, d);
 }
 
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
@@ -118,6 +116,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
+    dc->reset = piix4_isa_reset;
     dc->desc = "ISA bridge";
     dc->vmsd = &vmstate_piix4;
     /*
-- 
2.21.0

[PATCH v2 3/8] hw/ide/piix: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX3/IDE is a PCI device within the PIIX3 chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/piix.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index fba6bc8bff..18b2c3b722 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -30,7 +30,6 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/dma.h"
-#include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
 #include "trace.h"
@@ -103,9 +102,9 @@ static void bmdma_setup_bar(PCIIDEState *d)
     }
 }
 
-static void piix3_reset(void *opaque)
+static void piix3_ide_reset(DeviceState *dev)
 {
-    PCIIDEState *d = opaque;
+    PCIIDEState *d = PCI_IDE(dev);
     PCIDevice *pd = PCI_DEVICE(d);
     uint8_t *pci_conf = pd->config;
     int i;
@@ -154,8 +153,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
     pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
 
-    qemu_register_reset(piix3_reset, d);
-
     bmdma_setup_bar(d);
     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
 
@@ -247,6 +244,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+    dc->reset = piix3_ide_reset;
     k->realize = pci_piix_ide_realize;
     k->exit = pci_piix_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
-- 
2.21.0

[PATCH v2 4/8] hw/ide/sii3112: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The SiI3112A SATA controller is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/sii3112.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 2181260531..06605d7af2 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -15,7 +15,6 @@
 #include "qemu/osdep.h"
 #include "hw/ide/pci.h"
 #include "qemu/module.h"
-#include "sysemu/reset.h"
 #include "trace.h"
 
 #define TYPE_SII3112_PCI "sii3112"
@@ -237,9 +236,9 @@ static void sii3112_set_irq(void *opaque, int channel, int level)
     sii3112_update_irq(s);
 }
 
-static void sii3112_reset(void *opaque)
+static void sii3112_reset(DeviceState *dev)
 {
-    SiI3112PCIState *s = opaque;
+    SiI3112PCIState *s = SII3112_PCI(dev);
     int i;
 
     for (i = 0; i < 2; i++) {
@@ -290,7 +289,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
         s->bmdma[i].bus = &s->bus[i];
         ide_register_restart_cb(&s->bus[i]);
     }
-    qemu_register_reset(sii3112_reset, s);
 }
 
 static void sii3112_pci_class_init(ObjectClass *klass, void *data)
@@ -303,6 +301,7 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data)
     pd->class_id = PCI_CLASS_STORAGE_RAID;
     pd->revision = 1;
     pd->realize = sii3112_pci_realize;
+    dc->reset = sii3112_reset;
     dc->desc = "SiI3112A SATA controller";
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
-- 
2.21.0

[PATCH v2 5/8] hw/ide/via82c: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The VIA82C686B IDE controller is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/via.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 7087dc676e..053622bd82 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -29,7 +29,6 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "sysemu/dma.h"
-#include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
 #include "trace.h"
@@ -120,10 +119,10 @@ static void via_ide_set_irq(void *opaque, int n, int level)
     }
 }
 
-static void via_ide_reset(void *opaque)
+static void via_ide_reset(DeviceState *dev)
 {
-    PCIIDEState *d = opaque;
-    PCIDevice *pd = PCI_DEVICE(d);
+    PCIIDEState *d = PCI_IDE(dev);
+    PCIDevice *pd = PCI_DEVICE(dev);
     uint8_t *pci_conf = pd->config;
     int i;
 
@@ -172,8 +171,6 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
     pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
     dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
 
-    qemu_register_reset(via_ide_reset, d);
-
     memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
                           &d->bus[0], "via-ide0-data", 8);
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[0]);
@@ -229,6 +226,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+    dc->reset = via_ide_reset;
     k->realize = via_ide_realize;
     k->exit = via_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_VIA;
-- 
2.21.0

[PATCH v2 6/8] hw/isa/vt82c686: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The VIA VT82C686 Southbridge is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/isa/vt82c686.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 50bd28fa82..616f67f347 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,7 +23,6 @@
 #include "hw/isa/apm.h"
 #include "hw/acpi/acpi.h"
 #include "hw/i2c/pm_smbus.h"
-#include "sysemu/reset.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
@@ -116,11 +115,10 @@ static const MemoryRegionOps superio_ops = {
     },
 };
 
-static void vt82c686b_reset(void * opaque)
+static void vt82c686b_isa_reset(DeviceState *dev)
 {
-    PCIDevice *d = opaque;
-    uint8_t *pci_conf = d->config;
-    VT82C686BState *vt82c = VT82C686B_DEVICE(d);
+    VT82C686BState *vt82c = VT82C686B_DEVICE(dev);
+    uint8_t *pci_conf = vt82c->dev.config;
 
     pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
     pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
@@ -476,8 +474,6 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
      * But we do not emulate a floppy, so just set it here. */
     memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
                                 &vt82c->superio);
-
-    qemu_register_reset(vt82c686b_reset, d);
 }
 
 ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
@@ -501,6 +497,7 @@ static void via_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
     k->revision = 0x40;
+    dc->reset = vt82c686b_isa_reset;
     dc->desc = "ISA bridge";
     dc->vmsd = &vmstate_via;
     /*
-- 
2.21.0

[PATCH v2 7/8] hw/input/lm832x: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The LM8323 key-scan controller is a I2C device, it will be reset
when the I2C bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/input/lm832x.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
index a37eb854b9..aa629ddbf1 100644
--- a/hw/input/lm832x.c
+++ b/hw/input/lm832x.c
@@ -24,7 +24,6 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
-#include "sysemu/reset.h"
 #include "ui/console.h"
 
 #define TYPE_LM8323 "lm8323"
@@ -94,8 +93,10 @@ static void lm_kbd_gpio_update(LM823KbdState *s)
 {
 }
 
-static void lm_kbd_reset(LM823KbdState *s)
+static void lm_kbd_reset(DeviceState *dev)
 {
+    LM823KbdState *s = LM8323(dev);
+
     s->config = 0x80;
     s->status = INT_NOINIT;
     s->acttime = 125;
@@ -273,7 +274,7 @@ static void lm_kbd_write(LM823KbdState *s, int reg, int byte, uint8_t value)
 
     case LM832x_CMD_RESET:
         if (value == 0xaa)
-            lm_kbd_reset(s);
+            lm_kbd_reset(DEVICE(s));
         else
             lm_kbd_error(s, ERR_BADPAR);
         s->reg = LM832x_GENERAL_ERROR;
@@ -476,10 +477,6 @@ static void lm8323_realize(DeviceState *dev, Error **errp)
     s->pwm.tm[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm1_tick, s);
     s->pwm.tm[2] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm2_tick, s);
     qdev_init_gpio_out(dev, &s->nirq, 1);
-
-    lm_kbd_reset(s);
-
-    qemu_register_reset((void *) lm_kbd_reset, s);
 }
 
 void lm832x_key_event(DeviceState *dev, int key, int state)
@@ -507,6 +504,7 @@ static void lm8323_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
 
+    dc->reset = lm_kbd_reset;
     dc->realize = lm8323_realize;
     k->event = lm_i2c_event;
     k->recv = lm_i2c_rx;
-- 
2.21.0

[PATCH v2 8/8] hw/misc/vmcoreinfo: Add comment about reset handler

[Philippe Mathieu-Daudé]

The VM coreinfo device does not sit on a bus, so it won't be
reset automatically. This is why it calls qemu_register_reset().

Add a comment about it, so we don't convert its reset handler
to a DeviceReset method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/misc/vmcoreinfo.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c
index 326a3ce8f4..a9d718fc23 100644
--- a/hw/misc/vmcoreinfo.c
+++ b/hw/misc/vmcoreinfo.c
@@ -61,6 +61,10 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
                              NULL, fw_cfg_vmci_write, s,
                              &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
 
+    /*
+     * This device requires to register a global reset because it is
+     * not plugged to a bus (which, as its QOM parent, would reset it).
+     */
     qemu_register_reset(vmcoreinfo_reset, dev);
     vmcoreinfo_state = s;
 }
-- 
2.21.0

Re: [PATCH v2 8/8] hw/misc/vmcoreinfo: Add comment about reset handler

[Marc-André Lureau]

On Tue, Oct 8, 2019 at 6:35 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The VM coreinfo device does not sit on a bus, so it won't be
> reset automatically. This is why it calls qemu_register_reset().
>
> Add a comment about it, so we don't convert its reset handler
> to a DeviceReset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/misc/vmcoreinfo.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c
> index 326a3ce8f4..a9d718fc23 100644
> --- a/hw/misc/vmcoreinfo.c
> +++ b/hw/misc/vmcoreinfo.c
> @@ -61,6 +61,10 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
>                               NULL, fw_cfg_vmci_write, s,
>                               &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
>
> +    /*
> +     * This device requires to register a global reset because it is
> +     * not plugged to a bus (which, as its QOM parent, would reset it).
> +     */
>      qemu_register_reset(vmcoreinfo_reset, dev);
>      vmcoreinfo_state = s;
>  }
> --
> 2.21.0
>

Re: [PATCH v2 1/8] hw/acpi/piix4: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:28写道：

> The PIIX4/PM is a PCI device within the PIIX4 chipset, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>


Reviewed-by: Li Qiang <liq3ea@gmail.com>


> ---
>  hw/acpi/piix4.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 5742c3df87..4e079b39bd 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -27,7 +27,6 @@
>  #include "hw/pci/pci.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/acpi/acpi.h"
> -#include "sysemu/reset.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi/error.h"
> @@ -344,9 +343,9 @@ static const VMStateDescription vmstate_acpi = {
>      }
>  };
>
> -static void piix4_reset(void *opaque)
> +static void piix4_pm_reset(DeviceState *dev)
>  {
> -    PIIX4PMState *s = opaque;
> +    PIIX4PMState *s = PIIX4_PM(dev);
>      PCIDevice *d = PCI_DEVICE(s);
>      uint8_t *pci_conf = d->config;
>
> @@ -542,7 +541,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error
> **errp)
>
>      s->machine_ready.notify = piix4_pm_machine_ready;
>      qemu_add_machine_init_done_notifier(&s->machine_ready);
> -    qemu_register_reset(piix4_reset, s);
>
>      piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
>                                     pci_get_bus(dev), s);
> @@ -692,6 +690,7 @@ static void piix4_pm_class_init(ObjectClass *klass,
> void *data)
>      k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
>      k->revision = 0x03;
>      k->class_id = PCI_CLASS_BRIDGE_OTHER;
> +    dc->reset = piix4_pm_reset;
>      dc->desc = "PM";
>      dc->vmsd = &vmstate_acpi;
>      dc->props = piix4_pm_properties;
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2977 bytes --]

Re: [PATCH v2 2/8] hw/isa/piix4: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:49写道：

> The PIIX4/ISA is a PCI device within the PIIX4 chipset, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Reviewed-by: Li Qiang <liq3ea@gmail.com>



> ---
>  hw/isa/piix4.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 3294056cd5..890d999abf 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -28,7 +28,6 @@
>  #include "hw/isa/isa.h"
>  #include "hw/sysbus.h"
>  #include "migration/vmstate.h"
> -#include "sysemu/reset.h"
>
>  PCIDevice *piix4_dev;
>
> @@ -40,9 +39,9 @@ typedef struct PIIX4State {
>  #define PIIX4_PCI_DEVICE(obj) \
>      OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE)
>
> -static void piix4_reset(void *opaque)
> +static void piix4_isa_reset(DeviceState *dev)
>  {
> -    PIIX4State *d = opaque;
> +    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
>      uint8_t *pci_conf = d->dev.config;
>
>      pci_conf[0x04] = 0x07; // master, memory and I/O
> @@ -97,7 +96,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
>          return;
>      }
>      piix4_dev = &d->dev;
> -    qemu_register_reset(piix4_reset, d);
>  }
>
>  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
> @@ -118,6 +116,7 @@ static void piix4_class_init(ObjectClass *klass, void
> *data)
>      k->vendor_id = PCI_VENDOR_ID_INTEL;
>      k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
>      k->class_id = PCI_CLASS_BRIDGE_ISA;
> +    dc->reset = piix4_isa_reset;
>      dc->desc = "ISA bridge";
>      dc->vmsd = &vmstate_piix4;
>      /*
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2838 bytes --]

Re: [PATCH v2 3/8] hw/ide/piix: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:32写道：

> The PIIX3/IDE is a PCI device within the PIIX3 chipset, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ide/piix.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index fba6bc8bff..18b2c3b722 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -30,7 +30,6 @@
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/dma.h"
> -#include "sysemu/reset.h"
>
>  #include "hw/ide/pci.h"
>  #include "trace.h"
> @@ -103,9 +102,9 @@ static void bmdma_setup_bar(PCIIDEState *d)
>      }
>  }
>
> -static void piix3_reset(void *opaque)
> +static void piix3_ide_reset(DeviceState *dev)
>  {
> -    PCIIDEState *d = opaque;
> +    PCIIDEState *d = PCI_IDE(dev);
>      PCIDevice *pd = PCI_DEVICE(d);
>      uint8_t *pci_conf = pd->config;
>      int i;
> @@ -154,8 +153,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
> **errp)
>
>      pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
>
> -    qemu_register_reset(piix3_reset, d);
> -
>      bmdma_setup_bar(d);
>      pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>
> @@ -247,6 +244,7 @@ static void piix3_ide_class_init(ObjectClass *klass,
> void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> +    dc->reset = piix3_ide_reset;
>      k->realize = pci_piix_ide_realize;
>      k->exit = pci_piix_ide_exitfn;
>      k->vendor_id = PCI_VENDOR_ID_INTEL;
> --
>


Shouldn't we also add the reset callback for piix4 ide device?

Thanks,
Li Qiang



> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2749 bytes --]

Re: [PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:47写道：

> Since v1:
> - Removed the pci-host devices
>

Hello  I want to know why  remove this?

Thanks,
Li Qiang


> - Removed the vmcoreinfo conversion (elmarco) but add a comment.
> - Added Igor's R-b tag.
>
> Following the thread discussion between Peter/Markus/Damien about
> reset handlers:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
> I started to remove qemu_register_reset() calls from few qdevified
> devices (the trivial ones).
>
> Regards,
>
> Phil.
>
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html
>
> Philippe Mathieu-Daudé (8):
>   hw/acpi/piix4: Convert reset handler to DeviceReset
>   hw/isa/piix4: Convert reset handler to DeviceReset
>   hw/ide/piix: Convert reset handler to DeviceReset
>   hw/ide/sii3112: Convert reset handler to DeviceReset
>   hw/ide/via82c: Convert reset handler to DeviceReset
>   hw/isa/vt82c686: Convert reset handler to DeviceReset
>   hw/input/lm832x: Convert reset handler to DeviceReset
>   hw/misc/vmcoreinfo: Document its reset handler
>
>  hw/acpi/piix4.c      |  7 +++----
>  hw/ide/piix.c        |  8 +++-----
>  hw/ide/sii3112.c     |  7 +++----
>  hw/ide/via.c         | 10 ++++------
>  hw/input/lm832x.c    | 12 +++++-------
>  hw/isa/piix4.c       |  7 +++----
>  hw/isa/vt82c686.c    | 11 ++++-------
>  hw/misc/vmcoreinfo.c |  1 +
>  8 files changed, 26 insertions(+), 37 deletions(-)
>
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2421 bytes --]

Re: [PATCH v2 1/8] hw/acpi/piix4: Convert reset handler to DeviceReset

[Michael S. Tsirkin]

On Tue, Oct 08, 2019 at 04:25:32PM +0200, Philippe Mathieu-Daudé wrote:
> The PIIX4/PM is a PCI device within the PIIX4 chipset, it will be reset
> when the PCI bus it stands on is reset.
> 
> Convert its reset handler into a proper Device reset method.
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

feel free to take through the misc tree.


> ---
>  hw/acpi/piix4.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 5742c3df87..4e079b39bd 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -27,7 +27,6 @@
>  #include "hw/pci/pci.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/acpi/acpi.h"
> -#include "sysemu/reset.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi/error.h"
> @@ -344,9 +343,9 @@ static const VMStateDescription vmstate_acpi = {
>      }
>  };
>  
> -static void piix4_reset(void *opaque)
> +static void piix4_pm_reset(DeviceState *dev)
>  {
> -    PIIX4PMState *s = opaque;
> +    PIIX4PMState *s = PIIX4_PM(dev);
>      PCIDevice *d = PCI_DEVICE(s);
>      uint8_t *pci_conf = d->config;
>  
> @@ -542,7 +541,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
>  
>      s->machine_ready.notify = piix4_pm_machine_ready;
>      qemu_add_machine_init_done_notifier(&s->machine_ready);
> -    qemu_register_reset(piix4_reset, s);
>  
>      piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
>                                     pci_get_bus(dev), s);
> @@ -692,6 +690,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
>      k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
>      k->revision = 0x03;
>      k->class_id = PCI_CLASS_BRIDGE_OTHER;
> +    dc->reset = piix4_pm_reset;
>      dc->desc = "PM";
>      dc->vmsd = &vmstate_acpi;
>      dc->props = piix4_pm_properties;
> -- 
> 2.21.0

Re: [PATCH v2 3/8] hw/ide/piix: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

On 10/9/19 3:08 AM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于 
> 2019年10月8日周二 下午10:32写道：
> 
>     The PIIX3/IDE is a PCI device within the PIIX3 chipset, it will be reset
>     when the PCI bus it stands on is reset.
> 
>     Convert its reset handler into a proper Device reset method.
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>       hw/ide/piix.c | 8 +++-----
>       1 file changed, 3 insertions(+), 5 deletions(-)
> 
>     diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>     index fba6bc8bff..18b2c3b722 100644
>     --- a/hw/ide/piix.c
>     +++ b/hw/ide/piix.c
>     @@ -30,7 +30,6 @@
>       #include "sysemu/block-backend.h"
>       #include "sysemu/blockdev.h"
>       #include "sysemu/dma.h"
>     -#include "sysemu/reset.h"
> 
>       #include "hw/ide/pci.h"
>       #include "trace.h"
>     @@ -103,9 +102,9 @@ static void bmdma_setup_bar(PCIIDEState *d)
>           }
>       }
> 
>     -static void piix3_reset(void *opaque)
>     +static void piix3_ide_reset(DeviceState *dev)
>       {
>     -    PCIIDEState *d = opaque;
>     +    PCIIDEState *d = PCI_IDE(dev);
>           PCIDevice *pd = PCI_DEVICE(d);
>           uint8_t *pci_conf = pd->config;
>           int i;
>     @@ -154,8 +153,6 @@ static void pci_piix_ide_realize(PCIDevice *dev,
>     Error **errp)
> 
>           pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
> 
>     -    qemu_register_reset(piix3_reset, d);
>     -
>           bmdma_setup_bar(d);
>           pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO,
>     &d->bmdma_bar);
> 
>     @@ -247,6 +244,7 @@ static void piix3_ide_class_init(ObjectClass
>     *klass, void *data)
>           DeviceClass *dc = DEVICE_CLASS(klass);
>           PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> 
>     +    dc->reset = piix3_ide_reset;
>           k->realize = pci_piix_ide_realize;
>           k->exit = pci_piix_ide_exitfn;
>           k->vendor_id = PCI_VENDOR_ID_INTEL;
>     -- 
> 
> 
> 
> Shouldn't we also add the reset callback for piix4 ide device?

Yes! You catched a nice bug which would have rendered the Malta board 
broken on reboot, thanks a lot!

Phil.

Re: [PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Philippe Mathieu-Daudé]

Hi Li,

On 10/9/19 4:28 AM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于 
> 2019年10月8日周二 下午10:47写道：
> 
>     Since v1:
>     - Removed the pci-host devices
> 
> 
> Hello  I want to know why  remove this?

I haven't removed the devices, I simply remove the patches converting 
them to DeviceReset, basically because I've not enough time to check if 
the are on a bus that would reset them. I added these devices on my TODO 
list for later, so meanwhile the other devices can be easily reviewed 
and merged. When few patches from a series are not reviewed or 
incorrect, sometime the rest of the series is not merged, so I prefer to 
split it and get these patches merged.

> 
>     - Removed the vmcoreinfo conversion (elmarco) but add a comment.
>     - Added Igor's R-b tag.
> 
>     Following the thread discussion between Peter/Markus/Damien about
>     reset handlers:
>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
>     I started to remove qemu_register_reset() calls from few qdevified
>     devices (the trivial ones).
> 
>     Regards,
> 
>     Phil.
> 
>     v1: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html
> 
>     Philippe Mathieu-Daudé (8):
>        hw/acpi/piix4: Convert reset handler to DeviceReset
>        hw/isa/piix4: Convert reset handler to DeviceReset
>        hw/ide/piix: Convert reset handler to DeviceReset
>        hw/ide/sii3112: Convert reset handler to DeviceReset
>        hw/ide/via82c: Convert reset handler to DeviceReset
>        hw/isa/vt82c686: Convert reset handler to DeviceReset
>        hw/input/lm832x: Convert reset handler to DeviceReset
>        hw/misc/vmcoreinfo: Document its reset handler

Re: [PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月10日周四 上午3:54写道：

> Hi Li,
>
> On 10/9/19 4:28 AM, Li Qiang wrote:
> > Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于
> > 2019年10月8日周二 下午10:47写道：
> >
> >     Since v1:
> >     - Removed the pci-host devices
> >
> >
> > Hello  I want to know why  remove this?
>
> I haven't removed the devices, I simply remove the patches converting
> them to DeviceReset,


Yes, I mean the patches.


> basically because I've not enough time to check if
> the are on a bus that would reset them.


IIUC, they are right.


> I added these devices on my TODO
> list for later, so meanwhile the other devices can be easily reviewed
> and merged. When few patches from a series are not reviewed or
> incorrect, sometime the rest of the series is not merged, so I prefer to
> split it and get these patches merged.


As far as I can see, most of the devices' usage of qemu_register_reset
function can be
convert to 'dc->reset'. In the main function.

qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());

The 'qbus_reset_all_fn' calls 'qbus_reset_all' from the 'main-sys-bus'.
Then 'qdev_reset_one'
will call 'device_reset'. So IIUC every bus attached to 'main-sys-bus' can
be reset through 'dc->reset'

So I'm quite sure most of the cases that devices use 'qemu_register_reset'
can be changed to 'dc->reset'.
Seems you're busy,  If you don't mind, I can do some of the work to convert
'reset' callback(not a patchset, one by one).

Thanks,
Li Qiang




>
> >
> >     - Removed the vmcoreinfo conversion (elmarco) but add a comment.
> >     - Added Igor's R-b tag.
> >
> >     Following the thread discussion between Peter/Markus/Damien about
> >     reset handlers:
> >     https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
> >     I started to remove qemu_register_reset() calls from few qdevified
> >     devices (the trivial ones).
> >
> >     Regards,
> >
> >     Phil.
> >
> >     v1:
> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html
> >
> >     Philippe Mathieu-Daudé (8):
> >        hw/acpi/piix4: Convert reset handler to DeviceReset
> >        hw/isa/piix4: Convert reset handler to DeviceReset
> >        hw/ide/piix: Convert reset handler to DeviceReset
> >        hw/ide/sii3112: Convert reset handler to DeviceReset
> >        hw/ide/via82c: Convert reset handler to DeviceReset
> >        hw/isa/vt82c686: Convert reset handler to DeviceReset
> >        hw/input/lm832x: Convert reset handler to DeviceReset
> >        hw/misc/vmcoreinfo: Document its reset handler
>

[-- Attachment #2: Type: text/html, Size: 4393 bytes --]

Re: [PATCH v2 4/8] hw/ide/sii3112: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:32写道：

> The SiI3112A SATA controller is a PCI device, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Reviewed-by: Li Qiang <liq3ea@gmail.com>


> ---
>  hw/ide/sii3112.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 2181260531..06605d7af2 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -15,7 +15,6 @@
>  #include "qemu/osdep.h"
>  #include "hw/ide/pci.h"
>  #include "qemu/module.h"
> -#include "sysemu/reset.h"
>  #include "trace.h"
>
>  #define TYPE_SII3112_PCI "sii3112"
> @@ -237,9 +236,9 @@ static void sii3112_set_irq(void *opaque, int channel,
> int level)
>      sii3112_update_irq(s);
>  }
>
> -static void sii3112_reset(void *opaque)
> +static void sii3112_reset(DeviceState *dev)
>  {
> -    SiI3112PCIState *s = opaque;
> +    SiI3112PCIState *s = SII3112_PCI(dev);
>      int i;
>
>      for (i = 0; i < 2; i++) {
> @@ -290,7 +289,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
> **errp)
>          s->bmdma[i].bus = &s->bus[i];
>          ide_register_restart_cb(&s->bus[i]);
>      }
> -    qemu_register_reset(sii3112_reset, s);
>  }
>
>  static void sii3112_pci_class_init(ObjectClass *klass, void *data)
> @@ -303,6 +301,7 @@ static void sii3112_pci_class_init(ObjectClass *klass,
> void *data)
>      pd->class_id = PCI_CLASS_STORAGE_RAID;
>      pd->revision = 1;
>      pd->realize = sii3112_pci_realize;
> +    dc->reset = sii3112_reset;
>      dc->desc = "SiI3112A SATA controller";
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  }
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2747 bytes --]

Re: [PATCH v2 5/8] hw/ide/via82c: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:36写道：

> The VIA82C686B IDE controller is a PCI device, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Reviewed-by: Li Qiang <liq3ea@gmail.com>


> ---
>  hw/ide/via.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 7087dc676e..053622bd82 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -29,7 +29,6 @@
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
>  #include "sysemu/dma.h"
> -#include "sysemu/reset.h"
>
>  #include "hw/ide/pci.h"
>  #include "trace.h"
> @@ -120,10 +119,10 @@ static void via_ide_set_irq(void *opaque, int n, int
> level)
>      }
>  }
>
> -static void via_ide_reset(void *opaque)
> +static void via_ide_reset(DeviceState *dev)
>  {
> -    PCIIDEState *d = opaque;
> -    PCIDevice *pd = PCI_DEVICE(d);
> +    PCIIDEState *d = PCI_IDE(dev);
> +    PCIDevice *pd = PCI_DEVICE(dev);
>      uint8_t *pci_conf = pd->config;
>      int i;
>
> @@ -172,8 +171,6 @@ static void via_ide_realize(PCIDevice *dev, Error
> **errp)
>      pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
>      dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
>
> -    qemu_register_reset(via_ide_reset, d);
> -
>      memory_region_init_io(&d->data_bar[0], OBJECT(d),
> &pci_ide_data_le_ops,
>                            &d->bus[0], "via-ide0-data", 8);
>      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[0]);
> @@ -229,6 +226,7 @@ static void via_ide_class_init(ObjectClass *klass,
> void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> +    dc->reset = via_ide_reset;
>      k->realize = via_ide_realize;
>      k->exit = via_ide_exitfn;
>      k->vendor_id = PCI_VENDOR_ID_VIA;
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2945 bytes --]

Re: [PATCH v2 6/8] hw/isa/vt82c686: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:39写道：

> The VIA VT82C686 Southbridge is a PCI device, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Reviewed-by: Li Qiang <liq3ea@gmail.com>


> ---
>  hw/isa/vt82c686.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 50bd28fa82..616f67f347 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -23,7 +23,6 @@
>  #include "hw/isa/apm.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/i2c/pm_smbus.h"
> -#include "sysemu/reset.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
> @@ -116,11 +115,10 @@ static const MemoryRegionOps superio_ops = {
>      },
>  };
>
> -static void vt82c686b_reset(void * opaque)
> +static void vt82c686b_isa_reset(DeviceState *dev)
>  {
> -    PCIDevice *d = opaque;
> -    uint8_t *pci_conf = d->config;
> -    VT82C686BState *vt82c = VT82C686B_DEVICE(d);
> +    VT82C686BState *vt82c = VT82C686B_DEVICE(dev);
> +    uint8_t *pci_conf = vt82c->dev.config;
>
>      pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
>      pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO |
> PCI_COMMAND_MEMORY |
> @@ -476,8 +474,6 @@ static void vt82c686b_realize(PCIDevice *d, Error
> **errp)
>       * But we do not emulate a floppy, so just set it here. */
>      memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
>                                  &vt82c->superio);
> -
> -    qemu_register_reset(vt82c686b_reset, d);
>  }
>
>  ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
> @@ -501,6 +497,7 @@ static void via_class_init(ObjectClass *klass, void
> *data)
>      k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE;
>      k->class_id = PCI_CLASS_BRIDGE_ISA;
>      k->revision = 0x40;
> +    dc->reset = vt82c686b_isa_reset;
>      dc->desc = "ISA bridge";
>      dc->vmsd = &vmstate_via;
>      /*
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 3090 bytes --]

Re: [PATCH v2 7/8] hw/input/lm832x: Convert reset handler to DeviceReset

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:38写道：

> The LM8323 key-scan controller is a I2C device, it will be reset
> when the I2C bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Reviewed-by: Li Qiang <liq3ea@gmail.com>


> ---
>  hw/input/lm832x.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
> index a37eb854b9..aa629ddbf1 100644
> --- a/hw/input/lm832x.c
> +++ b/hw/input/lm832x.c
> @@ -24,7 +24,6 @@
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
> -#include "sysemu/reset.h"
>  #include "ui/console.h"
>
>  #define TYPE_LM8323 "lm8323"
> @@ -94,8 +93,10 @@ static void lm_kbd_gpio_update(LM823KbdState *s)
>  {
>  }
>
> -static void lm_kbd_reset(LM823KbdState *s)
> +static void lm_kbd_reset(DeviceState *dev)
>  {
> +    LM823KbdState *s = LM8323(dev);
> +
>      s->config = 0x80;
>      s->status = INT_NOINIT;
>      s->acttime = 125;
> @@ -273,7 +274,7 @@ static void lm_kbd_write(LM823KbdState *s, int reg,
> int byte, uint8_t value)
>
>      case LM832x_CMD_RESET:
>          if (value == 0xaa)
> -            lm_kbd_reset(s);
> +            lm_kbd_reset(DEVICE(s));
>          else
>              lm_kbd_error(s, ERR_BADPAR);
>          s->reg = LM832x_GENERAL_ERROR;
> @@ -476,10 +477,6 @@ static void lm8323_realize(DeviceState *dev, Error
> **errp)
>      s->pwm.tm[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm1_tick, s);
>      s->pwm.tm[2] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm2_tick, s);
>      qdev_init_gpio_out(dev, &s->nirq, 1);
> -
> -    lm_kbd_reset(s);
> -
> -    qemu_register_reset((void *) lm_kbd_reset, s);
>  }
>
>  void lm832x_key_event(DeviceState *dev, int key, int state)
> @@ -507,6 +504,7 @@ static void lm8323_class_init(ObjectClass *klass, void
> *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>
> +    dc->reset = lm_kbd_reset;
>      dc->realize = lm8323_realize;
>      k->event = lm_i2c_event;
>      k->recv = lm_i2c_rx;
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 3334 bytes --]

Re: [PATCH v2 8/8] hw/misc/vmcoreinfo: Add comment about reset handler

[Li Qiang]

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

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年10月8日周二 下午10:40写道：

> The VM coreinfo device does not sit on a bus, so it won't be
> reset automatically. This is why it calls qemu_register_reset().
>
> Add a comment about it, so we don't convert its reset handler
> to a DeviceReset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>


Reviewed-by: Li Qiang <liq3ea@gmail.com>

There are some other device-related code using 'qemu_register_reset'.
Maybe we can also add theses comment.

Thanks,
Li Qiang


> ---
>  hw/misc/vmcoreinfo.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c
> index 326a3ce8f4..a9d718fc23 100644
> --- a/hw/misc/vmcoreinfo.c
> +++ b/hw/misc/vmcoreinfo.c
> @@ -61,6 +61,10 @@ static void vmcoreinfo_realize(DeviceState *dev, Error
> **errp)
>                               NULL, fw_cfg_vmci_write, s,
>                               &s->vmcoreinfo, sizeof(s->vmcoreinfo),
> false);
>
> +    /*
> +     * This device requires to register a global reset because it is
> +     * not plugged to a bus (which, as its QOM parent, would reset it).
> +     */
>      qemu_register_reset(vmcoreinfo_reset, dev);
>      vmcoreinfo_state = s;
>  }
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2150 bytes --]

Re: [PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Philippe Mathieu-Daudé]

On 10/10/19 3:05 AM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于 
> 2019年10月10日周四 上午3:54写道：
> 
>     Hi Li,
> 
>     On 10/9/19 4:28 AM, Li Qiang wrote:
>      > Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com> <mailto:philmd@redhat.com
>     <mailto:philmd@redhat.com>>> 于
>      > 2019年10月8日周二 下午10:47写道：
>      >
>      >     Since v1:
>      >     - Removed the pci-host devices
>      >
>      >
>      > Hello  I want to know why  remove this?
> 
>     I haven't removed the devices, I simply remove the patches converting
>     them to DeviceReset, 
> 
> 
> Yes, I mean the patches.
> 
>     basically because I've not enough time to check if
>     the are on a bus that would reset them. 
> 
> 
> IIUC, they are right.
> 
>     I added these devices on my TODO
>     list for later, so meanwhile the other devices can be easily reviewed
>     and merged. When few patches from a series are not reviewed or
>     incorrect, sometime the rest of the series is not merged, so I
>     prefer to
>     split it and get these patches merged.
> 
> 
> As far as I can see, most of the devices' usage of qemu_register_reset 
> function can be
> convert to 'dc->reset'. In the main function.
> 
> qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());
> 
> The 'qbus_reset_all_fn' calls 'qbus_reset_all' from the 'main-sys-bus'. 
> Then 'qdev_reset_one'
> will call 'device_reset'. So IIUC every bus attached to 'main-sys-bus' 
> can be reset through 'dc->reset'
> 
> So I'm quite sure most of the cases that devices use 
> 'qemu_register_reset' can be changed to 'dc->reset'.
> Seems you're busy,  If you don't mind, I can do some of the work to 
> convert 'reset' callback(not a patchset, one by one).

I guess we are all busy ;) I'm just trying to prioritize here.
This series is not very important because what we have today works, and 
I would rather not introduce regressions. What happened then is it is 
the 3rd time at least I get confuse with the qemu_register_reset() 
function while reviewing code. Then my rule of thumb is if an 
improvement takes less than 1h, then I just do it and keep going, else 
if I postpone it I'll never go back to it. When a series start to take 
too much rework it means I might started it wrongly, or I underestimated 
the time required. Time that is taken of other more important tasks.
Today I prefer merged a subset of the series that is correct, rather 
than aiming for the whole and never get it merged.

If you are interested in respining/fixing the pci-host devices I'd be 
very thankful! I appreciate your help (and reviews).

Regards,

Phil.

PD: I'll respin as v3 with your tags and the PIIX4_IDE fix.

>      >
>      >     - Removed the vmcoreinfo conversion (elmarco) but add a comment.
>      >     - Added Igor's R-b tag.
>      >
>      >     Following the thread discussion between Peter/Markus/Damien about
>      >     reset handlers:
>      > https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
>      >     I started to remove qemu_register_reset() calls from few
>     qdevified
>      >     devices (the trivial ones).
>      >
>      >     Regards,
>      >
>      >     Phil.
>      >
>      >     v1:
>     https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html
>      >
>      >     Philippe Mathieu-Daudé (8):
>      >        hw/acpi/piix4: Convert reset handler to DeviceReset
>      >        hw/isa/piix4: Convert reset handler to DeviceReset
>      >        hw/ide/piix: Convert reset handler to DeviceReset
>      >        hw/ide/sii3112: Convert reset handler to DeviceReset
>      >        hw/ide/via82c: Convert reset handler to DeviceReset
>      >        hw/isa/vt82c686: Convert reset handler to DeviceReset
>      >        hw/input/lm832x: Convert reset handler to DeviceReset
>      >        hw/misc/vmcoreinfo: Document its reset handler
>