"Gao,Shiyuan" via <qemu-devel@nongnu.org> writes: >> > >> > Introduce the virtio-pci and pci_bridge_pci address spaces to solve this >> > problem. >> > >> > Before: >> > memory-region: pci_bridge_pci >> > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci >> > 00000000fe200000-00000000fe200fff (prio 1, i/o): virtio-blk-pci-msix >> > 00000000fe200000-00000000fe20016f (prio 0, i/o): msix-table >> > 00000000fe200800-00000000fe200807 (prio 0, i/o): msix-pba >> > 000000a000400000-000000a000403fff (prio 1, i/o): virtio-pci >> > 000000a000400000-000000a000400fff (prio 0, i/o): >> >virtio-pci-common-virtio-blk >> > 000000a000401000-000000a000401fff (prio 0, i/o): >> >virtio-pci-isr-virtio-blk >> > 000000a000402000-000000a000402fff (prio 0, i/o): >> >virtio-pci-device-virtio-blk >> > 000000a000403000-000000a000403fff (prio 0, i/o): >> >virtio-pci-notify-virtio-blk >> > >> > After: >> > address-space: pci_bridge_pci >> > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci >> > 00000000fe200000-00000000fe200fff (prio 1, i/o): virtio-blk-pci-msix >> > 00000000fe200000-00000000fe20016f (prio 0, i/o): msix-table >> > 00000000fe200800-00000000fe200807 (prio 0, i/o): msix-pba >> > 000000a000400000-000000a000403fff (prio 1, i/o): virtio-pci >> > 000000a000400000-000000a000400fff (prio 0, i/o): >> >virtio-pci-common-virtio-blk >> > 000000a000401000-000000a000401fff (prio 0, i/o): >> >virtio-pci-isr-virtio-blk >> > 000000a000402000-000000a000402fff (prio 0, i/o): >> >virtio-pci-device-virtio-blk >> > 000000a000403000-000000a000403fff (prio 0, i/o): >> >virtio-pci-notify-virtio-blk >> > >> > address-space: virtio-pci >> > 000000a000400000-000000a000403fff (prio 1, i/o): virtio-pci >> > 000000a000400000-000000a000400fff (prio 0, i/o): >> >virtio-pci-common-virtio-blk >> > 000000a000401000-000000a000401fff (prio 0, i/o): >> >virtio-pci-isr-virtio-blk >> > 000000a000402000-000000a000402fff (prio 0, i/o): >> >virtio-pci-device-virtio-blk >> > 000000a000403000-000000a000403fff (prio 0, i/o): >> >virtio-pci-notify-virtio-blk >> > >> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 >> > Fixes: ffa8a3e ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") >> >> Commit id is not unique. Use 12 digits please. > > Thanks, I will make changes in the next version. > >> >> I'm still not quite sure if memory_region_find() is really the right >> thing to use here, but I'm no expert on that so I'm hoping virtio/PCI >> people can review. >> > > Directly traversing the subregions of VirtIOPCIRegion's MR is a relatively > simple method(Now only notify region MR has subregion and only has one layer). > > But if want to be more general, we need to consider multiple layers and > the priority of subregions, which adds complexity. > > I think it would be better to use memory_region_find. > > Does anyone have any opinions?
I'm new to QEMU memory API, but here's my two cents. What is special here is that virtio PCI transport provides a "PCI configuration access capability" that allows accessing BAR-mapped virtio config regions *BEFORE* BARs are initialized. At that time, the BAR MemoryRegion is not yet added to the PCI MemoryRegion (and thus the memory AddressSpace), and that prevents memory_region_find() from working because the API requires the given MemoryRegion to be in an AddressSpace. I think it is reasonable to add one (or two, one for mmio and other other port i/o?) per-virtio-device address space for such virtio config regions. The PCI capability can be regarded as a window to a register space that is inaccessible otherwise under certain conditions. Also, device-specific address spaces are not rare today. Directly travering the subregions does not look like a good approach as it breaks abstraction. Accesses to memory regions require extra care about RCU and refcount. Scatter such operations in multiple files will make the abstraction harder to maintain. Adding another API for finding a subregion within a region not in any address space is an alternative, but I'm not sure if that looks like an overkill. -- Best Regards Junjie Mao
