pci_bridge: fix abort due to memory region API violation

Submitted by Michael Roth on Oct. 29, 2012, 3:36 p.m.

Details

Message ID 1351525013-9829-1-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth Oct. 29, 2012, 3:36 p.m.
2be0e25f added an assertion that memory_region_destroy() is not called
during a transaction. pci_bridge_update_mappings() wants to do this so
that it can remove existing subregions and add new ones in their
place as an atomic operation.

Work around this by storing away the old subregions and destroying them
after the transaction is complete.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/pci_bridge.c |   41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

Comments

Avi Kivity Oct. 29, 2012, 3:57 p.m.
On 10/29/2012 05:36 PM, Michael Roth wrote:
> 2be0e25f added an assertion that memory_region_destroy() is not called
> during a transaction. pci_bridge_update_mappings() wants to do this so
> that it can remove existing subregions and add new ones in their
> place as an atomic operation.
> 
> Work around this by storing away the old subregions and destroying them
> after the transaction is complete.
> 

An equivalent patch was already posted, see
http://thread.gmane.org/gmane.comp.emulators.qemu/177418/focus=177853.

Patch hide | download patch | download mbox

diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 5c6455f..06f0082 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -151,10 +151,14 @@  static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
     memory_region_add_subregion_overlap(parent_space, base, alias, 1);
 }
 
-static void pci_bridge_cleanup_alias(MemoryRegion *alias,
-                                     MemoryRegion *parent_space)
+static void pci_bridge_remove_alias(MemoryRegion *alias,
+                                    MemoryRegion *parent_space)
 {
     memory_region_del_subregion(parent_space, alias);
+}
+
+static void pci_bridge_destroy_alias(MemoryRegion *alias)
+{
     memory_region_destroy(alias);
 }
 
@@ -187,22 +191,41 @@  static void pci_bridge_region_init(PCIBridge *br)
 static void pci_bridge_region_cleanup(PCIBridge *br)
 {
     PCIBus *parent = br->dev.bus;
-    pci_bridge_cleanup_alias(&br->alias_io,
-                             parent->address_space_io);
-    pci_bridge_cleanup_alias(&br->alias_mem,
-                             parent->address_space_mem);
-    pci_bridge_cleanup_alias(&br->alias_pref_mem,
-                             parent->address_space_mem);
+    pci_bridge_remove_alias(&br->alias_io,
+                            parent->address_space_io);
+    pci_bridge_destroy_alias(&br->alias_io);
+    pci_bridge_remove_alias(&br->alias_mem,
+                            parent->address_space_mem);
+    pci_bridge_destroy_alias(&br->alias_mem);
+    pci_bridge_remove_alias(&br->alias_pref_mem,
+                            parent->address_space_mem);
+    pci_bridge_destroy_alias(&br->alias_pref_mem);
 }
 
 static void pci_bridge_update_mappings(PCIBridge *br)
 {
+    PCIBus *parent = br->dev.bus;
+    MemoryRegion alias_io_old, alias_mem_old, alias_pref_mem_old;
+
+    alias_io_old = br->alias_io;
+    alias_mem_old = br->alias_mem;
+    alias_pref_mem_old = br->alias_pref_mem;
+
     /* Make updates atomic to: handle the case of one VCPU updating the bridge
      * while another accesses an unaffected region. */
     memory_region_transaction_begin();
-    pci_bridge_region_cleanup(br);
+    pci_bridge_remove_alias(&br->alias_io,
+                            parent->address_space_io);
+    pci_bridge_remove_alias(&br->alias_mem,
+                            parent->address_space_mem);
+    pci_bridge_remove_alias(&br->alias_pref_mem,
+                            parent->address_space_mem);
     pci_bridge_region_init(br);
     memory_region_transaction_commit();
+
+    pci_bridge_destroy_alias(&alias_io_old);
+    pci_bridge_destroy_alias(&alias_mem_old);
+    pci_bridge_destroy_alias(&alias_pref_mem_old);
 }
 
 /* default write_config function for PCI-to-PCI bridge */