[Qemu-devel] [PATCH 0/5] piix_pci cleanup

[Qemu-devel] [PATCH 0/5] piix_pci cleanup

[Juan Quintela]

Hi

This series:
- split piix4 from piix_pci.  The only shared code where piix_save/load, i.e.
  almost nothing.  Once there, compile piix4.o only for mips (it was not used
  anywhere else).
- Move global variables to PCII440FXState.  Nice and clean until....
- pci_irq_levels: This is great:
  * it is saved/loaded from i440fx
  * it is cleaned during reset in piix3
  * it is used in piix3_set_irq, that don't receive neither i440fx nor
    piix state, it needs to be a global variable. (created a global link
    until a better solution appear).

I looked where to "hide" pci_irq_levels and piix3_dev (both needed in
piix3_set_irq), and didn't found where to pass them, out of:
- hack i8259 to hide it into PicState2 (that one got passed through the pic)
  ugly for words, but will work
- try to add <something> at setup_irq time, but at that point we have:
  * opaque -> pci_dev of device that we are working with
  * from there we can get to the bus that the device is attached to,
    but no way to go from there to the Host bridge (that is what we wanted
    in the 1st place)

Notice that this is needed for both pc and mips_malta.c

Ideas on where to hide a pci_dev inside a bus?

Later, Juan.

Juan Quintela (5):
  piix4 don't use pci_irq_levels at all
  Split piix4 support from piix_pci.c
  Use PCII440FXState instead of generic PCIDevice
  Move smm_enabled and isa_memory_mappings to PCII440FXState
  Cleanup of pci_irq_levels belong to i440fx

 Makefile.target |    2 +-
 hw/pc.c         |    2 +-
 hw/pc.h         |   10 +++-
 hw/piix4.c      |  127 +++++++++++++++++++++++++++++++++++++++++++
 hw/piix_pci.c   |  161 +++++++++++++++++-------------------------------------
 5 files changed, 187 insertions(+), 115 deletions(-)
 create mode 100644 hw/piix4.c

[Qemu-devel] [PATCH 1/5] piix4 don't use pci_irq_levels at all

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/piix_pci.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index e2ddf4b..7cf1d99 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -321,8 +321,6 @@ static void piix4_reset(void *opaque)
     pci_conf[0xab] = 0x00;
     pci_conf[0xac] = 0x00;
     pci_conf[0xae] = 0x00;
-
-    memset(pci_irq_levels, 0, sizeof(pci_irq_levels));
 }

 static void piix_save(QEMUFile* f, void *opaque)
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/5] Split piix4 support from piix_pci.c

[Juan Quintela]

Now mips_malta uses piix4 and pc's use piix_pci definitions

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 Makefile.target |    2 +-
 hw/pc.h         |    1 +
 hw/piix4.c      |  127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/piix_pci.c   |   71 -------------------------------
 4 files changed, 129 insertions(+), 72 deletions(-)
 create mode 100644 hw/piix4.c

diff --git a/Makefile.target b/Makefile.target
index 066af8d..4a5d917 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -217,7 +217,7 @@ obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
 obj-mips-y += mips_timer.o mips_int.o dma.o vga.o serial.o i8254.o i8259.o rc4030.o
 obj-mips-y += g364fb.o jazz_led.o dp8393x.o
 obj-mips-y += ide.o gt64xxx.o pckbd.o fdc.o mc146818rtc.o usb-uhci.o acpi.o ds1225y.o
-obj-mips-y += piix_pci.o parallel.o cirrus_vga.o isa-bus.o pcspk.o $(sound-obj-y)
+obj-mips-y += piix4.o parallel.o cirrus_vga.o isa-bus.o pcspk.o $(sound-obj-y)
 obj-mips-y += mipsnet.o
 obj-mips-y += pflash_cfi01.o
 obj-mips-y += vmware_vga.o
