From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58672) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bojTm-0006Kt-1H for qemu-devel@nongnu.org; Mon, 26 Sep 2016 23:54:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bojTj-0002u0-OA for qemu-devel@nongnu.org; Mon, 26 Sep 2016 23:54:50 -0400 Date: Tue, 27 Sep 2016 13:48:12 +1000 From: David Gibson Message-ID: <20160927034812.GF15376@umbus.fritz.box> References: <1474899049-12506-1-git-send-email-lvivier@redhat.com> <1474899049-12506-3-git-send-email-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="udcq9yAoWb9A4FsZ" Content-Disposition: inline In-Reply-To: <1474899049-12506-3-git-send-email-lvivier@redhat.com> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] libqos: add PCI management in qtest_vboot()/qtest_shutdown() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: qemu-devel@nongnu.org, thuth@redhat.com, Greg Kurz , qemu-ppc@nongnu.org, Gerd Hoffmann , dgibson@redhat.com --udcq9yAoWb9A4FsZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 26, 2016 at 04:10:47PM +0200, Laurent Vivier wrote: > Signed-off-by: Laurent Vivier > --- > tests/e1000e-test.c | 2 +- > tests/i440fx-test.c | 2 +- > tests/ide-test.c | 2 +- > tests/ivshmem-test.c | 2 +- > tests/libqos/ahci.c | 2 +- > tests/libqos/libqos-pc.c | 5 ++++- > tests/libqos/libqos-spapr.c | 5 ++++- > tests/libqos/libqos.c | 21 ++++++++++++++++----- > tests/libqos/libqos.h | 3 +++ > tests/libqos/pci-pc.c | 2 +- > tests/libqos/pci-pc.h | 3 ++- > tests/q35-test.c | 2 +- > tests/rtl8139-test.c | 2 +- > tests/tco-test.c | 2 +- > tests/usb-hcd-ehci-test.c | 2 +- > tests/usb-hcd-uhci-test.c | 2 +- > tests/vhost-user-test.c | 2 +- > tests/virtio-9p-test.c | 2 +- > tests/virtio-blk-test.c | 2 +- > tests/virtio-net-test.c | 2 +- > tests/virtio-scsi-test.c | 2 +- > 21 files changed, 45 insertions(+), 24 deletions(-) Couple of queries below. >=20 > diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c > index d497b08..3979b20 100644 > --- a/tests/e1000e-test.c > +++ b/tests/e1000e-test.c > @@ -390,7 +390,7 @@ static void data_test_init(e1000e_device *d) > qtest_start(cmdline); > g_free(cmdline); > =20 > - test_bus =3D qpci_init_pc(); > + test_bus =3D qpci_init_pc(NULL); > g_assert_nonnull(test_bus); > =20 > test_alloc =3D pc_alloc_init(); > diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c > index 3542ad1..da2d5a5 100644 > --- a/tests/i440fx-test.c > +++ b/tests/i440fx-test.c > @@ -38,7 +38,7 @@ static QPCIBus *test_start_get_bus(const TestData *s) > cmdline =3D g_strdup_printf("-smp %d", s->num_cpus); > qtest_start(cmdline); > g_free(cmdline); > - return qpci_init_pc(); > + return qpci_init_pc(NULL); > } > =20 > static void test_i440fx_defaults(gconstpointer opaque) > diff --git a/tests/ide-test.c b/tests/ide-test.c > index 1e51af2..a8a4081 100644 > --- a/tests/ide-test.c > +++ b/tests/ide-test.c > @@ -143,7 +143,7 @@ static QPCIDevice *get_pci_device(uint16_t *bmdma_bas= e) > uint16_t vendor_id, device_id; > =20 > if (!pcibus) { > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > } > =20 > /* Find PCI device and verify it's the right one */ > diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c > index 0957ee7..f36bfe7 100644 > --- a/tests/ivshmem-test.c > +++ b/tests/ivshmem-test.c > @@ -105,7 +105,7 @@ static void setup_vm_cmd(IVState *s, const char *cmd,= bool msix) > uint64_t barsize; > =20 > s->qtest =3D qtest_start(cmd); > - s->pcibus =3D qpci_init_pc(); > + s->pcibus =3D qpci_init_pc(NULL); > s->dev =3D get_device(s->pcibus); > =20 > s->reg_base =3D qpci_iomap(s->dev, 0, &barsize); > diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c > index f3be550..716ab79 100644 > --- a/tests/libqos/ahci.c > +++ b/tests/libqos/ahci.c > @@ -128,7 +128,7 @@ QPCIDevice *get_ahci_device(uint32_t *fingerprint) > uint32_t ahci_fingerprint; > QPCIBus *pcibus; > =20 > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > =20 > /* Find the AHCI PCI device and verify it's the right one. */ > ahci =3D qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02)); > diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c > index df34092..aa17c98 100644 > --- a/tests/libqos/libqos-pc.c > +++ b/tests/libqos/libqos-pc.c > @@ -1,10 +1,13 @@ > #include "qemu/osdep.h" > #include "libqos/libqos-pc.h" > #include "libqos/malloc-pc.h" > +#include "libqos/pci-pc.h" > =20 > static QOSOps qos_ops =3D { > .init_allocator =3D pc_alloc_init_flags, > - .uninit_allocator =3D pc_alloc_uninit > + .uninit_allocator =3D pc_alloc_uninit, > + .qpci_init =3D qpci_init_pc, > + .qpci_free =3D qpci_free_pc, > }; > =20 > QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap) > diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c > index f19408b..125c6b3 100644 > --- a/tests/libqos/libqos-spapr.c > +++ b/tests/libqos/libqos-spapr.c > @@ -1,10 +1,13 @@ > #include "qemu/osdep.h" > #include "libqos/libqos-spapr.h" > #include "libqos/malloc-spapr.h" > +#include "libqos/pci-spapr.h" > =20 > static QOSOps qos_ops =3D { > .init_allocator =3D spapr_alloc_init_flags, > - .uninit_allocator =3D spapr_alloc_uninit > + .uninit_allocator =3D spapr_alloc_uninit, > + .qpci_init =3D qpci_init_spapr, > + .qpci_free =3D qpci_free_spapr > }; > =20 > QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap) > diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c > index a852dc5..332d60e 100644 > --- a/tests/libqos/libqos.c > +++ b/tests/libqos/libqos.c > @@ -20,8 +20,13 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline= _fmt, va_list ap) > cmdline =3D g_strdup_vprintf(cmdline_fmt, ap); > qs->qts =3D qtest_start(cmdline); > qs->ops =3D ops; > - if (ops && ops->init_allocator) { > - qs->alloc =3D ops->init_allocator(ALLOC_NO_FLAGS); > + if (ops) { > + if (ops->init_allocator) { > + qs->alloc =3D ops->init_allocator(ALLOC_NO_FLAGS); > + } > + if (ops->qpci_init && qs->alloc) { > + qs->pcibus =3D ops->qpci_init(qs->alloc); > + } > } > =20 > g_free(cmdline); > @@ -49,9 +54,15 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_= fmt, ...) > */ > void qtest_shutdown(QOSState *qs) > { > - if (qs->alloc && qs->ops && qs->ops->uninit_allocator) { > - qs->ops->uninit_allocator(qs->alloc); > - qs->alloc =3D NULL; > + if (qs->ops) { > + if (qs->alloc && qs->ops->uninit_allocator) { > + qs->ops->uninit_allocator(qs->alloc); > + qs->alloc =3D NULL; > + } > + if (qs->pcibus && qs->ops->qpci_free) { > + qs->ops->qpci_free(qs->pcibus); > + qs->pcibus =3D NULL; > + } Is it safe to cleanup the allocator before the PCI stuff? Usually cleanups want to go in the opposite order to initialization. > } > qtest_quit(qs->qts); > g_free(qs); > diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h > index 604980d..a9f6990 100644 > --- a/tests/libqos/libqos.h > +++ b/tests/libqos/libqos.h > @@ -8,11 +8,14 @@ > typedef struct QOSOps { > QGuestAllocator *(*init_allocator)(QAllocOpts); > void (*uninit_allocator)(QGuestAllocator *); > + QPCIBus *(*qpci_init)(QGuestAllocator *alloc); > + void (*qpci_free)(QPCIBus *bus); > } QOSOps; > =20 > typedef struct QOSState { > QTestState *qts; > QGuestAllocator *alloc; > + QPCIBus *pcibus; > QOSOps *ops; > } QOSState; > =20 > diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c > index 82066b8..9600ed6 100644 > --- a/tests/libqos/pci-pc.c > +++ b/tests/libqos/pci-pc.c > @@ -212,7 +212,7 @@ static void qpci_pc_iounmap(QPCIBus *bus, void *data) > /* FIXME */ > } > =20 > -QPCIBus *qpci_init_pc(void) > +QPCIBus *qpci_init_pc(QGuestAllocator *alloc) > { > QPCIBusPC *ret; > You've added the alloc parameter, but you don't actually appear to use it.. > diff --git a/tests/libqos/pci-pc.h b/tests/libqos/pci-pc.h > index 2621179..9479b51 100644 > --- a/tests/libqos/pci-pc.h > +++ b/tests/libqos/pci-pc.h > @@ -14,8 +14,9 @@ > #define LIBQOS_PCI_PC_H > =20 > #include "libqos/pci.h" > +#include "libqos/malloc.h" > =20 > -QPCIBus *qpci_init_pc(void); > +QPCIBus *qpci_init_pc(QGuestAllocator *alloc); > void qpci_free_pc(QPCIBus *bus); > =20 > #endif > diff --git a/tests/q35-test.c b/tests/q35-test.c > index 71538fc..763fe3d 100644 > --- a/tests/q35-test.c > +++ b/tests/q35-test.c > @@ -42,7 +42,7 @@ static void test_smram_lock(void) > QPCIDevice *pcidev; > QDict *response; > =20 > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > g_assert(pcibus !=3D NULL); > =20 > pcidev =3D qpci_device_find(pcibus, 0); > diff --git a/tests/rtl8139-test.c b/tests/rtl8139-test.c > index 13de7ee..c2f601a 100644 > --- a/tests/rtl8139-test.c > +++ b/tests/rtl8139-test.c > @@ -35,7 +35,7 @@ static QPCIDevice *get_device(void) > { > QPCIDevice *dev; > =20 > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, &dev); > g_assert(dev !=3D NULL); > =20 > diff --git a/tests/tco-test.c b/tests/tco-test.c > index 0d13aa8..0d201b1 100644 > --- a/tests/tco-test.c > +++ b/tests/tco-test.c > @@ -57,7 +57,7 @@ static void test_init(TestData *d) > qtest_irq_intercept_in(qs, "ioapic"); > g_free(s); > =20 > - bus =3D qpci_init_pc(); > + bus =3D qpci_init_pc(NULL); > d->dev =3D qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00)); > g_assert(d->dev !=3D NULL); > =20 > diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c > index eb247ad..a4ceeaa 100644 > --- a/tests/usb-hcd-ehci-test.c > +++ b/tests/usb-hcd-ehci-test.c > @@ -56,7 +56,7 @@ static void pci_init(void) > if (pcibus) { > return; > } > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > g_assert(pcibus !=3D NULL); > =20 > qusb_pci_init_one(pcibus, &uhci1, QPCI_DEVFN(0x1d, 0), 4); > diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c > index 5cd59ad..c24063e 100644 > --- a/tests/usb-hcd-uhci-test.c > +++ b/tests/usb-hcd-uhci-test.c > @@ -23,7 +23,7 @@ static void test_port(int port) > struct qhc uhci; > =20 > g_assert(port > 0); > - pcibus =3D qpci_init_pc(); > + pcibus =3D qpci_init_pc(NULL); > g_assert(pcibus !=3D NULL); > qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4); > uhci_port_test(&uhci, port - 1, UHCI_PORT_CCS); > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c > index b89a551..300308c 100644 > --- a/tests/vhost-user-test.c > +++ b/tests/vhost-user-test.c > @@ -146,7 +146,7 @@ static void init_virtio_dev(TestServer *s) > QVirtioPCIDevice *dev; > uint32_t features; > =20 > - bus =3D qpci_init_pc(); > + bus =3D qpci_init_pc(NULL); > g_assert_nonnull(bus); > =20 > dev =3D qvirtio_pci_device_find(bus, VIRTIO_ID_NET); > diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > index b8fb6cd..e8b2196 100644 > --- a/tests/virtio-9p-test.c > +++ b/tests/virtio-9p-test.c > @@ -63,7 +63,7 @@ static QVirtIO9P *qvirtio_9p_pci_init(void) > =20 > v9p =3D g_new0(QVirtIO9P, 1); > v9p->alloc =3D pc_alloc_init(); > - v9p->bus =3D qpci_init_pc(); > + v9p->bus =3D qpci_init_pc(NULL); > =20 > dev =3D qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P); > g_assert_nonnull(dev); > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > index 811cf75..3c4fecc 100644 > --- a/tests/virtio-blk-test.c > +++ b/tests/virtio-blk-test.c > @@ -75,7 +75,7 @@ static QPCIBus *pci_test_start(void) > g_free(tmp_path); > g_free(cmdline); > =20 > - return qpci_init_pc(); > + return qpci_init_pc(NULL); > } > =20 > static void arm_test_start(void) > diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c > index 361506f..a343a6b 100644 > --- a/tests/virtio-net-test.c > +++ b/tests/virtio-net-test.c > @@ -62,7 +62,7 @@ static QPCIBus *pci_test_start(int socket) > qtest_start(cmdline); > g_free(cmdline); > =20 > - return qpci_init_pc(); > + return qpci_init_pc(NULL); > } > =20 > static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev) > diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c > index f1489e6..79088bb 100644 > --- a/tests/virtio-scsi-test.c > +++ b/tests/virtio-scsi-test.c > @@ -146,7 +146,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot) > =20 > vs =3D g_new0(QVirtIOSCSI, 1); > vs->alloc =3D pc_alloc_init(); > - vs->bus =3D qpci_init_pc(); > + vs->bus =3D qpci_init_pc(NULL); > =20 > dev =3D qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI); > vs->dev =3D (QVirtioDevice *)dev; --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --udcq9yAoWb9A4FsZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX6ev8AAoJEGw4ysog2bOS2GYQANb/tJzhgHh2lzB4V1GRu8VR 2b2w20acBZSXqUpk5QvSrCZrDIaGm2CAksudZyEDbGPoBvAftpn53YpgJ/ZTT3ud XvXSB5OquSmDM1q9OcLM7chINlYkAlOAt47MX2f1TjKR6Nd2Jv80IjHbijJOJ9px DEjnwSbL5hivVZMDMxcSuPx/qjtP3fJZ0ZyvSIHvymtF725SaKiw3iIshaB35S+n YyGF1b1RoQd3zFYZf6SHjZNXFYRRgzqj8iaefAjGFi2LYpWae8xelxL1SJi0RyxY PcCIn8Hhox9ZO9vzi3/RGSfQonFyXjGAXvLJOQfizy3BrfnEmzeTLaiCN/ZEuJKU Q8Ea0Tw9fMEhrAvS0x+mdDuSJvpkZ8oXgp7PVxdTJke1XLSCIR9/qgLGFqYGf2YN V1PTsIp9HweJwX4cJRdNwVM/EN9wZkURw2ci7beNw2f1P/uSOfQMhXpNW73aB1QG 7k9XEJ5V+5MirFo5GTKzbnHfLPrliIcgIqjsPV27BLZy4uO2CUFqWY7cxC1jNC4k 6rdvXxXTuemEdGCrHMb4hd9rvYf6xFvp7PRlKzx4Dxu6yPGH2F1kSi49JcRGvT+T KiSM+A8NTa5LBWAvAGyyRr+EZ7/HQyy2hqSnnxDi5h5JoUeZyEU63r7B1+izRhdf UbVXieX1POofmcvuP41H =sZCU -----END PGP SIGNATURE----- --udcq9yAoWb9A4FsZ--