Patchwork pci_bridge: fix abort due to memory region API violation

login
register
mail settings
Submitter Michael Roth
Date Oct. 29, 2012, 3:36 p.m.
Message ID <1351525013-9829-1-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/195055/
State New
Headers show

Comments

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(-)
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

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 */