diff --git a/hw/pc.h b/hw/pc.h
index 58d569b..bfa52d6 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -124,6 +124,7 @@ void i440fx_set_smm(PCIDevice *d, int val);
 int piix3_init(PCIBus *bus, int devfn);
 void i440fx_init_memory_mappings(PCIDevice *d);

+/* piix4.c */
 extern PCIDevice *piix4_dev;
 int piix4_init(PCIBus *bus, int devfn);

diff --git a/hw/piix4.c b/hw/piix4.c
new file mode 100644
index 0000000..c489f13
--- /dev/null
+++ b/hw/piix4.c
@@ -0,0 +1,127 @@
+/*
+ * QEMU PIIX4 PCI Bridge Emulation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw.h"
+#include "pc.h"
+#include "pci.h"
+#include "isa.h"
+#include "sysbus.h"
+
+PCIDevice *piix4_dev;
+
+static void piix4_reset(void *opaque)
+{
+    PCIDevice *d = opaque;
+    uint8_t *pci_conf = d->config;
+
+    pci_conf[0x04] = 0x07; // master, memory and I/O
+    pci_conf[0x05] = 0x00;
+    pci_conf[0x06] = 0x00;
+    pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
+    pci_conf[0x4c] = 0x4d;
+    pci_conf[0x4e] = 0x03;
+    pci_conf[0x4f] = 0x00;
+    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
+    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
+    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
+    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
+    pci_conf[0x69] = 0x02;
+    pci_conf[0x70] = 0x80;
+    pci_conf[0x76] = 0x0c;
+    pci_conf[0x77] = 0x0c;
+    pci_conf[0x78] = 0x02;
+    pci_conf[0x79] = 0x00;
+    pci_conf[0x80] = 0x00;
+    pci_conf[0x82] = 0x00;
+    pci_conf[0xa0] = 0x08;
+    pci_conf[0xa2] = 0x00;
+    pci_conf[0xa3] = 0x00;
+    pci_conf[0xa4] = 0x00;
+    pci_conf[0xa5] = 0x00;
+    pci_conf[0xa6] = 0x00;
+    pci_conf[0xa7] = 0x00;
+    pci_conf[0xa8] = 0x0f;
+    pci_conf[0xaa] = 0x00;
+    pci_conf[0xab] = 0x00;
+    pci_conf[0xac] = 0x00;
+    pci_conf[0xae] = 0x00;
+}
+
+static void piix_save(QEMUFile* f, void *opaque)
+{
+    PCIDevice *d = opaque;
+    pci_device_save(d, f);
+}
+
+static int piix_load(QEMUFile* f, void *opaque, int version_id)
+{
+    PCIDevice *d = opaque;
+    if (version_id != 2)
+        return -EINVAL;
+    return pci_device_load(d, f);
+}
+
+static void piix4_initfn(PCIDevice *d)
+{
+    uint8_t *pci_conf;
+
+    register_savevm("PIIX4", 0, 2, piix_save, piix_load, d);
+
+    pci_conf = d->config;
+    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
+    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_0); // 82371AB/EB/MB PIIX4 PCI-to-ISA bridge
+    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
+    pci_conf[PCI_HEADER_TYPE] =
+        PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
+
+    piix4_dev = d;
+    piix4_reset(d);
+    qemu_register_reset(piix4_reset, d);
+}
+
+int piix4_init(PCIBus *bus, int devfn)
+{
+    PCIDevice *d;
+
+    d = pci_create_simple(bus, devfn, "PIIX4");
+    return d->devfn;
+}
+
+static PCIDeviceInfo piix4_info[] = {
+    {
+        .qdev.name    = "PIIX4",
+        .qdev.desc    = "ISA bridge",
+        .qdev.size    = sizeof(PCIDevice),
+        .qdev.no_user = 1,
+        .init         = piix4_initfn,
+    },{
+        /* end of list */
+    }
+};
+
+static void piix4_register(void)
+{
+    pci_qdev_register_many(piix4_info);
+}
+device_init(piix4_register);
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7cf1d99..86db2fc 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -222,7 +222,6 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
 /* PIIX3 PCI to ISA bridge */

 static PCIDevice *piix3_dev;
