All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
@ 2012-03-26  2:06 Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 1/6] eliminate piix_pci.c and module i440fx and piix3 Wanpeng Li
                   ` (9 more replies)
  0 siblings, 10 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>


This series aggressively refactors the PC machine initialization to be more
modelled and less ad-hoc.  The highlights of this series are:

 1) Things like -m and -bios-name are now device model properties

 2) The i440fx and piix3 are now modelled in a thorough fashion

 3) Most of the chipset features of the piix3 are modelled through composition

 4) i440fx_init is trivialized to creating devices and setting properties

 5) convert MemoryRegion to QOM

 6) convert PCI host bridge to QOM

The point (4) is the most important one.  As we refactor in this fashion,
we should quickly get to the point where machine->init disappears completely in
favor of just creating a handful of devices.

The two stage initialization of QOM is important here.  instance_init() is when
composed devices are created which means that after you've created a device, all
of its children are visible in the device model.  This lets you set properties
of the parent and its children.

realize() (which is still called DeviceState::init today) will be called right
before the guest starts up for the first time.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 Makefile.target     |    3 +-
 hw/hpet.c           |   39 +---
 hw/hpet_emul.h      |   41 +++
 hw/i440fx.c         |  431 ++++++++++++++++++++++++++
 hw/i440fx.h         |   78 +++++
 hw/i8254_internal.h |    2 +-
 hw/mc146818rtc.c    |   26 --
 hw/mc146818rtc.h    |   29 ++
 hw/pc.c             |  838 +++++++++++++++++++++++++++++++++++++++++++++------
 hw/pc.h             |   46 +---
 hw/pc_piix.c        |  762 ----------------------------------------------
 hw/pci_host.c       |   26 ++
 hw/pci_host.h       |    5 +
 hw/piix3.c          |  274 +++++++++++++++++
 hw/piix3.h          |   79 +++++
 hw/piix_pci.c       |  600 ------------------------------------
 memory.c            |   94 +++++--
 memory.h            |    8 +
 18 files changed, 1795 insertions(+), 1586 deletions(-)
 create mode 100644 hw/i440fx.c
 create mode 100644 hw/i440fx.h
 delete mode 100644 hw/pc_piix.c
 create mode 100644 hw/piix3.c
 create mode 100644 hw/piix3.h
 delete mode 100644 hw/piix_pci.c
--

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 1/6] eliminate piix_pci.c and module i440fx and piix3
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 2/6] convert MemoryRegion to QOM Wanpeng Li
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>


The big picture about the patch is shown as follows:

1) pc_init creates an I440FX, any bus devices (ISA serial port, PCI
vga and nics, etc.), sets properties appropriately, and realizes the
devices.
2) I440FX is-a PCIHost, has-a I440FX-PMC, has-a PIIX3
3) PIIX3 has-a RTC, has-a I8042, has-a DMAController, etc.

i440fx-pcihost => i440fx
i440fx => i440fx-pmc

i440fx_pmc is Programmable Memory Controller which integrated in I440FX chipset,
and move ram initialization into i440fx-pmc.

It might seem like a small change, but it better reflects the fact that the PMC
is contained within the i440fx which we will now reflect in composition in the
next few changesets.


Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 Makefile.target |    2 +-
 hw/i440fx.c     |  431 +++++++++++++++++++++++++++++++++++++++
 hw/i440fx.h     |   78 +++++++
 hw/piix3.c      |  274 +++++++++++++++++++++++++
 hw/piix3.h      |   79 ++++++++
 hw/piix_pci.c   |  600 -------------------------------------------------------
 6 files changed, 863 insertions(+), 601 deletions(-)
 create mode 100644 hw/i440fx.c
 create mode 100644 hw/i440fx.h
 create mode 100644 hw/piix3.c
 create mode 100644 hw/piix3.h
 delete mode 100644 hw/piix_pci.c

diff --git a/Makefile.target b/Makefile.target
index 63cf769..24fb0c0 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -247,7 +247,7 @@ obj-y += device-hotplug.o
 # Hardware support
 obj-i386-y += mc146818rtc.o pc.o
 obj-i386-y += apic_common.o apic.o kvmvapic.o
-obj-i386-y += sga.o ioapic_common.o ioapic.o piix_pci.o
+obj-i386-y += sga.o ioapic_common.o ioapic.o i440fx.o piix3.o
 obj-i386-y += vmport.o
 obj-i386-y += pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/i440fx.c b/hw/i440fx.c
new file mode 100644
index 0000000..3658740
--- /dev/null
+++ b/hw/i440fx.c
@@ -0,0 +1,431 @@
+/*
+ * QEMU i440FX PCI Host 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 "i440fx.h"
+#include "range.h"
+#include "xen.h"
+#include "loader.h"
+#include "pc.h"
+
+#define BIOS_FILENAME "bios.bin"
+
+/*
+ * I440FX chipset data sheet.
+ * http://download.intel.com/design/chipsets/datashts/29054901.pdf
+ *
+ * The I440FX is a package that contains an integrated PCI Host controller,
+ * memory controller, and is usually packaged with a PCI-ISA bus and super I/O
+ * chipset.
+ *
+ * The "i440FX" device is the PCI host controller.  On function 0.0, there is a
+ * memory controller called the Programmable Memory Controller (PMC).  On
+ * function 1.0, there is the PCI-ISA bus/super I/O chip called the PIIX3.
+ */
+
+#define I440FX_PMC_PCI_HOLE     0xE0000000ULL
+#define I440FX_PMC_PCI_HOLE_END 0x100000000ULL
+
+#define I440FX_PAM      0x59
+#define I440FX_PAM_SIZE 7
+#define I440FX_SMRAM    0x72
+
+static void piix3_set_irq(void *opaque, int pirq, int level)
+{
+    PIIX3State *piix3 = opaque;
+    piix3_set_irq_level(piix3, pirq, level);
+}
+
+/* return the global irq number corresponding to a given device irq
+   pin. We could also use the bus number to have a more precise
+   mapping. */
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+{
+    int slot_addend;
+    slot_addend = (pci_dev->devfn >> 3) - 1;
+    return (pci_intx + slot_addend) & 3;
+}
+
+static void update_pam(I440FXPMCState *d, uint32_t start, uint32_t end, int r,
+                       PAMMemoryRegion *mem)
+{
+    if (mem->initialized) {
+        memory_region_del_subregion(d->system_memory, &mem->mem);
+        memory_region_destroy(&mem->mem);
+    }
+
+    //    printf("ISA mapping %08x-0x%08x: %d\n", start, end, r);
+    switch(r) {
+    case 3:
+        /* RAM */
+        memory_region_init_alias(&mem->mem, "pam-ram", d->ram_memory,
+                                 start, end - start);
+        break;
+    case 1:
+        /* ROM (XXX: not quite correct) */
+        memory_region_init_alias(&mem->mem, "pam-rom", d->ram_memory,
+                                 start, end - start);
+        memory_region_set_readonly(&mem->mem, true);
+        break;
+    case 2:
+    case 0:
+        /* XXX: should distinguish read/write cases */
+        memory_region_init_alias(&mem->mem, "pam-pci", d->pci_address_space,
+                                 start, end - start);
+        break;
+    }
+    memory_region_add_subregion_overlap(d->system_memory,
+                                        start, &mem->mem, 1);
+    mem->initialized = true;
+}
+
+static void i440fx_pmc_update_memory_mappings(I440FXPMCState *d)
+{
+    int i, r;
+    uint32_t smram;
+    bool smram_enabled;
+
+    memory_region_transaction_begin();
+    update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3,
+               &d->pam_regions[0]);
+    for(i = 0; i < 12; i++) {
+        r = (d->dev.config[(i >> 1) + (I440FX_PAM + 1)] >> ((i & 1) * 4)) & 3;
+        update_pam(d, 0xc0000 + 0x4000 * i, 0xc0000 + 0x4000 * (i + 1), r,
+                   &d->pam_regions[i+1]);
+    }
+    smram = d->dev.config[I440FX_SMRAM];
+    smram_enabled = (d->smm_enabled && (smram & 0x08)) || (smram & 0x40);
+    memory_region_set_enabled(&d->smram_region, !smram_enabled);
+    memory_region_transaction_commit();
+}
+
+static void i440fx_pmc_set_smm(int val, void *arg)
+{
+    I440FXPMCState *d = arg;
+
+    val = (val != 0);
+    if (d->smm_enabled != val) {
+        d->smm_enabled = val;
+        i440fx_pmc_update_memory_mappings(d);
+    }
+}
+
+
+static void i440fx_pmc_write_config(PCIDevice *dev,
+                                    uint32_t address, uint32_t val, int len)
+{
+    I440FXPMCState *d = DO_UPCAST(I440FXPMCState, dev, dev);
+
+    /* XXX: implement SMRAM.D_LOCK */
+    pci_default_write_config(dev, address, val, len);
+    if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
+        range_covers_byte(address, len, I440FX_SMRAM)) {
+        i440fx_pmc_update_memory_mappings(d);
+    }
+}
+
+static int i440fx_pmc_load_old(QEMUFile* f, void *opaque, int version_id)
+{
+    I440FXPMCState *d = opaque;
+    int ret, i;
+
+    ret = pci_device_load(&d->dev, f);
+    if (ret < 0)
+        return ret;
+    i440fx_pmc_update_memory_mappings(d);
+    qemu_get_8s(f, &d->smm_enabled);
+
+    if (version_id == 2) {
+        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+            qemu_get_be32(f); /* dummy load for compatibility */
+        }
+    }
+
+    return 0;
+}
+
+static int i440fx_pmc_post_load(void *opaque, int version_id)
+{
+    I440FXPMCState *d = opaque;
+
+    i440fx_pmc_update_memory_mappings(d);
+    return 0;
+}
+
+static const VMStateDescription vmstate_i440fx_pmc = {
+    .name = "I440FX", /* this is wrong but we can't change it */
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 1,
+    .load_state_old = i440fx_pmc_load_old,
+    .post_load = i440fx_pmc_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(dev, I440FXPMCState),
+        VMSTATE_UINT8(smm_enabled, I440FXPMCState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int i440fx_realize(SysBusDevice *dev)
+{
+    I440FXState *s = I440FX(dev);
+    PCIHostState *h = PCI_HOST(s);
+    int bios_size, isa_bios_size;
+    char *filename;
+    int ret;
+
+    g_assert(h->address_space != NULL);
+    g_assert(s->address_space_io != NULL);
+
+    h->bus = pci_bus_new(DEVICE(s), NULL, &s->pci_address_space,
+                         s->address_space_io, 0);
+
+    memory_region_init_io(&h->conf_mem, &pci_host_conf_le_ops, s,
+                          "pci-conf-idx", 4);
+    sysbus_add_io(dev, 0xcf8, &h->conf_mem);
+    sysbus_init_ioports(&h->busdev, 0xcf8, 4);
+
+    memory_region_init_io(&h->data_mem, &pci_host_data_le_ops, s,
+                          "pci-conf-data", 4);
+    sysbus_add_io(dev, 0xcfc, &h->data_mem);
+    sysbus_init_ioports(&h->busdev, 0xcfc, 4);
+
+    s->pmc.system_memory = h->address_space;
+    s->pmc.pci_address_space = &s->pci_address_space;
+
+    qdev_set_parent_bus(DEVICE(&s->pmc), &(h->bus->qbus));
+    qdev_prop_set_uint32(DEVICE(&s->pmc), "addr", PCI_DEVFN(0, 0));
+    qdev_init_nofail(DEVICE(&s->pmc));
+
+    qdev_set_parent_bus(DEVICE(&s->piix3), &(h->bus->qbus));
+    qdev_init_nofail(DEVICE(&s->piix3));
+
+    if (xen_enabled()) {
+        pci_bus_irqs(h->bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
+                     &s->piix3, XEN_PIIX_NUM_PIRQS);
+    } else {
+        pci_bus_irqs(h->bus, piix3_set_irq, pci_slot_get_pirq, &s->piix3,
+                PIIX_NUM_PIRQS);
+    }
+
+    /* BIOS load */
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, s->bios_name);
+    if (filename) {
+        bios_size = get_image_size(filename);
+    } else {
+        bios_size = -1;
+    }
+    if (bios_size <= 0 ||
+        (bios_size % 65536) != 0) {
+        goto bios_error;
+    }
+    memory_region_init_ram(&s->bios, "pc.bios", bios_size);
+    vmstate_register_ram_global(&s->bios);
+    memory_region_set_readonly(&s->bios, true);
+    ret = rom_add_file_fixed(s->bios_name, (uint32_t)(-bios_size), -1);
+    if (ret != 0) {
+    bios_error:
+        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", s->bios_name);
+        exit(1);
+    }
+    if (filename) {
+        g_free(filename);
+    }
+
+    /* map the last 128KB of the BIOS in ISA space */
+    isa_bios_size = bios_size;
+    if (isa_bios_size > (128 * 1024)) {
+        isa_bios_size = 128 * 1024;
+    }
+    memory_region_init_alias(&s->isa_bios, "isa-bios", &s->bios,
+                             bios_size - isa_bios_size, isa_bios_size);
+    memory_region_add_subregion_overlap(&s->pci_address_space,
+                                        0x100000 - isa_bios_size,
+                                        &s->isa_bios,
+                                        1);
+    memory_region_set_readonly(&s->isa_bios, true);
+
+    memory_region_init_ram(&s->option_roms, "pc.rom", PC_ROM_SIZE);
+    vmstate_register_ram_global(&s->option_roms);
+    memory_region_add_subregion_overlap(&s->pci_address_space,
+                                        PC_ROM_MIN_VGA,
+                                        &s->option_roms,
+                                        1);
+
+    /* map all the bios at the top of memory */
+    memory_region_add_subregion(&s->pci_address_space,
+                                (uint32_t)(-bios_size),
+                                &s->bios);
+
+    return 0;
+}
+
+static void i440fx_initfn(Object *obj)
+{
+    I440FXState *s = I440FX(obj);
+
+    object_initialize(&s->pmc, TYPE_I440FX_PMC);
+    object_property_add_child(obj, "pmc", OBJECT(&s->pmc), NULL);
+
+    /* Xen supports additional interrupt routes from the PCI devices to
+     * the IOAPIC: the four pins of each PCI device on the bus are also
+     * connected to the IOAPIC directly.
+     * These additional routes can be discovered through ACPI. */
+    if (xen_enabled()) {
+        object_initialize(&s->piix3, "PIIX3-xen");
+    } else {
+        object_initialize(&s->piix3, "PIIX3");
+    }
+    object_property_add_child(OBJECT(s), "piix3", OBJECT(&s->piix3), NULL);
+
+    s->bios_name = g_strdup(BIOS_FILENAME);
+
+    memory_region_init(&s->pci_address_space, "pci", INT64_MAX);
+}
+
+static int i440fx_pmc_realize(PCIDevice *dev)
+{
+    I440FXPMCState *d = DO_UPCAST(I440FXPMCState, dev, dev);
+    ram_addr_t ram_size;
+    uint64_t below_4g_mem_size, above_4g_mem_size;
+    uint64_t pci_hole_start, pci_hole_size;
+    uint64_t pci_hole64_start, pci_hole64_size;
+
+    g_assert(d->ram_size != 0);
+    g_assert(d->system_memory != NULL);
+    g_assert(d->pci_address_space != NULL);
+
+    /* Calculate memory geometry from RAM size */
+    if (d->ram_size > I440FX_PMC_PCI_HOLE) {
+        below_4g_mem_size = I440FX_PMC_PCI_HOLE;
+        above_4g_mem_size = d->ram_size - I440FX_PMC_PCI_HOLE;
+    } else {
+        below_4g_mem_size = d->ram_size;
+        above_4g_mem_size = 0;
+    }
+
+    /* Allocate RAM.  We allocate it as a single memory region and use
+     * aliases to address portions of it, mostly for backwards compatibility
+     * with older qemus that used qemu_ram_alloc().
+     */
+    memory_region_init_ram(&d->ram, "pc.ram",
+                           below_4g_mem_size + above_4g_mem_size);
+    vmstate_register_ram_global(&d->ram);
+
+    memory_region_init_alias(&d->ram_below_4g, "ram-below-4g", &d->ram,
+                             0, below_4g_mem_size);
+    memory_region_add_subregion(d->system_memory, 0, &d->ram_below_4g);
+    if (above_4g_mem_size > 0) {
+        memory_region_init_alias(&d->ram_above_4g, "ram-above-4g", &d->ram,
+                                 below_4g_mem_size, above_4g_mem_size);
+        memory_region_add_subregion(d->system_memory, 0x100000000ULL,
+                                    &d->ram_above_4g);
+    }
+
+    pci_hole_start = below_4g_mem_size;
+    pci_hole_size = I440FX_PMC_PCI_HOLE_END - pci_hole_start;
+
+    pci_hole64_start = I440FX_PMC_PCI_HOLE_END + d->ram_size - pci_hole_start;
+    if (sizeof(target_phys_addr_t) == 4) {
+        pci_hole64_size = 0;
+    } else {
+        pci_hole64_size = (1ULL << 62);
+    }
+
+    memory_region_init_alias(&d->pci_hole, "pci-hole", d->pci_address_space,
+                             pci_hole_start, pci_hole_size);
+    memory_region_add_subregion(d->system_memory, pci_hole_start, &d->pci_hole);
+    memory_region_init_alias(&d->pci_hole_64bit, "pci-hole64",
+                             d->pci_address_space,
+                             pci_hole64_start, pci_hole64_size);
+    if (pci_hole64_size) {
+        memory_region_add_subregion(d->system_memory, pci_hole64_start,
+                                    &d->pci_hole_64bit);
+    }
+    memory_region_init_alias(&d->smram_region, "smram-region",
+                             d->pci_address_space, 0xa0000, 0x20000);
+    memory_region_add_subregion_overlap(d->system_memory, 0xa0000,
+                                        &d->smram_region, 1);
+    memory_region_set_enabled(&d->smram_region, false);
+
+    ram_size = d->ram_size / 8 / 1024 / 1024;
+    if (ram_size > 255)
+        ram_size = 255;
+    d->dev.config[0x57] = ram_size;
+
+    i440fx_pmc_update_memory_mappings(d);
+
+    d->dev.config[I440FX_SMRAM] = 0x02;
+
+    cpu_smm_register(&i440fx_pmc_set_smm, d);
+    return 0;
+}
+
+static void i440fx_pmc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->no_hotplug = 1;
+    k->init = i440fx_pmc_realize;
+    k->config_write = i440fx_pmc_write_config;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
+    k->class_id = PCI_CLASS_BRIDGE_HOST;
+    k->revision = 0x02;
+    dc->desc = "Host bridge";
+    dc->no_user = 1;
+    dc->vmsd = &vmstate_i440fx_pmc;
+}
+
+static TypeInfo i440fx_pmc_info = {
+    .name          = TYPE_I440FX_PMC,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(I440FXPMCState),
+    .class_init    = i440fx_pmc_class_init,
+};
+
+static void i440fx_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = i440fx_realize;
+    dc->fw_name = "pci";
+    dc->no_user = 1;
+}
+
+static TypeInfo i440fx_info = {
+    .name          = TYPE_I440FX,
+    .parent        = TYPE_PCI_HOST,
+    .instance_size = sizeof(I440FXState),
+    .instance_init = i440fx_initfn,
+    .class_init    = i440fx_class_init,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&i440fx_info);
+    type_register_static(&i440fx_pmc_info);
+}
+type_init(register_devices);
diff --git a/hw/i440fx.h b/hw/i440fx.h
new file mode 100644
index 0000000..aaee522
--- /dev/null
+++ b/hw/i440fx.h
@@ -0,0 +1,78 @@
+/*
+ * QEMU i440FX PCI Host 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.
+ */
+
+#ifndef QEMU_I440FX_H
+#define QEMU_I440FX_H
+
+#include "pci_host.h"
+#include "piix3.h"
+#include "pci_internals.h"
+
+#define TYPE_I440FX_PMC "i440FX-PMC"
+#define I440FX_PMC(obj) OBJECT_CHECK(I440FXPMCState, (obj), TYPE_I440FX_PMC)
+
+typedef struct PAMMemoryRegion {
+    MemoryRegion mem;
+    bool initialized;
+} PAMMemoryRegion;
+
+typedef struct I440FXPMCState {
+    PCIDevice dev;
+    MemoryRegion *system_memory;
+    MemoryRegion *pci_address_space;
+    MemoryRegion *ram_memory;
+    MemoryRegion pci_hole;
+    MemoryRegion pci_hole_64bit;
+    PAMMemoryRegion pam_regions[13];
+    MemoryRegion smram_region;
+    uint8_t smm_enabled;
+
+    ram_addr_t ram_size;
+    MemoryRegion ram;
+    MemoryRegion ram_below_4g;
+    MemoryRegion ram_above_4g;
+} I440FXPMCState;
+
+#define TYPE_I440FX "i440FX"
+#define I440FX(obj) OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX)
+
+typedef struct I440FXState
+{
+    PCIHostState parent;
+
+    MemoryRegion *address_space_io;
+    MemoryRegion pci_address_space;
+
+    I440FXPMCState pmc;
+    PIIX3State piix3;
+
+    /* Is this more appropriate for the PMC? */
+    MemoryRegion bios;
+    MemoryRegion isa_bios;
+    MemoryRegion option_roms;
+
+    char *bios_name;
+} I440FXState;
+
+#endif
diff --git a/hw/piix3.c b/hw/piix3.c
new file mode 100644
index 0000000..bbdcb24
--- /dev/null
+++ b/hw/piix3.c
@@ -0,0 +1,274 @@
+/*
+ * QEMU PIIX3 PCI-ISA Bridge Emulation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * 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 "piix3.h"
+#include "range.h"
+#include "kvm.h"
+#include "xen.h"
+#include "pc.h"
+#include "pcspk.h"
+
+/* PIIX3 PCI to ISA bridge */
+static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+{
+    qemu_set_irq(piix3->pic[pic_irq],
+                 !!(piix3->pic_levels &
+                    (((1ULL << PIIX_NUM_PIRQS) - 1) <<
+                     (pic_irq * PIIX_NUM_PIRQS))));
+}
+
+void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
+{
+    int pic_irq;
+    uint64_t mask;
+
+    pic_irq = piix3->dev.config[PIIX_PIRQC + pirq];
+    if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+        return;
+    }
+
+    mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
+    piix3->pic_levels &= ~mask;
+    piix3->pic_levels |= mask * !!level;
+
+    piix3_set_irq_pic(piix3, pic_irq);
+}
+
+/* irq routing is changed. so rebuild bitmap */
+static void piix3_update_irq_levels(PIIX3State *piix3)
+{
+    int pirq;
+
+    piix3->pic_levels = 0;
+    for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
+        piix3_set_irq_level(piix3, pirq,
+                            pci_bus_get_irq_level(piix3->dev.bus, pirq));
+    }
+}
+
+static void piix3_write_config(PCIDevice *dev,
+                               uint32_t address, uint32_t val, int len)
+{
+    pci_default_write_config(dev, address, val, len);
+    if (ranges_overlap(address, len, PIIX_PIRQC, 4)) {
+        PIIX3State *piix3 = DO_UPCAST(PIIX3State, dev, dev);
+        int pic_irq;
+        piix3_update_irq_levels(piix3);
+        for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
+            piix3_set_irq_pic(piix3, pic_irq);
+        }
+    }
+}
+
+static void piix3_write_config_xen(PCIDevice *dev,
+                               uint32_t address, uint32_t val, int len)
+{
+    xen_piix_pci_write_config_client(address, val, len);
+    piix3_write_config(dev, address, val, len);
+}
+
+static void piix3_reset(DeviceState *dev)
+{
+    PIIX3State *d = PIIX3(dev);
+    uint8_t *pci_conf = d->dev.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] = 0x80;
+    pci_conf[0x61] = 0x80;
+    pci_conf[0x62] = 0x80;
+    pci_conf[0x63] = 0x80;
+    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;
+
+    d->pic_levels = 0;
+}
+
+static int piix3_post_load(void *opaque, int version_id)
+{
+    PIIX3State *piix3 = opaque;
+    piix3_update_irq_levels(piix3);
+    return 0;
+}
+
+static void piix3_pre_save(void *opaque)
+{
+    int i;
+    PIIX3State *piix3 = opaque;
+
+    for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
+        piix3->pci_irq_levels_vmstate[i] =
+            pci_bus_get_irq_level(piix3->dev.bus, i);
+    }
+}
+
+static const VMStateDescription vmstate_piix3 = {
+    .name = TYPE_PIIX3,
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .post_load = piix3_post_load,
+    .pre_save = piix3_pre_save,
+    .fields      = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(dev, PIIX3State),
+        VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
+                              PIIX_NUM_PIRQS, 3),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int piix3_realize(PCIDevice *dev)
+{
+    PIIX3State *s = PIIX3(dev);
+    qemu_irq rtc_irq;
+
+    /* Initialize ISA Bus */
+    s->bus = isa_bus_new(DEVICE(dev), pci_address_space_io(dev));
+    isa_bus_irqs(s->bus, s->pic);
+
+    /* Realize the RTC */
+    qdev_set_parent_bus(DEVICE(&s->rtc), &(s->bus->qbus));
+    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
+    qdev_init_nofail(DEVICE(&s->rtc));
+
+    /* Realize HPET */
+    if (s->hpet_enable) {
+        int i;
+
+        /* We need to introduce a proper IRQ and Memory QOM infrastructure
+         * so that the HPET isn't a sysbus device */
+        qdev_set_parent_bus(DEVICE(&s->hpet), sysbus_get_default());
+        qdev_init_nofail(DEVICE(&s->hpet));
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->hpet), 0, HPET_BASE);
+        for (i = 0; i < GSI_NUM_PINS; i++) {
+            sysbus_connect_irq(SYS_BUS_DEVICE(&s->hpet), i, s->pic[i]);
+        }
+
+        rtc_irq = qdev_get_gpio_in(DEVICE(&s->hpet), 0);
+    } else {
+        isa_init_irq(ISA_DEVICE(&s->rtc), &rtc_irq, RTC_ISA_IRQ);
+    }
+
+    /* Setup the RTC IRQ */
+    s->rtc.irq = rtc_irq;
+
+    /* Realize the PIT */
+    qdev_set_parent_bus(DEVICE(&s->pit), &(s->bus->qbus));
+    qdev_prop_set_uint32(DEVICE(&s->pit), "iobase", 0x40);
+	qdev_connect_gpio_out(DEVICE(&s->pit), 0, isa_get_irq(ISA_DEVICE(&s->pit), 0));
+    qdev_init_nofail(DEVICE(&s->pit));
+
+    /* FIXME this should be refactored */
+    pcspk_init(s->bus, ISA_DEVICE(&s->pit));
+
+    return 0;
+}
+
+static void piix3_initfn(Object *obj)
+{
+    PIIX3State *s = PIIX3(obj);
+
+    object_initialize(&s->hpet, TYPE_HPET);
+    object_property_add_child(obj, "hpet", OBJECT(&s->hpet), NULL);
+
+    object_initialize(&s->rtc, TYPE_RTC);
+    object_property_add_child(obj, "rtc", OBJECT(&s->rtc), NULL);
+
+    object_initialize(&s->pit, TYPE_PIT);
+    object_property_add_child(obj, "pit", OBJECT(&s->pit), NULL);
+}
+
+static void piix3_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    dc->desc        = "ISA bridge";
+    dc->vmsd        = &vmstate_piix3;
+    dc->no_user     = 1;
+    dc->reset       = piix3_reset;
+    k->no_hotplug   = 1;
+    k->init         = piix3_realize;
+    k->config_write = piix3_write_config;
+    k->vendor_id    = PCI_VENDOR_ID_INTEL;
+    k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0; // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
+    k->class_id     = PCI_CLASS_BRIDGE_ISA;
+}
+
+static TypeInfo piix3_info = {
+    .name          = TYPE_PIIX3,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PIIX3State),
+    .instance_init = piix3_initfn,
+    .class_init    = piix3_class_init,
+};
+
+static void piix3_xen_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->config_write = piix3_write_config_xen;
+};
+
+static TypeInfo piix3_xen_info = {
+    .name          = "PIIX3-xen",
+    .parent        = TYPE_PIIX3,
+    .instance_size = sizeof(PIIX3State),
+    .class_init    = piix3_xen_class_init,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&piix3_info);
+    type_register_static(&piix3_xen_info);
+}
+type_init(register_devices);
diff --git a/hw/piix3.h b/hw/piix3.h
new file mode 100644
index 0000000..18f08ec
--- /dev/null
+++ b/hw/piix3.h
@@ -0,0 +1,79 @@
+/*
+ * QEMU PIIX3 PCI-ISA Bridge Emulation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * 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.
+ */
+
+#ifndef QEMU_PIIX3_H
+#define QEMU_PIIX3_H
+
+#include "pci.h"
+#include "hpet_emul.h"
+#include "mc146818rtc.h"
+#include "i8254.h"
+#include "i8254_internal.h"
+
+#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
+#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
+#define XEN_PIIX_NUM_PIRQS      128ULL
+#define PIIX_PIRQC              0x60
+
+#define TYPE_PIIX3 "PIIX3"
+#define PIIX3(obj) OBJECT_CHECK(PIIX3State, (obj), TYPE_PIIX3)
+
+typedef struct PIIX3State {
+    PCIDevice dev;
+
+    /*
+     * bitmap to track pic levels.
+     * The pic level is the logical OR of all the PCI irqs mapped to it
+     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
+     *
+     * PIRQ is mapped to PIC pins, we track it by
+     * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+     * pic_irq * PIIX_NUM_PIRQS + pirq
+     */
+#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#error "unable to encode pic state in 64bit in pic_levels."
+#endif
+    uint64_t pic_levels;
+
+    bool hpet_enable;
+
+    HPETState hpet;
+    RTCState rtc;
+    PITCommonState pit;
+
+    ISABus *bus;
+
+    qemu_irq *pic;
+
+    /* This member isn't used. Just for save/load compatibility */
+    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
+} PIIX3State;
+
+void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level);
+
+#endif
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
deleted file mode 100644
index e0268fe..0000000
--- a/hw/piix_pci.c
+++ /dev/null
@@ -1,600 +0,0 @@
-/*
- * QEMU i440FX/PIIX3 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 "pci_host.h"
-#include "isa.h"
-#include "sysbus.h"
-#include "range.h"
-#include "xen.h"
-
-/*
- * I440FX chipset data sheet.
- * http://download.intel.com/design/chipsets/datashts/29054901.pdf
- */
-
-typedef PCIHostState I440FXState;
-
-#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
-#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
-#define XEN_PIIX_NUM_PIRQS      128ULL
-#define PIIX_PIRQC              0x60
-
-typedef struct PIIX3State {
-    PCIDevice dev;
-
-    /*
-     * bitmap to track pic levels.
-     * The pic level is the logical OR of all the PCI irqs mapped to it
-     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
-     *
-     * PIRQ is mapped to PIC pins, we track it by
-     * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
-     * pic_irq * PIIX_NUM_PIRQS + pirq
-     */
-#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
-#error "unable to encode pic state in 64bit in pic_levels."
-#endif
-    uint64_t pic_levels;
-
-    qemu_irq *pic;
-
-    /* This member isn't used. Just for save/load compatibility */
-    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
-} PIIX3State;
-
-typedef struct PAMMemoryRegion {
-    MemoryRegion mem;
-    bool initialized;
-} PAMMemoryRegion;
-
-struct PCII440FXState {
-    PCIDevice dev;
-    MemoryRegion *system_memory;
-    MemoryRegion *pci_address_space;
-    MemoryRegion *ram_memory;
-    MemoryRegion pci_hole;
-    MemoryRegion pci_hole_64bit;
-    PAMMemoryRegion pam_regions[13];
-    MemoryRegion smram_region;
-    uint8_t smm_enabled;
-};
-
-
-#define I440FX_PAM      0x59
-#define I440FX_PAM_SIZE 7
-#define I440FX_SMRAM    0x72
-
-static void piix3_set_irq(void *opaque, int pirq, int level);
-static void piix3_write_config_xen(PCIDevice *dev,
-                               uint32_t address, uint32_t val, int len);
-
-/* return the global irq number corresponding to a given device irq
-   pin. We could also use the bus number to have a more precise
-   mapping. */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
-{
-    int slot_addend;
-    slot_addend = (pci_dev->devfn >> 3) - 1;
-    return (pci_intx + slot_addend) & 3;
-}
-
-static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r,
-                       PAMMemoryRegion *mem)
-{
-    if (mem->initialized) {
-        memory_region_del_subregion(d->system_memory, &mem->mem);
-        memory_region_destroy(&mem->mem);
-    }
-
-    //    printf("ISA mapping %08x-0x%08x: %d\n", start, end, r);
-    switch(r) {
-    case 3:
-        /* RAM */
-        memory_region_init_alias(&mem->mem, "pam-ram", d->ram_memory,
-                                 start, end - start);
-        break;
-    case 1:
-        /* ROM (XXX: not quite correct) */
-        memory_region_init_alias(&mem->mem, "pam-rom", d->ram_memory,
-                                 start, end - start);
-        memory_region_set_readonly(&mem->mem, true);
-        break;
-    case 2:
-    case 0:
-        /* XXX: should distinguish read/write cases */
-        memory_region_init_alias(&mem->mem, "pam-pci", d->pci_address_space,
-                                 start, end - start);
-        break;
-    }
-    memory_region_add_subregion_overlap(d->system_memory,
-                                        start, &mem->mem, 1);
-    mem->initialized = true;
-}
-
-static void i440fx_update_memory_mappings(PCII440FXState *d)
-{
-    int i, r;
-    uint32_t smram;
-    bool smram_enabled;
-
-    memory_region_transaction_begin();
-    update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3,
-               &d->pam_regions[0]);
-    for(i = 0; i < 12; i++) {
-        r = (d->dev.config[(i >> 1) + (I440FX_PAM + 1)] >> ((i & 1) * 4)) & 3;
-        update_pam(d, 0xc0000 + 0x4000 * i, 0xc0000 + 0x4000 * (i + 1), r,
-                   &d->pam_regions[i+1]);
-    }
-    smram = d->dev.config[I440FX_SMRAM];
-    smram_enabled = (d->smm_enabled && (smram & 0x08)) || (smram & 0x40);
-    memory_region_set_enabled(&d->smram_region, !smram_enabled);
-    memory_region_transaction_commit();
-}
-
-static void i440fx_set_smm(int val, void *arg)
-{
-    PCII440FXState *d = arg;
-
-    val = (val != 0);
-    if (d->smm_enabled != val) {
-        d->smm_enabled = val;
-        i440fx_update_memory_mappings(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(dev, address, val, len);
-    if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
-        range_covers_byte(address, len, I440FX_SMRAM)) {
-        i440fx_update_memory_mappings(d);
-    }
-}
-
-static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
-{
-    PCII440FXState *d = opaque;
-    int ret, i;
-
-    ret = pci_device_load(&d->dev, f);
-    if (ret < 0)
-        return ret;
-    i440fx_update_memory_mappings(d);
-    qemu_get_8s(f, &d->smm_enabled);
-
-    if (version_id == 2) {
-        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-            qemu_get_be32(f); /* dummy load for compatibility */
-        }
-    }
-
-    return 0;
-}
-
-static int i440fx_post_load(void *opaque, int version_id)
-{
-    PCII440FXState *d = opaque;
-
-    i440fx_update_memory_mappings(d);
-    return 0;
-}
-
-static const VMStateDescription vmstate_i440fx = {
-    .name = "I440FX",
-    .version_id = 3,
-    .minimum_version_id = 3,
-    .minimum_version_id_old = 1,
-    .load_state_old = i440fx_load_old,
-    .post_load = i440fx_post_load,
-    .fields      = (VMStateField []) {
-        VMSTATE_PCI_DEVICE(dev, PCII440FXState),
-        VMSTATE_UINT8(smm_enabled, PCII440FXState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static int i440fx_pcihost_initfn(SysBusDevice *dev)
-{
-    I440FXState *s = FROM_SYSBUS(I440FXState, dev);
-
-    memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
-                          "pci-conf-idx", 4);
-    sysbus_add_io(dev, 0xcf8, &s->conf_mem);
-    sysbus_init_ioports(&s->busdev, 0xcf8, 4);
-
-    memory_region_init_io(&s->data_mem, &pci_host_data_le_ops, s,
-                          "pci-conf-data", 4);
-    sysbus_add_io(dev, 0xcfc, &s->data_mem);
-    sysbus_init_ioports(&s->busdev, 0xcfc, 4);
-
-    return 0;
-}
-
-static int i440fx_initfn(PCIDevice *dev)
-{
-    PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
-
-    d->dev.config[I440FX_SMRAM] = 0x02;
-
-    cpu_smm_register(&i440fx_set_smm, d);
-    return 0;
-}
-
-static PCIBus *i440fx_common_init(const char *device_name,
-                                  PCII440FXState **pi440fx_state,
-                                  int *piix3_devfn,
-                                  ISABus **isa_bus, qemu_irq *pic,
-                                  MemoryRegion *address_space_mem,
-                                  MemoryRegion *address_space_io,
-                                  ram_addr_t ram_size,
-                                  target_phys_addr_t pci_hole_start,
-                                  target_phys_addr_t pci_hole_size,
-                                  target_phys_addr_t pci_hole64_start,
-                                  target_phys_addr_t pci_hole64_size,
-                                  MemoryRegion *pci_address_space,
-                                  MemoryRegion *ram_memory)
-{
-    DeviceState *dev;
-    PCIBus *b;
-    PCIDevice *d;
-    I440FXState *s;
-    PIIX3State *piix3;
-    PCII440FXState *f;
-
-    dev = qdev_create(NULL, "i440FX-pcihost");
-    s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
-    s->address_space = address_space_mem;
-    b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
-                    address_space_io, 0);
-    s->bus = b;
-    qdev_init_nofail(dev);
-    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
-
-    d = pci_create_simple(b, 0, device_name);
-    *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
-    f = *pi440fx_state;
-    f->system_memory = address_space_mem;
-    f->pci_address_space = pci_address_space;
-    f->ram_memory = ram_memory;
-    memory_region_init_alias(&f->pci_hole, "pci-hole", f->pci_address_space,
-                             pci_hole_start, pci_hole_size);
-    memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
-    memory_region_init_alias(&f->pci_hole_64bit, "pci-hole64",
-                             f->pci_address_space,
-                             pci_hole64_start, pci_hole64_size);
-    if (pci_hole64_size) {
-        memory_region_add_subregion(f->system_memory, pci_hole64_start,
-                                    &f->pci_hole_64bit);
-    }
-    memory_region_init_alias(&f->smram_region, "smram-region",
-                             f->pci_address_space, 0xa0000, 0x20000);
-    memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
-                                        &f->smram_region, 1);
-    memory_region_set_enabled(&f->smram_region, false);
-
-    /* Xen supports additional interrupt routes from the PCI devices to
-     * the IOAPIC: the four pins of each PCI device on the bus are also
-     * connected to the IOAPIC directly.
-     * These additional routes can be discovered through ACPI. */
-    if (xen_enabled()) {
-        piix3 = DO_UPCAST(PIIX3State, dev,
-                pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
-        pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
-                piix3, XEN_PIIX_NUM_PIRQS);
-    } else {
-        piix3 = DO_UPCAST(PIIX3State, dev,
-                pci_create_simple_multifunction(b, -1, true, "PIIX3"));
-        pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
-                PIIX_NUM_PIRQS);
-    }
-    object_property_add_child(OBJECT(dev), "piix3", OBJECT(piix3), NULL);
-    piix3->pic = pic;
-    *isa_bus = DO_UPCAST(ISABus, qbus,
-                         qdev_get_child_bus(&piix3->dev.qdev, "isa.0"));
-
-    *piix3_devfn = piix3->dev.devfn;
-
-    ram_size = ram_size / 8 / 1024 / 1024;
-    if (ram_size > 255)
-        ram_size = 255;
-    (*pi440fx_state)->dev.config[0x57]=ram_size;
-
-    i440fx_update_memory_mappings(f);
-
-    return b;
-}
-
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn,
-                    ISABus **isa_bus, qemu_irq *pic,
-                    MemoryRegion *address_space_mem,
-                    MemoryRegion *address_space_io,
-                    ram_addr_t ram_size,
-                    target_phys_addr_t pci_hole_start,
-                    target_phys_addr_t pci_hole_size,
-                    target_phys_addr_t pci_hole64_start,
-                    target_phys_addr_t pci_hole64_size,
-                    MemoryRegion *pci_memory, MemoryRegion *ram_memory)
-
-{
-    PCIBus *b;
-
-    b = i440fx_common_init("i440FX", pi440fx_state, piix3_devfn, isa_bus, pic,
-                           address_space_mem, address_space_io, ram_size,
-                           pci_hole_start, pci_hole_size,
-                           pci_hole64_size, pci_hole64_size,
-                           pci_memory, ram_memory);
-    return b;
-}
-
-/* PIIX3 PCI to ISA bridge */
-static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
-{
-    qemu_set_irq(piix3->pic[pic_irq],
-                 !!(piix3->pic_levels &
-                    (((1ULL << PIIX_NUM_PIRQS) - 1) <<
-                     (pic_irq * PIIX_NUM_PIRQS))));
-}
-
-static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
-{
-    int pic_irq;
-    uint64_t mask;
-
-    pic_irq = piix3->dev.config[PIIX_PIRQC + pirq];
-    if (pic_irq >= PIIX_NUM_PIC_IRQS) {
-        return;
-    }
-
-    mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
-    piix3->pic_levels &= ~mask;
-    piix3->pic_levels |= mask * !!level;
-
-    piix3_set_irq_pic(piix3, pic_irq);
-}
-
-static void piix3_set_irq(void *opaque, int pirq, int level)
-{
-    PIIX3State *piix3 = opaque;
-    piix3_set_irq_level(piix3, pirq, level);
-}
-
-/* irq routing is changed. so rebuild bitmap */
-static void piix3_update_irq_levels(PIIX3State *piix3)
-{
-    int pirq;
-
-    piix3->pic_levels = 0;
-    for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-        piix3_set_irq_level(piix3, pirq,
-                            pci_bus_get_irq_level(piix3->dev.bus, pirq));
-    }
-}
-
-static void piix3_write_config(PCIDevice *dev,
-                               uint32_t address, uint32_t val, int len)
-{
-    pci_default_write_config(dev, address, val, len);
-    if (ranges_overlap(address, len, PIIX_PIRQC, 4)) {
-        PIIX3State *piix3 = DO_UPCAST(PIIX3State, dev, dev);
-        int pic_irq;
-        piix3_update_irq_levels(piix3);
-        for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
-            piix3_set_irq_pic(piix3, pic_irq);
-        }
-    }
-}
-
-static void piix3_write_config_xen(PCIDevice *dev,
-                               uint32_t address, uint32_t val, int len)
-{
-    xen_piix_pci_write_config_client(address, val, len);
-    piix3_write_config(dev, address, val, len);
-}
-
-static void piix3_reset(void *opaque)
-{
-    PIIX3State *d = opaque;
-    uint8_t *pci_conf = d->dev.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] = 0x80;
-    pci_conf[0x61] = 0x80;
-    pci_conf[0x62] = 0x80;
-    pci_conf[0x63] = 0x80;
-    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;
-
-    d->pic_levels = 0;
-}
-
-static int piix3_post_load(void *opaque, int version_id)
-{
-    PIIX3State *piix3 = opaque;
-    piix3_update_irq_levels(piix3);
-    return 0;
-}
-
-static void piix3_pre_save(void *opaque)
-{
-    int i;
-    PIIX3State *piix3 = opaque;
-
-    for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
-        piix3->pci_irq_levels_vmstate[i] =
-            pci_bus_get_irq_level(piix3->dev.bus, i);
-    }
-}
-
-static const VMStateDescription vmstate_piix3 = {
-    .name = "PIIX3",
-    .version_id = 3,
-    .minimum_version_id = 2,
-    .minimum_version_id_old = 2,
-    .post_load = piix3_post_load,
-    .pre_save = piix3_pre_save,
-    .fields      = (VMStateField []) {
-        VMSTATE_PCI_DEVICE(dev, PIIX3State),
-        VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
-                              PIIX_NUM_PIRQS, 3),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static int piix3_initfn(PCIDevice *dev)
-{
-    PIIX3State *d = DO_UPCAST(PIIX3State, dev, dev);
-
-    isa_bus_new(&d->dev.qdev, pci_address_space_io(dev));
-    qemu_register_reset(piix3_reset, d);
-    return 0;
-}
-
-static void piix3_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    dc->desc        = "ISA bridge";
-    dc->vmsd        = &vmstate_piix3;
-    dc->no_user     = 1,
-    k->no_hotplug   = 1;
-    k->init         = piix3_initfn;
-    k->config_write = piix3_write_config;
-    k->vendor_id    = PCI_VENDOR_ID_INTEL;
-    k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0; // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
-    k->class_id     = PCI_CLASS_BRIDGE_ISA;
-}
-
-static TypeInfo piix3_info = {
-    .name          = "PIIX3",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX3State),
-    .class_init    = piix3_class_init,
-};
-
-static void piix3_xen_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    dc->desc        = "ISA bridge";
-    dc->vmsd        = &vmstate_piix3;
-    dc->no_user     = 1;
-    k->no_hotplug   = 1;
-    k->init         = piix3_initfn;
-    k->config_write = piix3_write_config_xen;
-    k->vendor_id    = PCI_VENDOR_ID_INTEL;
-    k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0; // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
-    k->class_id     = PCI_CLASS_BRIDGE_ISA;
-};
-
-static TypeInfo piix3_xen_info = {
-    .name          = "PIIX3-xen",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX3State),
-    .class_init    = piix3_xen_class_init,
-};
-
-static void i440fx_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->no_hotplug = 1;
-    k->init = i440fx_initfn;
-    k->config_write = i440fx_write_config;
-    k->vendor_id = PCI_VENDOR_ID_INTEL;
-    k->device_id = PCI_DEVICE_ID_INTEL_82441;
-    k->revision = 0x02;
-    k->class_id = PCI_CLASS_BRIDGE_HOST;
-    dc->desc = "Host bridge";
-    dc->no_user = 1;
-    dc->vmsd = &vmstate_i440fx;
-}
-
-static TypeInfo i440fx_info = {
-    .name          = "i440FX",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCII440FXState),
-    .class_init    = i440fx_class_init,
-};
-
-static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-    k->init = i440fx_pcihost_initfn;
-    dc->fw_name = "pci";
-    dc->no_user = 1;
-}
-
-static TypeInfo i440fx_pcihost_info = {
-    .name          = "i440FX-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(I440FXState),
-    .class_init    = i440fx_pcihost_class_init,
-};
-
-static void i440fx_register_types(void)
-{
-    type_register_static(&i440fx_info);
-    type_register_static(&piix3_info);
-    type_register_static(&piix3_xen_info);
-    type_register_static(&i440fx_pcihost_info);
-}
-
-type_init(i440fx_register_types)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 2/6] convert MemoryRegion to QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 1/6] eliminate piix_pci.c and module i440fx and piix3 Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>


Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 memory.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++----------------
 memory.h |    8 +++++
 2 files changed, 78 insertions(+), 24 deletions(-)

diff --git a/memory.c b/memory.c
index 22b0352..ec1a4ae 100644
--- a/memory.c
+++ b/memory.c
@@ -797,35 +797,26 @@ static bool memory_region_wrong_endianness(MemoryRegion *mr)
 #endif
 }
 
-void memory_region_init(MemoryRegion *mr,
-                        const char *name,
-                        uint64_t size)
+void memory_region_set_name(MemoryRegion *mr, const char *name)
+{
+    mr->name = g_strdup(name);
+}
+
+void memory_region_set_size(MemoryRegion *mr, uint64_t size)
 {
-    mr->ops = NULL;
-    mr->parent = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
     }
-    mr->addr = 0;
-    mr->subpage = false;
-    mr->enabled = true;
-    mr->terminates = false;
-    mr->ram = false;
-    mr->readable = true;
-    mr->readonly = false;
-    mr->rom_device = false;
-    mr->destructor = memory_region_destructor_none;
-    mr->priority = 0;
-    mr->may_overlap = false;
-    mr->alias = NULL;
-    QTAILQ_INIT(&mr->subregions);
-    memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
-    QTAILQ_INIT(&mr->coalesced);
-    mr->name = g_strdup(name);
-    mr->dirty_log_mask = 0;
-    mr->ioeventfd_nb = 0;
-    mr->ioeventfds = NULL;
+}
+
+void memory_region_init(MemoryRegion *mr,
+                        const char *name,
+                        uint64_t size)
+{
+    object_initialize(mr, TYPE_MEMORY_REGION);
+    memory_region_set_name(mr, name);
+    memory_region_set_size(mr, size);
 }
 
 static bool memory_region_access_valid(MemoryRegion *mr,
@@ -1640,3 +1631,58 @@ void mtree_info(fprintf_function mon_printf, void *f)
         mtree_print_mr(mon_printf, f, address_space_io.root, 0, 0, &ml_head);
     }
 }
+
+static void memory_region_initfn(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    mr->ops = NULL;
+    mr->parent = NULL;
+    mr->size = int128_2_64();
+    mr->addr = 0;
+    mr->subpage = false;
+    mr->enabled = true;
+    mr->terminates = false;
+    mr->ram = false;
+    mr->readable = true;
+    mr->readonly = false;
+    mr->rom_device = false;
+    mr->destructor = memory_region_destructor_none;
+    mr->priority = 0;
+    mr->may_overlap = false;
+    mr->alias = NULL;
+    mr->name = NULL;
+    QTAILQ_INIT(&mr->subregions);
+    memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
+    QTAILQ_INIT(&mr->coalesced);
+    mr->dirty_log_mask = 0;
+    mr->ioeventfd_nb = 0;
+    mr->ioeventfds = NULL;
+}
+
+static void memory_region_finalize(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    assert(QTAILQ_EMPTY(&mr->subregions));
+    mr->destructor(mr);
+    memory_region_clear_coalescing(mr);
+    if (mr->name) {
+        g_free((char *)mr->name);
+    }
+    g_free(mr->ioeventfds);
+}
+
+static TypeInfo memory_region_type = {
+    .name = TYPE_MEMORY_REGION,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(MemoryRegion),
+    .instance_init = memory_region_initfn,
+    .instance_finalize = memory_region_finalize,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&memory_region_type);
+}
+
+type_init(register_devices);
diff --git a/memory.h b/memory.h
index 53ff62b..d4d5600 100644
--- a/memory.h
+++ b/memory.h
@@ -25,6 +25,7 @@
 #include "iorange.h"
 #include "ioport.h"
 #include "int128.h"
+#include "qemu/object.h"
 
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegion MemoryRegion;
@@ -116,6 +117,9 @@ struct MemoryRegionOps {
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
+#define TYPE_MEMORY_REGION "memory-region"
+#define MEMORY_REGION(obj) OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
+
 struct MemoryRegion {
     /* All fields are private - violators will be prosecuted */
     const MemoryRegionOps *ops;
@@ -719,6 +723,10 @@ void memory_global_dirty_log_stop(void);
 
 void mtree_info(fprintf_function mon_printf, void *f);
 
+void memory_region_set_name(MemoryRegion *mr, const char *name);
+
+void memory_region_set_size(MemoryRegion *mr, uint64_t size);
+
 #endif
 
 #endif
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 3/6] convert pci-host to QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 1/6] eliminate piix_pci.c and module i440fx and piix3 Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 2/6] convert MemoryRegion to QOM Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26  7:32   ` Stefan Hajnoczi
                     ` (2 more replies)
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 4/6] prepare to create HPET, RTC and i8254 through composition Wanpeng Li
                   ` (6 subsequent siblings)
  9 siblings, 3 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>


Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 hw/pci_host.c |   26 ++++++++++++++++++++++++++
 hw/pci_host.h |    5 +++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..44d7e55 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+	object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+    PCIHostState *s = PCI_HOST(obj);
