[Qemu-devel] [PATCH v2 0/5] PCI hotplug fixes/cleanup

[Qemu-devel] [PATCH v2 0/5] PCI hotplug fixes/cleanup

[Alex Williamson]

Incorporating feedback from v1.  Re-ordered so first 2 patches are fixes,
allowing easier backport.  Instead of declaring the eject register is
write-only we define it to be a feature bits register and define the base
feature set in the docs (a feature register seems easier to maintain than
a version register).  Other patches remain the same.  Thanks,

Alex

---

Alex Williamson (5):
      acpi_piix4: Use pci_get/set_byte
      acpi_piix4: Re-define PCI hotplug eject register read
      acpi_piix4: Remove PCI_RMV_BASE write code
      acpi_piix4: Fix PCI hotplug race
      acpi_piix4: Disallow write to up/down PCI hotplug registers


 docs/specs/acpi_pci_hotplug.txt |   18 +++-
 hw/acpi_piix4.c                 |  180 +++++++++++++++++++++------------------
 2 files changed, 109 insertions(+), 89 deletions(-)

[Qemu-devel] [PATCH v2 1/5] acpi_piix4: Disallow write to up/down PCI hotplug registers

[Alex Williamson]

The write side of these registers is never used and actually can't be
used as defined because any read/modify/write sequence from the guest
potentially races with qemu.  Drop the write support and define these
as read-only registers.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 docs/specs/acpi_pci_hotplug.txt |    4 ++--
 hw/acpi_piix4.c                 |   44 ++++++++++++---------------------------
 2 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
index f0f74a7..1e2c8a2 100644
--- a/docs/specs/acpi_pci_hotplug.txt
+++ b/docs/specs/acpi_pci_hotplug.txt
@@ -15,14 +15,14 @@ PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte access):
 Slot injection notification pending. One bit per slot.
 
 Read by ACPI BIOS GPE.1 handler to notify OS of injection
-events.
+events.  Read-only.
 
 PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
 -----------------------------------------------------
 Slot removal notification pending. One bit per slot.
 
 Read by ACPI BIOS GPE.1 handler to notify OS of removal
-events.
+events.  Read-only.
 
 PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
 ----------------------------------------
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 797ed24..5e8b261 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -40,7 +40,8 @@
 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
-#define PCI_BASE 0xae00
+#define PCI_UP_BASE 0xae00
+#define PCI_DOWN_BASE 0xae04
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c
 
@@ -448,38 +449,22 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
     PIIX4_DPRINTF("gpe write %x <== %d\n", addr, val);
 }
 
-static uint32_t pcihotplug_read(void *opaque, uint32_t addr)
+static uint32_t pci_up_read(void *opaque, uint32_t addr)
 {
-    uint32_t val = 0;
-    struct pci_status *g = opaque;
-    switch (addr) {
-        case PCI_BASE:
-            val = g->up;
-            break;
-        case PCI_BASE + 4:
-            val = g->down;
-            break;
-        default:
-            break;
-    }
+    PIIX4PMState *s = opaque;
+    uint32_t val = s->pci0_status.up;
 
-    PIIX4_DPRINTF("pcihotplug read %x == %x\n", addr, val);
+    PIIX4_DPRINTF("pci_up_read %x\n", val);
     return val;
 }
 
-static void pcihotplug_write(void *opaque, uint32_t addr, uint32_t val)
+static uint32_t pci_down_read(void *opaque, uint32_t addr)
 {
-    struct pci_status *g = opaque;
-    switch (addr) {
-        case PCI_BASE:
-            g->up = val;
-            break;
-        case PCI_BASE + 4:
-            g->down = val;
-            break;
-   }
-
-    PIIX4_DPRINTF("pcihotplug write %x <== %d\n", addr, val);
+    PIIX4PMState *s = opaque;
+    uint32_t val = s->pci0_status.down;
+
+    PIIX4_DPRINTF("pci_down_read %x\n", val);
+    return val;
 }
 
 static uint32_t pciej_read(void *opaque, uint32_t addr)
@@ -523,14 +508,13 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
 
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
 {
-    struct pci_status *pci0_status = &s->pci0_status;
 
     register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
     register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
     acpi_gpe_blk(&s->ar, GPE_BASE);
 
-    register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status);
-    register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, pci0_status);
+    register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
+    register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
 
     register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
     register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);

[Qemu-devel] [PATCH v2 2/5] acpi_piix4: Fix PCI hotplug race

[Alex Williamson]

As Michael Tsirkin demonstrated, current PCI hotplug is vulnerable
to a few races.  The first is a race with other hotplug operations
because we clear the up & down registers at each event.  If a new
event comes before the last is processed, up/down is cleared and
the event is lost.