-PCIDevice *piix4_dev;

 static void piix3_set_irq(qemu_irq *pic, int irq_num, int level)
 {
@@ -285,44 +284,6 @@ static void piix3_reset(void *opaque)
     memset(pci_irq_levels, 0, sizeof(pci_irq_levels));
 }

-static void piix4_reset(void *opaque)
-{
-    PCIDevice *d = opaque;
-    uint8_t *pci_conf = d->config;
-
-    pci_conf[0x04] = 0x07; // master, memory and I/O
-    pci_conf[0x05] = 0x00;
-    pci_conf[0x06] = 0x00;
-    pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
-    pci_conf[0x4c] = 0x4d;
-    pci_conf[0x4e] = 0x03;
-    pci_conf[0x4f] = 0x00;
-    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
-    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
-    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
-    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
-    pci_conf[0x69] = 0x02;
-    pci_conf[0x70] = 0x80;
-    pci_conf[0x76] = 0x0c;
-    pci_conf[0x77] = 0x0c;
-    pci_conf[0x78] = 0x02;
-    pci_conf[0x79] = 0x00;
-    pci_conf[0x80] = 0x00;
-    pci_conf[0x82] = 0x00;
-    pci_conf[0xa0] = 0x08;
-    pci_conf[0xa2] = 0x00;
-    pci_conf[0xa3] = 0x00;
-    pci_conf[0xa4] = 0x00;
-    pci_conf[0xa5] = 0x00;
-    pci_conf[0xa6] = 0x00;
-    pci_conf[0xa7] = 0x00;
-    pci_conf[0xa8] = 0x0f;
-    pci_conf[0xaa] = 0x00;
-    pci_conf[0xab] = 0x00;
-    pci_conf[0xac] = 0x00;
-    pci_conf[0xae] = 0x00;
-}
-
 static void piix_save(QEMUFile* f, void *opaque)
 {
     PCIDevice *d = opaque;
@@ -356,24 +317,6 @@ static void piix3_initfn(PCIDevice *d)
     qemu_register_reset(piix3_reset, d);
 }

-static void piix4_initfn(PCIDevice *d)
-{
-    uint8_t *pci_conf;
-
-    register_savevm("PIIX4", 0, 2, piix_save, piix_load, d);
-
-    pci_conf = d->config;
-    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_0); // 82371AB/EB/MB PIIX4 PCI-to-ISA bridge
-    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
-    pci_conf[PCI_HEADER_TYPE] =
-        PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
-
-    piix4_dev = d;
-    piix4_reset(d);
-    qemu_register_reset(piix4_reset, d);
-}
-
 int piix3_init(PCIBus *bus, int devfn)
 {
     PCIDevice *d;
@@ -382,14 +325,6 @@ int piix3_init(PCIBus *bus, int devfn)
     return d->devfn;
 }

-int piix4_init(PCIBus *bus, int devfn)
-{
-    PCIDevice *d;
-
-    d = pci_create_simple(bus, devfn, "PIIX4");
-    return d->devfn;
-}
-
 static PCIDeviceInfo i440fx_info[] = {
     {
         .qdev.name    = "i440FX",
@@ -405,12 +340,6 @@ static PCIDeviceInfo i440fx_info[] = {
         .qdev.no_user = 1,
         .init         = piix3_initfn,
     },{
-        .qdev.name    = "PIIX4",
-        .qdev.desc    = "ISA bridge",
-        .qdev.size    = sizeof(PCIDevice),
-        .qdev.no_user = 1,
-        .init         = piix4_initfn,
-    },{
         /* end of list */
     }
 };
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/5] Use PCII440FXState instead of generic PCIDevice

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/pc.c       |    2 +-
 hw/pc.h       |    9 ++++++---
 hw/piix_pci.c |   54 +++++++++++++++++++++++++++++++-----------------------
 3 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index cc6e7e8..a908f8b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -61,7 +61,7 @@ static fdctrl_t *floppy_controller;
 static RTCState *rtc_state;
 static PITState *pit;
 static IOAPICState *ioapic;
