[PATCH 0/7] vt82c686b clean ups - part II

[PATCH 0/7] vt82c686b clean ups - part II

[BALATON Zoltan via]

This is continuing cleaning up the vt82c686b model with the aim to add
vt8231 emulation without too much duplication, reusing or fixing the
existing model where possible. (DISCALIMER: It does not aim to fix all
existing bugs or make the model perfectly emulate the real chip.) This
series continues general clean up and finishes the power management
part. It is based on previous series and also needs Bonito fix
(currently tested with Jiaxun's proposal).

Still to go are superio and isa bridge parts that will be addressed in
some later series but this and previous series could be merged so far
independently of those future series so I'd appreciate if this could
be reviewed and merged to keep outstanding patches managable.

I can submit this and previous series together as one series if that
helps.

Regards,
BALATON Zoltan

BALATON Zoltan (7):
  vt82c686: Use shorter name for local variable holding object state
  vt82c686: Rename superio config related parts
  vt82c686: Move superio memory region to SuperIOConfig struct
  vt82c686: Reorganise code
  vt82c686: Fix SMBus IO base and configuration registers
  vt82c686: Fix up power management io base and config
  vt82c686: Make vt82c686b-pm an abstract base class and add vt8231-pm
    based on it

 hw/isa/trace-events       |   2 +
 hw/isa/vt82c686.c         | 417 ++++++++++++++++++++++----------------
 hw/mips/fuloong2e.c       |   4 +-
 include/hw/isa/vt82c686.h |   1 +
 4 files changed, 251 insertions(+), 173 deletions(-)

-- 
2.21.3

[PATCH 2/7] vt82c686: Rename superio config related parts

[BALATON Zoltan via]

Use less confusing naming for superio config register handling related
parts that makes it clearer what belongs to this part.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 48 +++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2633cfe7dc..a6f5a0843d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,7 +27,7 @@
 #include "trace.h"
 
 typedef struct SuperIOConfig {
-    uint8_t config[0x100];
+    uint8_t regs[0x100];
     uint8_t index;
     uint8_t data;
 } SuperIOConfig;
@@ -35,23 +35,23 @@ typedef struct SuperIOConfig {
 struct VT82C686BISAState {
     PCIDevice dev;
     MemoryRegion superio;
-    SuperIOConfig superio_conf;
+    SuperIOConfig superio_cfg;
 };
 
 OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
 
-static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
-                                  unsigned size)
+static void superio_cfg_write(void *opaque, hwaddr addr, uint64_t data,
+                              unsigned size)
 {
-    SuperIOConfig *superio_conf = opaque;
+    SuperIOConfig *sc = opaque;
 
     if (addr == 0x3f0) { /* config index register */
-        superio_conf->index = data & 0xff;
+        sc->index = data & 0xff;
     } else {
         bool can_write = true;
         /* 0x3f1, config data register */
-        trace_via_superio_write(superio_conf->index, data & 0xff);
-        switch (superio_conf->index) {
+        trace_via_superio_write(sc->index, data & 0xff);
+        switch (sc->index) {
         case 0x00 ... 0xdf:
         case 0xe4:
         case 0xe5:
@@ -69,23 +69,23 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
 
         }
         if (can_write) {
-            superio_conf->config[superio_conf->index] = data & 0xff;
+            sc->regs[sc->index] = data & 0xff;
         }
     }
 }
 
-static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
+static uint64_t superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
 {
-    SuperIOConfig *superio_conf = opaque;
-    uint8_t val = superio_conf->config[superio_conf->index];
+    SuperIOConfig *sc = opaque;
+    uint8_t val = sc->regs[sc->index];
 
-    trace_via_superio_read(superio_conf->index, val);
+    trace_via_superio_read(sc->index, val);
     return val;
 }
 
-static const MemoryRegionOps superio_ops = {
-    .read = superio_ioport_readb,
-    .write = superio_ioport_writeb,
+static const MemoryRegionOps superio_cfg_ops = {
+    .read = superio_cfg_read,
+    .write = superio_cfg_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .impl = {
         .min_access_size = 1,
@@ -112,12 +112,12 @@ static void vt82c686b_isa_reset(DeviceState *dev)
     pci_conf[0x5f] = 0x04;
     pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 
-    s->superio_conf.config[0xe0] = 0x3c;
-    s->superio_conf.config[0xe2] = 0x03;
-    s->superio_conf.config[0xe3] = 0xfc;
-    s->superio_conf.config[0xe6] = 0xde;
-    s->superio_conf.config[0xe7] = 0xfe;
-    s->superio_conf.config[0xe8] = 0xbe;
+    s->superio_cfg.regs[0xe0] = 0x3c; /* Device ID */
+    s->superio_cfg.regs[0xe2] = 0x03; /* Function select */
+    s->superio_cfg.regs[0xe3] = 0xfc; /* Floppy ctrl base addr */
+    s->superio_cfg.regs[0xe6] = 0xde; /* Parallel port base addr */
+    s->superio_cfg.regs[0xe7] = 0xfe; /* Serial port 1 base addr */
+    s->superio_cfg.regs[0xe8] = 0xbe; /* Serial port 2 base addr */
 }
 
 /* write config pci function0 registers. PCI-ISA bridge */
@@ -311,8 +311,8 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
         }
     }
 
-    memory_region_init_io(&s->superio, OBJECT(d), &superio_ops,
-                          &s->superio_conf, "superio", 2);
+    memory_region_init_io(&s->superio, OBJECT(d), &superio_cfg_ops,
+                          &s->superio_cfg, "superio", 2);
     memory_region_set_enabled(&s->superio, false);
     /*
      * The floppy also uses 0x3f0 and 0x3f1.
-- 
2.21.3

[PATCH 1/7] vt82c686: Use shorter name for local variable holding object state

[BALATON Zoltan via]

Rename local variable holding object state for readability and
consistency.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 02d6759c00..2633cfe7dc 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -95,8 +95,8 @@ static const MemoryRegionOps superio_ops = {
 
 static void vt82c686b_isa_reset(DeviceState *dev)
 {
-    VT82C686BISAState *vt82c = VT82C686B_ISA(dev);
-    uint8_t *pci_conf = vt82c->dev.config;
+    VT82C686BISAState *s = VT82C686B_ISA(dev);
+    uint8_t *pci_conf = s->dev.config;
 
     pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
     pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
@@ -112,24 +112,24 @@ static void vt82c686b_isa_reset(DeviceState *dev)
     pci_conf[0x5f] = 0x04;
     pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 
-    vt82c->superio_conf.config[0xe0] = 0x3c;
-    vt82c->superio_conf.config[0xe2] = 0x03;
-    vt82c->superio_conf.config[0xe3] = 0xfc;
-    vt82c->superio_conf.config[0xe6] = 0xde;
-    vt82c->superio_conf.config[0xe7] = 0xfe;
-    vt82c->superio_conf.config[0xe8] = 0xbe;
+    s->superio_conf.config[0xe0] = 0x3c;
+    s->superio_conf.config[0xe2] = 0x03;
+    s->superio_conf.config[0xe3] = 0xfc;
+    s->superio_conf.config[0xe6] = 0xde;
+    s->superio_conf.config[0xe7] = 0xfe;
+    s->superio_conf.config[0xe8] = 0xbe;
 }
 
 /* write config pci function0 registers. PCI-ISA bridge */
 static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
                                    uint32_t val, int len)
 {
-    VT82C686BISAState *vt686 = VT82C686B_ISA(d);
+    VT82C686BISAState *s = VT82C686B_ISA(d);
 
     trace_via_isa_write(addr, val, len);
     pci_default_write_config(d, addr, val, len);
     if (addr == 0x85) {  /* enable or disable super IO configure */
-        memory_region_set_enabled(&vt686->superio, val & 0x2);
+        memory_region_set_enabled(&s->superio, val & 0x2);
     }
 }
 
@@ -289,7 +289,7 @@ static const VMStateDescription vmstate_via = {
 /* init the PCI-to-ISA bridge */
 static void vt82c686b_realize(PCIDevice *d, Error **errp)
 {
-    VT82C686BISAState *vt82c = VT82C686B_ISA(d);
+    VT82C686BISAState *s = VT82C686B_ISA(d);
     uint8_t *pci_conf;
     ISABus *isa_bus;
     uint8_t *wmask;
@@ -311,15 +311,15 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
         }
     }
 
-    memory_region_init_io(&vt82c->superio, OBJECT(d), &superio_ops,
-                          &vt82c->superio_conf, "superio", 2);
-    memory_region_set_enabled(&vt82c->superio, false);
+    memory_region_init_io(&s->superio, OBJECT(d), &superio_ops,
+                          &s->superio_conf, "superio", 2);
+    memory_region_set_enabled(&s->superio, false);
     /*
      * The floppy also uses 0x3f0 and 0x3f1.
      * But we do not emulate a floppy, so just set it here.
      */
     memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
-                                &vt82c->superio);
+                                &s->superio);
 }
 
 static void via_class_init(ObjectClass *klass, void *data)
