[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq se
From: |
Paul Durrant |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server |
Date: |
Tue, 9 Jun 2015 09:05:13 +0000 |
> -----Original Message-----
> From: Don Slutz [mailto:address@hidden
> Sent: 08 June 2015 22:19
> To: address@hidden; address@hidden
> Cc: Michael S. Tsirkin; Paul Durrant; Stefano Stabellini; Don Slutz; Don Slutz
> Subject: [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server
>
> The commit 707ff80021ccd7a68f4b3d2c44eebf87efbb41c4 added usage of
> xc_hvm_map_pcidev_from_ioreq_server() and
> xc_hvm_unmap_pcidev_from_ioreq_server(). These routines only
> correctly work if the PCI BDF of a PCI device is unique. The 3
> parts (bus, device, and function) of a PCI BDF are not required to
> be unique.
>
> The PCI BDF is accessed in QEMU:
> bus pci_bus_num()
> device PCI_SLOT()
> function PCI_FUNC()
>
> Add a hash table to track the current set of PCI BDFs and only call
> on xc_hvm_map_pcidev_from_ioreq_server() and
> xc_hvm_unmap_pcidev_from_ioreq_server() when needed.
>
This seems to imply that the devices are being realized multiple times without
being unrealized? Is that really the case?
Paul
> Also fix the PCI BDF default stderr trace output: bus is seperated
> by ':', and function is only 1 digit.
>
> Here is an example of stderr trace output:
>
> xen_map_pcidev id: 1 bdf: 00:00.0
> xen_map_pcidev id: 1 bdf: 00:01.0
> xen_map_pcidev id: 1 bdf: 00:01.1
> xen_map_pcidev id: 1 bdf: 00:01.3
> xen_map_pcidev id: 1 bdf: 00:02.0
> xen_map_pcidev id: 1 bdf: 00:03.0
> xen_map_pcidev id: 1 bdf: 00:04.0
> xen_map_pcidev id: 1 bdf: 00:05.0
> xen_map_pcidev id: 1 bdf: 00:11.0
> xen_map_pcidev id: 1 bdf: 00:15.0
> xen_map_pcidev id: 1 bdf: 00:16.0
> xen_map_pcidev id: 1 bdf: 00:17.0
> xen_map_pcidev id: 1 bdf: 00:18.0
> xen_map_pcidev_dup id: 1 bdf: 00:01.0 dup: 2
> xen_map_pcidev_dup id: 1 bdf: 00:01.0 dup: 3
> xen_map_pcidev_dup id: 1 bdf: 00:03.0 dup: 2
> xen_map_pcidev_dup id: 1 bdf: 00:03.0 dup: 3
> xen_map_pcidev_dup id: 1 bdf: 00:04.0 dup: 2
> xen_unmap_pcidev_dup id: 1 bdf: 00:03.0 dup: 2
> xen_map_pcidev id: 1 bdf: ff:03.0
> xen_unmap_pcidev id: 1 bdf: 00:17.0
> xen_map_pcidev id: 1 bdf: ff:17.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:01.0 dup: 2
> xen_map_pcidev id: 1 bdf: ff:01.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:03.0 dup: 1
> xen_map_pcidev_dup id: 1 bdf: ff:03.0 dup: 2
> xen_unmap_pcidev_dup id: 1 bdf: 00:04.0 dup: 1
> xen_map_pcidev id: 1 bdf: ff:04.0
> xen_unmap_pcidev_dup id: 1 bdf: ff:03.0 dup: 1
> xen_map_pcidev id: 1 bdf: 01:03.0
> xen_unmap_pcidev id: 1 bdf: ff:17.0
> xen_map_pcidev id: 1 bdf: 01:17.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:01.0 dup: 1
> xen_map_pcidev_dup id: 1 bdf: ff:01.0 dup: 2
> xen_unmap_pcidev_dup id: 1 bdf: ff:01.0 dup: 1
> xen_map_pcidev id: 1 bdf: 02:01.0
> xen_unmap_pcidev id: 1 bdf: ff:01.0
> xen_map_pcidev id: 1 bdf: 03:01.0
> xen_unmap_pcidev id: 1 bdf: ff:03.0
> xen_map_pcidev id: 1 bdf: 03:03.0
> xen_unmap_pcidev id: 1 bdf: ff:04.0
> xen_map_pcidev id: 1 bdf: 03:04.0
> xen_unmap_pcidev id: 1 bdf: 00:04.0
> xen_unmap_pcidev id: 1 bdf: 00:05.0
> xen_unmap_pcidev id: 1 bdf: 01:03.0
> xen_map_pcidev_dup id: 1 bdf: 00:03.0 dup: 2
> xen_unmap_pcidev id: 1 bdf: 01:17.0
> xen_map_pcidev id: 1 bdf: 00:17.0
> xen_unmap_pcidev id: 1 bdf: 03:01.0
> xen_map_pcidev_dup id: 1 bdf: 00:01.0 dup: 2
> xen_unmap_pcidev id: 1 bdf: 03:03.0
> xen_map_pcidev_dup id: 1 bdf: 00:03.0 dup: 3
> xen_unmap_pcidev id: 1 bdf: 03:04.0
> xen_map_pcidev id: 1 bdf: 00:04.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:03.0 dup: 2
> xen_map_pcidev id: 1 bdf: 01:03.0
> xen_unmap_pcidev id: 1 bdf: 00:17.0
> xen_map_pcidev id: 1 bdf: 01:17.0
> xen_unmap_pcidev id: 1 bdf: 02:01.0
> xen_map_pcidev_dup id: 1 bdf: 00:01.0 dup: 3
> xen_unmap_pcidev_dup id: 1 bdf: 00:01.0 dup: 2
> xen_map_pcidev id: 1 bdf: 02:01.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:01.0 dup: 1
> xen_map_pcidev id: 1 bdf: 03:01.0
> xen_unmap_pcidev_dup id: 1 bdf: 00:03.0 dup: 1
> xen_map_pcidev id: 1 bdf: 03:03.0
> xen_unmap_pcidev id: 1 bdf: 00:04.0
> xen_map_pcidev id: 1 bdf: 03:04.0
>
> Signed-off-by: Don Slutz <address@hidden>
> CC: Don Slutz <address@hidden>
> ---
> include/hw/xen/xen_common.h | 53
> +++++++++++++++++++++++++++++++++++----------
> trace-events | 6 +++--
> xen-hvm.c | 15 ++++++++-----
> 3 files changed, 55 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> index 6579b78..260ee58 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -223,12 +223,14 @@ static inline void xen_unmap_io_section(XenXC xc,
> domid_t dom,
>
> static inline void xen_map_pcidev(XenXC xc, domid_t dom,
> ioservid_t ioservid,
> + GHashTable *pci_bdf,
> PCIDevice *pci_dev)
> {
> }
>
> static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
> ioservid_t ioservid,
> + GHashTable *pci_bdf,
> PCIDevice *pci_dev,
> uint8_t oldbus)
> {
> @@ -346,27 +348,54 @@ static inline void xen_unmap_io_section(XenXC xc,
> domid_t dom,
>
> static inline void xen_map_pcidev(XenXC xc, domid_t dom,
> ioservid_t ioservid,
> + GHashTable *pci_bdf,
> PCIDevice *pci_dev)
> {
> - trace_xen_map_pcidev(ioservid, pci_bus_num(pci_dev->bus),
> - PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn));
> - xc_hvm_map_pcidev_to_ioreq_server(xc, dom, ioservid,
> - 0, pci_bus_num(pci_dev->bus),
> - PCI_SLOT(pci_dev->devfn),
> - PCI_FUNC(pci_dev->devfn));
> + gpointer bdf_key = GINT_TO_POINTER((pci_bus_num(pci_dev->bus) <<
> 8) |
> + pci_dev->devfn);
> + gint bdf_cnt = GPOINTER_TO_INT(g_hash_table_lookup(pci_bdf,
> bdf_key));
> +
> + if (!bdf_cnt) {
> + trace_xen_map_pcidev(ioservid, pci_bus_num(pci_dev->bus),
> + PCI_SLOT(pci_dev->devfn),
> + PCI_FUNC(pci_dev->devfn));
> + g_hash_table_insert(pci_bdf, bdf_key, GINT_TO_POINTER(1));
> + xc_hvm_map_pcidev_to_ioreq_server(xc, dom, ioservid,
> + 0, pci_bus_num(pci_dev->bus),
> + PCI_SLOT(pci_dev->devfn),
> + PCI_FUNC(pci_dev->devfn));
> + } else {
> + trace_xen_map_pcidev_dup(ioservid, pci_bus_num(pci_dev->bus),
> + PCI_SLOT(pci_dev->devfn),
> + PCI_FUNC(pci_dev->devfn),
> + bdf_cnt + 1);
> + g_hash_table_replace(pci_bdf, bdf_key, GINT_TO_POINTER(bdf_cnt +
> 1));
> + }
> }
>
> static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
> ioservid_t ioservid,
> + GHashTable *pci_bdf,
> PCIDevice *pci_dev,
> uint8_t oldbus)
> {
> - trace_xen_unmap_pcidev(ioservid, oldbus,
> - PCI_SLOT(pci_dev->devfn),
> PCI_FUNC(pci_dev->devfn));
> - xc_hvm_unmap_pcidev_from_ioreq_server(xc, dom, ioservid,
> - 0, oldbus,
> - PCI_SLOT(pci_dev->devfn),
> - PCI_FUNC(pci_dev->devfn));
> + gpointer bdf_key = GINT_TO_POINTER((oldbus << 8) | pci_dev->devfn);
> + gint bdf_cnt = GPOINTER_TO_INT(g_hash_table_lookup(pci_bdf,
> bdf_key));
> +
> + if (bdf_cnt == 1) {
> + trace_xen_unmap_pcidev(ioservid, oldbus,
> + PCI_SLOT(pci_dev->devfn),
> + PCI_FUNC(pci_dev->devfn));
> + g_hash_table_remove(pci_bdf, bdf_key);
> + xc_hvm_unmap_pcidev_from_ioreq_server(xc, dom, ioservid,
> + 0, oldbus,
> + PCI_SLOT(pci_dev->devfn),
> + PCI_FUNC(pci_dev->devfn));
> + } else {
> + trace_xen_unmap_pcidev_dup(ioservid, oldbus, PCI_SLOT(pci_dev-
> >devfn),
> + PCI_FUNC(pci_dev->devfn), bdf_cnt - 1);
> + g_hash_table_replace(pci_bdf, bdf_key, GINT_TO_POINTER(bdf_cnt -
> 1));
> + }
> }
>
> static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
> diff --git a/trace-events b/trace-events
> index a589650..58deeaf 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -930,8 +930,10 @@ xen_map_mmio_range(uint32_t id, uint64_t
> start_addr, uint64_t end_addr) "id: %u
> xen_unmap_mmio_range(uint32_t id, uint64_t start_addr, uint64_t
> end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
> xen_map_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr)
> "id: %u start: %#"PRIx64" end: %#"PRIx64
> xen_unmap_portio_range(uint32_t id, uint64_t start_addr, uint64_t
> end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
> -xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u
> bdf: %02x.%02x.%02x"
> -xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id:
> %u bdf: %02x.%02x.%02x"
> +xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u
> bdf: %02x:%02x.%x"
> +xen_map_pcidev_dup(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func,
> int dup) "id: %u bdf: %02x:%02x.%x dup: %d"
> +xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id:
> %u bdf: %02x:%02x.%x"
> +xen_unmap_pcidev_dup(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func,
> int dup) "id: %u bdf: %02x:%02x.%x dup: %d"
>
> # xen-mapcache.c
> xen_map_cache(uint64_t phys_addr) "want %#"PRIx64
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 7b6d8f1..f041909 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -123,6 +123,7 @@ typedef struct XenIOState {
> MemoryListener memory_listener;
> MemoryListener io_listener;
> DeviceListener device_listener;
> + GHashTable *pci_bdf;
> QLIST_HEAD(, XenPhysmap) physmap;
> hwaddr free_phys_offset;
> const XenPhysmap *log_for_dirtybit;
> @@ -578,7 +579,8 @@ static void xen_device_realize(DeviceListener
> *listener,
> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> PCIDevice *pci_dev = PCI_DEVICE(dev);
>
> - xen_map_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev);
> + xen_map_pcidev(xen_xc, xen_domid, state->ioservid, state->pci_bdf,
> + pci_dev);
> }
> }
>
> @@ -590,8 +592,8 @@ static void xen_device_unrealize(DeviceListener
> *listener,
> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> PCIDevice *pci_dev = PCI_DEVICE(dev);
>
> - xen_unmap_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev,
> - pci_bus_num(pci_dev->bus));
> + xen_unmap_pcidev(xen_xc, xen_domid, state->ioservid, state-
> >pci_bdf,
> + pci_dev, pci_bus_num(pci_dev->bus));
> }
> }
>
> @@ -600,8 +602,10 @@ static void
> xen_device_change_pci_bus_num(DeviceListener *listener,
> {
> XenIOState *state = container_of(listener, XenIOState, device_listener);
>
> - xen_unmap_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev,
> oldbus);
> - xen_map_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev);
> + xen_unmap_pcidev(xen_xc, xen_domid, state->ioservid, state->pci_bdf,
> + pci_dev, oldbus);
> + xen_map_pcidev(xen_xc, xen_domid, state->ioservid, state->pci_bdf,
> + pci_dev);
> }
>
> static void xen_sync_dirty_bitmap(XenIOState *state,
> @@ -1318,6 +1322,7 @@ int xen_hvm_init(ram_addr_t
> *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> memory_listener_register(&state->io_listener, &address_space_io);
>
> state->device_listener = xen_device_listener;
> + state->pci_bdf = g_hash_table_new(NULL, NULL);
> device_listener_register(&state->device_listener);
>
> /* Initialize backend core & drivers */
> --
> 1.8.4
[Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Don Slutz, 2015/06/08
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server,
Paul Durrant <=
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Don Slutz, 2015/06/09
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Paul Durrant, 2015/06/09
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Don Slutz, 2015/06/09
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Paul Durrant, 2015/06/09
- Re: [Qemu-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server, Don Slutz, 2015/06/09
Re: [Qemu-devel] [PATCH v2 0/4] Fix device listener interface for PCI to PCI bridges, Michael S. Tsirkin, 2015/06/09