-static PCIDevice *i440fx_state;
+static PCII440FXState *i440fx_state;

 typedef struct rom_reset_data {
     uint8_t *data;
diff --git a/hw/pc.h b/hw/pc.h
index bfa52d6..7733d1a 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -119,10 +119,13 @@ void pcspk_init(PITState *);
 int pcspk_audio_init(qemu_irq *pic);

 /* piix_pci.c */
-PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic);
-void i440fx_set_smm(PCIDevice *d, int val);
+struct PCII440FXState;
+typedef struct PCII440FXState PCII440FXState;
+
+PCIBus *i440fx_init(PCII440FXState **pi440fx_state, qemu_irq *pic);
+void i440fx_set_smm(PCII440FXState *d, int val);
 int piix3_init(PCIBus *bus, int devfn);
-void i440fx_init_memory_mappings(PCIDevice *d);
+void i440fx_init_memory_mappings(PCII440FXState *d);

 /* piix4.c */
 extern PCIDevice *piix4_dev;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 86db2fc..2450c31 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -33,6 +33,10 @@ typedef uint32_t pci_addr_t;

 typedef PCIHostState I440FXState;

+struct PCII440FXState {
+    PCIDevice dev;
+};
+
 static void i440fx_addr_writel(void* opaque, uint32_t addr, uint32_t val)
 {
     I440FXState *s = opaque;
@@ -61,7 +65,7 @@ static target_phys_addr_t isa_page_descs[384 / 4];
 static uint8_t smm_enabled;
 static int pci_irq_levels[4];

-static void update_pam(PCIDevice *d, uint32_t start, uint32_t end, int r)
+static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
 {
     uint32_t addr;

@@ -88,17 +92,17 @@ static void update_pam(PCIDevice *d, uint32_t start, uint32_t end, int r)
     }
 }

-static void i440fx_update_memory_mappings(PCIDevice *d)
+static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i, r;
     uint32_t smram, addr;

-    update_pam(d, 0xf0000, 0x100000, (d->config[0x59] >> 4) & 3);
+    update_pam(d, 0xf0000, 0x100000, (d->dev.config[0x59] >> 4) & 3);
     for(i = 0; i < 12; i++) {
-        r = (d->config[(i >> 1) + 0x5a] >> ((i & 1) * 4)) & 3;
+        r = (d->dev.config[(i >> 1) + 0x5a] >> ((i & 1) * 4)) & 3;
         update_pam(d, 0xc0000 + 0x4000 * i, 0xc0000 + 0x4000 * (i + 1), r);
     }
-    smram = d->config[0x72];
+    smram = d->dev.config[0x72];
     if ((smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
         cpu_register_physical_memory(0xa0000, 0x20000, 0xa0000);
     } else {
@@ -109,7 +113,7 @@ static void i440fx_update_memory_mappings(PCIDevice *d)
     }
 }