-- 
2.21.3

[PATCH 4/7] vt82c686: Reorganise code

[BALATON Zoltan via]

Move lines around so that object definitions become consecutive and
not scattered around. This brings functions belonging to an object
together so it's clearer what's defined and what parts belong to which
object.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 279 +++++++++++++++++++++++-----------------------
 1 file changed, 140 insertions(+), 139 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 30fe02f4c6..fe8961b057 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -26,112 +26,7 @@
 #include "exec/address-spaces.h"
 #include "trace.h"
 
-typedef struct SuperIOConfig {
-    uint8_t regs[0x100];
-    uint8_t index;
-    MemoryRegion io;
-} SuperIOConfig;
-
-struct VT82C686BISAState {
-    PCIDevice dev;
-    SuperIOConfig superio_cfg;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
-
-static void superio_cfg_write(void *opaque, hwaddr addr, uint64_t data,
-                              unsigned size)
-{
-    SuperIOConfig *sc = opaque;
-
-    if (addr == 0x3f0) { /* config index register */
-        sc->index = data & 0xff;
-    } else {
-        bool can_write = true;
-        /* 0x3f1, config data register */
-        trace_via_superio_write(sc->index, data & 0xff);
-        switch (sc->index) {
-        case 0x00 ... 0xdf:
-        case 0xe4:
-        case 0xe5:
-        case 0xe9 ... 0xed:
-        case 0xf3:
-        case 0xf5:
-        case 0xf7:
-        case 0xf9 ... 0xfb:
-        case 0xfd ... 0xff:
-            can_write = false;
-            break;
-        /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
-        default:
-            break;
-
-        }
-        if (can_write) {
-            sc->regs[sc->index] = data & 0xff;
-        }
-    }
-}
-
-static uint64_t superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
-{
-    SuperIOConfig *sc = opaque;
-    uint8_t val = sc->regs[sc->index];
-
-    trace_via_superio_read(sc->index, val);
-    return val;
-}
-
-static const MemoryRegionOps superio_cfg_ops = {
-    .read = superio_cfg_read,
-    .write = superio_cfg_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-};
-
-static void vt82c686b_isa_reset(DeviceState *dev)
-{
-    VT82C686BISAState *s = VT82C686B_ISA(dev);
-    uint8_t *pci_conf = s->dev.config;
-
-    pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
-    pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
-                 PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL);
-    pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
-
-    pci_conf[0x48] = 0x01; /* Miscellaneous Control 3 */
-    pci_conf[0x4a] = 0x04; /* IDE interrupt Routing */
-    pci_conf[0x4f] = 0x03; /* DMA/Master Mem Access Control 3 */
-    pci_conf[0x50] = 0x2d; /* PnP DMA Request Control */
-    pci_conf[0x59] = 0x04;
-    pci_conf[0x5a] = 0x04; /* KBC/RTC Control*/
-    pci_conf[0x5f] = 0x04;
-    pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
-
-    s->superio_cfg.regs[0xe0] = 0x3c; /* Device ID */
-    s->superio_cfg.regs[0xe2] = 0x03; /* Function select */
-    s->superio_cfg.regs[0xe3] = 0xfc; /* Floppy ctrl base addr */
-    s->superio_cfg.regs[0xe6] = 0xde; /* Parallel port base addr */
-    s->superio_cfg.regs[0xe7] = 0xfe; /* Serial port 1 base addr */
-    s->superio_cfg.regs[0xe8] = 0xbe; /* Serial port 2 base addr */
-}
-
-/* write config pci function0 registers. PCI-ISA bridge */
-static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
-                                   uint32_t val, int len)
-{
-    VT82C686BISAState *s = VT82C686B_ISA(d);
-
-    trace_via_isa_write(addr, val, len);
-    pci_default_write_config(d, addr, val, len);
-    if (addr == 0x85) {
-        /* BIT(1): enable or disable superio config io ports */
-        memory_region_set_enabled(&s->superio_cfg.io, val & BIT(1));
-    }
-}
+OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
 
 struct VT686PMState {
     PCIDevice dev;
@@ -142,30 +37,6 @@ struct VT686PMState {
     uint32_t smb_io_base;
 };
 
-OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
-
-static void pm_update_sci(VT686PMState *s)
-{
-    int sci_level, pmsts;
-
-    pmsts = acpi_pm1_evt_get_sts(&s->ar);
-    sci_level = (((pmsts & s->ar.pm1.evt.en) &
-                  (ACPI_BITMASK_RT_CLOCK_ENABLE |
-                   ACPI_BITMASK_POWER_BUTTON_ENABLE |
-                   ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
-                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
-    pci_set_irq(&s->dev, sci_level);
-    /* schedule a timer interruption if needed */
-    acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) &&
-                       !(pmsts & ACPI_BITMASK_TIMER_STATUS));
-}
-
-static void pm_tmr_timer(ACPIREGS *ar)
-{
-    VT686PMState *s = container_of(ar, VT686PMState, ar);
-    pm_update_sci(s);
-}
-
 static void pm_io_space_update(VT686PMState *s)
 {
     uint32_t pm_io_base;
@@ -179,12 +50,6 @@ static void pm_io_space_update(VT686PMState *s)
     memory_region_transaction_commit();
 }
 
-static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
-{
-    trace_via_pm_write(addr, val, len);
-    pci_default_write_config(d, addr, val, len);
-}
-
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
     VT686PMState *s = opaque;
@@ -210,7 +75,34 @@ static const VMStateDescription vmstate_acpi = {
     }
 };
 
-/* vt82c686 pm init */
+static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
+{
+    trace_via_pm_write(addr, val, len);
+    pci_default_write_config(d, addr, val, len);
+}
+
+static void pm_update_sci(VT686PMState *s)
+{
+    int sci_level, pmsts;
+
+    pmsts = acpi_pm1_evt_get_sts(&s->ar);
+    sci_level = (((pmsts & s->ar.pm1.evt.en) &
+                  (ACPI_BITMASK_RT_CLOCK_ENABLE |
+                   ACPI_BITMASK_POWER_BUTTON_ENABLE |
+                   ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
+                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
+    pci_set_irq(&s->dev, sci_level);
+    /* schedule a timer interruption if needed */
+    acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) &&
+                       !(pmsts & ACPI_BITMASK_TIMER_STATUS));
+}
+
+static void pm_tmr_timer(ACPIREGS *ar)
+{
+    VT686PMState *s = container_of(ar, VT686PMState, ar);
+    pm_update_sci(s);
+}
+
 static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
     VT686PMState *s = VT82C686B_PM(dev);
@@ -276,6 +168,87 @@ static const TypeInfo via_pm_info = {
     },
 };
 
+
+typedef struct SuperIOConfig {
+    uint8_t regs[0x100];
+    uint8_t index;
+    MemoryRegion io;
+} SuperIOConfig;
+
+static void superio_cfg_write(void *opaque, hwaddr addr, uint64_t data,
+                              unsigned size)
+{
+    SuperIOConfig *sc = opaque;
+
+    if (addr == 0x3f0) { /* config index register */
+        sc->index = data & 0xff;
+    } else {
+        bool can_write = true;
+        /* 0x3f1, config data register */
+        trace_via_superio_write(sc->index, data & 0xff);
+        switch (sc->index) {
+        case 0x00 ... 0xdf:
+        case 0xe4:
+        case 0xe5:
+        case 0xe9 ... 0xed:
+        case 0xf3:
+        case 0xf5:
+        case 0xf7:
+        case 0xf9 ... 0xfb:
+        case 0xfd ... 0xff:
+            can_write = false;
+            break;
+        /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
+        default:
+            break;
+
+        }
+        if (can_write) {
+            sc->regs[sc->index] = data & 0xff;
+        }
+    }
+}
+
+static uint64_t superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
+{
+    SuperIOConfig *sc = opaque;
+    uint8_t val = sc->regs[sc->index];
+
+    trace_via_superio_read(sc->index, val);
+    return val;
+}
+
+static const MemoryRegionOps superio_cfg_ops = {
+    .read = superio_cfg_read,
+    .write = superio_cfg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+
+OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
+
+struct VT82C686BISAState {
+    PCIDevice dev;
+    SuperIOConfig superio_cfg;
+};
+
+static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
+                                   uint32_t val, int len)
+{
+    VT82C686BISAState *s = VT82C686B_ISA(d);
+
+    trace_via_isa_write(addr, val, len);
+    pci_default_write_config(d, addr, val, len);
+    if (addr == 0x85) {
+        /* BIT(1): enable or disable superio config io ports */
+        memory_region_set_enabled(&s->superio_cfg.io, val & BIT(1));
+    }
+}
+
 static const VMStateDescription vmstate_via = {
     .name = "vt82c686b",
     .version_id = 1,
@@ -286,7 +259,33 @@ static const VMStateDescription vmstate_via = {
     }
 };
 
-/* init the PCI-to-ISA bridge */
+static void vt82c686b_isa_reset(DeviceState *dev)
+{
+    VT82C686BISAState *s = VT82C686B_ISA(dev);
+    uint8_t *pci_conf = s->dev.config;
+
+    pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
+    pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
+                 PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL);
+    pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
+
+    pci_conf[0x48] = 0x01; /* Miscellaneous Control 3 */
+    pci_conf[0x4a] = 0x04; /* IDE interrupt Routing */
+    pci_conf[0x4f] = 0x03; /* DMA/Master Mem Access Control 3 */
+    pci_conf[0x50] = 0x2d; /* PnP DMA Request Control */
+    pci_conf[0x59] = 0x04;
+    pci_conf[0x5a] = 0x04; /* KBC/RTC Control*/
+    pci_conf[0x5f] = 0x04;
+    pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
+
+    s->superio_cfg.regs[0xe0] = 0x3c; /* Device ID */
+    s->superio_cfg.regs[0xe2] = 0x03; /* Function select */
+    s->superio_cfg.regs[0xe3] = 0xfc; /* Floppy ctrl base addr */
+    s->superio_cfg.regs[0xe6] = 0xde; /* Parallel port base addr */
+    s->superio_cfg.regs[0xe7] = 0xfe; /* Serial port 1 base addr */
+    s->superio_cfg.regs[0xe8] = 0xbe; /* Serial port 2 base addr */
+}
+
 static void vt82c686b_realize(PCIDevice *d, Error **errp)
 {
     VT82C686BISAState *s = VT82C686B_ISA(d);
@@ -354,6 +353,7 @@ static const TypeInfo via_info = {
     },
 };
 