+
+	object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
+					         (Object **)&s->address_space, NULL);
+}
+
+static TypeInfo pci_host_type = {
+    .name = TYPE_PCI_HOST,
+    .parent = TYPE_SYS_BUS_DEVICE,
+	.instance_size = sizeof(PCIHostState),
+	.instance_init = pci_host_initfn,
+};
+
+static void register_devices(void)
+{
+	type_register_static(&pci_host_type);
+}
+
+type_init(register_devices);
 
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..084e15c 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
     SysBusDevice busdev;
     MemoryRegion conf_mem;
@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 4/6] prepare to create HPET, RTC and i8254 through composition
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (2 preceding siblings ...)
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c Wanpeng Li
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>

The HPET usually sits on the LPC bus (which replaces ISA in modern systems).  
It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.  
I think in terms of where it would live in this hypothetical device model,
putting it in the PIIX is rational.


Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 hw/hpet.c           |   39 ++-------------------------------------
 hw/hpet_emul.h      |   41 +++++++++++++++++++++++++++++++++++++++++
 hw/i8254_internal.h |    2 +-
 hw/mc146818rtc.c    |   26 --------------------------
 hw/mc146818rtc.h    |   29 +++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 64 deletions(-)

diff --git a/hw/hpet.c b/hw/hpet.c
index fd3ddca..fc0ff6c 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -42,41 +42,6 @@
 
 #define HPET_MSI_SUPPORT        0
 
