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

[Michael S. Tsirkin]

On Sat, Dec 08, 2018 at 11:58:59AM +0000, xuyandong wrote:
> 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

This one is empty though right?

>  
> 
> 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.


OK however as you show below the BAR being sized is the BAR
if a bridge. Are you then adding a bridge device by hotplug?



> 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.

For sure. But I'm still puzzled as to why does Linux try to
size the BAR of the bridge while a device behind it is
used.

Can you pls post your QEMU command line?



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