To fix this for the down register, we create a life cycle for
the event request that starts with the hot unplug request in
piix4_device_hotplug() and ends when the device is ejected.
This allows us to mask and clear individual bits, preserving them
against races.  For the up register, we have no clear end point
for when the event is finished.  We could modify the BIOS to
acknowledge the bit and clear it, but this creates BIOS compatibiliy
issues without offering a complete solution.  Instead we note that
gratuitous ACPI device checks are not harmful, which allows us to
issue a device check for every slot.  We know which slots are present
and we know which slots are hotpluggable, so we can easily reduce
this to a more manageable set for the guest.

The other race Michael noted was that an unplug request followed
by reset may also lose the eject notification, which may also
result in the eject request being lost which a subsequent add
or remove.  Once we're in reset, the device is unused and we can
flush the queue of device removals ourselves.  Previously if a
device_del was issued to a guest without ACPI PCI hotplug support,
it was necessary to shutdown the guest to recover the device.
With this, a guest reboot is sufficient.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/acpi_piix4.c |   74 +++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 5e8b261..0e7af51 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -48,7 +48,7 @@
 #define PIIX4_PCI_HOTPLUG_STATUS 2
 
 struct pci_status {
-    uint32_t up;
+    uint32_t up; /* deprecated, maintained for migration compatibility */
     uint32_t down;
 };
 
@@ -70,6 +70,7 @@ typedef struct PIIX4PMState {
     /* for pci hotplug */
     struct pci_status pci0_status;
     uint32_t pci0_hotplug_enable;
+    uint32_t pci0_slot_device_present;
 } PIIX4PMState;
 
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
@@ -205,6 +206,17 @@ static void pm_write_config(PCIDevice *d,
         pm_io_space_update((PIIX4PMState *)d);
 }
 
+static void vmstate_pci_status_pre_save(void *opaque)
+{
+    struct pci_status *pci0_status = opaque;
+    PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status);
+
+    /* We no longer track up, so build a safe value for migrating
+     * to a version that still does... of course these might get lost
+     * by an old buggy implementation, but we try. */
+    pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable;
+}
+
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;
@@ -241,6 +253,7 @@ static const VMStateDescription vmstate_pci_status = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .pre_save = vmstate_pci_status_pre_save,
     .fields      = (VMStateField []) {
         VMSTATE_UINT32(up, struct pci_status),
         VMSTATE_UINT32(down, struct pci_status),
@@ -269,13 +282,38 @@ static const VMStateDescription vmstate_acpi = {
     }
 };
 
+static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
+{
+    DeviceState *qdev, *next;
+    BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
+    int slot = ffs(slots) - 1;
+
+    /* Mark request as complete */
+    s->pci0_status.down &= ~(1U << slot);
+
+    QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+        PCIDevice *dev = PCI_DEVICE(qdev);
+        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+        if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) {
+            s->pci0_slot_device_present &= ~(1U << slot);
+            qdev_free(qdev);
+        }
+    }
+}
+
 static void piix4_update_hotplug(PIIX4PMState *s)
 {
     PCIDevice *dev = &s->dev;
     BusState *bus = qdev_get_parent_bus(&dev->qdev);
     DeviceState *qdev, *next;
 
+    /* Execute any pending removes during reset */
+    while (s->pci0_status.down) {
+        acpi_piix_eject_slot(s, s->pci0_status.down);
+    }
+
     s->pci0_hotplug_enable = ~0;
+    s->pci0_slot_device_present = 0;
 
     QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
         PCIDevice *pdev = PCI_DEVICE(qdev);
@@ -283,8 +321,10 @@ static void piix4_update_hotplug(PIIX4PMState *s)
         int slot = PCI_SLOT(pdev->devfn);
 
         if (pc->no_hotplug) {
-            s->pci0_hotplug_enable &= ~(1 << slot);
+            s->pci0_hotplug_enable &= ~(1U << slot);
         }
+
+        s->pci0_slot_device_present |= (1U << slot);
     }
 }
 
@@ -452,7 +492,11 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
 static uint32_t pci_up_read(void *opaque, uint32_t addr)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val = s->pci0_status.up;
+    uint32_t val;
+
+    /* Manufacture an "up" value to cause a device check on any hotplug
+     * slot with a device.  Extra device checks are harmless. */
+    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
 
     PIIX4_DPRINTF("pci_up_read %x\n", val);
     return val;
@@ -475,18 +519,7 @@ static uint32_t pciej_read(void *opaque, uint32_t addr)
 
 static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
 {
-    BusState *bus = opaque;
-    DeviceState *qdev, *next;
-    int slot = ffs(val) - 1;
-
-    QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
-        PCIDevice *dev = PCI_DEVICE(qdev);
-        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
-        if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) {
-            qdev_free(qdev);
-        }
-    }
-
+    acpi_piix_eject_slot(opaque, val);
 
     PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
 }