-struct HPETState;
-typedef struct HPETTimer {  /* timers */
-    uint8_t tn;             /*timer number*/
-    QEMUTimer *qemu_timer;
-    struct HPETState *state;
-    /* Memory-mapped, software visible timer registers */
-    uint64_t config;        /* configuration/cap */
-    uint64_t cmp;           /* comparator */
-    uint64_t fsb;           /* FSB route */
-    /* Hidden register state */
-    uint64_t period;        /* Last value written to comparator */
-    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
-                             * mode. Next pop will be actual timer expiration.
-                             */
-} HPETTimer;
-
-typedef struct HPETState {
-    SysBusDevice busdev;
-    MemoryRegion iomem;
-    uint64_t hpet_offset;
-    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
-    uint32_t flags;
-    uint8_t rtc_irq_level;
-    qemu_irq pit_enabled;
-    uint8_t num_timers;
-    HPETTimer timer[HPET_MAX_TIMERS];
-
-    /* Memory-mapped, software visible registers */
-    uint64_t capability;        /* capabilities */
-    uint64_t config;            /* configuration */
-    uint64_t isr;               /* interrupt status reg */
-    uint64_t hpet_counter;      /* main counter */
-    uint8_t  hpet_id;           /* instance id */
-} HPETState;
-
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {
     return s->config & HPET_CFG_LEGACY;
@@ -278,7 +243,7 @@ static const VMStateDescription vmstate_hpet_timer = {
 };
 
 static const VMStateDescription vmstate_hpet = {
-    .name = "hpet",
+    .name = TYPE_HPET,
     .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
@@ -746,7 +711,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
 }
 
 static TypeInfo hpet_device_info = {
-    .name          = "hpet",
+    .name          = TYPE_HPET,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(HPETState),
     .class_init    = hpet_device_class_init,
diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h
index 757f79f..5808a19 100644
--- a/hw/hpet_emul.h
+++ b/hw/hpet_emul.h
@@ -13,6 +13,9 @@
 #ifndef QEMU_HPET_EMUL_H
 #define QEMU_HPET_EMUL_H
 
+#include "hw.h"
+#include "sysbus.h"
+
 #define HPET_BASE               0xfed00000
 #define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds == 10ns*/
 
@@ -71,4 +74,42 @@ struct hpet_fw_config
 } QEMU_PACKED;
 
 extern struct hpet_fw_config hpet_cfg;
+
+#define TYPE_HPET "hpet"
+
+struct HPETState;
+typedef struct HPETTimer {  /* timers */
+    uint8_t tn;             /*timer number*/
+    QEMUTimer *qemu_timer;
+    struct HPETState *state;
+    /* Memory-mapped, software visible timer registers */
+    uint64_t config;        /* configuration/cap */
+    uint64_t cmp;           /* comparator */
+    uint64_t fsb;           /* FSB route */
+    /* Hidden register state */
+    uint64_t period;        /* Last value written to comparator */
+    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
+                             * mode. Next pop will be actual timer expiration.
+                             */
+} HPETTimer;
+
+typedef struct HPETState {
+    SysBusDevice busdev;
+    MemoryRegion iomem;
+    uint64_t hpet_offset;
+    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
+    uint32_t flags;
+    uint8_t rtc_irq_level;
+	qemu_irq pit_enabled;
+    uint8_t num_timers;
+    HPETTimer timer[HPET_MAX_TIMERS];
+
+    /* Memory-mapped, software visible registers */
+    uint64_t capability;        /* capabilities */
+    uint64_t config;            /* configuration */
+    uint64_t isr;               /* interrupt status reg */
+    uint64_t hpet_counter;      /* main counter */
+    uint8_t  hpet_id;           /* instance id */
+} HPETState;
+
 #endif
diff --git a/hw/i8254_internal.h b/hw/i8254_internal.h
index 686f0c2..542f7c1 100644
--- a/hw/i8254_internal.h
+++ b/hw/i8254_internal.h
@@ -26,7 +26,6 @@
 #define QEMU_I8254_INTERNAL_H
 
 #include "hw.h"
-#include "pc.h"
 #include "isa.h"
 
 typedef struct PITChannelState {
@@ -57,6 +56,7 @@ typedef struct PITCommonState {
     PITChannelState channels[3];
 } PITCommonState;
 
+#define TYPE_PIT "isa-pit"
 #define TYPE_PIT_COMMON "pit-common"
 #define PIT_COMMON(obj) \
      OBJECT_CHECK(PITCommonState, (obj), TYPE_PIT_COMMON)
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2b59c36..5375f96 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -80,32 +80,6 @@
 #define REG_C_PF   0x40
 #define REG_C_AF   0x20
 
-typedef struct RTCState {
-    ISADevice dev;
-    MemoryRegion io;
-    uint8_t cmos_data[128];
-    uint8_t cmos_index;
-    struct tm current_tm;
-    int32_t base_year;
-    qemu_irq irq;
-    qemu_irq sqw_irq;
-    int it_shift;
-    /* periodic timer */
-    QEMUTimer *periodic_timer;
-    int64_t next_periodic_time;
-    /* second update */
-    int64_t next_second_time;
-    uint16_t irq_reinject_on_ack_count;
-    uint32_t irq_coalesced;
-    uint32_t period;
-    QEMUTimer *coalesced_timer;
-    QEMUTimer *second_timer;
-    QEMUTimer *second_timer2;
-    Notifier clock_reset_notifier;
-    LostTickPolicy lost_tick_policy;
-    Notifier suspend_notifier;
-} RTCState;
-
 static void rtc_set_time(RTCState *s);
 static void rtc_copy_date(RTCState *s);
 
diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
index f119930..60a4b4e 100644
--- a/hw/mc146818rtc.h
+++ b/hw/mc146818rtc.h
@@ -2,9 +2,38 @@
 #define MC146818RTC_H
 
 #include "isa.h"
+#include "notify.h"
 
 #define RTC_ISA_IRQ 8
 
+#define TYPE_RTC "mc146818rtc"
+
+typedef struct RTCState {
+    ISADevice dev;
+    MemoryRegion io;
+    uint8_t cmos_data[128];
+    uint8_t cmos_index;
+    struct tm current_tm;
+    int32_t base_year;
+    qemu_irq irq;
+    qemu_irq sqw_irq;
+    int it_shift;
+    /* periodic timer */
+    QEMUTimer *periodic_timer;
+    int64_t next_periodic_time;
+    /* second update */
+    int64_t next_second_time;
+    uint16_t irq_reinject_on_ack_count;
+    uint32_t irq_coalesced;
+    uint32_t period;
+    QEMUTimer *coalesced_timer;
+    QEMUTimer *second_timer;
+    QEMUTimer *second_timer2;
+    Notifier clock_reset_notifier;
+    LostTickPolicy lost_tick_policy;
+	Notifier suspend_notifier;
+} RTCState;
+
 ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
 void rtc_set_memory(ISADevice *dev, int addr, int val);
 void rtc_set_date(ISADevice *dev, const struct tm *tm);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (3 preceding siblings ...)
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 4/6] prepare to create HPET, RTC and i8254 through composition Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26 12:42   ` Avi Kivity
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 6/6] make some functions static Wanpeng Li
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 Makefile.target |    1 -
 hw/pc.c         |  816 +++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/pc.h         |   20 +-
 hw/pc_piix.c    |  762 ---------------------------------------------------
 4 files changed, 739 insertions(+), 860 deletions(-)
 delete mode 100644 hw/pc_piix.c

diff --git a/Makefile.target b/Makefile.target
index 24fb0c0..5c4605f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -251,7 +251,6 @@ obj-i386-y += sga.o ioapic_common.o ioapic.o i440fx.o piix3.o
 obj-i386-y += vmport.o
 obj-i386-y += pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
-obj-i386-y += pc_piix.o
 obj-i386-y += pc_sysfw.o
 obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o kvm/i8254.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
diff --git a/hw/pc.c b/hw/pc.c
index 83a1b5b..d5a557e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -36,8 +36,6 @@
 #include "elf.h"
 #include "multiboot.h"
 #include "mc146818rtc.h"
-#include "i8254.h"
-#include "pcspk.h"
 #include "msi.h"
 #include "sysbus.h"
 #include "sysemu.h"
@@ -46,6 +44,11 @@
 #include "ui/qemu-spice.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "kvm/clock.h"
+#include "xen.h"
+#include "arch_init.h"
+#include "smbus.h"
+#include "boards.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -60,6 +63,8 @@
 #define DPRINTF(fmt, ...)
 #endif
 
+#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
+
 /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
 #define ACPI_DATA_SIZE       0x10000
 #define BIOS_CFG_IOPORT 0x510
@@ -73,6 +78,8 @@
 
 #define E820_NR_ENTRIES		16
 
+#define MAX_IDE_BUS 2
+
 struct e820_entry {
     uint64_t address;
     uint64_t length;
@@ -84,6 +91,10 @@ struct e820_table {
     struct e820_entry entry[E820_NR_ENTRIES];
 } QEMU_PACKED __attribute((__aligned__(4)));
 
+static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
+static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
+static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
+
 static struct e820_table e820_table;
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
@@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
     DeviceState *dev;
     static int apic_mapped;
 
-    if (kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         dev = qdev_create(NULL, "kvm-apic");
     } else {
         dev = qdev_create(NULL, "apic");
@@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
     }
 
     /* KVM does not support MSI yet. */
-    if (!kvm_irqchip_in_kernel()) {
+    if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
         msi_supported = true;
     }
 
@@ -972,50 +983,13 @@ void pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_cmdline,
                     const char *initrd_filename,
                     ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory)
+                    ram_addr_t above_4g_mem_size)
 {
     int linux_boot, i;
-    MemoryRegion *ram, *option_rom_mr;
-    MemoryRegion *ram_below_4g, *ram_above_4g;
     void *fw_cfg;
 
     linux_boot = (kernel_filename != NULL);
 
-    /* Allocate RAM.  We allocate it as a single memory region and use
-     * aliases to address portions of it, mostly for backwards compatibility
-     * with older qemus that used qemu_ram_alloc().
-     */
-    ram = g_malloc(sizeof(*ram));
-    memory_region_init_ram(ram, "pc.ram",
-                           below_4g_mem_size + above_4g_mem_size);
-    vmstate_register_ram_global(ram);
-    *ram_memory = ram;
-    ram_below_4g = g_malloc(sizeof(*ram_below_4g));
-    memory_region_init_alias(ram_below_4g, "ram-below-4g", ram,
-                             0, below_4g_mem_size);
-    memory_region_add_subregion(system_memory, 0, ram_below_4g);
-    if (above_4g_mem_size > 0) {
-        ram_above_4g = g_malloc(sizeof(*ram_above_4g));
-        memory_region_init_alias(ram_above_4g, "ram-above-4g", ram,
-                                 below_4g_mem_size, above_4g_mem_size);
-        memory_region_add_subregion(system_memory, 0x100000000ULL,
-                                    ram_above_4g);
-    }
-
-
-    /* Initialize PC system firmware */
-    pc_system_firmware_init(rom_memory);
-
-    option_rom_mr = g_malloc(sizeof(*option_rom_mr));
-    memory_region_init_ram(option_rom_mr, "pc.rom", PC_ROM_SIZE);
-    vmstate_register_ram_global(option_rom_mr);
-    memory_region_add_subregion_overlap(rom_memory,
-                                        PC_ROM_MIN_VGA,
-                                        option_rom_mr,
-                                        1);
-
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
@@ -1078,57 +1052,19 @@ static void cpu_request_exit(void *opaque, int irq, int level)
 }
 
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
                           ISADevice **floppy,
                           bool no_vmport)
 {
     int i;
     DriveInfo *fd[MAX_FD];
-    DeviceState *hpet = NULL;
-    int pit_isa_irq = 0;
-    qemu_irq pit_alt_irq = NULL;
-    qemu_irq rtc_irq = NULL;
     qemu_irq *a20_line;
-    ISADevice *i8042, *port92, *vmmouse, *pit;
+    ISADevice *i8042, *port92, *vmmouse;
     qemu_irq *cpu_exit_irq;
 
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
 
     register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL);
 
-    /*
-     * Check if an HPET shall be created.
-     *
-     * Without KVM_CAP_PIT_STATE2, we cannot switch off the in-kernel PIT
-     * when the HPET wants to take over. Thus we have to disable the latter.
-     */
-    if (!no_hpet && (!kvm_irqchip_in_kernel() || kvm_has_pit_state2())) {
-        hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL);
-
-        if (hpet) {
-            for (i = 0; i < GSI_NUM_PINS; i++) {
-                sysbus_connect_irq(sysbus_from_qdev(hpet), i, gsi[i]);
-            }
-            pit_isa_irq = -1;
-            pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
-            rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
-        }
-    }
-    *rtc_state = rtc_init(isa_bus, 2000, rtc_irq);
-
-    qemu_register_boot_set(pc_boot_set, *rtc_state);
-
-    if (kvm_irqchip_in_kernel()) {
-        pit = kvm_pit_init(isa_bus, 0x40);
-    } else {
-        pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq);
-    }
-    if (hpet) {
-        /* connect PIT to output control line of the HPET */
-        qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(&pit->qdev, 0));
-    }
-    pcspk_init(isa_bus, pit);
-
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
             serial_isa_init(isa_bus, i, serial_hds[i]);
@@ -1176,3 +1112,721 @@ void pc_pci_device_init(PCIBus *pci_bus)
         pci_create_simple(pci_bus, -1, "lsi53c895a");
     }
 }
+
+static void kvm_piix3_setup_irq_routing(bool pci_enabled)
+{
+#ifdef CONFIG_KVM
+    KVMState *s = kvm_state;
+    int ret, i;
+
+    if (kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
+        for (i = 0; i < 8; ++i) {
+            if (i == 2) {
+                continue;
+            }
+            kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_MASTER, i);
+        }
+        for (i = 8; i < 16; ++i) {
+            kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
+        }
+        if (pci_enabled) {
+            for (i = 0; i < 24; ++i) {
+                if (i == 0) {
+                    kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, 2);
+                } else if (i != 2) {
+                    kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, i);
+                }
+            }
+        }
+        ret = kvm_irqchip_commit_routes(s);
+        if (ret < 0) {
+            hw_error("KVM IRQ routing setup failed");
+        }
+    }
+#endif /* CONFIG_KVM */
+}
+
+static void kvm_piix3_gsi_handler(void *opaque, int n, int level)
+{
+    GSIState *s = opaque;
+
+    if (n < ISA_NUM_IRQS) {
+        /* Kernel will forward to both PIC and IOAPIC */
+        qemu_set_irq(s->i8259_irq[n], level);
+    } else {
+        qemu_set_irq(s->ioapic_irq[n], level);
+    }
+}
+
+static void ioapic_init(GSIState *gsi_state)
+{
+    DeviceState *dev;
+    SysBusDevice *d;
+    unsigned int i;
+
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        dev = qdev_create(NULL, "kvm-ioapic");
+    } else {
+        dev = qdev_create(NULL, "ioapic");
+    }
+    qdev_init_nofail(dev);
+    d = sysbus_from_qdev(dev);
+    sysbus_mmio_map(d, 0, 0xfec00000);
+
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
+    }
+}
+
+static PCIBus *i440fx_init(I440FXPMCState **pi440fx_state, int *piix3_devfn,
+                           ISABus **isa_bus, qemu_irq *pic,
+                           MemoryRegion *address_space_mem,
+                           MemoryRegion *address_space_io,
+                           ram_addr_t ram_size,
+                           const char *bios_name)
+{
+    I440FXState *s;
+    PCIHostState *h;
+
+    s = I440FX(object_new(TYPE_I440FX));
+    h = PCI_HOST(s);
+
+    /* FIXME make a properties */
+    h->address_space = address_space_mem;
+    s->address_space_io = address_space_io;
+    s->piix3.pic = pic;
+    if (bios_name) {
+        g_free(s->bios_name);
+        s->bios_name = g_strdup(bios_name);
+    }
+    s->pmc.ram_size = ram_size;
+
+    qdev_set_parent_bus(DEVICE(s), sysbus_get_default());
+    qdev_init_nofail(DEVICE(s));
+
+    object_property_add_child(object_get_root(), "i440fx", OBJECT(s), NULL);
+
+    *isa_bus = s->piix3.bus;
+    *pi440fx_state = &s->pmc;
+    *piix3_devfn = s->piix3.dev.devfn;
+
+    return h->bus;
+}
+
+/* PC hardware initialisation */
+static void pc_init1(MemoryRegion *system_memory,
+                     MemoryRegion *system_io,
+                     ram_addr_t ram_size,
+                     const char *boot_device,
+                     const char *kernel_filename,
+                     const char *kernel_cmdline,
+                     const char *initrd_filename,
+                     const char *cpu_model,
+                     int pci_enabled,
+                     int kvmclock_enabled)
+{
+    int i;
+    ram_addr_t below_4g_mem_size, above_4g_mem_size;
+    PCIBus *pci_bus;
+    ISABus *isa_bus;
+    I440FXPMCState *i440fx_state;
+    int piix3_devfn = -1;
+    qemu_irq *cpu_irq;
+    qemu_irq *gsi;
+    qemu_irq *i8259;
+    qemu_irq *smi_irq;
+    GSIState *gsi_state;
+    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+    BusState *idebus[MAX_IDE_BUS];
+    ISADevice *rtc_state;
+    ISADevice *floppy;
+    DeviceState *dev;
+
+    pc_cpus_init(cpu_model);
+
+    if (kvmclock_enabled) {
+        kvmclock_create();
+    }
+
+    if (ram_size >= 0xe0000000 ) {
+        above_4g_mem_size = ram_size - 0xe0000000;
+        below_4g_mem_size = 0xe0000000;
+    } else {
+        above_4g_mem_size = 0;
+        below_4g_mem_size = ram_size;
+    }
+
+    /* allocate ram and load rom/bios */
+    if (!xen_enabled()) {
+        pc_memory_init(system_memory,
+                       kernel_filename, kernel_cmdline, initrd_filename,
+                       below_4g_mem_size, above_4g_mem_size);
+    }
+
+    gsi_state = g_malloc0(sizeof(*gsi_state));
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        kvm_piix3_setup_irq_routing(pci_enabled);
+        gsi = qemu_allocate_irqs(kvm_piix3_gsi_handler, gsi_state,
+                                 GSI_NUM_PINS);
+    } else {
+        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
+    }
+
+    if (pci_enabled) {
+        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
+                              system_memory, system_io, ram_size,
+                              bios_name);
+    } else {
+        pci_bus = NULL;
+        i440fx_state = NULL;
+        isa_bus = isa_bus_new(NULL, system_io);
+        no_hpet = 1;
+    }
+
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        i8259 = kvm_i8259_init(isa_bus);
+    } else if (xen_enabled()) {
+        i8259 = xen_interrupt_controller_init();
+    } else {
+        cpu_irq = pc_allocate_cpu_irq();
+        i8259 = i8259_init(isa_bus, cpu_irq[0]);
+    }
+
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        gsi_state->i8259_irq[i] = i8259[i];
+    }
+    if (pci_enabled) {
+        ioapic_init(gsi_state);
+    }
+
+    pc_register_ferr_irq(gsi[13]);
+
+    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
+    if (dev) {
+        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
+    }
+
+    if (xen_enabled()) {
+        pci_create_simple(pci_bus, -1, "xen-platform");
+    }
+
+    /* init basic PC hardware */
+    pc_basic_device_init(isa_bus, gsi, &floppy, xen_enabled());
+
+    for(i = 0; i < nb_nics; i++) {
+        NICInfo *nd = &nd_table[i];
+
+        if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
+            pc_init_ne2k_isa(isa_bus, nd);
+        else
+            pci_nic_init_nofail(nd, "e1000", NULL);
+    }
+
+    ide_drive_get(hd, MAX_IDE_BUS);
+    if (pci_enabled) {
+        PCIDevice *dev;
+        if (xen_enabled()) {
+            dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
+        } else {
+            dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
+        }
+        idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
+        idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
+    } else {
+        for(i = 0; i < MAX_IDE_BUS; i++) {
+            ISADevice *dev;
+            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+                               ide_irq[i],
+                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+            idebus[i] = qdev_get_child_bus(&dev->qdev, "ide.0");
+        }
+    }
+
+    /* FIXME */
+    rtc_state = ISA_DEVICE(object_resolve_path("rtc", NULL));
+    qemu_register_boot_set(pc_boot_set, rtc_state);
+
+    audio_init(isa_bus, pci_enabled ? pci_bus : NULL);
+
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+                 floppy, idebus[0], idebus[1], rtc_state);
+
+    if (pci_enabled && usb_enabled) {
+        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
+    }
+
+    if (pci_enabled && acpi_enabled) {
+        i2c_bus *smbus;
+
+        smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
+        /* TODO: Populate SPD eeprom data.  */
+        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
+                              gsi[9], *smi_irq,
+                              kvm_enabled());
+        smbus_eeprom_init(smbus, 8, NULL, 0);
+    }
+
+    if (pci_enabled) {
+        pc_pci_device_init(pci_bus);
+    }
+}
+
+static void pc_init_pci(ram_addr_t ram_size,
+                        const char *boot_device,
+                        const char *kernel_filename,
+                        const char *kernel_cmdline,
+                        const char *initrd_filename,
+                        const char *cpu_model)
+{
+    pc_init1(get_system_memory(),
+             get_system_io(),
+             ram_size, boot_device,
+             kernel_filename, kernel_cmdline,
+             initrd_filename, cpu_model, 1, 1);
+}
+
+static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
+                                    const char *boot_device,
+                                    const char *kernel_filename,
+                                    const char *kernel_cmdline,
+                                    const char *initrd_filename,
+                                    const char *cpu_model)
+{
+    pc_init1(get_system_memory(),
+             get_system_io(),
+             ram_size, boot_device,
+             kernel_filename, kernel_cmdline,
+             initrd_filename, cpu_model, 1, 0);
+}
+
+static void pc_init_isa(ram_addr_t ram_size,
+                        const char *boot_device,
+                        const char *kernel_filename,
+                        const char *kernel_cmdline,
+                        const char *initrd_filename,
+                        const char *cpu_model)
+{
+    if (cpu_model == NULL)
+        cpu_model = "486";
+    pc_init1(get_system_memory(),
+             get_system_io(),
+             ram_size, boot_device,
+             kernel_filename, kernel_cmdline,
+             initrd_filename, cpu_model, 0, 1);
+}
+
+#ifdef CONFIG_XEN
+static void pc_xen_hvm_init(ram_addr_t ram_size,
+                            const char *boot_device,
+                            const char *kernel_filename,
+                            const char *kernel_cmdline,
+                            const char *initrd_filename,
+                            const char *cpu_model)
+{
+    if (xen_hvm_init() != 0) {
+        hw_error("xen hardware virtual machine initialisation failed");
+    }
+    pc_init_pci_no_kvmclock(ram_size, boot_device,
+                            kernel_filename, kernel_cmdline,
+                            initrd_filename, cpu_model);
+    xen_vcpu_init();
+}
+#endif
+
+static QEMUMachine pc_machine_v1_1 = {
+    .name = "pc-1.1",
+    .alias = "pc",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+    .is_default = 1,
+};
+
+static QEMUMachine pc_machine_v1_0 = {
+    .name = "pc-1.0",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        }, {
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        { /* end of list */ }
+    },
+};
+
+static QEMUMachine pc_machine_v0_15 = {
+    .name = "pc-0.15",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        }, {
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        { /* end of list */ }
+    },
+};
+
+static QEMUMachine pc_machine_v0_14 = {
+    .name = "pc-0.14",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "qxl",
+            .property = "revision",
+            .value    = stringify(2),
+        },{
+            .driver   = "qxl-vga",
+            .property = "revision",
+            .value    = stringify(2),
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-balloon-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    },
+};
+
+static QEMUMachine pc_machine_v0_13 = {
+    .name = "pc-0.13",
+    .desc = "Standard PC",
+    .init = pc_init_pci_no_kvmclock,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "virtio-9p-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "VGA",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "vmware-svga",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-balloon-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "AC97",
+            .property = "use_broken_id",
+            .value    = stringify(1),
+        },{
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    },
+};
+
+static QEMUMachine pc_machine_v0_12 = {
+    .name = "pc-0.12",
+    .desc = "Standard PC",
+    .init = pc_init_pci_no_kvmclock,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "virtio-serial-pci",
+            .property = "max_ports",
+            .value    = stringify(1),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "VGA",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "vmware-svga",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-balloon-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "AC97",
+            .property = "use_broken_id",
+            .value    = stringify(1),
+        },{
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    }
+};
+
+static QEMUMachine pc_machine_v0_11 = {
+    .name = "pc-0.11",
+    .desc = "Standard PC, qemu 0.11",
+    .init = pc_init_pci_no_kvmclock,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "virtio-blk-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "max_ports",
+            .value    = stringify(1),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "ide-drive",
+            .property = "ver",
+            .value    = "0.11",
+        },{
+            .driver   = "scsi-disk",
+            .property = "ver",
+            .value    = "0.11",
+        },{
+            .driver   = "PCI",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-balloon-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "AC97",
+            .property = "use_broken_id",
+            .value    = stringify(1),
+        },{
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    }
+};
+
+static QEMUMachine pc_machine_v0_10 = {
+    .name = "pc-0.10",
+    .desc = "Standard PC, qemu 0.10",
+    .init = pc_init_pci_no_kvmclock,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "virtio-blk-pci",
+            .property = "class",
+            .value    = stringify(PCI_CLASS_STORAGE_OTHER),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "class",
+            .value    = stringify(PCI_CLASS_DISPLAY_OTHER),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "max_ports",
+            .value    = stringify(1),
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "vectors",
+            .value    = stringify(0),
+        },{
+            .driver   = "ide-drive",
+            .property = "ver",
+            .value    = "0.10",
+        },{
+            .driver   = "scsi-disk",
+            .property = "ver",
+            .value    = "0.10",
+        },{
+            .driver   = "PCI",
+            .property = "rombar",
+            .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
+        },{
+            .driver   = "virtio-blk-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-net-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "virtio-balloon-pci",
+            .property = "event_idx",
+            .value    = "off",
+        },{
+            .driver   = "AC97",
+            .property = "use_broken_id",
+            .value    = stringify(1),
+        },{
+            .driver   = "isa-fdc",
+            .property = "check_media_rate",
+            .value    = "off",
+        },
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    },
+};
+
+static QEMUMachine isapc_machine = {
+    .name = "isapc",
+    .desc = "ISA-only PC",
+    .init = pc_init_isa,
+    .max_cpus = 1,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "pc-sysfw",
+            .property = "rom_only",
+            .value    = stringify(1),
+        },
+        { /* end of list */ }
+    },
+};
+
+#ifdef CONFIG_XEN
+static QEMUMachine xenfv_machine = {
+    .name = "xenfv",
+    .desc = "Xen Fully-virtualized PC",
+    .init = pc_xen_hvm_init,
+    .max_cpus = HVM_MAX_VCPUS,
+    .default_machine_opts = "accel=xen",
+};
+#endif
+
+static void pc_machine_init(void)
+{
+    qemu_register_machine(&pc_machine_v1_1);
+    qemu_register_machine(&pc_machine_v1_0);
+    qemu_register_machine(&pc_machine_v0_15);
+    qemu_register_machine(&pc_machine_v0_14);
+    qemu_register_machine(&pc_machine_v0_13);
+    qemu_register_machine(&pc_machine_v0_12);
+    qemu_register_machine(&pc_machine_v0_11);
+    qemu_register_machine(&pc_machine_v0_10);
+    qemu_register_machine(&isapc_machine);
+#ifdef CONFIG_XEN
+    qemu_register_machine(&xenfv_machine);
+#endif
+}
+
+machine_init(pc_machine_init);
diff --git a/hw/pc.h b/hw/pc.h
index 74d3369..7348da2 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -9,6 +9,7 @@
 #include "net.h"
 #include "memory.h"
 #include "ioapic.h"
+#include "i440fx.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -117,7 +118,6 @@ void pc_memory_init(MemoryRegion *system_memory,
 qemu_irq *pc_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
                           ISADevice **floppy,
                           bool no_vmport);
 void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
@@ -148,21 +148,9 @@ void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
 /* hpet.c */
 extern int no_hpet;
 
-/* piix_pci.c */
-struct PCII440FXState;
-typedef struct PCII440FXState PCII440FXState;
-
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
-                    ISABus **isa_bus, qemu_irq *pic,
-                    MemoryRegion *address_space_mem,
-                    MemoryRegion *address_space_io,
-                    ram_addr_t ram_size,
-                    target_phys_addr_t pci_hole_start,
-                    target_phys_addr_t pci_hole_size,
-                    target_phys_addr_t pci_hole64_start,
-                    target_phys_addr_t pci_hole64_size,
-                    MemoryRegion *pci_memory,
-                    MemoryRegion *ram_memory);
+/* pcspk.c */
+void pcskp_init(ISADevice *pit);
+int pcskp_audio_init(qemu_irq *pic);
 
 /* piix4.c */
 extern PCIDevice *piix4_dev;
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
deleted file mode 100644
index 3f99f9a..0000000
--- a/hw/pc_piix.c
+++ /dev/null
@@ -1,762 +0,0 @@
-/*
- * QEMU PC System Emulator
- *
- * Copyright (c) 2003-2004 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 <glib.h>
-
-#include "hw.h"
-#include "pc.h"
-#include "apic.h"
-#include "pci.h"
-#include "net.h"
-#include "boards.h"
-#include "ide.h"
-#include "kvm.h"
-#include "kvm/clock.h"
-#include "sysemu.h"
-#include "sysbus.h"
-#include "arch_init.h"
-#include "blockdev.h"
-#include "smbus.h"
-#include "xen.h"
-#include "memory.h"
-#include "exec-memory.h"
-#ifdef CONFIG_XEN
-#  include <xen/hvm/hvm_info_table.h>
-#endif
-
-#define MAX_IDE_BUS 2
-
-static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
-static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
-static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
-
-static void kvm_piix3_setup_irq_routing(bool pci_enabled)
-{
-#ifdef CONFIG_KVM
-    KVMState *s = kvm_state;
-    int ret, i;
-
-    if (kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
-        for (i = 0; i < 8; ++i) {
-            if (i == 2) {
-                continue;
-            }
-            kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_MASTER, i);
-        }
-        for (i = 8; i < 16; ++i) {
-            kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
-        }
-        if (pci_enabled) {
-            for (i = 0; i < 24; ++i) {
-                if (i == 0) {
-                    kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, 2);
-                } else if (i != 2) {
-                    kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, i);
-                }
-            }
-        }
-        ret = kvm_irqchip_commit_routes(s);
-        if (ret < 0) {
-            hw_error("KVM IRQ routing setup failed");
-        }
-    }
-#endif /* CONFIG_KVM */
-}
-
-static void kvm_piix3_gsi_handler(void *opaque, int n, int level)
-{
-    GSIState *s = opaque;
-
-    if (n < ISA_NUM_IRQS) {
-        /* Kernel will forward to both PIC and IOAPIC */
-        qemu_set_irq(s->i8259_irq[n], level);
-    } else {
-        qemu_set_irq(s->ioapic_irq[n], level);
-    }
-}
-
-static void ioapic_init(GSIState *gsi_state)
-{
-    DeviceState *dev;
-    SysBusDevice *d;
-    unsigned int i;
-
-    if (kvm_irqchip_in_kernel()) {
-        dev = qdev_create(NULL, "kvm-ioapic");
-    } else {
-        dev = qdev_create(NULL, "ioapic");
-    }
-    qdev_init_nofail(dev);
-    d = sysbus_from_qdev(dev);
-    sysbus_mmio_map(d, 0, 0xfec00000);
-
-    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
-    }
-}
-
-/* PC hardware initialisation */
-static void pc_init1(MemoryRegion *system_memory,
-                     MemoryRegion *system_io,
-                     ram_addr_t ram_size,
-                     const char *boot_device,
-                     const char *kernel_filename,
-                     const char *kernel_cmdline,
-                     const char *initrd_filename,
-                     const char *cpu_model,
-                     int pci_enabled,
-                     int kvmclock_enabled)
-{
-    int i;
-    ram_addr_t below_4g_mem_size, above_4g_mem_size;
-    PCIBus *pci_bus;
-    ISABus *isa_bus;
-    PCII440FXState *i440fx_state;
-    int piix3_devfn = -1;
-    qemu_irq *cpu_irq;
-    qemu_irq *gsi;
-    qemu_irq *i8259;
-    qemu_irq *smi_irq;
-    GSIState *gsi_state;
-    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BusState *idebus[MAX_IDE_BUS];
-    ISADevice *rtc_state;
-    ISADevice *floppy;
-    MemoryRegion *ram_memory;
-    MemoryRegion *pci_memory;
-    MemoryRegion *rom_memory;
-    DeviceState *dev;
-
-    pc_cpus_init(cpu_model);
-
-    if (kvmclock_enabled) {
-        kvmclock_create();
-    }
-
-    if (ram_size >= 0xe0000000 ) {
-        above_4g_mem_size = ram_size - 0xe0000000;
-        below_4g_mem_size = 0xe0000000;
-    } else {
-        above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
-    }
-
-    if (pci_enabled) {
-        pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, "pci", INT64_MAX);
-        rom_memory = pci_memory;
-    } else {
-        pci_memory = NULL;
-        rom_memory = system_memory;
-    }
-
-    /* allocate ram and load rom/bios */
-    if (!xen_enabled()) {
-        pc_memory_init(system_memory,
-                       kernel_filename, kernel_cmdline, initrd_filename,
-                       below_4g_mem_size, above_4g_mem_size,
-                       pci_enabled ? rom_memory : system_memory, &ram_memory);
-    }
-
-    gsi_state = g_malloc0(sizeof(*gsi_state));
-    if (kvm_irqchip_in_kernel()) {
-        kvm_piix3_setup_irq_routing(pci_enabled);
-        gsi = qemu_allocate_irqs(kvm_piix3_gsi_handler, gsi_state,
-                                 GSI_NUM_PINS);
-    } else {
-        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
-    }
-
-    if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
-                              system_memory, system_io, ram_size,
-                              below_4g_mem_size,
-                              0x100000000ULL - below_4g_mem_size,
-                              0x100000000ULL + above_4g_mem_size,
-                              (sizeof(target_phys_addr_t) == 4
-                               ? 0
-                               : ((uint64_t)1 << 62)),
-                              pci_memory, ram_memory);
-    } else {
-        pci_bus = NULL;
-        i440fx_state = NULL;
-        isa_bus = isa_bus_new(NULL, system_io);
-        no_hpet = 1;
-    }
-    isa_bus_irqs(isa_bus, gsi);
-
-    if (kvm_irqchip_in_kernel()) {
-        i8259 = kvm_i8259_init(isa_bus);
-    } else if (xen_enabled()) {
-        i8259 = xen_interrupt_controller_init();
-    } else {
-        cpu_irq = pc_allocate_cpu_irq();
-        i8259 = i8259_init(isa_bus, cpu_irq[0]);
-    }
-
-    for (i = 0; i < ISA_NUM_IRQS; i++) {
-        gsi_state->i8259_irq[i] = i8259[i];
-    }
-    if (pci_enabled) {
-        ioapic_init(gsi_state);
-    }
-
-    pc_register_ferr_irq(gsi[13]);
-
-    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
-    if (dev) {
-        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
-    }
-
-    if (xen_enabled()) {
-        pci_create_simple(pci_bus, -1, "xen-platform");
-    }
-
-    /* init basic PC hardware */
-    pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled());
-
-    for(i = 0; i < nb_nics; i++) {
-        NICInfo *nd = &nd_table[i];
-
-        if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
-            pc_init_ne2k_isa(isa_bus, nd);
-        else
-            pci_nic_init_nofail(nd, "e1000", NULL);
-    }
-
-    ide_drive_get(hd, MAX_IDE_BUS);
-    if (pci_enabled) {
-        PCIDevice *dev;
-        if (xen_enabled()) {
-            dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
-        } else {
-            dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
-        }
-        idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
-        idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
-
-        /* FIXME there's some major spaghetti here.  Somehow we create the
-         * devices on the PIIX before we actually create it.  We create the
-         * PIIX3 deep in the recess of the i440fx creation too and then lose
-         * the DeviceState.
-         *
-         * For now, let's "fix" this by making judicious use of paths.  This
-         * is not generally the right way to do this.
-         */
-        object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
-                                  "rtc", (Object *)rtc_state, NULL);
-    } else {
-        for(i = 0; i < MAX_IDE_BUS; i++) {
-            ISADevice *dev;
-            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
-                               ide_irq[i],
-                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
-            idebus[i] = qdev_get_child_bus(&dev->qdev, "ide.0");
-        }
-    }
-
-    audio_init(isa_bus, pci_enabled ? pci_bus : NULL);
-
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
-                 floppy, idebus[0], idebus[1], rtc_state);
-
-    if (pci_enabled && usb_enabled) {
-        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
-    }
-
-    if (pci_enabled && acpi_enabled) {
-        i2c_bus *smbus;
-
-        smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
-        /* TODO: Populate SPD eeprom data.  */
-        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
-                              gsi[9], *smi_irq,
-                              kvm_enabled());
-        smbus_eeprom_init(smbus, 8, NULL, 0);
-    }
-
-    if (pci_enabled) {
-        pc_pci_device_init(pci_bus);
-    }
-}
-
-static void pc_init_pci(ram_addr_t ram_size,
-                        const char *boot_device,
-                        const char *kernel_filename,
-                        const char *kernel_cmdline,
-                        const char *initrd_filename,
-                        const char *cpu_model)
-{
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 1);
-}
-
-static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
-                                    const char *boot_device,
-                                    const char *kernel_filename,
-                                    const char *kernel_cmdline,
-                                    const char *initrd_filename,
-                                    const char *cpu_model)
-{
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 0);
-}
-
-static void pc_init_isa(ram_addr_t ram_size,
-                        const char *boot_device,
-                        const char *kernel_filename,
-                        const char *kernel_cmdline,
-                        const char *initrd_filename,
-                        const char *cpu_model)
-{
-    if (cpu_model == NULL)
-        cpu_model = "486";
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 0, 1);
-}
-
-#ifdef CONFIG_XEN
-static void pc_xen_hvm_init(ram_addr_t ram_size,
-                            const char *boot_device,
-                            const char *kernel_filename,
-                            const char *kernel_cmdline,
-                            const char *initrd_filename,
-                            const char *cpu_model)
-{
-    if (xen_hvm_init() != 0) {
-        hw_error("xen hardware virtual machine initialisation failed");
-    }
-    pc_init_pci_no_kvmclock(ram_size, boot_device,
-                            kernel_filename, kernel_cmdline,
-                            initrd_filename, cpu_model);
-    xen_vcpu_init();
-}
-#endif
-
-static QEMUMachine pc_machine_v1_1 = {
-    .name = "pc-1.1",
-    .alias = "pc",
-    .desc = "Standard PC",
-    .init = pc_init_pci,
-    .max_cpus = 255,
-    .is_default = 1,
-};
-
-static QEMUMachine pc_machine_v1_0 = {
-    .name = "pc-1.0",
-    .desc = "Standard PC",
-    .init = pc_init_pci,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        }, {
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        { /* end of list */ }
-    },
-};
-
-static QEMUMachine pc_machine_v0_15 = {
-    .name = "pc-0.15",
-    .desc = "Standard PC",
-    .init = pc_init_pci,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        }, {
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        { /* end of list */ }
-    },
-};
-
-static QEMUMachine pc_machine_v0_14 = {
-    .name = "pc-0.14",
-    .desc = "Standard PC",
-    .init = pc_init_pci,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "qxl",
-            .property = "revision",
-            .value    = stringify(2),
-        },{
-            .driver   = "qxl-vga",
-            .property = "revision",
-            .value    = stringify(2),
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-balloon-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    },
-};
-
-static QEMUMachine pc_machine_v0_13 = {
-    .name = "pc-0.13",
-    .desc = "Standard PC",
-    .init = pc_init_pci_no_kvmclock,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "virtio-9p-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "VGA",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "vmware-svga",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "PCI",
-            .property = "command_serr_enable",
-            .value    = "off",
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-balloon-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "AC97",
-            .property = "use_broken_id",
-            .value    = stringify(1),
-        },{
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    },
-};
-
-static QEMUMachine pc_machine_v0_12 = {
-    .name = "pc-0.12",
-    .desc = "Standard PC",
-    .init = pc_init_pci_no_kvmclock,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "virtio-serial-pci",
-            .property = "max_ports",
-            .value    = stringify(1),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "VGA",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "vmware-svga",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "PCI",
-            .property = "command_serr_enable",
-            .value    = "off",
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-balloon-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "AC97",
-            .property = "use_broken_id",
-            .value    = stringify(1),
-        },{
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    }
-};
-
-static QEMUMachine pc_machine_v0_11 = {
-    .name = "pc-0.11",
-    .desc = "Standard PC, qemu 0.11",
-    .init = pc_init_pci_no_kvmclock,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "virtio-blk-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "max_ports",
-            .value    = stringify(1),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "ide-drive",
-            .property = "ver",
-            .value    = "0.11",
-        },{
-            .driver   = "scsi-disk",
-            .property = "ver",
-            .value    = "0.11",
-        },{
-            .driver   = "PCI",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "PCI",
-            .property = "command_serr_enable",
-            .value    = "off",
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-balloon-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "AC97",
-            .property = "use_broken_id",
-            .value    = stringify(1),
-        },{
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    }
-};
-
-static QEMUMachine pc_machine_v0_10 = {
-    .name = "pc-0.10",
-    .desc = "Standard PC, qemu 0.10",
-    .init = pc_init_pci_no_kvmclock,
-    .max_cpus = 255,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "virtio-blk-pci",
-            .property = "class",
-            .value    = stringify(PCI_CLASS_STORAGE_OTHER),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "class",
-            .value    = stringify(PCI_CLASS_DISPLAY_OTHER),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "max_ports",
-            .value    = stringify(1),
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "vectors",
-            .value    = stringify(0),
-        },{
-            .driver   = "ide-drive",
-            .property = "ver",
-            .value    = "0.10",
-        },{
-            .driver   = "scsi-disk",
-            .property = "ver",
-            .value    = "0.10",
-        },{
-            .driver   = "PCI",
-            .property = "rombar",
-            .value    = stringify(0),
-        },{
-            .driver   = "PCI",
-            .property = "command_serr_enable",
-            .value    = "off",
-        },{
-            .driver   = "virtio-blk-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-serial-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-net-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "virtio-balloon-pci",
-            .property = "event_idx",
-            .value    = "off",
-        },{
-            .driver   = "AC97",
-            .property = "use_broken_id",
-            .value    = stringify(1),
-        },{
-            .driver   = "isa-fdc",
-            .property = "check_media_rate",
-            .value    = "off",
-        },
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    },
-};
-
-static QEMUMachine isapc_machine = {
-    .name = "isapc",
-    .desc = "ISA-only PC",
-    .init = pc_init_isa,
-    .max_cpus = 1,
-    .compat_props = (GlobalProperty[]) {
-        {
-            .driver   = "pc-sysfw",
-            .property = "rom_only",
-            .value    = stringify(1),
-        },
-        { /* end of list */ }
-    },
-};
-
-#ifdef CONFIG_XEN
-static QEMUMachine xenfv_machine = {
-    .name = "xenfv",
-    .desc = "Xen Fully-virtualized PC",
-    .init = pc_xen_hvm_init,
-    .max_cpus = HVM_MAX_VCPUS,
-    .default_machine_opts = "accel=xen",
-};
-#endif
-
-static void pc_machine_init(void)
-{
-    qemu_register_machine(&pc_machine_v1_1);
-    qemu_register_machine(&pc_machine_v1_0);
-    qemu_register_machine(&pc_machine_v0_15);
-    qemu_register_machine(&pc_machine_v0_14);
-    qemu_register_machine(&pc_machine_v0_13);
-    qemu_register_machine(&pc_machine_v0_12);
-    qemu_register_machine(&pc_machine_v0_11);
-    qemu_register_machine(&pc_machine_v0_10);
-    qemu_register_machine(&isapc_machine);
-#ifdef CONFIG_XEN
-    qemu_register_machine(&xenfv_machine);
-#endif
-}
-
-machine_init(pc_machine_init);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Qemu-devel] [PATCH 6/6] make some functions static
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (4 preceding siblings ...)
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c Wanpeng Li
@ 2012-03-26  2:06 ` Wanpeng Li
  2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  2:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Wanpeng Li, Anthony Liguori, Avi Kivity, Gavin Shan


From: Anthony Liguori <aliguori@us.ibm.com>

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 hw/pc.c |   22 +++++++++++-----------
 hw/pc.h |   26 --------------------------
 2 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index d5a557e..5f93643 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -98,7 +98,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 static struct e820_table e820_table;
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
-void gsi_handler(void *opaque, int n, int level)
+static void gsi_handler(void *opaque, int n, int level)
 {
     GSIState *s = opaque;
 
@@ -116,7 +116,7 @@ static void ioport80_write(void *opaque, uint32_t addr, uint32_t data)
 /* MSDOS compatibility mode FPU exception support */
 static qemu_irq ferr_irq;
 
-void pc_register_ferr_irq(qemu_irq irq)
+static void pc_register_ferr_irq(qemu_irq irq)
 {
     ferr_irq = irq;
 }
@@ -339,7 +339,7 @@ static void pc_cmos_init_late(void *opaque)
     qemu_unregister_reset(pc_cmos_init_late, opaque);
 }
 
-void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
+static void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
                   const char *boot_device,
                   ISADevice *floppy, BusState *idebus0, BusState *idebus1,
                   ISADevice *s)
