|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server
> -----Original Message----- > From: Don Slutz [mailto:dslutz@xxxxxxxxxxx] > Sent: 09 June 2015 14:51 > To: Paul Durrant; Slutz, Donald Christopher; qemu-devel@xxxxxxxxxx; xen- > devel@xxxxxxxxxxxxx > Cc: Michael S. Tsirkin; Stefano Stabellini; Don Slutz > Subject: Re: [PATCH v2 4/4] xen: Fix map/unmap of pcidev to ioreq server > > On 06/09/15 05:05, Paul Durrant wrote: > >> -----Original Message----- > >> From: Don Slutz [mailto:dslutz@xxxxxxxxxxx] > >> Sent: 08 June 2015 22:19 > >> To: qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx > >> 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? > > Not directly. The devices are not being "realized" in QEMU terms. The > PCI to PCI brige is changing state, and the BDF for all PCI devices on > the secondary bus changes when the PCI config named > PCI_SECONDARY_BUS on > the PCI to PCI bridge is changed. This is what patch #2 is all about. > > > Is that really the case? > > > > That is what it looks like to Xen. But the device listener callbacks > are not called. Then what's calling xc_hvm_map_pcidev_from_ioreq_server()? I don't understand why you are having to introduce this to avoid duplicate calls? Paul > > -Don Slutz > > > 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 <dslutz@xxxxxxxxxxx> > >> CC: Don Slutz <don.slutz@xxxxxxxxx> > >> --- > >> 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 > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |