| Message ID | 20241009095827.67393-1-gaoshiyuan@baidu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR | expand |
On 09.10.24 11:58, Gao Shiyuan 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 address spaces to solve this problem. > > Before: > memory-region: pci_bridge_pci > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > After: > address-space: virtio-pci-cfg-mem-as > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > address-space: pci_bridge_pci_mem > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > Signed-off-by: Wang Liang <wangliang44@baidu.com> Why the two additional SOB? You're the author and you sent out the patch. If the other two were co-authors, it should be something like Co-developed-by: Zuo Boqun <zuoboqun@baidu.com> Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> Co-developed-by: Wang Liang <wangliang44@baidu.com> Signed-off-by: Wang Liang <wangliang44@baidu.com> Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> [...] > > static void virtio_pci_reset(DeviceState *qdev) > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 5cd452115a..bd12fbe4ef 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -72,6 +72,8 @@ struct PCIBridge { > */ > MemoryRegion address_space_mem; > MemoryRegion address_space_io; > + AddressSpace as_mem; > + AddressSpace as_io; address_space_mem vs. as_mem is really confusing, likely "address_space_mem" shpuld be called "mem_mr" ot sth. like that. Anyhow, that's independent of your patch Acked-by: David Hildenbrand <david@redhat.com>
Gao Shiyuan via <qemu-devel@nongnu.org> writes: > 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 address spaces to solve this problem. > > Before: > memory-region: pci_bridge_pci > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > After: > address-space: virtio-pci-cfg-mem-as > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > address-space: pci_bridge_pci_mem > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > Signed-off-by: Wang Liang <wangliang44@baidu.com> Reviewed-by: Junjie Mao <junjie.mao@hotmail.com> Thanks! -- Best Regards Junjie Mao
Ping. > 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 address spaces to solve this problem. > > Before: > memory-region: pci_bridge_pci > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > After: > address-space: virtio-pci-cfg-mem-as > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > address-space: pci_bridge_pci_mem > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > Signed-off-by: Wang Liang <wangliang44@baidu.com> > --- > hw/pci/pci_bridge.c | 4 ++++ > hw/virtio/virtio-pci.c | 5 +++++ > include/hw/pci/pci_bridge.h | 2 ++ > include/hw/virtio/virtio-pci.h | 3 +++ > 4 files changed, 14 insertions(+) > > v1 -> v2: > * modify commit message > * add address space for port I/O notification config region > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 6a4e38856d..099ea9e24e 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -380,9 +380,11 @@ 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_mem"); > sec_bus->address_space_io = &br->address_space_io; > memory_region_init(&br->address_space_io, OBJECT(br), "pci_bridge_io", > 4 * GiB); > + address_space_init(&br->as_io, &br->address_space_io, "pci_bridge_pci_io"); > pci_bridge_region_init(br); > QLIST_INIT(&sec_bus->child); > QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling); > @@ -399,6 +401,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev) > PCIBridge *s = PCI_BRIDGE(pci_dev); > assert(QLIST_EMPTY(&s->sec_bus.child)); > QLIST_REMOVE(&s->sec_bus, sibling); > + address_space_destroy(&s->as_mem); > + address_space_destroy(&s->as_io); > pci_bridge_region_del(s, &s->windows); > pci_bridge_region_cleanup(s, &s->windows); > /* object_unparent() is called automatically during device deletion */ > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 4d832fe845..1e862dd0df 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -2057,6 +2057,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > if (modern_pio) { > memory_region_init(&proxy->io_bar, OBJECT(proxy), > "virtio-pci-io", 0x4); > + address_space_init(&proxy->modern_cfg_io_as, &proxy->io_bar, "virtio-pci-cfg-io-as"); > > pci_register_bar(&proxy->pci_dev, proxy->modern_io_bar_idx, > PCI_BASE_ADDRESS_SPACE_IO, &proxy->io_bar); > @@ -2180,6 +2181,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) > /* PCI BAR regions must be powers of 2 */ > pow2ceil(proxy->notify.offset + proxy->notify.size)); > > + address_space_init(&proxy->modern_cfg_mem_as, &proxy->modern_bar, "virtio-pci-cfg-mem-as"); > + > if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } > @@ -2275,6 +2278,8 @@ static void virtio_pci_exit(PCIDevice *pci_dev) > pci_is_express(pci_dev)) { > pcie_aer_exit(pci_dev); > } > + address_space_destroy(&proxy->modern_cfg_mem_as); > + address_space_destroy(&proxy->modern_cfg_io_as); > } > > static void virtio_pci_reset(DeviceState *qdev) > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 5cd452115a..bd12fbe4ef 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -72,6 +72,8 @@ struct PCIBridge { > */ > MemoryRegion address_space_mem; > MemoryRegion address_space_io; > + AddressSpace as_mem; > + AddressSpace as_io; > > PCIBridgeWindows windows; > > diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h > index 9e67ba38c7..971c5fabd4 100644 > --- a/include/hw/virtio/virtio-pci.h > +++ b/include/hw/virtio/virtio-pci.h > @@ -147,6 +147,9 @@ struct VirtIOPCIProxy { > }; > MemoryRegion modern_bar; > MemoryRegion io_bar; > + /* address space for VirtIOPCIRegions */ > + AddressSpace modern_cfg_mem_as; > + AddressSpace modern_cfg_io_as; > uint32_t legacy_io_bar_idx; > uint32_t msix_bar_idx; > uint32_t modern_io_bar_idx; > -- > 2.34.1
On Wed, Oct 09, 2024 at 05:58:27PM +0800, Gao Shiyuan via 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 address spaces to solve this problem. > > Before: > memory-region: pci_bridge_pci > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > After: > address-space: virtio-pci-cfg-mem-as > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > address-space: pci_bridge_pci_mem > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > Signed-off-by: Wang Liang <wangliang44@baidu.com> > --- > hw/pci/pci_bridge.c | 4 ++++ > hw/virtio/virtio-pci.c | 5 +++++ > include/hw/pci/pci_bridge.h | 2 ++ > include/hw/virtio/virtio-pci.h | 3 +++ > 4 files changed, 14 insertions(+) Should we add some qtest coverage for this perhaps. eg per the bug report, something like this should demonstrate the crash: static void test_addr_space_crash(void) { QTestState *s = qtest_init("-device virtio-balloon -machine q35 -nodefaults"); qtest_outl(s, 0xcf8, 0x80000890); qtest_outl(s, 0xcfc, 0x2); qtest_outl(s, 0xcf8, 0x80000891); qtest_inl(s, 0xcfc); qtest_quit(s); } if we add that to one of the tests/qtest/*.c files as appropriate, or create a new qtest file ? With regards, Daniel
On Wed, Oct 09, 2024 at 05:58:27PM +0800, Gao Shiyuan 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 address spaces to solve this problem. > > Before: > memory-region: pci_bridge_pci > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > After: > address-space: virtio-pci-cfg-mem-as > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > address-space: pci_bridge_pci_mem > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > no empty lines between headers pls. > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > Signed-off-by: Wang Liang <wangliang44@baidu.com> Daniel can you pls confirm it fixes the bug you reported with bsd? > --- > hw/pci/pci_bridge.c | 4 ++++ > hw/virtio/virtio-pci.c | 5 +++++ > include/hw/pci/pci_bridge.h | 2 ++ > include/hw/virtio/virtio-pci.h | 3 +++ > 4 files changed, 14 insertions(+) > > v1 -> v2: > * modify commit message > * add address space for port I/O notification config region > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 6a4e38856d..099ea9e24e 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -380,9 +380,11 @@ 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_mem"); > sec_bus->address_space_io = &br->address_space_io; > memory_region_init(&br->address_space_io, OBJECT(br), "pci_bridge_io", > 4 * GiB); > + address_space_init(&br->as_io, &br->address_space_io, "pci_bridge_pci_io"); > pci_bridge_region_init(br); > QLIST_INIT(&sec_bus->child); > QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling); > @@ -399,6 +401,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev) > PCIBridge *s = PCI_BRIDGE(pci_dev); > assert(QLIST_EMPTY(&s->sec_bus.child)); > QLIST_REMOVE(&s->sec_bus, sibling); > + address_space_destroy(&s->as_mem); > + address_space_destroy(&s->as_io); > pci_bridge_region_del(s, &s->windows); > pci_bridge_region_cleanup(s, &s->windows); > /* object_unparent() is called automatically during device deletion */ > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 4d832fe845..1e862dd0df 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -2057,6 +2057,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > if (modern_pio) { > memory_region_init(&proxy->io_bar, OBJECT(proxy), > "virtio-pci-io", 0x4); > + address_space_init(&proxy->modern_cfg_io_as, &proxy->io_bar, "virtio-pci-cfg-io-as"); > > pci_register_bar(&proxy->pci_dev, proxy->modern_io_bar_idx, > PCI_BASE_ADDRESS_SPACE_IO, &proxy->io_bar); > @@ -2180,6 +2181,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) > /* PCI BAR regions must be powers of 2 */ > pow2ceil(proxy->notify.offset + proxy->notify.size)); > > + address_space_init(&proxy->modern_cfg_mem_as, &proxy->modern_bar, "virtio-pci-cfg-mem-as"); > + > if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } > @@ -2275,6 +2278,8 @@ static void virtio_pci_exit(PCIDevice *pci_dev) > pci_is_express(pci_dev)) { > pcie_aer_exit(pci_dev); > } > + address_space_destroy(&proxy->modern_cfg_mem_as); > + address_space_destroy(&proxy->modern_cfg_io_as); > } > > static void virtio_pci_reset(DeviceState *qdev) > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 5cd452115a..bd12fbe4ef 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -72,6 +72,8 @@ struct PCIBridge { > */ > MemoryRegion address_space_mem; > MemoryRegion address_space_io; > + AddressSpace as_mem; > + AddressSpace as_io; > > PCIBridgeWindows windows; > > diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h > index 9e67ba38c7..971c5fabd4 100644 > --- a/include/hw/virtio/virtio-pci.h > +++ b/include/hw/virtio/virtio-pci.h > @@ -147,6 +147,9 @@ struct VirtIOPCIProxy { > }; > MemoryRegion modern_bar; > MemoryRegion io_bar; > + /* address space for VirtIOPCIRegions */ > + AddressSpace modern_cfg_mem_as; > + AddressSpace modern_cfg_io_as; > uint32_t legacy_io_bar_idx; > uint32_t msix_bar_idx; > uint32_t modern_io_bar_idx; > -- > 2.34.1
On Tue, Oct 29, 2024 at 03:44:01PM -0400, Michael S. Tsirkin wrote: > On Wed, Oct 09, 2024 at 05:58:27PM +0800, Gao Shiyuan 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 address spaces to solve this problem. > > > > Before: > > memory-region: pci_bridge_pci > > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > > > After: > > address-space: virtio-pci-cfg-mem-as > > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > > > address-space: pci_bridge_pci_mem > > 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci > > 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix > > 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table > > 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba > > 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci > > 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net > > 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net > > 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net > > 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 > > Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") > > > > no empty lines between headers pls. > > > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> > > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> > > Signed-off-by: Wang Liang <wangliang44@baidu.com> > > > Daniel can you pls confirm it fixes the bug you reported with bsd? yes, it works Tested-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 6a4e38856d..099ea9e24e 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -380,9 +380,11 @@ 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_mem"); sec_bus->address_space_io = &br->address_space_io; memory_region_init(&br->address_space_io, OBJECT(br), "pci_bridge_io", 4 * GiB); + address_space_init(&br->as_io, &br->address_space_io, "pci_bridge_pci_io"); pci_bridge_region_init(br); QLIST_INIT(&sec_bus->child); QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling); @@ -399,6 +401,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev) PCIBridge *s = PCI_BRIDGE(pci_dev); assert(QLIST_EMPTY(&s->sec_bus.child)); QLIST_REMOVE(&s->sec_bus, sibling); + address_space_destroy(&s->as_mem); + address_space_destroy(&s->as_io); pci_bridge_region_del(s, &s->windows); pci_bridge_region_cleanup(s, &s->windows); /* object_unparent() is called automatically during device deletion */ diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 4d832fe845..1e862dd0df 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -2057,6 +2057,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) if (modern_pio) { memory_region_init(&proxy->io_bar, OBJECT(proxy), "virtio-pci-io", 0x4); + address_space_init(&proxy->modern_cfg_io_as, &proxy->io_bar, "virtio-pci-cfg-io-as"); pci_register_bar(&proxy->pci_dev, proxy->modern_io_bar_idx, PCI_BASE_ADDRESS_SPACE_IO, &proxy->io_bar); @@ -2180,6 +2181,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) /* PCI BAR regions must be powers of 2 */ pow2ceil(proxy->notify.offset + proxy->notify.size)); + address_space_init(&proxy->modern_cfg_mem_as, &proxy->modern_bar, "virtio-pci-cfg-mem-as"); + if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } @@ -2275,6 +2278,8 @@ static void virtio_pci_exit(PCIDevice *pci_dev) pci_is_express(pci_dev)) { pcie_aer_exit(pci_dev); } + address_space_destroy(&proxy->modern_cfg_mem_as); + address_space_destroy(&proxy->modern_cfg_io_as); } static void virtio_pci_reset(DeviceState *qdev) diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 5cd452115a..bd12fbe4ef 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -72,6 +72,8 @@ struct PCIBridge { */ MemoryRegion address_space_mem; MemoryRegion address_space_io; + AddressSpace as_mem; + AddressSpace as_io; PCIBridgeWindows windows; diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h index 9e67ba38c7..971c5fabd4 100644 --- a/include/hw/virtio/virtio-pci.h +++ b/include/hw/virtio/virtio-pci.h @@ -147,6 +147,9 @@ struct VirtIOPCIProxy { }; MemoryRegion modern_bar; MemoryRegion io_bar; + /* address space for VirtIOPCIRegions */ + AddressSpace modern_cfg_mem_as; + AddressSpace modern_cfg_io_as; uint32_t legacy_io_bar_idx; uint32_t msix_bar_idx; uint32_t modern_io_bar_idx;
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 address spaces to solve this problem. Before: memory-region: pci_bridge_pci 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net After: address-space: virtio-pci-cfg-mem-as 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net address-space: pci_bridge_pci_mem 0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci 00000000fe840000-00000000fe840fff (prio 1, i/o): virtio-net-pci-msix 00000000fe840000-00000000fe84003f (prio 0, i/o): msix-table 00000000fe840800-00000000fe840807 (prio 0, i/o): msix-pba 0000380000000000-0000380000003fff (prio 1, i/o): virtio-pci 0000380000000000-0000380000000fff (prio 0, i/o): virtio-pci-common-virtio-net 0000380000001000-0000380000001fff (prio 0, i/o): virtio-pci-isr-virtio-net 0000380000002000-0000380000002fff (prio 0, i/o): virtio-pci-device-virtio-net 0000380000003000-0000380000003fff (prio 0, i/o): virtio-pci-notify-virtio-net Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2576 Fixes: ffa8a3e3b2e6 ("virtio-pci: Add lookup subregion of VirtIOPCIRegion MR") Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com> Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> Signed-off-by: Wang Liang <wangliang44@baidu.com> --- hw/pci/pci_bridge.c | 4 ++++ hw/virtio/virtio-pci.c | 5 +++++ include/hw/pci/pci_bridge.h | 2 ++ include/hw/virtio/virtio-pci.h | 3 +++ 4 files changed, 14 insertions(+) v1 -> v2: * modify commit message * add address space for port I/O notification config region