-void i440fx_set_smm(PCIDevice *d, int val)
+void i440fx_set_smm(PCII440FXState *d, int val)
 {
     val = (val != 0);
     if (smm_enabled != val) {
@@ -122,7 +126,7 @@ void i440fx_set_smm(PCIDevice *d, int val)
 /* XXX: suppress when better memory API. We make the assumption that
    no device (in particular the VGA) changes the memory mappings in
    the 0xa0000-0x100000 range */
-void i440fx_init_memory_mappings(PCIDevice *d)
+void i440fx_init_memory_mappings(PCII440FXState *d)
 {
     int i;
     for(i = 0; i < 96; i++) {
@@ -130,21 +134,23 @@ void i440fx_init_memory_mappings(PCIDevice *d)
     }
 }

-static void i440fx_write_config(PCIDevice *d,
+static void i440fx_write_config(PCIDevice *dev,
                                 uint32_t address, uint32_t val, int len)
 {
+    PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
+
     /* XXX: implement SMRAM.D_LOCK */
-    pci_default_write_config(d, address, val, len);
+    pci_default_write_config(dev, address, val, len);
     if ((address >= 0x59 && address <= 0x5f) || address == 0x72)
         i440fx_update_memory_mappings(d);
 }

 static void i440fx_save(QEMUFile* f, void *opaque)
 {
-    PCIDevice *d = opaque;
+    PCII440FXState *d = opaque;
     int i;

-    pci_device_save(d, f);
+    pci_device_save(&d->dev, f);
     qemu_put_8s(f, &smm_enabled);

     for (i = 0; i < 4; i++)
@@ -153,12 +159,12 @@ static void i440fx_save(QEMUFile* f, void *opaque)

 static int i440fx_load(QEMUFile* f, void *opaque, int version_id)
 {
-    PCIDevice *d = opaque;
+    PCII440FXState *d = opaque;
     int ret, i;

     if (version_id > 2)
         return -EINVAL;
-    ret = pci_device_load(d, f);
+    ret = pci_device_load(&d->dev, f);
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
@@ -186,20 +192,22 @@ static void i440fx_pcihost_initfn(SysBusDevice *dev)
     register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
 }

-static void i440fx_initfn(PCIDevice *d)
+static void i440fx_initfn(PCIDevice *dev)
 {
-    pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
-    d->config[0x08] = 0x02; // revision
-    pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
-    d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
+    PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);

-    d->config[0x72] = 0x02; /* SMRAM */
+    pci_config_set_vendor_id(d->dev.config, PCI_VENDOR_ID_INTEL);
+    pci_config_set_device_id(d->dev.config, PCI_DEVICE_ID_INTEL_82441);
+    d->dev.config[0x08] = 0x02; // revision
+    pci_config_set_class(d->dev.config, PCI_CLASS_BRIDGE_HOST);
+    d->dev.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
+
+    d->dev.config[0x72] = 0x02; /* SMRAM */

     register_savevm("I440FX", 0, 2, i440fx_save, i440fx_load, d);
 }

-PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
+PCIBus *i440fx_init(PCII440FXState **pi440fx_state, qemu_irq *pic)
 {
     DeviceState *dev;
     PCIBus *b;
@@ -214,7 +222,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     qdev_init(dev);

     d = pci_create_simple(b, 0, "i440FX");
-    *pi440fx_state = d;
+    *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);

     return b;
 }
@@ -329,7 +337,7 @@ static PCIDeviceInfo i440fx_info[] = {
     {
         .qdev.name    = "i440FX",
         .qdev.desc    = "Host bridge",
-        .qdev.size    = sizeof(PCIDevice),
+        .qdev.size    = sizeof(PCII440FXState),
         .qdev.no_user = 1,
         .init         = i440fx_initfn,
         .config_write = i440fx_write_config,
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/5] Move smm_enabled and isa_memory_mappings to PCII440FXState

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/piix_pci.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 2450c31..d8aeeec 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -35,6 +35,8 @@ typedef PCIHostState I440FXState;

 struct PCII440FXState {
     PCIDevice dev;
+    target_phys_addr_t isa_page_descs[384 / 4];
+    uint8_t smm_enabled;
 };

 static void i440fx_addr_writel(void* opaque, uint32_t addr, uint32_t val)
@@ -61,8 +63,6 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return (irq_num + slot_addend) & 3;
 }

-static target_phys_addr_t isa_page_descs[384 / 4];
-static uint8_t smm_enabled;
 static int pci_irq_levels[4];

 static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
@@ -86,7 +86,7 @@ static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
         /* XXX: should distinguish read/write cases */
         for(addr = start; addr < end; addr += 4096) {
             cpu_register_physical_memory(addr, 4096,
-                                         isa_page_descs[(addr - 0xa0000) >> 12]);
+                                         d->isa_page_descs[(addr - 0xa0000) >> 12]);
         }
         break;
     }