@@ -869,7 +869,7 @@ static const int ne2000_irq[NE2000_NB_MAX] = { 9, 10, 11, 3, 4, 5 };
 static const int parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 static const int parallel_irq[MAX_PARALLEL_PORTS] = { 7, 7, 7 };
 
-void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
+static void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
 {
     static int nb_ne2k = 0;
 
@@ -926,7 +926,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
     return dev;
 }
 
-void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
+static void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 {
     CPUX86State *s = opaque;
 
@@ -960,7 +960,7 @@ static CPUX86State *pc_new_cpu(const char *cpu_model)
     return env;
 }
 
-void pc_cpus_init(const char *cpu_model)
+static void pc_cpus_init(const char *cpu_model)
 {
     int i;
 
@@ -978,7 +978,7 @@ void pc_cpus_init(const char *cpu_model)
     }
 }
 
-void pc_memory_init(MemoryRegion *system_memory,
+static void pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
                     const char *kernel_cmdline,
                     const char *initrd_filename,
@@ -1002,12 +1002,12 @@ void pc_memory_init(MemoryRegion *system_memory,
     }
 }
 
-qemu_irq *pc_allocate_cpu_irq(void)
+static qemu_irq *pc_allocate_cpu_irq(void)
 {
     return qemu_allocate_irqs(pic_irq_request, NULL, 1);
 }
 
-DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
+static DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
 {
     DeviceState *dev = NULL;
 
@@ -1051,7 +1051,7 @@ static void cpu_request_exit(void *opaque, int irq, int level)
     }
 }
 
-void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
+static void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **floppy,
                           bool no_vmport)
 {
@@ -1102,7 +1102,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     *floppy = fdctrl_init_isa(isa_bus, fd);
 }
 
-void pc_pci_device_init(PCIBus *pci_bus)
+static void pc_pci_device_init(PCIBus *pci_bus)
 {
     int max_bus;
     int bus;
diff --git a/hw/pc.h b/hw/pc.h
index 7348da2..89f78dd 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -80,8 +80,6 @@ typedef struct GSIState {
     qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
 } GSIState;
 
-void gsi_handler(void *opaque, int n, int level);
-
 /* vmport.c */
 static inline void vmport_init(ISABus *bus)
 {
@@ -103,30 +101,6 @@ void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out);
 /* pc.c */
 extern int fd_bootchk;
 
-void pc_register_ferr_irq(qemu_irq irq);
-void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-
-void pc_cpus_init(const char *cpu_model);
-void pc_memory_init(MemoryRegion *system_memory,
-                    const char *kernel_filename,
-                    const char *kernel_cmdline,
-                    const char *initrd_filename,
-                    ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory);
-qemu_irq *pc_allocate_cpu_irq(void);
-DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
-void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **floppy,
-                          bool no_vmport);
-void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
-void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
-                  const char *boot_device,
-                  ISADevice *floppy, BusState *ide0, BusState *ide1,
-                  ISADevice *s);
-void pc_pci_device_init(PCIBus *pci_bus);
-
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
 void cpu_smm_register(cpu_set_smm_t callback, void *arg);
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
@ 2012-03-26  7:32   ` Stefan Hajnoczi
  2012-03-26  9:22   ` Wanpeng Li
  2012-03-26 14:25   ` Andreas Färber
  2 siblings, 0 replies; 42+ messages in thread
From: Stefan Hajnoczi @ 2012-03-26  7:32 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Gavin Shan, Anthony Liguori, qemu-devel, Avi Kivity

On Mon, Mar 26, 2012 at 10:06:45AM +0800, Wanpeng Li wrote:
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 44c6c20..44d7e55 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
> +{
> +	object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
> +}
> +
> +static void pci_host_initfn(Object *obj)
> +{
> +    PCIHostState *s = PCI_HOST(obj);
> +
> +	object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
> +					         (Object **)&s->address_space, NULL);

This patch has tabs instead of 4-space indent.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
  2012-03-26  7:32   ` Stefan Hajnoczi
@ 2012-03-26  9:22   ` Wanpeng Li
  2012-03-26 14:25   ` Andreas Färber
  2 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26  9:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gavin Shan, Stefan Hajnoczi, Anthony Liguori, Avi Kivity, liwp

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

On Mon, Mar 26, 2012 at 10:06:45AM +0800, Wanpeng Li wrote:
>
>From: Anthony Liguori <aliguori@us.ibm.com>
>
>
>Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>
>---
> hw/pci_host.c |   26 ++++++++++++++++++++++++++
> hw/pci_host.h |    5 +++++
> 2 files changed, 31 insertions(+), 0 deletions(-)
>
>diff --git a/hw/pci_host.c b/hw/pci_host.c
>index 44c6c20..44d7e55 100644
>--- a/hw/pci_host.c
>+++ b/hw/pci_host.c
>@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
>     .endianness = DEVICE_BIG_ENDIAN,
> };
>
>+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
>+{
>+	object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
>+}
>+
>+static void pci_host_initfn(Object *obj)
>+{
>+    PCIHostState *s = PCI_HOST(obj);
>+
>+	object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
>+					         (Object **)&s->address_space, NULL);
>+}
>+
>+static TypeInfo pci_host_type = {
>+    .name = TYPE_PCI_HOST,
>+    .parent = TYPE_SYS_BUS_DEVICE,
>+	.instance_size = sizeof(PCIHostState),
>+	.instance_init = pci_host_initfn,
>+};
>+
>+static void register_devices(void)
>+{
>+	type_register_static(&pci_host_type);
>+}
>+
>+type_init(register_devices);
>
>diff --git a/hw/pci_host.h b/hw/pci_host.h
>index 359e38f..084e15c 100644
>--- a/hw/pci_host.h
>+++ b/hw/pci_host.h
>@@ -30,6 +30,9 @@
>
> #include "sysbus.h"
>
>+#define TYPE_PCI_HOST "pci-host"
>+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
>+
> struct PCIHostState {
>     SysBusDevice busdev;
>     MemoryRegion conf_mem;
>@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
>
>+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
>+
> extern const MemoryRegionOps pci_host_conf_le_ops;
> extern const MemoryRegionOps pci_host_conf_be_ops;
> extern const MemoryRegionOps pci_host_data_le_ops;
>-- 
>1.7.5.4
>



[-- Attachment #2: 0003.patch --]
[-- Type: text/x-diff, Size: 2161 bytes --]

>From 72bc193e6e25cb393437317843a701b82a9b9233 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <liwp@linux.vnet.ibm.com>
Date: Thu, 22 Mar 2012 17:57:30 +0800
Subject: [PATCH 3/6] convert pci-host to QOM


From: Anthony Liguori <aliguori@us.ibm.com>


Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

---
 hw/pci_host.c |   26 ++++++++++++++++++++++++++
 hw/pci_host.h |    5 +++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..44d7e55 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+    object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+    PCIHostState *s = PCI_HOST(obj);
+
+    object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
+					         (Object **)&s->address_space, NULL);
+}
+
+static TypeInfo pci_host_type = {
+    .name = TYPE_PCI_HOST,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PCIHostState),
+    .instance_init = pci_host_initfn,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&pci_host_type);
+}
+
+type_init(register_devices);
 
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..084e15c 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
     SysBusDevice busdev;
     MemoryRegion conf_mem;
