Re: [Qemu-devel] [PATCH] pci: add standard bridge device

[Michael S. Tsirkin]

On Sun, Sep 04, 2011 at 06:42:42PM +0300, Avi Kivity wrote:
> On 09/04/2011 06:26 PM, Michael S. Tsirkin wrote:
> >On Sun, Sep 04, 2011 at 05:36:56PM +0300, Avi Kivity wrote:
> >>  So long as we're nitpicking ...
> >
> >+static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
> >+                                  uint8_t type, const char *name,
> >+                                  MemoryRegion *space,
> >+                                  MemoryRegion *parent_space)
> >+{
> >+    pcibus_t base = pci_bridge_get_base(&bridge->dev, type);
> >+    pcibus_t limit = pci_bridge_get_limit(&bridge->dev, type);
> >+    /* TODO: this doesn't handle base = 0 limit = 2^64 - 1 correctly.
> >+     * Apparent_spacely no way to do this with existing memory APIs. */
> 
> Spelling out spaces is a new fashion?

Will fix.

> >+    pcibus_t size = limit>= base ? limit + 1 - base : 0;
> >+
> >+    memory_region_init_alias(alias, name, space, base, size);
> >+    memory_region_add_subregion_overlap(parent_space, base, alias, 1);
> >+}
> >+
> >
> >@@ -246,10 +312,14 @@ int pci_bridge_initfn(PCIDevice *dev)
> >                          br->bus_name);
> >      sec_bus->parent_dev = dev;
> >      sec_bus->map_irq = br->map_irq;
> >-    /* TODO: use memory API to perform memory filtering. */
> >-    sec_bus->address_space_mem = parent->address_space_mem;
> >-    sec_bus->address_space_io = parent->address_space_io;
> >-
> >+    sec_bus->address_space_mem = g_new(MemoryRegion, 1);
> >+    memory_region_init(sec_bus->address_space_mem, "pci_pridge_pci", 
> >INT64_MAX);
> >+    sec_bus->address_space_io = g_new(MemoryRegion, 1);
> >+    memory_region_init(sec_bus->address_space_io, "pci_bridge_io", 65536);
> >+    sec_bus->alias_pref_mem = g_new(MemoryRegion, 1);
> >+    sec_bus->alias_mem = g_new(MemoryRegion, 1);
> >+    sec_bus->alias_io = g_new(MemoryRegion, 1);
> 
> Why pointers?  Regular fields require less upkeep.

Good point. Why does PIIX use pointers? I just copied that ...

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