| Message ID | 20130116093638.GA6141@dhcp-192-168-178-175.profitbricks.localdomain |
|---|---|
| State | New |
| Headers | show |
Hi, Am 16.01.2013 10:36, schrieb Vasilis Liaskovitis: > On Wed, Jan 16, 2013 at 03:20:40PM +0800, Hu Tao wrote: >> On Tue, Dec 18, 2012 at 01:41:41PM +0100, Vasilis Liaskovitis wrote: >>> Refactor code so that chipset initialization is similar to q35. This will >>> allow memory map initialization at chipset qdev init time for both >>> machines, as well as more similar code structure overall. >>> >>> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> >>> --- >>> hw/pc_piix.c | 57 ++++++++++++--- >>> hw/piix_pci.c | 225 ++++++++++++++------------------------------------------- >>> 2 files changed, 100 insertions(+), 182 deletions(-) >>> >>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >>> index 19e342a..6a9b508 100644 >>> --- a/hw/pc_piix.c >>> +++ b/hw/pc_piix.c >>> @@ -47,6 +47,7 @@ >>> #ifdef CONFIG_XEN >>> # include <xen/hvm/hvm_info_table.h> >>> #endif >>> +#include "piix_pci.h" >> >> Can't find this file. Did you forget to add this file to git? > > sorry, you are right. Below is the corrected patch with the missing header Please take review comments on other similar series into account. You can also check if the QOM Vadis slides from KVM Forum are online somewhere. You are aware that there were two people previously working on QOM'ifying i440fx? > Refactor code so that chipset initialization is similar to q35. This will > allow memory map initialization at chipset qdev init time for both > machines, as well as more similar code structure overall. > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> > --- > hw/pc_piix.c | 57 ++++++++++++--- > hw/piix_pci.c | 225 ++++++++++++++------------------------------------------- > hw/piix_pci.h | 116 +++++++++++++++++++++++++++++ > 3 files changed, 216 insertions(+), 182 deletions(-) > create mode 100644 hw/piix_pci.h > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 19e342a..6a9b508 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c [...] > @@ -127,21 +130,53 @@ static void pc_init1(MemoryRegion *system_memory, > } > > 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(hwaddr) == 4 > - ? 0 > - : ((uint64_t)1 << 62)), > - pci_memory, ram_memory); > + i440fx_host = I440FX_HOST_DEVICE(qdev_create(NULL, > + TYPE_I440FX_HOST_DEVICE)); Elsewhere it was requested to use _HOST_BRIDGE wording. > + i440fx_host->mch.ram_memory = ram_memory; > + i440fx_host->mch.pci_address_space = pci_memory; > + i440fx_host->mch.system_memory = get_system_memory(); > + i440fx_host->mch.address_space_io = get_system_io();; > + i440fx_host->mch.below_4g_mem_size = below_4g_mem_size; > + i440fx_host->mch.above_4g_mem_size = above_4g_mem_size; > + > + qdev_init_nofail(DEVICE(i440fx_host)); > + i440fx_state = &i440fx_host->mch; > + pci_bus = i440fx_host->parent_obj.bus; Please don't access the parent field, in particular not "parent_obj". It was specifically renamed after checking that no more users exist. PCIHostState *phb = PCI_HOST_BRIDGE(i440fx_host); ... pci_bus = phb->bus; > + /* 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(pci_bus, -1, true, > + "PIIX3-xen")); Please don't introduce new usages of DO_UPCAST() with QOM types. Instead add QOM cast macros where needed and use them. > + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > + piix3, XEN_PIIX_NUM_PIRQS); > + } else { > + piix3 = DO_UPCAST(PIIX3State, dev, > + pci_create_simple_multifunction(pci_bus, -1, true, > + "PIIX3")); > + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, piix3, > + PIIX_NUM_PIRQS); > + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > + } > + piix3->pic = gsi; > + isa_bus = DO_UPCAST(ISABus, qbus, > + qdev_get_child_bus(&piix3->dev.qdev, "isa.0")); isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), ...)); > + > + piix3_devfn = piix3->dev.devfn; > + > + ram_size = ram_size / 8 / 1024 / 1024; > + if (ram_size > 255) { > + ram_size = 255; > + } > + i440fx_state->dev.config[0x57] = ram_size; > } 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()) { > @@ -157,7 +192,7 @@ static void pc_init1(MemoryRegion *system_memory, > gsi_state->i8259_irq[i] = i8259[i]; > } > if (pci_enabled) { > - ioapic_init_gsi(gsi_state, "i440fx"); > + ioapic_init_gsi(gsi_state, NULL); Unrelated? Why? > } > > pc_register_ferr_irq(gsi[13]); > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index ba1b3de..7ca3c73 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -31,70 +31,15 @@ > #include "range.h" > #include "xen.h" > #include "pam.h" > +#include "piix_pci.h" > > -/* > - * I440FX chipset data sheet. > - * http://download.intel.com/design/chipsets/datashts/29054901.pdf > - */ > - > -typedef struct I440FXState { > - PCIHostState parent_obj; > -} 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; > - > -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 PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); > 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 pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) > { > int slot_addend; > slot_addend = (pci_dev->devfn >> 3) - 1; > @@ -180,149 +125,86 @@ static const VMStateDescription vmstate_i440fx = { > } > }; > > -static int i440fx_pcihost_initfn(SysBusDevice *dev) > +static void i440fx_pcihost_initfn(Object *obj) > { > - PCIHostState *s = PCI_HOST_BRIDGE(dev); > + I440FXState *s = I440FX_HOST_DEVICE(obj); > + object_initialize(&s->mch, TYPE_I440FX_PCI_DEVICE); > + object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); Is there maybe a more readable property name? > +} > > - 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); > +static int i440fx_pcihost_init(SysBusDevice *dev) > +{ > + PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev); Don't use FROM_SYSBUS() either: PCIHostState *phb = PCI_HOST_BRIDGE(dev); > + I440FXState *s = I440FX_HOST_DEVICE(&dev->qdev); No need to access ->qdev, just use I440FX_...(dev); > + PCIBus *b; > + > + memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci, > + "pci-conf-idx", 4); > + sysbus_add_io(dev, 0xcf8, &pci->conf_mem); > + sysbus_init_ioports(&pci->busdev, 0xcf8, 4); > + memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci, > + "pci-conf-data", 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); > + sysbus_add_io(dev, 0xcfc, &pci->data_mem); > + sysbus_init_ioports(&pci->busdev, 0xcfc, 4); > + > + b = pci_bus_new(&s->parent_obj.busdev.qdev, NULL, s->mch.pci_address_space, DEVICE(dev) > + s->mch.address_space_io, 0); Initializing the bus in-place would be preferred. > + s->parent_obj.bus = b; > + qdev_set_parent_bus(DEVICE(&s->mch), BUS(b)); > + qdev_init_nofail(DEVICE(&s->mch)); When casts other than OBJECT() are used multiple times, a variable is preferred. > > return 0; > } > > static int i440fx_initfn(PCIDevice *dev) > { > - PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev); > + int i; > + PCII440FXState *f = DO_UPCAST(PCII440FXState, dev, dev); > + hwaddr pci_hole64_size; > > - d->dev.config[I440FX_SMRAM] = 0x02; > + f->dev.config[I440FX_SMRAM] = 0x02; > > - cpu_smm_register(&i440fx_set_smm, d); > - return 0; > -} > + cpu_smm_register(&i440fx_set_smm, f); Is all this d -> f variable renaming really necessary? I can understand the s -> pci (or for less ambiguity: phb) renaming above (I believe I left it s to keep my patch small ;)), but here no new variable is introduced so it just seems to enlarge the patch. [...] > @@ -550,7 +432,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data) > } > > static const TypeInfo i440fx_info = { > - .name = "i440FX", > + .name = TYPE_I440FX_PCI_DEVICE, > .parent = TYPE_PCI_DEVICE, This matches the _PCI_DEVICE naming in earlier series including prep_pci > .instance_size = sizeof(PCII440FXState), > .class_init = i440fx_class_init, > @@ -561,15 +443,16 @@ 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; > + k->init = i440fx_pcihost_init; > dc->fw_name = "pci"; > dc->no_user = 1; > } > > static const TypeInfo i440fx_pcihost_info = { > - .name = "i440FX-pcihost", > + .name = TYPE_I440FX_HOST_DEVICE, > .parent = TYPE_PCI_HOST_BRIDGE, whereas here you see the mentioned _HOST_DEVICE vs. _HOST_BRIDGE. > .instance_size = sizeof(I440FXState), > + .instance_init = i440fx_pcihost_initfn, > .class_init = i440fx_pcihost_class_init, > }; > [snip] Regards, Andreas
Hi, On Wed, Jan 16, 2013 at 12:17:05PM +0100, Andreas Färber wrote: > Hi, > > Am 16.01.2013 10:36, schrieb Vasilis Liaskovitis: > > On Wed, Jan 16, 2013 at 03:20:40PM +0800, Hu Tao wrote: > >> On Tue, Dec 18, 2012 at 01:41:41PM +0100, Vasilis Liaskovitis wrote: > >>> Refactor code so that chipset initialization is similar to q35. This will > >>> allow memory map initialization at chipset qdev init time for both > >>> machines, as well as more similar code structure overall. > >>> > >>> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> > >>> --- > >>> hw/pc_piix.c | 57 ++++++++++++--- > >>> hw/piix_pci.c | 225 ++++++++++++++------------------------------------------- > >>> 2 files changed, 100 insertions(+), 182 deletions(-) > >>> > >>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c > >>> index 19e342a..6a9b508 100644 > >>> --- a/hw/pc_piix.c > >>> +++ b/hw/pc_piix.c > >>> @@ -47,6 +47,7 @@ > >>> #ifdef CONFIG_XEN > >>> # include <xen/hvm/hvm_info_table.h> > >>> #endif > >>> +#include "piix_pci.h" > >> > >> Can't find this file. Did you forget to add this file to git? > > > > sorry, you are right. Below is the corrected patch with the missing header > > Please take review comments on other similar series into account. You > can also check if the QOM Vadis slides from KVM Forum are online somewhere. thanks, I will take a look. > > You are aware that there were two people previously working on > QOM'ifying i440fx? I am aware of Anthony's i440fx-pmc patchset from about a year ago (a few months ago I asked if it would be respinned, but got no response iirc, so I am not sure what the status is) What's the second effort you mention and its status? Are prep_pci patchsets going to address this in the future? I don't mean to step on other people's work-in-progress, so I don't mind dropping this patch if one of the other efforts is still active. > > --- > > hw/pc_piix.c | 57 ++++++++++++--- > > hw/piix_pci.c | 225 ++++++++++++++------------------------------------------- > > hw/piix_pci.h | 116 +++++++++++++++++++++++++++++ > > 3 files changed, 216 insertions(+), 182 deletions(-) > > create mode 100644 hw/piix_pci.h > > > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > > index 19e342a..6a9b508 100644 > > --- a/hw/pc_piix.c > > +++ b/hw/pc_piix.c > [...] > > @@ -127,21 +130,53 @@ static void pc_init1(MemoryRegion *system_memory, > > } > > > > 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(hwaddr) == 4 > > - ? 0 > > - : ((uint64_t)1 << 62)), > > - pci_memory, ram_memory); > > + i440fx_host = I440FX_HOST_DEVICE(qdev_create(NULL, > > + TYPE_I440FX_HOST_DEVICE)); > > Elsewhere it was requested to use _HOST_BRIDGE wording. ok > > > + i440fx_host->mch.ram_memory = ram_memory; > > + i440fx_host->mch.pci_address_space = pci_memory; > > + i440fx_host->mch.system_memory = get_system_memory(); > > + i440fx_host->mch.address_space_io = get_system_io();; > > + i440fx_host->mch.below_4g_mem_size = below_4g_mem_size; > > + i440fx_host->mch.above_4g_mem_size = above_4g_mem_size; > > + > > + qdev_init_nofail(DEVICE(i440fx_host)); > > + i440fx_state = &i440fx_host->mch; > > + pci_bus = i440fx_host->parent_obj.bus; > > Please don't access the parent field, in particular not "parent_obj". It > was specifically renamed after checking that no more users exist. ok > > PCIHostState *phb = PCI_HOST_BRIDGE(i440fx_host); > ... > pci_bus = phb->bus; > > > + /* 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(pci_bus, -1, true, > > + "PIIX3-xen")); > > Please don't introduce new usages of DO_UPCAST() with QOM types. Instead > add QOM cast macros where needed and use them. Any examples of this? I assume the new macros should't use DO_UPCAST themselves. > > > + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > > + piix3, XEN_PIIX_NUM_PIRQS); > > + } else { > > + piix3 = DO_UPCAST(PIIX3State, dev, > > + pci_create_simple_multifunction(pci_bus, -1, true, > > + "PIIX3")); > > + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, piix3, > > + PIIX_NUM_PIRQS); > > + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > > + } > > + piix3->pic = gsi; > > + isa_bus = DO_UPCAST(ISABus, qbus, > > + qdev_get_child_bus(&piix3->dev.qdev, "isa.0")); > > isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), ...)); > > > + > > + piix3_devfn = piix3->dev.devfn; > > + > > + ram_size = ram_size / 8 / 1024 / 1024; > > + if (ram_size > 255) { > > + ram_size = 255; > > + } > > + i440fx_state->dev.config[0x57] = ram_size; > > } 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()) { > > @@ -157,7 +192,7 @@ static void pc_init1(MemoryRegion *system_memory, > > gsi_state->i8259_irq[i] = i8259[i]; > > } > > if (pci_enabled) { > > - ioapic_init_gsi(gsi_state, "i440fx"); > > + ioapic_init_gsi(gsi_state, NULL); > > Unrelated? Why? I was getting a segfault in object_propert_add_child because the path to "i440fx" was resolved to a NULL object. This was just a quick fix - I think the real issue is that the name of the i440fx-host device is not yet set in the new code. I 'll try to fix. > > > } > > > > pc_register_ferr_irq(gsi[13]); > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > > index ba1b3de..7ca3c73 100644 > > --- a/hw/piix_pci.c > > +++ b/hw/piix_pci.c > > @@ -31,70 +31,15 @@ > > #include "range.h" > > #include "xen.h" > > #include "pam.h" > > +#include "piix_pci.h" > > > > -/* > > - * I440FX chipset data sheet. > > - * http://download.intel.com/design/chipsets/datashts/29054901.pdf > > - */ > > - > > -typedef struct I440FXState { > > - PCIHostState parent_obj; > > -} 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; > > - > > -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 PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); > > 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 pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) > > { > > int slot_addend; > > slot_addend = (pci_dev->devfn >> 3) - 1; > > @@ -180,149 +125,86 @@ static const VMStateDescription vmstate_i440fx = { > > } > > }; > > > > -static int i440fx_pcihost_initfn(SysBusDevice *dev) > > +static void i440fx_pcihost_initfn(Object *obj) > > { > > - PCIHostState *s = PCI_HOST_BRIDGE(dev); > > + I440FXState *s = I440FX_HOST_DEVICE(obj); > > + object_initialize(&s->mch, TYPE_I440FX_PCI_DEVICE); > > + object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); > > Is there maybe a more readable property name? "mem-controller" maybe? although q35 also uses an "mch" property for its memory controller. > > > +} > > > > - 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); > > +static int i440fx_pcihost_init(SysBusDevice *dev) > > +{ > > + PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev); > > Don't use FROM_SYSBUS() either: Is a new macro required here as well? > > PCIHostState *phb = PCI_HOST_BRIDGE(dev); > > > + I440FXState *s = I440FX_HOST_DEVICE(&dev->qdev); > > No need to access ->qdev, just use I440FX_...(dev); ok. > > > + PCIBus *b; > > + > > + memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci, > > + "pci-conf-idx", 4); > > + sysbus_add_io(dev, 0xcf8, &pci->conf_mem); > > + sysbus_init_ioports(&pci->busdev, 0xcf8, 4); > > + memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci, > > + "pci-conf-data", 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); > > + sysbus_add_io(dev, 0xcfc, &pci->data_mem); > > + sysbus_init_ioports(&pci->busdev, 0xcfc, 4); > > + > > + b = pci_bus_new(&s->parent_obj.busdev.qdev, NULL, s->mch.pci_address_space, > > DEVICE(dev) > > > + s->mch.address_space_io, 0); > > Initializing the bus in-place would be preferred. Do you mean pci_bus_new_inplace? Why is this preferred? > > > + s->parent_obj.bus = b; > > + qdev_set_parent_bus(DEVICE(&s->mch), BUS(b)); > > + qdev_init_nofail(DEVICE(&s->mch)); > > When casts other than OBJECT() are used multiple times, a variable is > preferred. ok > > > > > return 0; > > } > > > > static int i440fx_initfn(PCIDevice *dev) > > { > > - PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev); > > + int i; > > + PCII440FXState *f = DO_UPCAST(PCII440FXState, dev, dev); > > + hwaddr pci_hole64_size; > > > > - d->dev.config[I440FX_SMRAM] = 0x02; > > + f->dev.config[I440FX_SMRAM] = 0x02; > > > > - cpu_smm_register(&i440fx_set_smm, d); > > - return 0; > > -} > > + cpu_smm_register(&i440fx_set_smm, f); > > Is all this d -> f variable renaming really necessary? I can understand > the s -> pci (or for less ambiguity: phb) renaming above (I believe I > left it s to keep my patch small ;)), but here no new variable is > introduced so it just seems to enlarge the patch. yes, this renaming here isn't needed, I 'll omit it. > > [...] > > @@ -550,7 +432,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data) > > } > > > > static const TypeInfo i440fx_info = { > > - .name = "i440FX", > > + .name = TYPE_I440FX_PCI_DEVICE, > > .parent = TYPE_PCI_DEVICE, > > This matches the _PCI_DEVICE naming in earlier series including prep_pci > > > .instance_size = sizeof(PCII440FXState), > > .class_init = i440fx_class_init, > > @@ -561,15 +443,16 @@ 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; > > + k->init = i440fx_pcihost_init; > > dc->fw_name = "pci"; > > dc->no_user = 1; > > } > > > > static const TypeInfo i440fx_pcihost_info = { > > - .name = "i440FX-pcihost", > > + .name = TYPE_I440FX_HOST_DEVICE, > > .parent = TYPE_PCI_HOST_BRIDGE, > > whereas here you see the mentioned _HOST_DEVICE vs. _HOST_BRIDGE. > so, TYPE_I440FX_HOST_BRIDGE for the name is preferred? thanks, - Vasilis
diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 19e342a..6a9b508 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -47,6 +47,7 @@ #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif +#include "piix_pci.h" #define MAX_IDE_BUS 2 @@ -85,6 +86,8 @@ static void pc_init1(MemoryRegion *system_memory, MemoryRegion *pci_memory; MemoryRegion *rom_memory; void *fw_cfg = NULL; + I440FXState *i440fx_host; + PIIX3State *piix3; pc_cpus_init(cpu_model); @@ -127,21 +130,53 @@ static void pc_init1(MemoryRegion *system_memory, } 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(hwaddr) == 4 - ? 0 - : ((uint64_t)1 << 62)), - pci_memory, ram_memory); + i440fx_host = I440FX_HOST_DEVICE(qdev_create(NULL, + TYPE_I440FX_HOST_DEVICE)); + i440fx_host->mch.ram_memory = ram_memory; + i440fx_host->mch.pci_address_space = pci_memory; + i440fx_host->mch.system_memory = get_system_memory(); + i440fx_host->mch.address_space_io = get_system_io();; + i440fx_host->mch.below_4g_mem_size = below_4g_mem_size; + i440fx_host->mch.above_4g_mem_size = above_4g_mem_size; + + qdev_init_nofail(DEVICE(i440fx_host)); + i440fx_state = &i440fx_host->mch; + pci_bus = i440fx_host->parent_obj.bus; + /* 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(pci_bus, -1, true, + "PIIX3-xen")); + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, + piix3, XEN_PIIX_NUM_PIRQS); + } else { + piix3 = DO_UPCAST(PIIX3State, dev, + pci_create_simple_multifunction(pci_bus, -1, true, + "PIIX3")); + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, piix3, + PIIX_NUM_PIRQS); + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); + } + piix3->pic = gsi; + 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; + } + i440fx_state->dev.config[0x57] = ram_size; } 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()) { @@ -157,7 +192,7 @@ static void pc_init1(MemoryRegion *system_memory, gsi_state->i8259_irq[i] = i8259[i]; } if (pci_enabled) { - ioapic_init_gsi(gsi_state, "i440fx"); + ioapic_init_gsi(gsi_state, NULL); } pc_register_ferr_irq(gsi[13]); diff --git a/hw/piix_pci.c b/hw/piix_pci.c index ba1b3de..7ca3c73 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -31,70 +31,15 @@ #include "range.h" #include "xen.h" #include "pam.h" +#include "piix_pci.h" -/* - * I440FX chipset data sheet. - * http://download.intel.com/design/chipsets/datashts/29054901.pdf - */ - -typedef struct I440FXState { - PCIHostState parent_obj; -} 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; - -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 PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); 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 pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) { int slot_addend; slot_addend = (pci_dev->devfn >> 3) - 1; @@ -180,149 +125,86 @@ static const VMStateDescription vmstate_i440fx = { } }; -static int i440fx_pcihost_initfn(SysBusDevice *dev) +static void i440fx_pcihost_initfn(Object *obj) { - PCIHostState *s = PCI_HOST_BRIDGE(dev); + I440FXState *s = I440FX_HOST_DEVICE(obj); + object_initialize(&s->mch, TYPE_I440FX_PCI_DEVICE); + object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); +} - 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); +static int i440fx_pcihost_init(SysBusDevice *dev) +{ + PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev); + I440FXState *s = I440FX_HOST_DEVICE(&dev->qdev); + PCIBus *b; + + memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci, + "pci-conf-idx", 4); + sysbus_add_io(dev, 0xcf8, &pci->conf_mem); + sysbus_init_ioports(&pci->busdev, 0xcf8, 4); + memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci, + "pci-conf-data", 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); + sysbus_add_io(dev, 0xcfc, &pci->data_mem); + sysbus_init_ioports(&pci->busdev, 0xcfc, 4); + + b = pci_bus_new(&s->parent_obj.busdev.qdev, NULL, s->mch.pci_address_space, + s->mch.address_space_io, 0); + s->parent_obj.bus = b; + qdev_set_parent_bus(DEVICE(&s->mch), BUS(b)); + qdev_init_nofail(DEVICE(&s->mch)); return 0; } static int i440fx_initfn(PCIDevice *dev) { - PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev); + int i; + PCII440FXState *f = DO_UPCAST(PCII440FXState, dev, dev); + hwaddr pci_hole64_size; - d->dev.config[I440FX_SMRAM] = 0x02; + f->dev.config[I440FX_SMRAM] = 0x02; - cpu_smm_register(&i440fx_set_smm, d); - return 0; -} + cpu_smm_register(&i440fx_set_smm, f); -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, - hwaddr pci_hole_start, - hwaddr pci_hole_size, - hwaddr pci_hole64_start, - hwaddr pci_hole64_size, - MemoryRegion *pci_address_space, - MemoryRegion *ram_memory) -{ - DeviceState *dev; - PCIBus *b; - PCIDevice *d; - PCIHostState *s; - PIIX3State *piix3; - PCII440FXState *f; - unsigned i; - - dev = qdev_create(NULL, "i440FX-pcihost"); - s = PCI_HOST_BRIDGE(dev); - s->address_space = address_space_mem; - b = pci_bus_new(dev, NULL, pci_address_space, - address_space_io, 0); - s->bus = b; - object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL); - qdev_init_nofail(dev); - - 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; + pci_hole64_size = (sizeof(hwaddr) == 4 ? 0 : + ((uint64_t)1 << 62)); 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); + f->below_4g_mem_size, + 0x100000000LL - f->below_4g_mem_size); + memory_region_add_subregion(f->system_memory, f->below_4g_mem_size, + &f->pci_hole); memory_region_init_alias(&f->pci_hole_64bit, "pci-hole64", f->pci_address_space, - pci_hole64_start, pci_hole64_size); + 0x100000000LL + f->above_4g_mem_size, + pci_hole64_size); if (pci_hole64_size) { - memory_region_add_subregion(f->system_memory, pci_hole64_start, + memory_region_add_subregion(f->system_memory, + 0x100000000LL + f->above_4g_mem_size, &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); + init_pam(f->ram_memory, f->system_memory, f->pci_address_space, &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE); for (i = 0; i < 12; ++i) { init_pam(f->ram_memory, f->system_memory, f->pci_address_space, - &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, + &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); } - - /* 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); - pci_bus_set_route_irq_fn(b, piix3_route_intx_pin_to_irq); - } - 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; - + f->dev.config[0x57] = f->below_4g_mem_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, - hwaddr pci_hole_start, - hwaddr pci_hole_size, - hwaddr pci_hole64_start, - hwaddr 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_start, pci_hole64_size, - pci_memory, ram_memory); - return b; + return 0; } /* PIIX3 PCI to ISA bridge */ -static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) +void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3->pic[pic_irq], !!(piix3->pic_levels & @@ -347,13 +229,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) piix3_set_irq_pic(piix3, pic_irq); } -static void piix3_set_irq(void *opaque, int pirq, int level) +void piix3_set_irq(void *opaque, int pirq, int level) { PIIX3State *piix3 = opaque; piix3_set_irq_level(piix3, pirq, level); } -static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin) +PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin) { PIIX3State *piix3 = opaque; int irq = piix3->dev.config[PIIX_PIRQC + pin]; @@ -550,7 +432,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data) } static const TypeInfo i440fx_info = { - .name = "i440FX", + .name = TYPE_I440FX_PCI_DEVICE, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(PCII440FXState), .class_init = i440fx_class_init, @@ -561,15 +443,16 @@ 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; + k->init = i440fx_pcihost_init; dc->fw_name = "pci"; dc->no_user = 1; } static const TypeInfo i440fx_pcihost_info = { - .name = "i440FX-pcihost", + .name = TYPE_I440FX_HOST_DEVICE, .parent = TYPE_PCI_HOST_BRIDGE, .instance_size = sizeof(I440FXState), + .instance_init = i440fx_pcihost_initfn, .class_init = i440fx_pcihost_class_init, }; diff --git a/hw/piix_pci.h b/hw/piix_pci.h new file mode 100644 index 0000000..26b3fa0 --- /dev/null +++ b/hw/piix_pci.h @@ -0,0 +1,116 @@ +/* + * piix_pci.h + * + * Copyright (c) 2009 Isaku Yamahata <yamahata at valinux co jp> + * VA Linux Systems Japan K.K. + * Copyright (C) 2012 Jason Baron <jbaron@redhat.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/> + */ + +#ifndef HW_I440FX_H +#define HW_I440FX_H + +#include "hw.h" +#include "range.h" +#include "isa.h" +#include "sysbus.h" +#include "pc.h" +#include "apm.h" +#include "apic.h" +#include "pci.h" +#include "pcie_host.h" +#include "acpi.h" +#include "pam.h" +#include "dimm.h" + +/* + * I440FX chipset data sheet. + * http://download.intel.com/design/chipsets/datashts/29054901.pdf + */ +#define I440FX_PCI_HOLE_START 0xe0000000 + +#define TYPE_I440FX_HOST_DEVICE "i440FX-pcihost" +#define I440FX_HOST_DEVICE(obj) \ + OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_HOST_DEVICE) + +#define TYPE_I440FX_PCI_DEVICE "i440FX" +#define I440FX_PCI_DEVICE(obj) \ + OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) + +typedef struct PCII440FXState { + PCIDevice dev; + MemoryRegion *system_memory; + MemoryRegion *pci_address_space; + MemoryRegion *ram_memory; + MemoryRegion *address_space_io; + PAMMemoryRegion pam_regions[13]; + MemoryRegion pci_hole; + MemoryRegion pci_hole_64bit; + MemoryRegion smram_region; + uint8_t smm_enabled; + ram_addr_t below_4g_mem_size; + ram_addr_t above_4g_mem_size; + /* i440fx allows for 1 DRAM channels x 8 DRAM ranks */ + DimmBus *dram_channel0; + /* paravirtual memory bus */ + DimmBus *pv_dram_channel; + void *fw_cfg; +} PCII440FXState; + +typedef struct I440FXState { + PCIHostState parent_obj; + PCII440FXState mch; +} 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; + + +int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx); +void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq); +void piix3_set_irq(void *opaque, int pirq, int level); +PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); +hwaddr i440fx_pmc_dimm_offset(DeviceState *dev, uint64_t size); + +#define I440FX_PAM 0x59 +#define I440FX_PAM_SIZE 7 +#define I440FX_SMRAM 0x72 + +#endif /* HW_I440FX_H */