@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (5 preceding siblings ...)
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 6/6] make some functions static Wanpeng Li
@ 2012-03-26 12:20 ` Jan Kiszka
  2012-03-26 15:54   ` Isaku Yamahata
                     ` (2 more replies)
  2012-03-26 12:47 ` Andreas Färber
                   ` (2 subsequent siblings)
  9 siblings, 3 replies; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 12:20 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Anthony Liguori, Gavin Shan, qemu-devel, Isaku Yamahata,
	Avi Kivity, Paolo Bonzini

On 2012-03-26 04:06, Wanpeng Li wrote:
> From: Anthony Liguori <aliguori@us.ibm.com>
> 
> 
> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:
> 
>  1) Things like -m and -bios-name are now device model properties
> 
>  2) The i440fx and piix3 are now modelled in a thorough fashion
> 
>  3) Most of the chipset features of the piix3 are modelled through composition
> 
>  4) i440fx_init is trivialized to creating devices and setting properties
> 
>  5) convert MemoryRegion to QOM
> 
>  6) convert PCI host bridge to QOM
> 
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely in
> favor of just creating a handful of devices.
> 
> The two stage initialization of QOM is important here.  instance_init() is when
> composed devices are created which means that after you've created a device, all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
> 
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.

While I see the value of the overall direction, I still disagree on
making internal data structures of HPET, RTC and 8254 publicly
available. That's a wrong step back. I'm sure there are smarter
solutions, alse as there were some proposals back then in the original
thread.

I'm also sure we will have to refactor the merge significantly again for
the introduction of additional chipsets and PC boards. But unless those
requirements can already be specified (Isaku?), that might be unavoidable.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c Wanpeng Li
@ 2012-03-26 12:42   ` Avi Kivity
  2012-03-26 12:47     ` Jan Kiszka
  0 siblings, 1 reply; 42+ messages in thread
From: Avi Kivity @ 2012-03-26 12:42 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Anthony Liguori, qemu-devel, Gavin Shan

On 03/26/2012 04:06 AM, Wanpeng Li wrote:
> From: Anthony Liguori <aliguori@us.ibm.com>
>
> @@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>      DeviceState *dev;
>      static int apic_mapped;
>  
> -    if (kvm_irqchip_in_kernel()) {
> +    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>          dev = qdev_create(NULL, "kvm-apic");
>      } else {
>          dev = qdev_create(NULL, "apic");
> @@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>      }
>  
>      /* KVM does not support MSI yet. */
> -    if (!kvm_irqchip_in_kernel()) {
> +    if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>          msi_supported = true;

Why these changes?

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (6 preceding siblings ...)
  2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
@ 2012-03-26 12:47 ` Andreas Färber
  2012-03-26 12:57   ` Wanpeng Li
  2012-03-26 17:09 ` Blue Swirl
  2012-03-26 17:25 ` Anthony Liguori
  9 siblings, 1 reply; 42+ messages in thread
From: Andreas Färber @ 2012-03-26 12:47 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Gavin Shan, Anthony Liguori, qemu-devel, Avi Kivity

Am 26.03.2012 04:06, schrieb Wanpeng Li:
> From: Anthony Liguori <aliguori@us.ibm.com>

Resending an old cover letter is not a good idea. This looks like a v2,
so please mark it as such in the subjects; it's missing a Change Log
against Anthony's version. I take it, some patches were dropped?

> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:
> 
>  1) Things like -m and -bios-name are now device model properties
> 
>  2) The i440fx and piix3 are now modelled in a thorough fashion
> 
>  3) Most of the chipset features of the piix3 are modelled through composition
> 
>  4) i440fx_init is trivialized to creating devices and setting properties
> 
>  5) convert MemoryRegion to QOM
> 
>  6) convert PCI host bridge to QOM
> 
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely in
> favor of just creating a handful of devices.

I agree that machine->init needs to be refactored, however I don't think
it'll disappear, just be moved into initfn/realize functions.

Andreas

> 
> The two stage initialization of QOM is important here.  instance_init() is when
> composed devices are created which means that after you've created a device, all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
> 
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> ---
>  Makefile.target     |    3 +-
>  hw/hpet.c           |   39 +---
>  hw/hpet_emul.h      |   41 +++
>  hw/i440fx.c         |  431 ++++++++++++++++++++++++++
>  hw/i440fx.h         |   78 +++++
>  hw/i8254_internal.h |    2 +-
>  hw/mc146818rtc.c    |   26 --
>  hw/mc146818rtc.h    |   29 ++
>  hw/pc.c             |  838 +++++++++++++++++++++++++++++++++++++++++++++------
>  hw/pc.h             |   46 +---
>  hw/pc_piix.c        |  762 ----------------------------------------------
>  hw/pci_host.c       |   26 ++
>  hw/pci_host.h       |    5 +
>  hw/piix3.c          |  274 +++++++++++++++++
>  hw/piix3.h          |   79 +++++
>  hw/piix_pci.c       |  600 ------------------------------------
>  memory.c            |   94 +++++--
>  memory.h            |    8 +
>  18 files changed, 1795 insertions(+), 1586 deletions(-)
>  create mode 100644 hw/i440fx.c
>  create mode 100644 hw/i440fx.h
>  delete mode 100644 hw/pc_piix.c
>  create mode 100644 hw/piix3.c
>  create mode 100644 hw/piix3.h
>  delete mode 100644 hw/piix_pci.c
> --

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c
  2012-03-26 12:42   ` Avi Kivity
@ 2012-03-26 12:47     ` Jan Kiszka
  2012-03-26 17:37       ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 12:47 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Gavin Shan, Anthony Liguori, Wanpeng Li, qemu-devel

On 2012-03-26 14:42, Avi Kivity wrote:
> On 03/26/2012 04:06 AM, Wanpeng Li wrote:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>>
>> @@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>      DeviceState *dev;
>>      static int apic_mapped;
>>  
>> -    if (kvm_irqchip_in_kernel()) {
>> +    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>>          dev = qdev_create(NULL, "kvm-apic");
>>      } else {
>>          dev = qdev_create(NULL, "apic");
>> @@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>      }
>>  
>>      /* KVM does not support MSI yet. */
>> -    if (!kvm_irqchip_in_kernel()) {
>> +    if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>>          msi_supported = true;
> 
> Why these changes?
> 

Yep, they are obsolete, likely related to the rebase of the original
patch. A lot of code is moved around here, and I bet there are more
artifacts...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 12:47 ` Andreas Färber
@ 2012-03-26 12:57   ` Wanpeng Li
  0 siblings, 0 replies; 42+ messages in thread
From: Wanpeng Li @ 2012-03-26 12:57 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel

On Mon, Mar 26, 2012 at 02:47:19PM +0200, Andreas Färber wrote:
>Am 26.03.2012 04:06, schrieb Wanpeng Li:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>
>Resending an old cover letter is not a good idea. This looks like a v2,
>so please mark it as such in the subjects; it's missing a Change Log
>against Anthony's version. I take it, some patches were dropped?
>
No, I just help him rebase his patches.

>> This series aggressively refactors the PC machine initialization to be more
>> modelled and less ad-hoc.  The highlights of this series are:
>> 
>>  1) Things like -m and -bios-name are now device model properties
>> 
>>  2) The i440fx and piix3 are now modelled in a thorough fashion
>> 
>>  3) Most of the chipset features of the piix3 are modelled through composition
>> 
>>  4) i440fx_init is trivialized to creating devices and setting properties
>> 
>>  5) convert MemoryRegion to QOM
>> 
>>  6) convert PCI host bridge to QOM
>> 
>> The point (4) is the most important one.  As we refactor in this fashion,
>> we should quickly get to the point where machine->init disappears completely in
>> favor of just creating a handful of devices.
>
>I agree that machine->init needs to be refactored, however I don't think
>it'll disappear, just be moved into initfn/realize functions.
>
>Andreas
>
>> 
>> The two stage initialization of QOM is important here.  instance_init() is when
>> composed devices are created which means that after you've created a device, all
>> of its children are visible in the device model.  This lets you set properties
>> of the parent and its children.
>> 
>> realize() (which is still called DeviceState::init today) will be called right
>> before the guest starts up for the first time.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> 
>> ---
>>  Makefile.target     |    3 +-
>>  hw/hpet.c           |   39 +---
>>  hw/hpet_emul.h      |   41 +++
>>  hw/i440fx.c         |  431 ++++++++++++++++++++++++++
>>  hw/i440fx.h         |   78 +++++
>>  hw/i8254_internal.h |    2 +-
>>  hw/mc146818rtc.c    |   26 --
>>  hw/mc146818rtc.h    |   29 ++
>>  hw/pc.c             |  838 +++++++++++++++++++++++++++++++++++++++++++++------
>>  hw/pc.h             |   46 +---
>>  hw/pc_piix.c        |  762 ----------------------------------------------
>>  hw/pci_host.c       |   26 ++
>>  hw/pci_host.h       |    5 +
>>  hw/piix3.c          |  274 +++++++++++++++++
>>  hw/piix3.h          |   79 +++++
>>  hw/piix_pci.c       |  600 ------------------------------------
>>  memory.c            |   94 +++++--
>>  memory.h            |    8 +
>>  18 files changed, 1795 insertions(+), 1586 deletions(-)
>>  create mode 100644 hw/i440fx.c
>>  create mode 100644 hw/i440fx.h
>>  delete mode 100644 hw/pc_piix.c
>>  create mode 100644 hw/piix3.c
>>  create mode 100644 hw/piix3.h
>>  delete mode 100644 hw/piix_pci.c
>> --
>
>-- 
>SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

-- 
LTC China, IBM, Shanghai

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM
  2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
  2012-03-26  7:32   ` Stefan Hajnoczi
  2012-03-26  9:22   ` Wanpeng Li
@ 2012-03-26 14:25   ` Andreas Färber
  2 siblings, 0 replies; 42+ messages in thread
From: Andreas Färber @ 2012-03-26 14:25 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Gavin Shan, Anthony Liguori, qemu-devel, Avi Kivity

Am 26.03.2012 04:06, schrieb Wanpeng Li:
> From: Anthony Liguori <aliguori@us.ibm.com>
> 
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>

Some minor formal comments inline, otherwise looks okay.

> ---
>  hw/pci_host.c |   26 ++++++++++++++++++++++++++
>  hw/pci_host.h |    5 +++++
>  2 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 44c6c20..44d7e55 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
> +{
> +	object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
> +}
> +
> +static void pci_host_initfn(Object *obj)
> +{
> +    PCIHostState *s = PCI_HOST(obj);
> +
> +	object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
> +					         (Object **)&s->address_space, NULL);
> +}
> +
> +static TypeInfo pci_host_type = {

I thought the convention was ..._type_info (in case we ever need to do a
mass conversion again). And please make it static const.

> +    .name = TYPE_PCI_HOST,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +	.instance_size = sizeof(PCIHostState),
> +	.instance_init = pci_host_initfn,
> +};
> +
> +static void register_devices(void)

pci_host_register_types please.

> +{
> +	type_register_static(&pci_host_type);
> +}
> +
> +type_init(register_devices);

No semicolon please, it's not a statement.

There's still some tabs left in the revised version, please run
script/checkpatch.pl and repost a v3 inline so that we can comment on it.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
@ 2012-03-26 15:54   ` Isaku Yamahata
  2012-03-26 17:29     ` Anthony Liguori
  2012-03-26 17:17   ` Blue Swirl
  2012-03-26 17:33   ` Anthony Liguori
  2 siblings, 1 reply; 42+ messages in thread
From: Isaku Yamahata @ 2012-03-26 15:54 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Anthony Liguori, Wanpeng Li, qemu-devel, Avi Kivity,
	Paolo Bonzini, Gavin Shan

On Mon, Mar 26, 2012 at 02:20:24PM +0200, Jan Kiszka wrote:
> I'm also sure we will have to refactor the merge significantly again for
> the introduction of additional chipsets and PC boards. But unless those
> requirements can already be specified (Isaku?), that might be unavoidable.

Agreed. At least I'd like pam/smram stuff decoupled from piix.
It is difficult to understand why patches of 5, 6 are needed without further
patches because they don't show how the code will look like eventually.
Anyway I expect some sort of refactor again and am waiting for QOM related
stuff stabilizing.

thanks,
-- 
yamahata

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (7 preceding siblings ...)
  2012-03-26 12:47 ` Andreas Färber
@ 2012-03-26 17:09 ` Blue Swirl
  2012-03-26 17:35   ` Anthony Liguori
  2012-03-26 17:25 ` Anthony Liguori
  9 siblings, 1 reply; 42+ messages in thread
From: Blue Swirl @ 2012-03-26 17:09 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Gavin Shan, Anthony Liguori, qemu-devel, Avi Kivity

On Mon, Mar 26, 2012 at 02:06, Wanpeng Li <liwp@linux.vnet.ibm.com> wrote:
>
> From: Anthony Liguori <aliguori@us.ibm.com>
>
>
> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:

Please fix coding style while moving.

>  1) Things like -m and -bios-name are now device model properties
>
>  2) The i440fx and piix3 are now modelled in a thorough fashion
>
>  3) Most of the chipset features of the piix3 are modelled through composition
>
>  4) i440fx_init is trivialized to creating devices and setting properties
>
>  5) convert MemoryRegion to QOM
>
>  6) convert PCI host bridge to QOM
>
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely in
> favor of just creating a handful of devices.
>
> The two stage initialization of QOM is important here.  instance_init() is when
> composed devices are created which means that after you've created a device, all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
>
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>
> ---
>  Makefile.target     |    3 +-
>  hw/hpet.c           |   39 +---
>  hw/hpet_emul.h      |   41 +++
>  hw/i440fx.c         |  431 ++++++++++++++++++++++++++
>  hw/i440fx.h         |   78 +++++
>  hw/i8254_internal.h |    2 +-
>  hw/mc146818rtc.c    |   26 --
>  hw/mc146818rtc.h    |   29 ++
>  hw/pc.c             |  838 +++++++++++++++++++++++++++++++++++++++++++++------
>  hw/pc.h             |   46 +---
>  hw/pc_piix.c        |  762 ----------------------------------------------
>  hw/pci_host.c       |   26 ++
>  hw/pci_host.h       |    5 +
>  hw/piix3.c          |  274 +++++++++++++++++
>  hw/piix3.h          |   79 +++++
>  hw/piix_pci.c       |  600 ------------------------------------
>  memory.c            |   94 +++++--
>  memory.h            |    8 +
>  18 files changed, 1795 insertions(+), 1586 deletions(-)
>  create mode 100644 hw/i440fx.c
>  create mode 100644 hw/i440fx.h
>  delete mode 100644 hw/pc_piix.c
>  create mode 100644 hw/piix3.c
>  create mode 100644 hw/piix3.h
>  delete mode 100644 hw/piix_pci.c
> --
>
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
  2012-03-26 15:54   ` Isaku Yamahata
@ 2012-03-26 17:17   ` Blue Swirl
  2012-03-26 17:33   ` Anthony Liguori
  2 siblings, 0 replies; 42+ messages in thread
From: Blue Swirl @ 2012-03-26 17:17 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Anthony Liguori, Wanpeng Li, qemu-devel, Isaku Yamahata,
	Avi Kivity, Paolo Bonzini, Gavin Shan

On Mon, Mar 26, 2012 at 12:20, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2012-03-26 04:06, Wanpeng Li wrote:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>>
>>
>> This series aggressively refactors the PC machine initialization to be more
>> modelled and less ad-hoc.  The highlights of this series are:
>>
>>  1) Things like -m and -bios-name are now device model properties
>>
>>  2) The i440fx and piix3 are now modelled in a thorough fashion
>>
>>  3) Most of the chipset features of the piix3 are modelled through composition
>>
>>  4) i440fx_init is trivialized to creating devices and setting properties
>>
>>  5) convert MemoryRegion to QOM
>>
>>  6) convert PCI host bridge to QOM
>>
>> The point (4) is the most important one.  As we refactor in this fashion,
>> we should quickly get to the point where machine->init disappears completely in
>> favor of just creating a handful of devices.
>>
>> The two stage initialization of QOM is important here.  instance_init() is when
>> composed devices are created which means that after you've created a device, all
>> of its children are visible in the device model.  This lets you set properties
>> of the parent and its children.
>>
>> realize() (which is still called DeviceState::init today) will be called right
>> before the guest starts up for the first time.
>
> While I see the value of the overall direction, I still disagree on
> making internal data structures of HPET, RTC and 8254 publicly
> available. That's a wrong step back. I'm sure there are smarter
> solutions, alse as there were some proposals back then in the original
> thread.

Fully agree. At least there should be something like this memory.c line:

/* All fields are private - violators will be prosecuted */

or other C++-ish comments about the fields being private to the class.
Device classes should not have friend classes except in exceptional
cases like LAPIC/RTC.

> I'm also sure we will have to refactor the merge significantly again for
> the introduction of additional chipsets and PC boards. But unless those
> requirements can already be specified (Isaku?), that might be unavoidable.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
                   ` (8 preceding siblings ...)
  2012-03-26 17:09 ` Blue Swirl
@ 2012-03-26 17:25 ` Anthony Liguori
  9 siblings, 0 replies; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:25 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Gavin Shan, Anthony Liguori, qemu-devel, Avi Kivity

On 03/25/2012 09:06 PM, Wanpeng Li wrote:
> From: Anthony Liguori<aliguori@us.ibm.com>
>
>
> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:
>
>   1) Things like -m and -bios-name are now device model properties
>
>   2) The i440fx and piix3 are now modelled in a thorough fashion
>
>   3) Most of the chipset features of the piix3 are modelled through composition
>
>   4) i440fx_init is trivialized to creating devices and setting properties
>
>   5) convert MemoryRegion to QOM
>
>   6) convert PCI host bridge to QOM
>
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely in
> favor of just creating a handful of devices.
>
> The two stage initialization of QOM is important here.  instance_init() is when
> composed devices are created which means that after you've created a device, all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
>
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>

I should add that there's a short term focus on refactoring the i440fx in such a 
way that we can introduce a new chipset (Q35).

Regards,

Anthony Liguori

>
> ---
>   Makefile.target     |    3 +-
>   hw/hpet.c           |   39 +---
>   hw/hpet_emul.h      |   41 +++
>   hw/i440fx.c         |  431 ++++++++++++++++++++++++++
>   hw/i440fx.h         |   78 +++++
>   hw/i8254_internal.h |    2 +-
>   hw/mc146818rtc.c    |   26 --
>   hw/mc146818rtc.h    |   29 ++
>   hw/pc.c             |  838 +++++++++++++++++++++++++++++++++++++++++++++------
>   hw/pc.h             |   46 +---
>   hw/pc_piix.c        |  762 ----------------------------------------------
>   hw/pci_host.c       |   26 ++
>   hw/pci_host.h       |    5 +
>   hw/piix3.c          |  274 +++++++++++++++++
>   hw/piix3.h          |   79 +++++
>   hw/piix_pci.c       |  600 ------------------------------------
>   memory.c            |   94 +++++--
>   memory.h            |    8 +
>   18 files changed, 1795 insertions(+), 1586 deletions(-)
>   create mode 100644 hw/i440fx.c
>   create mode 100644 hw/i440fx.h
>   delete mode 100644 hw/pc_piix.c
>   create mode 100644 hw/piix3.c
>   create mode 100644 hw/piix3.h
>   delete mode 100644 hw/piix_pci.c
> --
>
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 15:54   ` Isaku Yamahata
@ 2012-03-26 17:29     ` Anthony Liguori
  2012-03-27 10:31       ` Avi Kivity
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:29 UTC (permalink / raw)
  To: Isaku Yamahata
  Cc: Anthony Liguori, Wanpeng Li, Jan Kiszka, qemu-devel, Avi Kivity,
	Paolo Bonzini, Gavin Shan

On 03/26/2012 10:54 AM, Isaku Yamahata wrote:
> On Mon, Mar 26, 2012 at 02:20:24PM +0200, Jan Kiszka wrote:
>> I'm also sure we will have to refactor the merge significantly again for
>> the introduction of additional chipsets and PC boards. But unless those
>> requirements can already be specified (Isaku?), that might be unavoidable.
>
> Agreed. At least I'd like pam/smram stuff decoupled from piix.

s/piix/i440fx/

PAM/SRAM has nothing do to with the piix.  Part of the problem with the current 
layout is that the distinction between i440fx and piix is not clear.  The piix 
is just a SuperIO chip (and southbridge).

I think the better approach is to have a PCNorthBridge base-class that contains 
functionality like PAM/SRAM that both I440FX and Q35 inherit from.

Obviously, without getting the I440FX/PIIX3 modeling in shape, we can't look 
reasonable at how to share code.

Regards,

Anthony Liguori

> It is difficult to understand why patches of 5, 6 are needed without further
> patches because they don't show how the code will look like eventually.
> Anyway I expect some sort of refactor again and am waiting for QOM related
> stuff stabilizing.
>
> thanks,

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
  2012-03-26 15:54   ` Isaku Yamahata
  2012-03-26 17:17   ` Blue Swirl
@ 2012-03-26 17:33   ` Anthony Liguori
  2012-03-26 19:30     ` Jan Kiszka
  2 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:33 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Anthony Liguori, Wanpeng Li, qemu-devel, Isaku Yamahata,
	Avi Kivity, Paolo Bonzini, Gavin Shan

On 03/26/2012 07:20 AM, Jan Kiszka wrote:
> On 2012-03-26 04:06, Wanpeng Li wrote:
>> From: Anthony Liguori<aliguori@us.ibm.com>
>>
>>
>> This series aggressively refactors the PC machine initialization to be more
>> modelled and less ad-hoc.  The highlights of this series are:
>>
>>   1) Things like -m and -bios-name are now device model properties
>>
>>   2) The i440fx and piix3 are now modelled in a thorough fashion
>>
>>   3) Most of the chipset features of the piix3 are modelled through composition
>>
>>   4) i440fx_init is trivialized to creating devices and setting properties
>>
>>   5) convert MemoryRegion to QOM
>>
>>   6) convert PCI host bridge to QOM
>>
>> The point (4) is the most important one.  As we refactor in this fashion,
>> we should quickly get to the point where machine->init disappears completely in
>> favor of just creating a handful of devices.
>>
>> The two stage initialization of QOM is important here.  instance_init() is when
>> composed devices are created which means that after you've created a device, all
>> of its children are visible in the device model.  This lets you set properties
>> of the parent and its children.
>>
>> realize() (which is still called DeviceState::init today) will be called right
>> before the guest starts up for the first time.
>
> While I see the value of the overall direction, I still disagree on
> making internal data structures of HPET, RTC and 8254 publicly
> available. That's a wrong step back. I'm sure there are smarter
> solutions, alse as there were some proposals back then in the original
> thread.

I'm not fully decided myself.  A couple things are clear to me though:

1) We must expose type proper types in header files.  We need there to be a 
globally accessible RTCState type and functions that operate on it.

2) We can simplify memory management by knowing the size of the type in the 
header files too.

Since this is an easily refactorable thing to look at later, I think we should 
start with extracting the types.

>
> I'm also sure we will have to refactor the merge significantly again for
> the introduction of additional chipsets and PC boards. But unless those
> requirements can already be specified (Isaku?), that might be unavoidable.

We cannot introduce another chipset without properly refactoring the i440fx. 
The previous refactorings that were oriented around moving code into functions 
created a nasty spaghetti of reference passing.

Regards,

Anthony Liguori

> Jan
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:09 ` Blue Swirl
@ 2012-03-26 17:35   ` Anthony Liguori
  2012-03-26 17:43     ` Blue Swirl
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:35 UTC (permalink / raw)
  To: Blue Swirl
  Cc: qemu-devel, Anthony Liguori, Wanpeng Li, Avi Kivity, Gavin Shan