+
 static void vt82c686b_superio_class_init(ObjectClass *klass, void *data)
 {
     ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
@@ -372,11 +372,12 @@ static const TypeInfo via_superio_info = {
     .class_init    = vt82c686b_superio_class_init,
 };
 
+
 static void vt82c686b_register_types(void)
 {
     type_register_static(&via_pm_info);
-    type_register_static(&via_superio_info);
     type_register_static(&via_info);
+    type_register_static(&via_superio_info);
 }
 
 type_init(vt82c686b_register_types)
-- 
2.21.3

[PATCH 5/7] vt82c686: Fix SMBus IO base and configuration registers

[BALATON Zoltan via]

The base address of the SMBus io ports and its enabled status is set
by registers in the PCI config space but this was not correctly
emulated. Instead the SMBus registers were mapped on realize to the
base address set by a property to the address expected by fuloong2e
firmware.

Fix the base and config register handling to more closely model
hardware which allows to remove the property and allows the guest to
control this mapping. Do all this in reset instead of realize so it's
correctly updated on reset.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c   | 49 +++++++++++++++++++++++++++++++++------------
 hw/mips/fuloong2e.c |  4 +---
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index fe8961b057..9c4d153022 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -22,6 +22,7 @@
 #include "hw/i2c/pm_smbus.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/range.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
@@ -34,7 +35,6 @@ struct VT686PMState {
     ACPIREGS ar;
     APMState apm;
     PMSMBus smb;
-    uint32_t smb_io_base;
 };
 
 static void pm_io_space_update(VT686PMState *s)
@@ -50,11 +50,22 @@ static void pm_io_space_update(VT686PMState *s)
     memory_region_transaction_commit();
 }
 
