[Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug

* [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
@ 2018-12-08 11:58 xuyandong
  2018-12-09 14:26 ` Michael S. Tsirkin
  2018-12-10  2:22 ` Michael S. Tsirkin
  0 siblings, 2 replies; 18+ messages in thread
From: xuyandong @ 2018-12-08 11:58 UTC (permalink / raw)
  To: mst, marcel, Paolo Bonzini
  Cc: qemu-devel, Zhanghailiang, wangxin (U), Huangweidong (C)

Hi all,

In our test, we configured VM with several pci-bridges and a virtio-net nic been attached with bus 4,
After VM is startup, We ping this nic from host to judge if it is working normally. Then, we hot add pci devices to this VM with bus 0.
We  found the virtio-net NIC in bus 4 is not working (can not connect) occasionally, as it kick virtio backend failure with error below:
    Unassigned mem write 00000000fc803004 = 0x1

memory-region: pci_bridge_pci
  0000000000000000-ffffffffffffffff (prio 0, RW): pci_bridge_pci
    00000000fc800000-00000000fc803fff (prio 1, RW): virtio-pci
      00000000fc800000-00000000fc800fff (prio 0, RW): virtio-pci-common
      00000000fc801000-00000000fc801fff (prio 0, RW): virtio-pci-isr
      00000000fc802000-00000000fc802fff (prio 0, RW): virtio-pci-device
      00000000fc803000-00000000fc803fff (prio 0, RW): virtio-pci-notify  <- io mem unassigned
      …

We caught an exceptional address changing while this problem happened, show as follow:
Before pci_bridge_update_mappings：
      00000000fc000000-00000000fc1fffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fc000000-00000000fc1fffff
      00000000fc200000-00000000fc3fffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fc200000-00000000fc3fffff
      00000000fc400000-00000000fc5fffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fc400000-00000000fc5fffff
      00000000fc600000-00000000fc7fffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fc600000-00000000fc7fffff
      00000000fc800000-00000000fc9fffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fc800000-00000000fc9fffff <- correct Adress Spce
      00000000fca00000-00000000fcbfffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fca00000-00000000fcbfffff
      00000000fcc00000-00000000fcdfffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fcc00000-00000000fcdfffff
      00000000fce00000-00000000fcffffff (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci 00000000fce00000-00000000fcffffff

After pci_bridge_update_mappings：
      00000000fda00000-00000000fdbfffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fda00000-00000000fdbfffff
      00000000fdc00000-00000000fddfffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fdc00000-00000000fddfffff
      00000000fde00000-00000000fdffffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fde00000-00000000fdffffff
      00000000fe000000-00000000fe1fffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fe000000-00000000fe1fffff
      00000000fe200000-00000000fe3fffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fe200000-00000000fe3fffff
      00000000fe400000-00000000fe5fffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fe400000-00000000fe5fffff
      00000000fe600000-00000000fe7fffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fe600000-00000000fe7fffff
      00000000fe800000-00000000fe9fffff (prio 1, RW): alias pci_bridge_mem @pci_bridge_pci 00000000fe800000-00000000fe9fffff
      fffffffffc800000-fffffffffc800000 (prio 1, RW): alias pci_bridge_pref_mem @pci_bridge_pci fffffffffc800000-fffffffffc800000   <- Exceptional Adress Space

We have figured out why this address becomes this value,  according to pci spec,  pci driver can get BAR address size by writing 0xffffffff to
the pci register firstly, and then read back the value from this register.
We didn't handle this value  specially while process pci write in qemu, the function call stack is:
Pci_bridge_dev_write_config
-> pci_bridge_write_config
-> pci_default_write_config (we update the config[address] value here to fffffffffc800000, which should be 0xfc800000 )
-> pci_bridge_update_mappings
                ->pci_bridge_region_del(br, br->windows);
-> pci_bridge_region_init
                                                                ->pci_bridge_init_alias (here pci_bridge_get_base, we use the wrong value fffffffffc800000)
                                                -> memory_region_transaction_commit

So, as we can see, we use the wrong base address in qemu to update the memory regions, though, we update the base address to
The correct value after pci driver in VM write the original value back, the virtio NIC in bus 4 may still sends net packets concurrently with
The wrong memory region address.

We have tried to skip the memory region update action in qemu while detect pci write with 0xffffffff value, and it does work, but
This seems to be not gently.

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index b2e50c3..84b405d 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -256,7 +256,8 @@ void pci_bridge_write_config(PCIDevice *d,
     pci_default_write_config(d, address, val, len);
-    if (ranges_overlap(address, len, PCI_COMMAND, 2) ||
+    if ( (val != 0xffffffff) &&
+        (ranges_overlap(address, len, PCI_COMMAND, 2) ||
         /* io base/limit */
         ranges_overlap(address, len, PCI_IO_BASE, 2) ||
@@ -266,7 +267,7 @@ void pci_bridge_write_config(PCIDevice *d,
         ranges_overlap(address, len, PCI_MEMORY_BASE, 20) ||
         /* vga enable */
-        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 2)) {
+        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 2))) {
         pci_bridge_update_mappings(s);
     }

Thinks,
Xu

^ permalink raw reply related	[flat|nested] 18+ messages in thread