* [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
@ 2024-10-09 9:58 Gao Shiyuan via
2024-10-11 8:18 ` David Hildenbrand
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Gao Shiyuan via @ 2024-10-09 9:58 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum
Cc: gaoshiyuan, zuoboqun, david, qemu-devel, Junjie Mao, wangliang44
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
@ 2024-10-11 8:18 ` David Hildenbrand
2024-10-11 10:00 ` Junjie Mao
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand @ 2024-10-11 8:18 UTC (permalink / raw)
To: Gao Shiyuan, Michael S. Tsirkin, Marcel Apfelbaum
Cc: zuoboqun, qemu-devel, Junjie Mao, wangliang44
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>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
2024-10-11 8:18 ` David Hildenbrand
@ 2024-10-11 10:00 ` Junjie Mao
2024-10-29 15:21 ` Gao,Shiyuan via
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Junjie Mao @ 2024-10-11 10:00 UTC (permalink / raw)
To: Gao Shiyuan
Cc: Michael S. Tsirkin, Marcel Apfelbaum, zuoboqun, david,
wangliang44, qemu-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
2024-10-11 8:18 ` David Hildenbrand
2024-10-11 10:00 ` Junjie Mao
@ 2024-10-29 15:21 ` Gao,Shiyuan via
2024-10-29 17:07 ` Daniel P. Berrangé
2024-10-29 19:44 ` Michael S. Tsirkin
4 siblings, 0 replies; 7+ messages in thread
From: Gao,Shiyuan via @ 2024-10-29 15:21 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum
Cc: Zuo,Boqun, david@redhat.com, qemu-devel@nongnu.org, Junjie Mao,
Wang,Liang(ACG CCN)
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
` (2 preceding siblings ...)
2024-10-29 15:21 ` Gao,Shiyuan via
@ 2024-10-29 17:07 ` Daniel P. Berrangé
2024-10-29 19:44 ` Michael S. Tsirkin
4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2024-10-29 17:07 UTC (permalink / raw)
To: Gao Shiyuan
Cc: Michael S. Tsirkin, Marcel Apfelbaum, zuoboqun, david, qemu-devel,
Junjie Mao, wangliang44
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
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
` (3 preceding siblings ...)
2024-10-29 17:07 ` Daniel P. Berrangé
@ 2024-10-29 19:44 ` Michael S. Tsirkin
2024-10-30 11:36 ` Daniel P. Berrangé
4 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2024-10-29 19:44 UTC (permalink / raw)
To: Gao Shiyuan
Cc: Marcel Apfelbaum, zuoboqun, david, qemu-devel, Junjie Mao,
wangliang44, Daniel P. Berrangé
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR
2024-10-29 19:44 ` Michael S. Tsirkin
@ 2024-10-30 11:36 ` Daniel P. Berrangé
0 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2024-10-30 11:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gao Shiyuan, Marcel Apfelbaum, zuoboqun, david, qemu-devel,
Junjie Mao, wangliang44
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
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-30 11:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 9:58 [PATCH v2 1/1] virtio-pci: fix memory_region_find for VirtIOPCIRegion's MR Gao Shiyuan via
2024-10-11 8:18 ` David Hildenbrand
2024-10-11 10:00 ` Junjie Mao
2024-10-29 15:21 ` Gao,Shiyuan via
2024-10-29 17:07 ` Daniel P. Berrangé
2024-10-29 19:44 ` Michael S. Tsirkin
2024-10-30 11:36 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).