+static void smb_io_space_update(VT686PMState *s)
+{
+    uint32_t smbase = pci_get_long(s->dev.config + 0x90) & 0xfff0UL;
+
+    memory_region_transaction_begin();
+    memory_region_set_address(&s->smb.io, smbase);
+    memory_region_set_enabled(&s->smb.io, s->dev.config[0xd2] & BIT(0));
+    memory_region_transaction_commit();
+}
+
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
     VT686PMState *s = opaque;
 
     pm_io_space_update(s);
+    smb_io_space_update(s);
     return 0;
 }
 
@@ -77,8 +88,18 @@ static const VMStateDescription vmstate_acpi = {
 
 static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 {
+    VT686PMState *s = VT82C686B_PM(d);
+
     trace_via_pm_write(addr, val, len);
     pci_default_write_config(d, addr, val, len);
+    if (ranges_overlap(addr, len, 0x90, 4)) {
+        uint32_t v = pci_get_long(s->dev.config + 0x90);
+        pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1);
+    }
+    if (range_covers_byte(addr, len, 0xd2)) {
+        s->dev.config[0xd2] &= 0xf;
+        smb_io_space_update(s);
+    }
 }
 
 static void pm_update_sci(VT686PMState *s)
@@ -103,6 +124,17 @@ static void pm_tmr_timer(ACPIREGS *ar)
     pm_update_sci(s);
 }
 
