qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Avi Kivity
Subject: Re: [Qemu-devel] [PATCH] pci: add standard bridge device
Date: Sun, 04 Sep 2011 16:55:33 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0

On 09/04/2011 04:41 PM, Michael S. Tsirkin wrote:
On Sun, Sep 04, 2011 at 04:05:14PM +0300, Avi Kivity wrote:
>  It follows naturally:

OK, so it seems the following is more or less what you suggest?
I'm not sure I create/destroy subregions properly.
Both the alias and the subregion get the same start value?

Yes (so addresses are not shifted).

Is the region name for debugging only?

For everything except RAM regions (there, the name is also used for save/restore).

When does priority matter? In case of overlap?

Yes. In this case, since overlap resolution is not defined by the spec, the actual priority does not matter.

@@ -135,6 +135,75 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, 
uint8_t type)
      return limit;
  }

+static pcibus_t pci_bridge_get_size(const PCIDevice *bridge, uint8_t type)
+{
+    return pci_bridge_get_limit(bridge, type)>=
+        pci_bridge_get_base(bridge, type) ?
+        pci_bridge_get_limit(bridge, type) -
+        pci_bridge_get_base(bridge, type)  + 1 : 0;
+}

Correct but unreadable. Doesn't work for limit == 2^64-1, is this a possible value?

+
+static void pci_bridge_region_init(PCIBridge *br)
+{
+    PCIBus *sec_bus =&br->sec_bus;
+    PCIBus *parent = br->dev.bus;
+    memory_region_init_alias(sec_bus->alias_pref_mem, "pci_bridge_pref_mem",
+            sec_bus->address_space_mem,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_MEM_PREFETCH),
+            pci_bridge_get_size(&br->dev, PCI_BASE_ADDRESS_MEM_PREFETCH));
+    memory_region_add_subregion_overlap(parent->address_space_mem,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_MEM_PREFETCH),
+             sec_bus->alias_pref_mem, 1);
+    memory_region_init_alias(sec_bus->alias_mem, "pci_bridge_memory",
+            sec_bus->address_space_mem,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
+            pci_bridge_get_size(&br->dev, PCI_BASE_ADDRESS_SPACE_MEMORY));
+    memory_region_add_subregion_overlap(parent->address_space_mem,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
+             sec_bus->alias_mem, 1);
+    memory_region_init_alias(sec_bus->alias_io, "pci_bridge_io",
+            sec_bus->address_space_io,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_SPACE_IO),
+            pci_bridge_get_size(&br->dev, PCI_BASE_ADDRESS_SPACE_IO));
+    memory_region_add_subregion_overlap(parent->address_space_io,
+            pci_bridge_get_base(&br->dev, PCI_BASE_ADDRESS_SPACE_IO),
+             sec_bus->alias_io, 1);
+}

This looks right. Might want to use pci_address_space() instead of ->address_space_mem.

Don't you have to do something similar for the vga window?

+
+static void pci_bridge_region_cleanup(PCIBridge *br)
+{
+    PCIBus *sec_bus =&br->sec_bus;
+    PCIBus *parent = br->dev.bus;
+    memory_region_del_subregion(parent->address_space_mem,
+                                sec_bus->alias_pref_mem);
+    memory_region_destroy(sec_bus->alias_pref_mem);
+    memory_region_del_subregion(parent->address_space_mem,
+                                sec_bus->alias_mem);
+    memory_region_destroy(sec_bus->alias_mem);
+    memory_region_del_subregion(parent->address_space_io,
+                                sec_bus->alias_io);
+    memory_region_destroy(sec_bus->alias_io);
+}

This is fine too.

+
+static void pci_bridge_update_mappings(PCIBridge *br)
+{
+    /* TODO: this doesn't handle the case of one VCPU
+     * updating the bridge while another accesses an unaffected
+     * region. To fix we'll need new memory region APIs. */
+    pci_bridge_region_cleanup(br);
+    pci_bridge_region_init(br);

memory_region_transaction_{begin,commit}()

(isn't 100% implemented, but at least the API is in place)
+
+#if 0
+    TODO: do we need to propagate updates to child buses?
+
+    pci_for_each_device_under_bus(b, pci_bridge_update_mappings_fn);
+
+    QLIST_FOREACH(child,&b->child, sibling) {
+        pci_bridge_update_mappings(child);
+    }
+#endif
+}

Don't need this.

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]