@@ -516,8 +549,8 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
     register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
     register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
 
-    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);
+    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
+    register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, s);
 
     register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s);
     register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
@@ -528,13 +561,13 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
 static void enable_device(PIIX4PMState *s, int slot)
 {
     s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
-    s->pci0_status.up |= (1 << slot);
+    s->pci0_slot_device_present |= (1U << slot);
 }
 
 static void disable_device(PIIX4PMState *s, int slot)
 {
     s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
-    s->pci0_status.down |= (1 << slot);
+    s->pci0_status.down |= (1U << slot);
 }
 
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
@@ -548,11 +581,10 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
      * it is present on boot, no hotplug event is necessary. We do send an
      * event when the device is disabled later. */
     if (state == PCI_COLDPLUG_ENABLED) {
+        s->pci0_slot_device_present |= (1U << slot);
         return 0;
     }
 
-    s->pci0_status.up = 0;
-    s->pci0_status.down = 0;
     if (state == PCI_HOTPLUG_ENABLED) {
         enable_device(s, slot);
     } else {

[Qemu-devel] [PATCH v2 3/5] acpi_piix4: Remove PCI_RMV_BASE write code

[Alex Williamson]

Clarify this register as read-only and remove write code.  No
change in existing behavior.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 docs/specs/acpi_pci_hotplug.txt |    2 +-
 hw/acpi_piix4.c                 |    6 ------
 2 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
index 1e2c8a2..1883d63 100644
--- a/docs/specs/acpi_pci_hotplug.txt
+++ b/docs/specs/acpi_pci_hotplug.txt
@@ -34,4 +34,4 @@ PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
 -----------------------------------------------
 
 Used by ACPI BIOS _RMV method to indicate removability status to OS. One
-bit per slot.
+bit per slot.  Read-only
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0e7af51..5d3b0ba 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -531,11 +531,6 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr)
     return s->pci0_hotplug_enable;
 }
 
-static void pcirmv_write(void *opaque, uint32_t addr, uint32_t val)
-{
-    return;
-}
-
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
@@ -552,7 +547,6 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
     register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
     register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, s);
 
-    register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s);
     register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
 
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);

[Qemu-devel] [PATCH v2 4/5] acpi_piix4: Re-define PCI hotplug eject register read

[Alex Williamson]

The PCI hotplug eject register has always returned 0, so let's redefine
it as a hotplug feature register.  The existing model of using separate
up & down read-only registers and an eject via write to this register
becomes the base implementation.  As we make use of new interfaces we'll
set bits here to allow the BIOS and AML implementation to optimize for
the platform implementation.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 docs/specs/acpi_pci_hotplug.txt |   12 ++++++++++--
 hw/acpi_piix4.c                 |    7 ++++---
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
index 1883d63..a839434 100644
--- a/docs/specs/acpi_pci_hotplug.txt
+++ b/docs/specs/acpi_pci_hotplug.txt
@@ -27,8 +27,16 @@ events.  Read-only.
 PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
 ----------------------------------------
 
-Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
-Reads return 0.
+Write: Used by ACPI BIOS _EJ0 method to request device removal.
+One bit per slot.
+
+Read: Hotplug features register.  Used by platform to identify features
+available.  Current base feature set (no bits set):
+ - Read-only "up" register @0xae00, 4-byte access, bit per slot
+ - Read-only "down" register @0xae04, 4-byte access, bit per slot
+ - Read/write "eject" register @0xae08, 4-byte access,
+   write: bit per slot eject, read: hotplug feature set
+ - Read-only hotplug capable register @0xae0c, 4-byte access, bit per slot
 
 PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
 -----------------------------------------------
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 5d3b0ba..11c1f85 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -511,9 +511,10 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static uint32_t pciej_read(void *opaque, uint32_t addr)
+static uint32_t pci_features_read(void *opaque, uint32_t addr)
 {
-    PIIX4_DPRINTF("pciej read %x\n", addr);
+    /* No feature defined yet */
+    PIIX4_DPRINTF("pci_features_read %x\n", 0);
     return 0;
 }
 
@@ -545,7 +546,7 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
     register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
 
     register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, s);
+    register_ioport_read(PCI_EJ_BASE, 4, 4,  pci_features_read, s);
 
     register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
 

[Qemu-devel] [PATCH v2 5/5] acpi_piix4: Use pci_get/set_byte

[Alex Williamson]