+static void vt82c686b_pm_reset(DeviceState *d)
+{
+    VT686PMState *s = VT82C686B_PM(d);
+
+    /* SMBus IO base */
+    pci_set_long(s->dev.config + 0x90, 1);
+    s->dev.config[0xd2] = 0;
+
+    smb_io_space_update(s);
+}
+
 static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
     VT686PMState *s = VT82C686B_PM(dev);
@@ -116,13 +148,9 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
     /* 0x48-0x4B is Power Management I/O Base */
     pci_set_long(pci_conf + 0x48, 0x00000001);
 
-    /* SMB ports:0xeee0~0xeeef */
-    s->smb_io_base = ((s->smb_io_base & 0xfff0) + 0x0);
-    pci_conf[0x90] = s->smb_io_base | 1;
-    pci_conf[0x91] = s->smb_io_base >> 8;
-    pci_conf[0xd2] = 0x90;
     pm_smbus_init(DEVICE(s), &s->smb, false);
-    memory_region_add_subregion(get_system_io(), s->smb_io_base, &s->smb.io);
+    memory_region_add_subregion(pci_address_space_io(dev), 0, &s->smb.io);
+    memory_region_set_enabled(&s->smb.io, false);
 
     apm_init(dev, &s->apm, NULL, s);
 
@@ -135,11 +163,6 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
     acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2);
 }
 
-static Property via_pm_properties[] = {
-    DEFINE_PROP_UINT32("smb_io_base", VT686PMState, smb_io_base, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void via_pm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -151,10 +174,10 @@ static void via_pm_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_VIA_ACPI;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
     k->revision = 0x40;
+    dc->reset = vt82c686b_pm_reset;
     dc->desc = "PM";
     dc->vmsd = &vmstate_acpi;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    device_class_set_props(dc, via_pm_properties);
 }
 
 static const TypeInfo via_pm_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index f393509633..bf19b4603e 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -263,9 +263,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
     pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
 
-    dev = pci_new(PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
-    qdev_prop_set_uint32(DEVICE(dev), "smb_io_base", 0xeee1);
-    pci_realize_and_unref(dev, pci_bus, &error_fatal);
+    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
 
     /* Audio support */
-- 
2.21.3

[PATCH 3/7] vt82c686: Move superio memory region to SuperIOConfig struct

[BALATON Zoltan via]

The superio memory region holds the io space index/data registers used
to access the superio config registers that are implemented in struct
SuperIOConfig. To keep these related things together move the memory
region to SuperIOConfig and rename it accordingly.
Also remove the unused data member of SuperIOConfig which is not
needed as we store actual data values in the regs array.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index a6f5a0843d..30fe02f4c6 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -29,12 +29,11 @@
 typedef struct SuperIOConfig {
     uint8_t regs[0x100];
     uint8_t index;
-    uint8_t data;
+    MemoryRegion io;
 } SuperIOConfig;
 
 struct VT82C686BISAState {
     PCIDevice dev;
-    MemoryRegion superio;
     SuperIOConfig superio_cfg;
 };
 
@@ -128,8 +127,9 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
 
     trace_via_isa_write(addr, val, len);
     pci_default_write_config(d, addr, val, len);
-    if (addr == 0x85) {  /* enable or disable super IO configure */
-        memory_region_set_enabled(&s->superio, val & 0x2);
+    if (addr == 0x85) {
+        /* BIT(1): enable or disable superio config io ports */
+        memory_region_set_enabled(&s->superio_cfg.io, val & BIT(1));
     }
 }
 
@@ -311,15 +311,15 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
         }
     }
 
