[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 2/4] libqos: add PCI management in qtest_vboot()/q
|
From: |
David Gibson |
|
Subject: |
Re: [Qemu-ppc] [PATCH 2/4] libqos: add PCI management in qtest_vboot()/qtest_shutdown() |
|
Date: |
Tue, 27 Sep 2016 13:48:12 +1000 |
|
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Mon, Sep 26, 2016 at 04:10:47PM +0200, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
> 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.
>
> 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);
>
> - test_bus = qpci_init_pc();
> + test_bus = qpci_init_pc(NULL);
> g_assert_nonnull(test_bus);
>
> test_alloc = 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 = g_strdup_printf("-smp %d", s->num_cpus);
> qtest_start(cmdline);
> g_free(cmdline);
> - return qpci_init_pc();
> + return qpci_init_pc(NULL);
> }
>
> 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_base)
> uint16_t vendor_id, device_id;
>
> if (!pcibus) {
> - pcibus = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
> }
>
> /* 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;
>
> s->qtest = qtest_start(cmd);
> - s->pcibus = qpci_init_pc();
> + s->pcibus = qpci_init_pc(NULL);
> s->dev = get_device(s->pcibus);
>
> s->reg_base = 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;
>
> - pcibus = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
>
> /* Find the AHCI PCI device and verify it's the right one. */
> ahci = 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"
>
> static QOSOps qos_ops = {
> .init_allocator = pc_alloc_init_flags,
> - .uninit_allocator = pc_alloc_uninit
> + .uninit_allocator = pc_alloc_uninit,
> + .qpci_init = qpci_init_pc,
> + .qpci_free = qpci_free_pc,
> };
>
> 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"
>
> static QOSOps qos_ops = {
> .init_allocator = spapr_alloc_init_flags,
> - .uninit_allocator = spapr_alloc_uninit
> + .uninit_allocator = spapr_alloc_uninit,
> + .qpci_init = qpci_init_spapr,
> + .qpci_free = qpci_free_spapr
> };
>
> 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 = g_strdup_vprintf(cmdline_fmt, ap);
> qs->qts = qtest_start(cmdline);
> qs->ops = ops;
> - if (ops && ops->init_allocator) {
> - qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
> + if (ops) {
> + if (ops->init_allocator) {
> + qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
> + }
> + if (ops->qpci_init && qs->alloc) {
> + qs->pcibus = ops->qpci_init(qs->alloc);
> + }
> }
>
> 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 = NULL;
> + if (qs->ops) {
> + if (qs->alloc && qs->ops->uninit_allocator) {
> + qs->ops->uninit_allocator(qs->alloc);
> + qs->alloc = NULL;
> + }
> + if (qs->pcibus && qs->ops->qpci_free) {
> + qs->ops->qpci_free(qs->pcibus);
> + qs->pcibus = 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;
>
> typedef struct QOSState {
> QTestState *qts;
> QGuestAllocator *alloc;
> + QPCIBus *pcibus;
> QOSOps *ops;
> } QOSState;
>
> 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 */
> }
>
> -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
>
> #include "libqos/pci.h"
> +#include "libqos/malloc.h"
>
> -QPCIBus *qpci_init_pc(void);
> +QPCIBus *qpci_init_pc(QGuestAllocator *alloc);
> void qpci_free_pc(QPCIBus *bus);
>
> #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;
>
> - pcibus = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
> g_assert(pcibus != NULL);
>
> pcidev = 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;
>
> - pcibus = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
> qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, &dev);
> g_assert(dev != NULL);
>
> 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);
>
> - bus = qpci_init_pc();
> + bus = qpci_init_pc(NULL);
> d->dev = qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00));
> g_assert(d->dev != NULL);
>
> 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 = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
> g_assert(pcibus != NULL);
>
> 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;
>
> g_assert(port > 0);
> - pcibus = qpci_init_pc();
> + pcibus = qpci_init_pc(NULL);
> g_assert(pcibus != 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;
>
> - bus = qpci_init_pc();
> + bus = qpci_init_pc(NULL);
> g_assert_nonnull(bus);
>
> dev = 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)
>
> v9p = g_new0(QVirtIO9P, 1);
> v9p->alloc = pc_alloc_init();
> - v9p->bus = qpci_init_pc();
> + v9p->bus = qpci_init_pc(NULL);
>
> dev = 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);
>
> - return qpci_init_pc();
> + return qpci_init_pc(NULL);
> }
>
> 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);
>
> - return qpci_init_pc();
> + return qpci_init_pc(NULL);
> }
>
> 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)
>
> vs = g_new0(QVirtIOSCSI, 1);
> vs->alloc = pc_alloc_init();
> - vs->bus = qpci_init_pc();
> + vs->bus = qpci_init_pc(NULL);
>
> dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
> vs->dev = (QVirtioDevice *)dev;
--
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
signature.asc
Description: PGP signature