On 03/26/2012 12:09 PM, Blue Swirl wrote:
> On Mon, Mar 26, 2012 at 02:06, Wanpeng Li<liwp@linux.vnet.ibm.com>  wrote:
>>
>> From: Anthony Liguori<aliguori@us.ibm.com>
>>
>>
>> This series aggressively refactors the PC machine initialization to be more
>> modelled and less ad-hoc.  The highlights of this series are:
>
> Please fix coding style while moving.

I disagree.  That makes reviewing the movement and rebasing the movement pretty 
difficult.

If we were to fix the issues, it should before or after.  But in that context, I 
think it makes it orthogonal to moving the code and should be treated independently.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c
  2012-03-26 12:47     ` Jan Kiszka
@ 2012-03-26 17:37       ` Anthony Liguori
  0 siblings, 0 replies; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:37 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Wanpeng Li, Anthony Liguori, qemu-devel, Avi Kivity, Gavin Shan

On 03/26/2012 07:47 AM, Jan Kiszka wrote:
> On 2012-03-26 14:42, Avi Kivity wrote:
>> On 03/26/2012 04:06 AM, Wanpeng Li wrote:
>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>> @@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>>       DeviceState *dev;
>>>       static int apic_mapped;
>>>
>>> -    if (kvm_irqchip_in_kernel()) {
>>> +    if (kvm_enabled()&&  kvm_irqchip_in_kernel()) {
>>>           dev = qdev_create(NULL, "kvm-apic");
>>>       } else {
>>>           dev = qdev_create(NULL, "apic");
>>> @@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>>       }
>>>
>>>       /* KVM does not support MSI yet. */
>>> -    if (!kvm_irqchip_in_kernel()) {
>>> +    if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>>>           msi_supported = true;
>>
>> Why these changes?
>>
>
> Yep, they are obsolete, likely related to the rebase of the original
> patch. A lot of code is moved around here, and I bet there are more
> artifacts...

git should have thrown a rebase error here.  I think that means the conflicts 
were ignored.

For something like this, if there's a rebase error, you pretty much have to 
repeat the copy/paste of the code for the section that threw a rebase error.

I'd suggest rebasing again but this time, be a bit more careful when resolving 
conflicts.

Regards,

Anthony Liguori

>
> Jan
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:35   ` Anthony Liguori
@ 2012-03-26 17:43     ` Blue Swirl
  2012-03-26 17:45       ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Blue Swirl @ 2012-03-26 17:43 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: qemu-devel, Anthony Liguori, Wanpeng Li, Avi Kivity, Gavin Shan

On Mon, Mar 26, 2012 at 17:35, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/26/2012 12:09 PM, Blue Swirl wrote:
>>
>> On Mon, Mar 26, 2012 at 02:06, Wanpeng Li<liwp@linux.vnet.ibm.com>  wrote:
>>>
>>>
>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>>
>>> This series aggressively refactors the PC machine initialization to be
>>> more
>>> modelled and less ad-hoc.  The highlights of this series are:
>>
>>
>> Please fix coding style while moving.
>
>
> I disagree.  That makes reviewing the movement and rebasing the movement
> pretty difficult.

Yes, a separate step would be nice.

> If we were to fix the issues, it should before or after.  But in that
> context, I think it makes it orthogonal to moving the code and should be
> treated independently.

I'd fix the style in the first patch, then perform moves etc. That way
no patch would add noncompliant code, only remove.

> Regards,
>
> Anthony Liguori

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:43     ` Blue Swirl
@ 2012-03-26 17:45       ` Anthony Liguori
  2012-03-26 18:01         ` Blue Swirl
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 17:45 UTC (permalink / raw)
  To: Blue Swirl
  Cc: qemu-devel, Anthony Liguori, Wanpeng Li, Avi Kivity, Gavin Shan

On 03/26/2012 12:43 PM, Blue Swirl wrote:
> On Mon, Mar 26, 2012 at 17:35, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 03/26/2012 12:09 PM, Blue Swirl wrote:
>>>
>>> On Mon, Mar 26, 2012 at 02:06, Wanpeng Li<liwp@linux.vnet.ibm.com>    wrote:
>>>>
>>>>
>>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>>
>>>>
>>>> This series aggressively refactors the PC machine initialization to be
>>>> more
>>>> modelled and less ad-hoc.  The highlights of this series are:
>>>
>>>
>>> Please fix coding style while moving.
>>
>>
>> I disagree.  That makes reviewing the movement and rebasing the movement
>> pretty difficult.
>
> Yes, a separate step would be nice.
>
>> If we were to fix the issues, it should before or after.  But in that
>> context, I think it makes it orthogonal to moving the code and should be
>> treated independently.
>
> I'd fix the style in the first patch, then perform moves etc. That way
> no patch would add noncompliant code, only remove.

Is this something we universally want to do?  What would we do about patches to 
audio?

I'd prefer not to go down this road.  Let's keep discussion of fixing 
CODING_STYLE of existing code separate from rearchitecting/enhancing code.

Regards,

Anthony Liguori

>
>> Regards,
>>
>> Anthony Liguori

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:45       ` Anthony Liguori
@ 2012-03-26 18:01         ` Blue Swirl
  2012-03-26 18:07           ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Blue Swirl @ 2012-03-26 18:01 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: qemu-devel, Anthony Liguori, Wanpeng Li, Avi Kivity, Gavin Shan

On Mon, Mar 26, 2012 at 17:45, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/26/2012 12:43 PM, Blue Swirl wrote:
>>
>> On Mon, Mar 26, 2012 at 17:35, Anthony Liguori<anthony@codemonkey.ws>
>>  wrote:
>>>
>>> On 03/26/2012 12:09 PM, Blue Swirl wrote:
>>>>
>>>>
>>>> On Mon, Mar 26, 2012 at 02:06, Wanpeng Li<liwp@linux.vnet.ibm.com>
>>>>  wrote:
>>>>>
>>>>>
>>>>>
>>>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>>>
>>>>>
>>>>> This series aggressively refactors the PC machine initialization to be
>>>>> more
>>>>> modelled and less ad-hoc.  The highlights of this series are:
>>>>
>>>>
>>>>
>>>> Please fix coding style while moving.
>>>
>>>
>>>
>>> I disagree.  That makes reviewing the movement and rebasing the movement
>>> pretty difficult.
>>
>>
>> Yes, a separate step would be nice.
>>
>>> If we were to fix the issues, it should before or after.  But in that
>>> context, I think it makes it orthogonal to moving the code and should be
>>> treated independently.
>>
>>
>> I'd fix the style in the first patch, then perform moves etc. That way
>> no patch would add noncompliant code, only remove.
>
>
> Is this something we universally want to do?  What would we do about patches
> to audio?

I'd do it in cases when there is code movement, then git blame will
not be very useful anyway and other people have to rebase their
patches as well.

The audio case has an additional factor, namely maintainer disagreeing
with global style and consistency. There are several ways how to
handle that case, one of which is to maintain status quo.

> I'd prefer not to go down this road.  Let's keep discussion of fixing
> CODING_STYLE of existing code separate from rearchitecting/enhancing code.

When code is moved, rearchitected or enhanced, that would be a good
point when to fix style too. Though this assumes that just fixing
style without those events is evil, but is it? I think you have not
been fully consistent in this matter.

> Regards,
>
> Anthony Liguori
>
>>
>>> Regards,
>>>
>>> Anthony Liguori
>
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 18:01         ` Blue Swirl
@ 2012-03-26 18:07           ` Anthony Liguori
  2012-03-26 18:25             ` Blue Swirl
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 18:07 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel, Avi Kivity, Wanpeng Li, Gavin Shan

On 03/26/2012 01:01 PM, Blue Swirl wrote:
> On Mon, Mar 26, 2012 at 17:45, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>
>>
>> Is this something we universally want to do?  What would we do about patches
>> to audio?
>
> I'd do it in cases when there is code movement, then git blame will
> not be very useful anyway and other people have to rebase their
> patches as well.
>
> The audio case has an additional factor, namely maintainer disagreeing
> with global style and consistency. There are several ways how to
> handle that case, one of which is to maintain status quo.
>
>> I'd prefer not to go down this road.  Let's keep discussion of fixing
>> CODING_STYLE of existing code separate from rearchitecting/enhancing code.
>
> When code is moved, rearchitected or enhanced, that would be a good
> point when to fix style too. Though this assumes that just fixing
> style without those events is evil, but is it? I think you have not
> been fully consistent in this matter.

I think modifying coding style alone is evil.

But I'm also sick of arguing about coding style.  If you take this patch series 
as an example, this is the beginning of a fundamental refactoring to how we do 
machines and devices in QEMU--and yet, we're discussing coding style.

I don't see an obvious way to just get past the coding style discussions.  If 
there was a perfect way to automate fixing coding style, at this point, I would 
say let's do it.  But there is no way I want to spend the next two years taking 
coding style fixup patches.

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>
>>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 18:07           ` Anthony Liguori
@ 2012-03-26 18:25             ` Blue Swirl
  0 siblings, 0 replies; 42+ messages in thread
From: Blue Swirl @ 2012-03-26 18:25 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel, Avi Kivity, Wanpeng Li, Gavin Shan

On Mon, Mar 26, 2012 at 18:07, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/26/2012 01:01 PM, Blue Swirl wrote:
>>
>> On Mon, Mar 26, 2012 at 17:45, Anthony Liguori<anthony@codemonkey.ws>
>>  wrote:
>>>
>>>
>>>
>>> Is this something we universally want to do?  What would we do about
>>> patches
>>> to audio?
>>
>>
>> I'd do it in cases when there is code movement, then git blame will
>> not be very useful anyway and other people have to rebase their
>> patches as well.
>>
>> The audio case has an additional factor, namely maintainer disagreeing
>> with global style and consistency. There are several ways how to
>> handle that case, one of which is to maintain status quo.
>>
>>> I'd prefer not to go down this road.  Let's keep discussion of fixing
>>> CODING_STYLE of existing code separate from rearchitecting/enhancing
>>> code.
>>
>>
>> When code is moved, rearchitected or enhanced, that would be a good
>> point when to fix style too. Though this assumes that just fixing
>> style without those events is evil, but is it? I think you have not
>> been fully consistent in this matter.
>
>
> I think modifying coding style alone is evil.
>
> But I'm also sick of arguing about coding style.  If you take this patch
> series as an example, this is the beginning of a fundamental refactoring to
> how we do machines and devices in QEMU--and yet, we're discussing coding
> style.

Well, if for example this "fix style in the first patch, then move"
would be a widely accepted rule, there would be no discussion. Now we
can discuss forever if reformatting is needed or not.

> I don't see an obvious way to just get past the coding style discussions.
>  If there was a perfect way to automate fixing coding style, at this point,
> I would say let's do it.  But there is no way I want to spend the next two
> years taking coding style fixup patches.

I'd just reformat once, that would reduce fixup patches considerably.
I can also volunteer to review and apply all pure style fixup patches
if that helps.

> Regards,
>
> Anthony Liguori
>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>>
>>>>> Regards,
>>>>>
>>>>> Anthony Liguori
>>>
>>>
>>>
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:33   ` Anthony Liguori
@ 2012-03-26 19:30     ` Jan Kiszka
  2012-03-26 19:35       ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 19:30 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Anthony Liguori, Wanpeng Li, qemu-devel, Isaku Yamahata,
	Avi Kivity, Paolo Bonzini, Gavin Shan

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

On 2012-03-26 19:33, Anthony Liguori wrote:
> On 03/26/2012 07:20 AM, Jan Kiszka wrote:
>> On 2012-03-26 04:06, Wanpeng Li wrote:
>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>>
>>> This series aggressively refactors the PC machine initialization to be more
>>> modelled and less ad-hoc.  The highlights of this series are:
>>>
>>>   1) Things like -m and -bios-name are now device model properties
>>>
>>>   2) The i440fx and piix3 are now modelled in a thorough fashion
>>>
>>>   3) Most of the chipset features of the piix3 are modelled through composition
>>>
>>>   4) i440fx_init is trivialized to creating devices and setting properties
>>>
>>>   5) convert MemoryRegion to QOM
>>>
>>>   6) convert PCI host bridge to QOM
>>>
>>> The point (4) is the most important one.  As we refactor in this fashion,
>>> we should quickly get to the point where machine->init disappears completely in
>>> favor of just creating a handful of devices.
>>>
>>> The two stage initialization of QOM is important here.  instance_init() is when
>>> composed devices are created which means that after you've created a device, all
>>> of its children are visible in the device model.  This lets you set properties
>>> of the parent and its children.
>>>
>>> realize() (which is still called DeviceState::init today) will be called right
>>> before the guest starts up for the first time.
>>
>> While I see the value of the overall direction, I still disagree on
>> making internal data structures of HPET, RTC and 8254 publicly
>> available. That's a wrong step back. I'm sure there are smarter
>> solutions, alse as there were some proposals back then in the original
>> thread.
> 
> I'm not fully decided myself.  A couple things are clear to me though:
> 
> 1) We must expose type proper types in header files.  We need there to be a 
> globally accessible RTCState type and functions that operate on it.

I'm not sure what "proper type" means in this context, but I'm quite
sure that there should be no need for poking into the internal of the
class outside of mc146818rtc.c. We even abstracted the specifics of the
RTC away when it is embedded into a super-IO and interacts with an HPET.
If QOM requires such poking, then that requirement should be reassessed.

> 
> 2) We can simplify memory management by knowing the size of the type in the 
> header files too.

Is this more than a malloc-free pair?

> 
> Since this is an easily refactorable thing to look at later, I think we should 
> start with extracting the types.

My worry is that those three refactorings set bad examples for others.
So I'd like to avoid such back and forth if possible.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:30     ` Jan Kiszka
@ 2012-03-26 19:35       ` Anthony Liguori
  2012-03-26 19:37         ` Jan Kiszka
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 19:35 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

On 03/26/2012 02:30 PM, Jan Kiszka wrote:
> On 2012-03-26 19:33, Anthony Liguori wrote:
>> On 03/26/2012 07:20 AM, Jan Kiszka wrote:
>>> On 2012-03-26 04:06, Wanpeng Li wrote:
>>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>>
>>>>
>>>> This series aggressively refactors the PC machine initialization to be more
>>>> modelled and less ad-hoc.  The highlights of this series are:
>>>>
>>>>    1) Things like -m and -bios-name are now device model properties
>>>>
>>>>    2) The i440fx and piix3 are now modelled in a thorough fashion
>>>>
>>>>    3) Most of the chipset features of the piix3 are modelled through composition
>>>>
>>>>    4) i440fx_init is trivialized to creating devices and setting properties
>>>>
>>>>    5) convert MemoryRegion to QOM
>>>>
>>>>    6) convert PCI host bridge to QOM
>>>>
>>>> The point (4) is the most important one.  As we refactor in this fashion,
>>>> we should quickly get to the point where machine->init disappears completely in
>>>> favor of just creating a handful of devices.
>>>>
>>>> The two stage initialization of QOM is important here.  instance_init() is when
>>>> composed devices are created which means that after you've created a device, all
>>>> of its children are visible in the device model.  This lets you set properties
>>>> of the parent and its children.
>>>>
>>>> realize() (which is still called DeviceState::init today) will be called right
>>>> before the guest starts up for the first time.
>>>
>>> While I see the value of the overall direction, I still disagree on
>>> making internal data structures of HPET, RTC and 8254 publicly
>>> available. That's a wrong step back. I'm sure there are smarter
>>> solutions, alse as there were some proposals back then in the original
>>> thread.
>>
>> I'm not fully decided myself.  A couple things are clear to me though:
>>
>> 1) We must expose type proper types in header files.  We need there to be a
>> globally accessible RTCState type and functions that operate on it.
>
> I'm not sure what "proper type" means in this context, but I'm quite
> sure that there should be no need for poking into the internal of the
> class outside of mc146818rtc.c.

It needs to be at least a forward reference.  So we can avoid stuff like:

int apic_accept_pic_intr(DeviceState *s);

It should be:

int apic_accept_pic_intr(APICState *s);

So we can make use of the lovely type checking provided by the compiler to us.

> We even abstracted the specifics of the
> RTC away when it is embedded into a super-IO and interacts with an HPET.
> If QOM requires such poking, then that requirement should be reassessed.

There are a couple of ways to make types private while still having forward 
declarations.  None of them are straight forward.  That's why I suggest we save 
this for another day.

>>
>> 2) We can simplify memory management by knowing the size of the type in the
>> header files too.
>
> Is this more than a malloc-free pair?
>
>>
>> Since this is an easily refactorable thing to look at later, I think we should
>> start with extracting the types.
>
> My worry is that those three refactorings set bad examples for others.
> So I'd like to avoid such back and forth if possible.

I'm not really worried about it.  It's so easier to refactor this later.  Why 
rush it now?

Regards,

Anthony Liguori

> Jan

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:35       ` Anthony Liguori
@ 2012-03-26 19:37         ` Jan Kiszka
  2012-03-26 19:39           ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 19:37 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

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

On 2012-03-26 21:35, Anthony Liguori wrote:
> On 03/26/2012 02:30 PM, Jan Kiszka wrote:
>> On 2012-03-26 19:33, Anthony Liguori wrote:
>>> On 03/26/2012 07:20 AM, Jan Kiszka wrote:
>>>> On 2012-03-26 04:06, Wanpeng Li wrote:
>>>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>>>
>>>>>
>>>>> This series aggressively refactors the PC machine initialization to
>>>>> be more
>>>>> modelled and less ad-hoc.  The highlights of this series are:
>>>>>
>>>>>    1) Things like -m and -bios-name are now device model properties
>>>>>
>>>>>    2) The i440fx and piix3 are now modelled in a thorough fashion
>>>>>
>>>>>    3) Most of the chipset features of the piix3 are modelled
>>>>> through composition
>>>>>
>>>>>    4) i440fx_init is trivialized to creating devices and setting
>>>>> properties
>>>>>
>>>>>    5) convert MemoryRegion to QOM
>>>>>
>>>>>    6) convert PCI host bridge to QOM
>>>>>
>>>>> The point (4) is the most important one.  As we refactor in this
>>>>> fashion,
>>>>> we should quickly get to the point where machine->init disappears
>>>>> completely in
>>>>> favor of just creating a handful of devices.
>>>>>
>>>>> The two stage initialization of QOM is important here. 
>>>>> instance_init() is when
>>>>> composed devices are created which means that after you've created
>>>>> a device, all
>>>>> of its children are visible in the device model.  This lets you set
>>>>> properties
>>>>> of the parent and its children.
>>>>>
>>>>> realize() (which is still called DeviceState::init today) will be
>>>>> called right
>>>>> before the guest starts up for the first time.
>>>>
>>>> While I see the value of the overall direction, I still disagree on
>>>> making internal data structures of HPET, RTC and 8254 publicly
>>>> available. That's a wrong step back. I'm sure there are smarter
>>>> solutions, alse as there were some proposals back then in the original
>>>> thread.
>>>
>>> I'm not fully decided myself.  A couple things are clear to me though:
>>>
>>> 1) We must expose type proper types in header files.  We need there
>>> to be a
>>> globally accessible RTCState type and functions that operate on it.
>>
>> I'm not sure what "proper type" means in this context, but I'm quite
>> sure that there should be no need for poking into the internal of the
>> class outside of mc146818rtc.c.
> 
> It needs to be at least a forward reference.  So we can avoid stuff like:
> 
> int apic_accept_pic_intr(DeviceState *s);
> 
> It should be:
> 
> int apic_accept_pic_intr(APICState *s);
> 
> So we can make use of the lovely type checking provided by the compiler
> to us.

I do not disagree. A pointer is harmless.