-    memory_region_init_io(&s->superio, OBJECT(d), &superio_cfg_ops,
-                          &s->superio_cfg, "superio", 2);
-    memory_region_set_enabled(&s->superio, false);
+    memory_region_init_io(&s->superio_cfg.io, OBJECT(d), &superio_cfg_ops,
+                          &s->superio_cfg, "superio_cfg", 2);
+    memory_region_set_enabled(&s->superio_cfg.io, false);
     /*
      * The floppy also uses 0x3f0 and 0x3f1.
      * But we do not emulate a floppy, so just set it here.
      */
     memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
-                                &s->superio);
+                                &s->superio_cfg.io);
 }
 
 static void via_class_init(ObjectClass *klass, void *data)
-- 
2.21.3

[PATCH 7/7] vt82c686: Make vt82c686b-pm an abstract base class and add vt8231-pm based on it

[BALATON Zoltan via]

The vt82c686b-pm model can be shared between VT82C686B and VT8231. The
only difference between the two is the device id in what we model so
make an abstract via-pm model by renaming appropriately and add types
for vt82c686b-pm and vt8231-pm based on it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c         | 87 ++++++++++++++++++++++++++-------------
 include/hw/isa/vt82c686.h |  1 +
 2 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index fc2a1f4430..2e269a2c0f 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,9 +27,10 @@
 #include "exec/address-spaces.h"
 #include "trace.h"
 
-OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
+#define TYPE_VIA_PM "via-pm"
+OBJECT_DECLARE_SIMPLE_TYPE(ViaPMState, VIA_PM)
 