@@ -103,12 +103,12 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
         update_pam(d, 0xc0000 + 0x4000 * i, 0xc0000 + 0x4000 * (i + 1), r);
     }
     smram = d->dev.config[0x72];
-    if ((smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
+    if ((d->smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
         cpu_register_physical_memory(0xa0000, 0x20000, 0xa0000);
     } else {
         for(addr = 0xa0000; addr < 0xc0000; addr += 4096) {
             cpu_register_physical_memory(addr, 4096,
-                                         isa_page_descs[(addr - 0xa0000) >> 12]);
+                                         d->isa_page_descs[(addr - 0xa0000) >> 12]);
         }
     }
 }
@@ -116,8 +116,8 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
 void i440fx_set_smm(PCII440FXState *d, int val)
 {
     val = (val != 0);
-    if (smm_enabled != val) {
-        smm_enabled = val;
+    if (d->smm_enabled != val) {
+        d->smm_enabled = val;
         i440fx_update_memory_mappings(d);
     }
 }
@@ -130,7 +130,7 @@ void i440fx_init_memory_mappings(PCII440FXState *d)
 {
     int i;
     for(i = 0; i < 96; i++) {
-        isa_page_descs[i] = cpu_get_physical_page_desc(0xa0000 + i * 0x1000);
+        d->isa_page_descs[i] = cpu_get_physical_page_desc(0xa0000 + i * 0x1000);
     }
 }

@@ -151,7 +151,7 @@ static void i440fx_save(QEMUFile* f, void *opaque)
     int i;

     pci_device_save(&d->dev, f);
-    qemu_put_8s(f, &smm_enabled);
+    qemu_put_8s(f, &d->smm_enabled);

     for (i = 0; i < 4; i++)
         qemu_put_be32(f, pci_irq_levels[i]);
@@ -168,7 +168,7 @@ static int i440fx_load(QEMUFile* f, void *opaque, int version_id)
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
-    qemu_get_8s(f, &smm_enabled);
+    qemu_get_8s(f, &d->smm_enabled);

     if (version_id >= 2)
         for (i = 0; i < 4; i++)
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/5] Cleanup of pci_irq_levels belong to i440fx

[Juan Quintela]

Do there in a new reset function.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/piix_pci.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index d8aeeec..bd6ab9b 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -63,8 +63,6 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return (irq_num + slot_addend) & 3;
 }

-static int pci_irq_levels[4];
-
 static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
 {
     uint32_t addr;
@@ -177,6 +175,13 @@ static int i440fx_load(QEMUFile* f, void *opaque, int version_id)
     return 0;
 }

+static void i440fx_reset(void *opaque)
+{
+    PCII440FXState *d = opaque;
+
+    memset(d->pci_irq_levels, 0, sizeof(d->pci_irq_levels));
+}
+
 static void i440fx_pcihost_initfn(SysBusDevice *dev)
 {
     I440FXState *s = FROM_SYSBUS(I440FXState, dev);
@@ -205,6 +210,9 @@ static void i440fx_initfn(PCIDevice *dev)
     d->dev.config[0x72] = 0x02; /* SMRAM */

     register_savevm("I440FX", 0, 2, i440fx_save, i440fx_load, d);
+
+    i440fx_reset(d);
+    qemu_register_reset(i440fx_reset, d);
 }

 PCIBus *i440fx_init(PCII440FXState **pi440fx_state, qemu_irq *pic)
@@ -288,8 +296,6 @@ static void piix3_reset(void *opaque)
     pci_conf[0xab] = 0x00;
     pci_conf[0xac] = 0x00;
     pci_conf[0xae] = 0x00;
