"Zuo,Boqun" <zuobo...@baidu.com> writes:
> On Wed, Sep 25, 2024 8:58 PM Junjie Mao wrote: >> > As shown below, if a virtio PCI device is attached under a pci-bridge, >> > the MR of VirtIOPCIRegion does not belong to any address space. So >> > memory_region_find cannot be used to search for this MR. >> > >> > 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") >> > >> > Signed-off-by: Gao Shiyuan <gaoshiy...@baidu.com> >> > Signed-off-by: Zuo Boqun <zuobo...@baidu.com> >> > --- >> > hw/pci/pci_bridge.c | 2 ++ >> > hw/virtio/virtio-pci.c | 3 +++ >> > include/hw/pci/pci_bridge.h | 1 + >> > include/hw/virtio/virtio-pci.h | 1 + >> > 4 files changed, 7 insertions(+) >> > >> > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index >> > 6a4e38856d..74683e7445 100644 >> > --- a/hw/pci/pci_bridge.c >> > +++ b/hw/pci/pci_bridge.c >> > @@ -380,6 +380,7 @@ void pci_bridge_initfn(PCIDevice *dev, const char >> *typename) >> > sec_bus->map_irq = br->map_irq ? br->map_irq : >> pci_swizzle_map_irq_fn; >> > sec_bus->address_space_mem = &br->address_space_mem; >> > memory_region_init(&br->address_space_mem, OBJECT(br), >> > "pci_bridge_pci", UINT64_MAX); >> > + address_space_init(&br->as_mem, &br->address_space_mem, >> > + "pci_bridge_pci"); >> >> I don't think this "pci_bridge_pci" address space is necessary. The >> VirtIOPCIProxy.modern_as AddressSpace is sufficient for >> memory_region_add() to work. > > This is because memory_region_find_rcu () and > memory_region_to_address_space() will find the memory container > on the top level and then get the corresponding address space of it. > If we only add "VirtIOPCIProxy.modern_as AddressSpace", memory_region_find() > still cannot find a address space. > Because "pci_bridge_pci" is the memory container on top level and it doesn't > have a corresponding address space. Got your point. That address space can help when a so-ill-behaved guest enables both the bridge and the virtio device, clears the memory base of secondary bus and then accesses the virtio configurations via the PCI config access cap. Thanks for the clarification. As mentioned earlier, you may want to add the I/O space counterparts for both address spaces, as the cap can target modern I/O BARs (when they are present) as well. -- Best Regards Junjie Mao