-struct VT686PMState {
+struct ViaPMState {
     PCIDevice dev;
     MemoryRegion io;
     ACPIREGS ar;
@@ -37,7 +38,7 @@ struct VT686PMState {
     PMSMBus smb;
 };
 
-static void pm_io_space_update(VT686PMState *s)
+static void pm_io_space_update(ViaPMState *s)
 {
     uint32_t pmbase = pci_get_long(s->dev.config + 0x48) & 0xff80UL;
 
@@ -47,7 +48,7 @@ static void pm_io_space_update(VT686PMState *s)
     memory_region_transaction_commit();
 }
 
-static void smb_io_space_update(VT686PMState *s)
+static void smb_io_space_update(ViaPMState *s)
 {
     uint32_t smbase = pci_get_long(s->dev.config + 0x90) & 0xfff0UL;
 
@@ -59,7 +60,7 @@ static void smb_io_space_update(VT686PMState *s)
 
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
-    VT686PMState *s = opaque;
+    ViaPMState *s = opaque;
 
     pm_io_space_update(s);
     smb_io_space_update(s);
@@ -72,20 +73,20 @@ static const VMStateDescription vmstate_acpi = {
     .minimum_version_id = 1,
     .post_load = vmstate_acpi_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, VT686PMState),
-        VMSTATE_UINT16(ar.pm1.evt.sts, VT686PMState),
-        VMSTATE_UINT16(ar.pm1.evt.en, VT686PMState),
-        VMSTATE_UINT16(ar.pm1.cnt.cnt, VT686PMState),
-        VMSTATE_STRUCT(apm, VT686PMState, 0, vmstate_apm, APMState),
-        VMSTATE_TIMER_PTR(ar.tmr.timer, VT686PMState),
-        VMSTATE_INT64(ar.tmr.overflow_time, VT686PMState),
+        VMSTATE_PCI_DEVICE(dev, ViaPMState),
+        VMSTATE_UINT16(ar.pm1.evt.sts, ViaPMState),
+        VMSTATE_UINT16(ar.pm1.evt.en, ViaPMState),
+        VMSTATE_UINT16(ar.pm1.cnt.cnt, ViaPMState),
+        VMSTATE_STRUCT(apm, ViaPMState, 0, vmstate_apm, APMState),
+        VMSTATE_TIMER_PTR(ar.tmr.timer, ViaPMState),
+        VMSTATE_INT64(ar.tmr.overflow_time, ViaPMState),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 {
-    VT686PMState *s = VT82C686B_PM(d);
+    ViaPMState *s = VIA_PM(d);
 
     trace_via_pm_write(addr, val, len);
     pci_default_write_config(d, addr, val, len);
@@ -127,7 +128,7 @@ static const MemoryRegionOps pm_io_ops = {
     },
 };
 
-static void pm_update_sci(VT686PMState *s)
+static void pm_update_sci(ViaPMState *s)
 {
     int sci_level, pmsts;
 
@@ -145,13 +146,13 @@ static void pm_update_sci(VT686PMState *s)
 
 static void pm_tmr_timer(ACPIREGS *ar)
 {
-    VT686PMState *s = container_of(ar, VT686PMState, ar);
+    ViaPMState *s = container_of(ar, ViaPMState, ar);
     pm_update_sci(s);
 }
 
-static void vt82c686b_pm_reset(DeviceState *d)
+static void via_pm_reset(DeviceState *d)
 {
-    VT686PMState *s = VT82C686B_PM(d);
+    ViaPMState *s = VIA_PM(d);
 
     memset(s->dev.config + PCI_CONFIG_HEADER_SIZE, 0,
            PCI_CONFIG_SPACE_SIZE - PCI_CONFIG_HEADER_SIZE);
@@ -164,9 +165,9 @@ static void vt82c686b_pm_reset(DeviceState *d)
     smb_io_space_update(s);
 }
 
-static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
+static void via_pm_realize(PCIDevice *dev, Error **errp)
 {
-    VT686PMState *s = VT82C686B_PM(dev);
+    ViaPMState *s = VIA_PM(dev);
 
     pci_set_word(dev->config + PCI_STATUS, PCI_STATUS_FAST_BACK |
                  PCI_STATUS_DEVSEL_MEDIUM);
@@ -177,8 +178,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 
     apm_init(dev, &s->apm, NULL, s);
 
-    memory_region_init_io(&s->io, OBJECT(dev), &pm_io_ops, s,
-                          "vt82c686-pm", 0x100);
+    memory_region_init_io(&s->io, OBJECT(dev), &pm_io_ops, s, "via-pm", 0x100);
     memory_region_add_subregion(pci_address_space_io(dev), 0, &s->io);
     memory_region_set_enabled(&s->io, false);
 
@@ -187,34 +187,61 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
     acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2);
 }
 
+typedef struct via_pm_init_info {
+    uint16_t device_id;
+} ViaPMInitInfo;
+
 static void via_pm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    ViaPMInitInfo *info = data;
 
-    k->realize = vt82c686b_pm_realize;
+    k->realize = via_pm_realize;
     k->config_write = pm_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
-    k->device_id = PCI_DEVICE_ID_VIA_ACPI;
+    k->device_id = info->device_id;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
     k->revision = 0x40;
-    dc->reset = vt82c686b_pm_reset;
-    dc->desc = "PM";
+    dc->reset = via_pm_reset;
+    /* Reason: part of VIA south bridge, does not exist stand alone */
+    dc->user_creatable = false;
     dc->vmsd = &vmstate_acpi;
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
 static const TypeInfo via_pm_info = {
-    .name          = TYPE_VT82C686B_PM,
+    .name          = TYPE_VIA_PM,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(VT686PMState),
-    .class_init    = via_pm_class_init,
+    .instance_size = sizeof(ViaPMState),
+    .abstract      = true,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },
     },
 };
 
+static const ViaPMInitInfo vt82c686b_init_info = {
+    .device_id = PCI_DEVICE_ID_VIA_ACPI,
+};
+
+static const TypeInfo vt82c686b_pm_info = {
+    .name          = TYPE_VT82C686B_PM,
+    .parent        = TYPE_VIA_PM,
+    .class_init    = via_pm_class_init,
+    .class_data    = (void *)&vt82c686b_init_info,
+};
+
+static const ViaPMInitInfo vt8231_init_info = {
+    .device_id = 0x8235,
+};
+
+static const TypeInfo vt8231_pm_info = {
+    .name          = TYPE_VT8231_PM,
+    .parent        = TYPE_VIA_PM,
+    .class_init    = via_pm_class_init,
+    .class_data    = (void *)&vt8231_init_info,
+};
+
 
 typedef struct SuperIOConfig {
     uint8_t regs[0x100];
@@ -423,6 +450,8 @@ static const TypeInfo via_superio_info = {
 static void vt82c686b_register_types(void)
 {
     type_register_static(&via_pm_info);
+    type_register_static(&vt82c686b_pm_info);
+    type_register_static(&vt8231_pm_info);
     type_register_static(&via_info);
     type_register_static(&via_superio_info);
 }
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 5b0a1ffe72..9b6d610e83 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -4,6 +4,7 @@
 #define TYPE_VT82C686B_ISA "vt82c686b-isa"
 #define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
 #define TYPE_VT82C686B_PM "vt82c686b-pm"
+#define TYPE_VT8231_PM "vt8231-pm"
 #define TYPE_VIA_AC97 "via-ac97"
 #define TYPE_VIA_MC97 "via-mc97"
 
-- 
2.21.3

[PATCH 6/7] vt82c686: Fix up power management io base and config

[BALATON Zoltan via]

Similar to the SMBus io registers there is a power management io range
that's set via similar base address reg and enable bit. Some handling
of this was already there but with several problems: using the wrong
registers and bits, wrong size range, not acually updating mapping and
handling reset correctly or emulating any of the acrual io registers.
Some of these are fixed up here.

After this patch we use the correct base address register, enable bit
and region size and allow guests to map/unmap this region and
correctly reset all registers to default values on reset but we still
don't emulate any of the registers in this range.

Previously just an empty RAM region was mapped on realize, now I've
added empty io range logging access. I think the pm timer should be
hooked up here but not sure guests need it. PMON on fuloong2e sets
base address but does not seem to enable region; the pegasos2 firmware
pokes some regs but continues anyway so don't know if anything would
make use of these facilities. Therefore this is just a clean up of
previous state for now and not intending to fully implement missing
functionality which could be done later if some guests need it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/trace-events |  2 ++
 hw/isa/vt82c686.c   | 56 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/hw/isa/trace-events b/hw/isa/trace-events
index d267d3e652..641d69eedf 100644
--- a/hw/isa/trace-events
+++ b/hw/isa/trace-events
@@ -17,5 +17,7 @@ apm_io_write(uint8_t addr, uint8_t val) "write addr=0x%x val=0x%02x"
 # vt82c686.c
 via_isa_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
 via_pm_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
+via_pm_io_read(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
+via_pm_io_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
 via_superio_read(uint8_t addr, uint8_t val) "addr 0x%x val 0x%x"
 via_superio_write(uint8_t addr, uint32_t val) "addr 0x%x val 0x%x"
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9c4d153022..fc2a1f4430 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -39,14 +39,11 @@ struct VT686PMState {
 
 static void pm_io_space_update(VT686PMState *s)
 {
-    uint32_t pm_io_base;
-
-    pm_io_base = pci_get_long(s->dev.config + 0x40);
-    pm_io_base &= 0xffc0;
+    uint32_t pmbase = pci_get_long(s->dev.config + 0x48) & 0xff80UL;
 
     memory_region_transaction_begin();
-    memory_region_set_enabled(&s->io, s->dev.config[0x80] & 1);
-    memory_region_set_address(&s->io, pm_io_base);
+    memory_region_set_address(&s->io, pmbase);
+    memory_region_set_enabled(&s->io, s->dev.config[0x41] & BIT(7));
     memory_region_transaction_commit();
 }
 
@@ -92,6 +89,13 @@ static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 
     trace_via_pm_write(addr, val, len);
     pci_default_write_config(d, addr, val, len);
+    if (ranges_overlap(addr, len, 0x48, 4)) {
+        uint32_t v = pci_get_long(s->dev.config + 0x48);
+        pci_set_long(s->dev.config + 0x48, (v & 0xff80UL) | 1);
+    }
+    if (range_covers_byte(addr, len, 0x41)) {
+        pm_io_space_update(s);
+    }
     if (ranges_overlap(addr, len, 0x90, 4)) {
         uint32_t v = pci_get_long(s->dev.config + 0x90);
         pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1);
@@ -102,6 +106,27 @@ static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
     }
 }
 
+static void pm_io_write(void *op, hwaddr addr, uint64_t data, unsigned size)
+{
+    trace_via_pm_io_write(addr, data, size);
+}
+
+static uint64_t pm_io_read(void *op, hwaddr addr, unsigned size)
+{
+    trace_via_pm_io_read(addr, 0, size);
+    return 0;
+}
+
+static const MemoryRegionOps pm_io_ops = {
+    .read = pm_io_read,
+    .write = pm_io_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 static void pm_update_sci(VT686PMState *s)
 {
     int sci_level, pmsts;
@@ -128,35 +153,34 @@ static void vt82c686b_pm_reset(DeviceState *d)
 {
     VT686PMState *s = VT82C686B_PM(d);
 
+    memset(s->dev.config + PCI_CONFIG_HEADER_SIZE, 0,
+           PCI_CONFIG_SPACE_SIZE - PCI_CONFIG_HEADER_SIZE);
+    /* Power Management IO base */
+    pci_set_long(s->dev.config + 0x48, 1);
     /* SMBus IO base */
     pci_set_long(s->dev.config + 0x90, 1);
-    s->dev.config[0xd2] = 0;
 
+    pm_io_space_update(s);
     smb_io_space_update(s);
 }
 
 static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
     VT686PMState *s = VT82C686B_PM(dev);
-    uint8_t *pci_conf;
 
-    pci_conf = s->dev.config;
-    pci_set_word(pci_conf + PCI_COMMAND, 0);
-    pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_FAST_BACK |
+    pci_set_word(dev->config + PCI_STATUS, PCI_STATUS_FAST_BACK |
                  PCI_STATUS_DEVSEL_MEDIUM);
 
-    /* 0x48-0x4B is Power Management I/O Base */
-    pci_set_long(pci_conf + 0x48, 0x00000001);
-
     pm_smbus_init(DEVICE(s), &s->smb, false);
     memory_region_add_subregion(pci_address_space_io(dev), 0, &s->smb.io);
     memory_region_set_enabled(&s->smb.io, false);
 
     apm_init(dev, &s->apm, NULL, s);
 
-    memory_region_init(&s->io, OBJECT(dev), "vt82c686-pm", 64);
+    memory_region_init_io(&s->io, OBJECT(dev), &pm_io_ops, s,
+                          "vt82c686-pm", 0x100);
+    memory_region_add_subregion(pci_address_space_io(dev), 0, &s->io);
     memory_region_set_enabled(&s->io, false);
-    memory_region_add_subregion(get_system_io(), 0, &s->io);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer, &s->io);
     acpi_pm1_evt_init(&s->ar, pm_tmr_timer, &s->io);
-- 
2.21.3