-
-    memset(pci_irq_levels, 0, sizeof(pci_irq_levels));
 }

 static void piix_save(QEMUFile* f, void *opaque)
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 0/5] piix_pci cleanup

[Blue Swirl]

On Fri, Aug 21, 2009 at 8:36 PM, Juan Quintela<quintela@redhat.com> wrote:
> Hi
>
> This series:
> - split piix4 from piix_pci.  The only shared code where piix_save/load, i.e.
>  almost nothing.  Once there, compile piix4.o only for mips (it was not used
>  anywhere else).
> - Move global variables to PCII440FXState.  Nice and clean until....
> - pci_irq_levels: This is great:
>  * it is saved/loaded from i440fx
>  * it is cleaned during reset in piix3
>  * it is used in piix3_set_irq, that don't receive neither i440fx nor
>    piix state, it needs to be a global variable. (created a global link
>    until a better solution appear).
>
> I looked where to "hide" pci_irq_levels and piix3_dev (both needed in
> piix3_set_irq), and didn't found where to pass them, out of:
> - hack i8259 to hide it into PicState2 (that one got passed through the pic)
>  ugly for words, but will work
> - try to add <something> at setup_irq time, but at that point we have:
>  * opaque -> pci_dev of device that we are working with
>  * from there we can get to the bus that the device is attached to,
>    but no way to go from there to the Host bridge (that is what we wanted
>    in the 1st place)
>
> Notice that this is needed for both pc and mips_malta.c
>
> Ideas on where to hide a pci_dev inside a bus?

pci_set_irq_fn should take an opaque state pointer (given at
pci_register_bus time):

typedef void (*pci_set_irq_fn)(void *opaque, qemu_irq *pic, int
irq_num, int level);

PCIBus *pci_register_bus(DeviceState *parent, const char *name,
                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                         qemu_irq *pic, void *irq_opaque, int
devfn_min, int nirq);

[Qemu-devel] Re: [PATCH 0/5] piix_pci cleanup

[Juan Quintela]

Blue Swirl <blauwirbel@gmail.com> wrote:
> On Fri, Aug 21, 2009 at 8:36 PM, Juan Quintela<quintela@redhat.com> wrote:
>> Hi
>>
>> This series:
>> - split piix4 from piix_pci.  The only shared code where piix_save/load, i.e.
>>  almost nothing.  Once there, compile piix4.o only for mips (it was not used
>>  anywhere else).
>> - Move global variables to PCII440FXState.  Nice and clean until....
>> - pci_irq_levels: This is great:
>>  * it is saved/loaded from i440fx
>>  * it is cleaned during reset in piix3
>>  * it is used in piix3_set_irq, that don't receive neither i440fx nor
>>    piix state, it needs to be a global variable. (created a global link
>>    until a better solution appear).
>>
>> I looked where to "hide" pci_irq_levels and piix3_dev (both needed in
>> piix3_set_irq), and didn't found where to pass them, out of:
>> - hack i8259 to hide it into PicState2 (that one got passed through the pic)
>>  ugly for words, but will work
>> - try to add <something> at setup_irq time, but at that point we have:
>>  * opaque -> pci_dev of device that we are working with
>>  * from there we can get to the bus that the device is attached to,
>>    but no way to go from there to the Host bridge (that is what we wanted
>>    in the 1st place)
>>
>> Notice that this is needed for both pc and mips_malta.c
>>
>> Ideas on where to hide a pci_dev inside a bus?
>
> pci_set_irq_fn should take an opaque state pointer (given at
> pci_register_bus time):
>
> typedef void (*pci_set_irq_fn)(void *opaque, qemu_irq *pic, int
> irq_num, int level);
>
> PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                          qemu_irq *pic, void *irq_opaque, int
> devfn_min, int nirq);

I fixed it changing the type of qemu_irq *pic to void *irq_state.
See the v2 of the series.

Thanks very much for the suggestion.

later, Juan.