Remove stray direct access

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/acpi_piix4.c |   53 +++++++++++++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 11c1f85..30d9f12 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -167,11 +167,12 @@ static const IORangeOps pm_iorange_ops = {
 static void apm_ctrl_changed(uint32_t val, void *arg)
 {
     PIIX4PMState *s = arg;
+    PCIDevice *dev = &s->dev;
 
     /* ACPI specs 3.0, 4.7.2.5 */
     acpi_pm1_cnt_update(&s->ar, val == ACPI_ENABLE, val == ACPI_DISABLE);
 
-    if (s->dev.config[0x5b] & (1 << 1)) {
+    if (pci_get_byte(dev->config + 0x5b) & (1 << 1)) {
         if (s->smi_irq) {
             qemu_irq_raise(s->smi_irq);
         }
@@ -185,10 +186,11 @@ static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
 
 static void pm_io_space_update(PIIX4PMState *s)
 {
+    PCIDevice *dev = &s->dev;
     uint32_t pm_io_base;
 
-    if (s->dev.config[0x80] & 1) {
-        pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
+    if (pci_get_byte(dev->config + 0x80) & 1) {
+        pm_io_base = pci_get_long(dev->config + 0x40);
         pm_io_base &= 0xffc0;
 
         /* XXX: need to improve memory and ioport allocation */
@@ -331,16 +333,16 @@ static void piix4_update_hotplug(PIIX4PMState *s)
 static void piix4_reset(void *opaque)
 {
     PIIX4PMState *s = opaque;
-    uint8_t *pci_conf = s->dev.config;
+    PCIDevice *dev = &s->dev;
 
-    pci_conf[0x58] = 0;
-    pci_conf[0x59] = 0;
-    pci_conf[0x5a] = 0;
-    pci_conf[0x5b] = 0;
+    pci_set_byte(dev->config + 0x58, 0);
+    pci_set_byte(dev->config + 0x59, 0);
+    pci_set_byte(dev->config + 0x5a, 0);
+    pci_set_byte(dev->config + 0x5b, 0);
 
     if (s->kvm_enabled) {
         /* Mark SMM as already inited (until KVM supports SMM). */
-        pci_conf[0x5B] = 0x02;
+        pci_set_byte(dev->config + 0x5B, 0x02);
     }
     piix4_update_hotplug(s);
 }
@@ -356,28 +358,27 @@ static void piix4_powerdown(void *opaque, int irq, int power_failing)
 static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 {
     PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready);
-    uint8_t *pci_conf;
+    PCIDevice *dev = &s->dev;
 
-    pci_conf = s->dev.config;
-    pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10;
-    pci_conf[0x63] = 0x60;
-    pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) |
-	(isa_is_ioport_assigned(0x2f8) ? 0x90 : 0);
+    pci_set_byte(dev->config + 0x5f,
+                 (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10);
+    pci_set_byte(dev->config + 0x63, 0x60);
+    pci_set_byte(dev->config + 0x67,
+                 (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) |
+                 (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0));
 
 }
 
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
-    uint8_t *pci_conf;
 
-    pci_conf = s->dev.config;
-    pci_conf[0x06] = 0x80;
-    pci_conf[0x07] = 0x02;
-    pci_conf[0x09] = 0x00;
-    pci_conf[0x3d] = 0x01; // interrupt pin 1
+    pci_set_byte(dev->config + 0x06, 0x80);
+    pci_set_byte(dev->config + 0x07, 0x02);
+    pci_set_byte(dev->config + 0x09, 0x00);
+    pci_set_byte(dev->config + 0x3d, 0x01); /* interrupt pin 1 */
 
-    pci_conf[0x40] = 0x01; /* PM io base read only bit */
+    pci_set_byte(dev->config + 0x40, 0x01); /* PM io base read only bit */
 
     /* APM */
     apm_init(&s->apm, apm_ctrl_changed, s);
@@ -387,14 +388,14 @@ static int piix4_pm_initfn(PCIDevice *dev)
     if (s->kvm_enabled) {
         /* Mark SMM as already inited to prevent SMM from running.  KVM does not
          * support SMM mode. */
-        pci_conf[0x5B] = 0x02;
+        pci_set_byte(dev->config + 0x5B, 0x02);
     }
 
     /* XXX: which specification is used ? The i82731AB has different
        mappings */
-    pci_conf[0x90] = s->smb_io_base | 1;
-    pci_conf[0x91] = s->smb_io_base >> 8;
-    pci_conf[0xd2] = 0x09;
+    pci_set_byte(dev->config + 0x90, s->smb_io_base | 1);
+    pci_set_byte(dev->config + 0x91, s->smb_io_base >> 8);
+    pci_set_byte(dev->config + 0xd2, 0x09);
     register_ioport_write(s->smb_io_base, 64, 1, smb_ioport_writeb, &s->smb);
     register_ioport_read(s->smb_io_base, 64, 1, smb_ioport_readb, &s->smb);