> 
>> We even abstracted the specifics of the
>> RTC away when it is embedded into a super-IO and interacts with an HPET.
>> If QOM requires such poking, then that requirement should be reassessed.
> 
> There are a couple of ways to make types private while still having
> forward declarations.  None of them are straight forward.  That's why I
> suggest we save this for another day.
> 
>>>
>>> 2) We can simplify memory management by knowing the size of the type
>>> in the
>>> header files too.
>>
>> Is this more than a malloc-free pair?
>>
>>>
>>> Since this is an easily refactorable thing to look at later, I think
>>> we should
>>> start with extracting the types.
>>
>> My worry is that those three refactorings set bad examples for others.
>> So I'd like to avoid such back and forth if possible.
> 
> I'm not really worried about it.  It's so easier to refactor this
> later.  Why rush it now?

You rush changing the current layout, not me. :)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:37         ` Jan Kiszka
@ 2012-03-26 19:39           ` Anthony Liguori
  2012-03-26 19:44             ` Jan Kiszka
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 19:39 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

On 03/26/2012 02:37 PM, Jan Kiszka wrote:
> On 2012-03-26 21:35, Anthony Liguori wrote:
>>>> Since this is an easily refactorable thing to look at later, I think
>>>> we should
>>>> start with extracting the types.
>>>
>>> My worry is that those three refactorings set bad examples for others.
>>> So I'd like to avoid such back and forth if possible.
>>
>> I'm not really worried about it.  It's so easier to refactor this
>> later.  Why rush it now?
>
> You rush changing the current layout, not me. :)

No, I'm trying to do incremental changes without boiling the ocean in the process.

I think we all are in violent agreement about where we want to end up (as opaque 
types as possible).  I don't want to hold back additional refactoring on doing 
this right (and it's not just a matter of malloc/free).

Regards,

Anthony Liguori

> Jan
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:39           ` Anthony Liguori
@ 2012-03-26 19:44             ` Jan Kiszka
  2012-03-26 19:49               ` Anthony Liguori
  2012-03-26 19:52               ` Anthony Liguori
  0 siblings, 2 replies; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 19:44 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

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

On 2012-03-26 21:39, Anthony Liguori wrote:
> On 03/26/2012 02:37 PM, Jan Kiszka wrote:
>> On 2012-03-26 21:35, Anthony Liguori wrote:
>>>>> Since this is an easily refactorable thing to look at later, I think
>>>>> we should
>>>>> start with extracting the types.
>>>>
>>>> My worry is that those three refactorings set bad examples for others.
>>>> So I'd like to avoid such back and forth if possible.
>>>
>>> I'm not really worried about it.  It's so easier to refactor this
>>> later.  Why rush it now?
>>
>> You rush changing the current layout, not me. :)
> 
> No, I'm trying to do incremental changes without boiling the ocean in
> the process.
> 
> I think we all are in violent agreement about where we want to end up
> (as opaque types as possible).  I don't want to hold back additional
> refactoring on doing this right (and it's not just a matter of
> malloc/free).

Either I'm missing it in the code shuffling, or it's not part of this
series: Can you point out where more that a forward reference and
malloc/free is needed?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:44             ` Jan Kiszka
@ 2012-03-26 19:49               ` Anthony Liguori
  2012-03-26 20:10                 ` Jan Kiszka
  2012-03-26 19:52               ` Anthony Liguori
  1 sibling, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 19:49 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

On 03/26/2012 02:44 PM, Jan Kiszka wrote:
> On 2012-03-26 21:39, Anthony Liguori wrote:
>> On 03/26/2012 02:37 PM, Jan Kiszka wrote:
>>> On 2012-03-26 21:35, Anthony Liguori wrote:
>>>>>> Since this is an easily refactorable thing to look at later, I think
>>>>>> we should
>>>>>> start with extracting the types.
>>>>>
>>>>> My worry is that those three refactorings set bad examples for others.
>>>>> So I'd like to avoid such back and forth if possible.
>>>>
>>>> I'm not really worried about it.  It's so easier to refactor this
>>>> later.  Why rush it now?
>>>
>>> You rush changing the current layout, not me. :)
>>
>> No, I'm trying to do incremental changes without boiling the ocean in
>> the process.
>>
>> I think we all are in violent agreement about where we want to end up
>> (as opaque types as possible).  I don't want to hold back additional
>> refactoring on doing this right (and it's not just a matter of
>> malloc/free).
>
> Either I'm missing it in the code shuffling, or it's not part of this
> series: Can you point out where more that a forward reference and
> malloc/free is needed?

QOM is built around the concept that the type size is know (as is GObject). 
type_initialize() assumes that the pointer passed is an adequate size.

You would either need to move to a model where the memory was completely owned 
by QOM (which would mean folding type_new into type_initialize) or have a way to 
query instance size for a given type.

This would also mean that reference counting should be revisited although with 
how dereferencing a parent affects the child.

It's not rocket science, but it's also something that needs to be done carefully.

Regards,

Anthony Liguori

>
> Jan
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:44             ` Jan Kiszka
  2012-03-26 19:49               ` Anthony Liguori
@ 2012-03-26 19:52               ` Anthony Liguori
  1 sibling, 0 replies; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 19:52 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Wanpeng Li, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Gavin Shan

On 03/26/2012 02:44 PM, Jan Kiszka wrote:
> On 2012-03-26 21:39, Anthony Liguori wrote:
>> On 03/26/2012 02:37 PM, Jan Kiszka wrote:
>>> On 2012-03-26 21:35, Anthony Liguori wrote:
>>>>>> Since this is an easily refactorable thing to look at later, I think
>>>>>> we should
>>>>>> start with extracting the types.
>>>>>
>>>>> My worry is that those three refactorings set bad examples for others.
>>>>> So I'd like to avoid such back and forth if possible.
>>>>
>>>> I'm not really worried about it.  It's so easier to refactor this
>>>> later.  Why rush it now?
>>>
>>> You rush changing the current layout, not me. :)
>>
>> No, I'm trying to do incremental changes without boiling the ocean in
>> the process.
>>
>> I think we all are in violent agreement about where we want to end up
>> (as opaque types as possible).  I don't want to hold back additional
>> refactoring on doing this right (and it's not just a matter of
>> malloc/free).
>
> Either I'm missing it in the code shuffling, or it's not part of this
> series: Can you point out where more that a forward reference and
> malloc/free is needed?

Inheritance is the other nasty case.

To inherit from a type, you need to have the type definition.  This is a pretty 
common problem with Object systems and the typical solution is PIMPL[1].

So maybe that's the right thing to do here, but that would have a significant 
affect on the code.  That's what I mean by rushing how to handle this.  There 
are multiple possible solutions and they need to be considered.

The problem is purely aesthetic too, so I don't see a rush to fix it.

[1] http://c2.com/cgi/wiki?PimplIdiom

Regards,

Anthony Liguori

>
> Jan

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 19:49               ` Anthony Liguori
@ 2012-03-26 20:10                 ` Jan Kiszka
  2012-03-26 20:13                   ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 20:10 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

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

On 2012-03-26 21:49, Anthony Liguori wrote:
> On 03/26/2012 02:44 PM, Jan Kiszka wrote:
>> On 2012-03-26 21:39, Anthony Liguori wrote:
>>> On 03/26/2012 02:37 PM, Jan Kiszka wrote:
>>>> On 2012-03-26 21:35, Anthony Liguori wrote:
>>>>>>> Since this is an easily refactorable thing to look at later, I think
>>>>>>> we should
>>>>>>> start with extracting the types.
>>>>>>
>>>>>> My worry is that those three refactorings set bad examples for
>>>>>> others.
>>>>>> So I'd like to avoid such back and forth if possible.
>>>>>
>>>>> I'm not really worried about it.  It's so easier to refactor this
>>>>> later.  Why rush it now?
>>>>
>>>> You rush changing the current layout, not me. :)
>>>
>>> No, I'm trying to do incremental changes without boiling the ocean in
>>> the process.
>>>
>>> I think we all are in violent agreement about where we want to end up
>>> (as opaque types as possible).  I don't want to hold back additional
>>> refactoring on doing this right (and it's not just a matter of
>>> malloc/free).
>>
>> Either I'm missing it in the code shuffling, or it's not part of this
>> series: Can you point out where more that a forward reference and
>> malloc/free is needed?
> 
> QOM is built around the concept that the type size is know (as is
> GObject). type_initialize() assumes that the pointer passed is an
> adequate size.
> 
> You would either need to move to a model where the memory was completely
> owned by QOM (which would mean folding type_new into type_initialize) or
> have a way to query instance size for a given type.
> 
> This would also mean that reference counting should be revisited
> although with how dereferencing a parent affects the child.
> 
> It's not rocket science, but it's also something that needs to be done
> carefully.

But all this is only a problem for these three here (PIT, RTC, HPET) as
their objects shall be embedded into the super-IO. If you only embed an
object pointer, it shouldn't be an issue anymore, no?

Also inheritance is not a problem here as we do not derive from the
three types in question. If there is a super/sub-class relation, those
need to share an internal header, of course.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 20:10                 ` Jan Kiszka
@ 2012-03-26 20:13                   ` Anthony Liguori
  2012-03-26 20:30                     ` Jan Kiszka
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 20:13 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Wanpeng Li, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Gavin Shan

On 03/26/2012 03:10 PM, Jan Kiszka wrote:
> On 2012-03-26 21:49, Anthony Liguori wrote:
>> On 03/26/2012 02:44 PM, Jan Kiszka wrote:
>> This would also mean that reference counting should be revisited
>> although with how dereferencing a parent affects the child.
>>
>> It's not rocket science, but it's also something that needs to be done
>> carefully.
>
> But all this is only a problem for these three here (PIT, RTC, HPET) as
> their objects shall be embedded into the super-IO. If you only embed an
> object pointer, it shouldn't be an issue anymore, no?

The reference counting stuff obviously needs to be looked at even in this case.

> Also inheritance is not a problem here as we do not derive from the
> three types in question. If there is a super/sub-class relation, those
> need to share an internal header, of course.

Yes, but then you have two headers for every type.  Is that really a good thing?

I think I prefer to just mark fields as /*< private >*/ and scold people if they 
violate that convention.

Regards,

Anthony Liguori

>
> Jan

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 20:13                   ` Anthony Liguori
@ 2012-03-26 20:30                     ` Jan Kiszka
  2012-03-26 21:00                       ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Jan Kiszka @ 2012-03-26 20:30 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Wanpeng Li, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Gavin Shan

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

On 2012-03-26 22:13, Anthony Liguori wrote:
> On 03/26/2012 03:10 PM, Jan Kiszka wrote:
>> On 2012-03-26 21:49, Anthony Liguori wrote:
>>> On 03/26/2012 02:44 PM, Jan Kiszka wrote:
>>> This would also mean that reference counting should be revisited
>>> although with how dereferencing a parent affects the child.
>>>
>>> It's not rocket science, but it's also something that needs to be done
>>> carefully.
>>
>> But all this is only a problem for these three here (PIT, RTC, HPET) as
>> their objects shall be embedded into the super-IO. If you only embed an
>> object pointer, it shouldn't be an issue anymore, no?
> 
> The reference counting stuff obviously needs to be looked at even in
> this case.

A composite object is owned by its container. So it should go when the
container leaves.

> 
>> Also inheritance is not a problem here as we do not derive from the
>> three types in question. If there is a super/sub-class relation, those
>> need to share an internal header, of course.
> 
> Yes, but then you have two headers for every type.  Is that really a
> good thing?

It's cleaner and more explicit than tagging members with comments. And
it's nothing we will have for each and every type as only a small subset
is actually inheriting, the mass is finalizing.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 20:30                     ` Jan Kiszka
@ 2012-03-26 21:00                       ` Anthony Liguori
  0 siblings, 0 replies; 42+ messages in thread
From: Anthony Liguori @ 2012-03-26 21:00 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Gavin Shan, qemu-devel, Isaku Yamahata, Avi Kivity,
	Paolo Bonzini, Wanpeng Li

On 03/26/2012 03:30 PM, Jan Kiszka wrote:
> On 2012-03-26 22:13, Anthony Liguori wrote:
>> On 03/26/2012 03:10 PM, Jan Kiszka wrote:
>>> On 2012-03-26 21:49, Anthony Liguori wrote:
>>>> On 03/26/2012 02:44 PM, Jan Kiszka wrote:
>>>> This would also mean that reference counting should be revisited
>>>> although with how dereferencing a parent affects the child.
>>>>
>>>> It's not rocket science, but it's also something that needs to be done
>>>> carefully.
>>>
>>> But all this is only a problem for these three here (PIT, RTC, HPET) as
>>> their objects shall be embedded into the super-IO. If you only embed an
>>> object pointer, it shouldn't be an issue anymore, no?
>>
>> The reference counting stuff obviously needs to be looked at even in
>> this case.
>
> A composite object is owned by its container. So it should go when the
> container leaves.

It's more complicated than that because it's a tree, not a graph.  So there may 
be additional references held beyond the parent.

>>> Also inheritance is not a problem here as we do not derive from the
>>> three types in question. If there is a super/sub-class relation, those
>>> need to share an internal header, of course.
>>
>> Yes, but then you have two headers for every type.  Is that really a
>> good thing?
>
> It's cleaner and more explicit than tagging members with comments. And
> it's nothing we will have for each and every type as only a small subset
> is actually inheriting, the mass is finalizing.

I don't fundamentally disagree with anything you're saying here.  I'm only 
pointing out that (1) it's not a trivial problem and (2) it's not an urgent problem.

Regards,

Anthony Liguori

>
> Jan

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-26 17:29     ` Anthony Liguori
@ 2012-03-27 10:31       ` Avi Kivity
  2012-03-27 13:52         ` Anthony Liguori
  0 siblings, 1 reply; 42+ messages in thread
From: Avi Kivity @ 2012-03-27 10:31 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Anthony Liguori, Wanpeng Li, Jan Kiszka, qemu-devel,
	Isaku Yamahata, Paolo Bonzini, Gavin Shan

On 03/26/2012 07:29 PM, Anthony Liguori wrote:
> On 03/26/2012 10:54 AM, Isaku Yamahata wrote:
>> On Mon, Mar 26, 2012 at 02:20:24PM +0200, Jan Kiszka wrote:
>>> I'm also sure we will have to refactor the merge significantly again
>>> for
>>> the introduction of additional chipsets and PC boards. But unless those
>>> requirements can already be specified (Isaku?), that might be
>>> unavoidable.
>>
>> Agreed. At least I'd like pam/smram stuff decoupled from piix.
>
> s/piix/i440fx/
>
> PAM/SRAM has nothing do to with the piix.  Part of the problem with
> the current layout is that the distinction between i440fx and piix is
> not clear.  The piix is just a SuperIO chip (and southbridge).

Right.

>
> I think the better approach is to have a PCNorthBridge base-class that
> contains functionality like PAM/SRAM that both I440FX and Q35 inherit
> from.

I hate to transform this into a languagey discussion, but I don't think
inheritance is the right thing here.  While both 440fx and q35 are north
bridges, the similar implementation of PAM/SMRAM is not part of that. 
It's just a random result of the chips' evolution.  I think the code for
PAM/SMRAM can be reused if the specs match, but using a has-a instead of
an is-a relationship.

As a counterexample, consider a northbridge that implements PAM/SMRAM
differently.  You'd have to refactor PCNorthBridge into two separate
classes.  With the other approach the new northbridge simply doesn't
include the existing PAM/SMRAM implementation and instead implements its
own.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-27 10:31       ` Avi Kivity
@ 2012-03-27 13:52         ` Anthony Liguori
  2012-03-27 14:18           ` Avi Kivity
  0 siblings, 1 reply; 42+ messages in thread
From: Anthony Liguori @ 2012-03-27 13:52 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Anthony Liguori, Wanpeng Li, Jan Kiszka, qemu-devel,
	Isaku Yamahata, Paolo Bonzini, Gavin Shan

On 03/27/2012 05:31 AM, Avi Kivity wrote:
>>
>> I think the better approach is to have a PCNorthBridge base-class that
>> contains functionality like PAM/SRAM that both I440FX and Q35 inherit
>> from.
>
> I hate to transform this into a languagey discussion, but I don't think
> inheritance is the right thing here.  While both 440fx and q35 are north
> bridges, the similar implementation of PAM/SMRAM is not part of that.
> It's just a random result of the chips' evolution.  I think the code for
> PAM/SMRAM can be reused if the specs match, but using a has-a instead of
> an is-a relationship.

The direction I was heading with the i440fx, the i440fx has-a i440fx-pmc and it 
was the pmc that actually did PAM/SMRAM.

I don't recall there being significant i440fx specific logic in the i440fx-pmc 
so it's entirely possible the the i440fx-pmc could be renamed to PCNorthBridge 
and then both the i440fx and q35 could has-a the same PCNorthBridge (possible 
with some properties to control certain behaviors.

We really need to push forward with the refactoring to find the right model here.

Regards,

Anthony Liguori

>
> As a counterexample, consider a northbridge that implements PAM/SMRAM
> differently.  You'd have to refactor PCNorthBridge into two separate
> classes.  With the other approach the new northbridge simply doesn't
> include the existing PAM/SMRAM implementation and instead implements its
> own.
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
  2012-03-27 13:52         ` Anthony Liguori
@ 2012-03-27 14:18           ` Avi Kivity
  0 siblings, 0 replies; 42+ messages in thread
From: Avi Kivity @ 2012-03-27 14:18 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Anthony Liguori, Wanpeng Li, Jan Kiszka, qemu-devel,
	Isaku Yamahata, Paolo Bonzini, Gavin Shan

On 03/27/2012 03:52 PM, Anthony Liguori wrote:
> On 03/27/2012 05:31 AM, Avi Kivity wrote:
>>>
>>> I think the better approach is to have a PCNorthBridge base-class that
>>> contains functionality like PAM/SRAM that both I440FX and Q35 inherit
>>> from.
>>
>> I hate to transform this into a languagey discussion, but I don't think
>> inheritance is the right thing here.  While both 440fx and q35 are north
>> bridges, the similar implementation of PAM/SMRAM is not part of that.
>> It's just a random result of the chips' evolution.  I think the code for
>> PAM/SMRAM can be reused if the specs match, but using a has-a instead of
>> an is-a relationship.
>
> The direction I was heading with the i440fx, the i440fx has-a
> i440fx-pmc and it was the pmc that actually did PAM/SMRAM.
>
> I don't recall there being significant i440fx specific logic in the
> i440fx-pmc so it's entirely possible the the i440fx-pmc could be
> renamed to PCNorthBridge and then both the i440fx and q35 could has-a
> the same PCNorthBridge (possible with some properties to control
> certain behaviors.
>

Sounds about right.

> We really need to push forward with the refactoring to find the right
> model here.
>

Yes.  This is opportunistic reuse - we find some sections of both specs
that are identical, and we reuse the code there.

In fact code duplication is also justified here.  The specs are
duplicated, why not the code.  Not against deduplication of course, but
we need not go out of our way here.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2012-03-27 14:18 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-26  2:06 [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Wanpeng Li
2012-03-26  2:06 ` [Qemu-devel] [PATCH 1/6] eliminate piix_pci.c and module i440fx and piix3 Wanpeng Li
2012-03-26  2:06 ` [Qemu-devel] [PATCH 2/6] convert MemoryRegion to QOM Wanpeng Li
2012-03-26  2:06 ` [Qemu-devel] [PATCH 3/6] convert pci-host " Wanpeng Li
2012-03-26  7:32   ` Stefan Hajnoczi
2012-03-26  9:22   ` Wanpeng Li
2012-03-26 14:25   ` Andreas Färber
2012-03-26  2:06 ` [Qemu-devel] [PATCH 4/6] prepare to create HPET, RTC and i8254 through composition Wanpeng Li
2012-03-26  2:06 ` [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c Wanpeng Li
2012-03-26 12:42   ` Avi Kivity
2012-03-26 12:47     ` Jan Kiszka
2012-03-26 17:37       ` Anthony Liguori
2012-03-26  2:06 ` [Qemu-devel] [PATCH 6/6] make some functions static Wanpeng Li
2012-03-26 12:20 ` [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM Jan Kiszka
2012-03-26 15:54   ` Isaku Yamahata
2012-03-26 17:29     ` Anthony Liguori
2012-03-27 10:31       ` Avi Kivity
2012-03-27 13:52         ` Anthony Liguori
2012-03-27 14:18           ` Avi Kivity
2012-03-26 17:17   ` Blue Swirl
2012-03-26 17:33   ` Anthony Liguori
2012-03-26 19:30     ` Jan Kiszka
2012-03-26 19:35       ` Anthony Liguori
2012-03-26 19:37         ` Jan Kiszka
2012-03-26 19:39           ` Anthony Liguori
2012-03-26 19:44             ` Jan Kiszka
2012-03-26 19:49               ` Anthony Liguori
2012-03-26 20:10                 ` Jan Kiszka
2012-03-26 20:13                   ` Anthony Liguori
2012-03-26 20:30                     ` Jan Kiszka
2012-03-26 21:00                       ` Anthony Liguori
2012-03-26 19:52               ` Anthony Liguori
2012-03-26 12:47 ` Andreas Färber
2012-03-26 12:57   ` Wanpeng Li
2012-03-26 17:09 ` Blue Swirl
2012-03-26 17:35   ` Anthony Liguori
2012-03-26 17:43     ` Blue Swirl
2012-03-26 17:45       ` Anthony Liguori
2012-03-26 18:01         ` Blue Swirl
2012-03-26 18:07           ` Anthony Liguori
2012-03-26 18:25             ` Blue Swirl
2012-03-26 17:25 ` Anthony Liguori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.