| Message ID | 1475169307-1510-4-git-send-email-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote: > but disable MSI-X tests on SPAPR as we can't check the result > (the memory region used on PC is not readable on SPAPR). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > tests/Makefile.include | 3 ++- > tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > tests/virtio-9p-test.c | 11 ++++++++++- > tests/virtio-blk-test.c | 22 +++++++++++++++++----- > tests/virtio-net-test.c | 17 +++++++++++++++-- > tests/virtio-rng-test.c | 7 ++++++- > tests/virtio-scsi-test.c | 10 +++++++++- > 7 files changed, 79 insertions(+), 13 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index c46a32d..1e4a3d5 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF) > gcov-files-ppc64-y += hw/usb/hcd-uhci.c > check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF) > gcov-files-ppc64-y += hw/usb/hcd-xhci.c > +check-qtest-ppc64-y += $(check-qtest-virtio-y) > > check-qtest-sh4-y = tests/endianness-test$(EXESUF) > > @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o > libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o > libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o > libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o > -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > > tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o > tests/rtc-test$(EXESUF): tests/rtc-test.o > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > index 6e005c1..ed81df6 100644 > --- a/tests/libqos/virtio-pci.c > +++ b/tests/libqos/virtio-pci.c > @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) > return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr); > } > > +/* PCI is always read in little-endian order > + * but virtio ( < 1.0) is in guest order > + * so with a big-endian guest the order has been reversed > + * reverse it again > + */ > + > static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr) > { > QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > - return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > + uint16_t value; > + > + value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > + if (target_big_endian()) { > + value = bswap16(value); > + } Don't you need some sort of test to distinguish the virtio < 1.0 and virtio-1.0 cases? > + return value; > } > > static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr) > { > QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > - return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); > + uint32_t value; > + > + value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); > + if (target_big_endian()) { > + value = bswap32(value); > + } > + return value; > } > > static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr) > diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > index 28d7f5b..a73bccb 100644 > --- a/tests/virtio-9p-test.c > +++ b/tests/virtio-9p-test.c > @@ -11,6 +11,7 @@ > #include "libqtest.h" > #include "qemu-common.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "standard-headers/linux/virtio_ids.h" > @@ -22,12 +23,20 @@ static char *test_share; > > static QOSState *qvirtio_9p_start(void) > { > + const char *arch = qtest_get_arch(); > + QOSState *qs = NULL; > test_share = g_strdup("/tmp/qtest.XXXXXX"); > g_assert_nonnull(mkdtemp(test_share)); > const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " > "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; > > - return qtest_pc_boot(cmd, test_share, mount_tag); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qs = qtest_pc_boot(cmd, test_share, mount_tag); > + } else if (strcmp(arch, "ppc64") == 0) { > + qs = qtest_spapr_boot(cmd, test_share, mount_tag); > + } > + > + return qs; > } > > static void qvirtio_9p_stop(QOSState *qs) > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > index a4de3e4..a032f8e 100644 > --- a/tests/virtio-blk-test.c > +++ b/tests/virtio-blk-test.c > @@ -11,6 +11,7 @@ > #include "qemu/osdep.h" > #include "libqtest.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "libqos/virtio-mmio.h" > @@ -58,6 +59,7 @@ static char *drive_create(void) > > static QOSState *pci_test_start(void) > { > + const char *arch = qtest_get_arch(); > QOSState *qs = NULL; > char *tmp_path; > const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw " > @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void) > > tmp_path = drive_create(); > > - qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + } else if (strcmp(arch, "ppc64") == 0) { > + qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + } > unlink(tmp_path); > g_free(tmp_path); > return qs; > @@ -676,6 +682,7 @@ static void pci_hotplug(void) > { > QVirtioPCIDevice *dev; > QOSState *qs; > + const char *arch = qtest_get_arch(); > > qs = pci_test_start(); > g_assert(qs); > @@ -690,7 +697,9 @@ static void pci_hotplug(void) > g_free(dev); > > /* unplug secondary disk */ > - qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); > + } > qtest_shutdown(qs); > } > > @@ -741,12 +750,15 @@ int main(int argc, char **argv) > > g_test_init(&argc, &argv, NULL); > > - if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 || > + strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) { > qtest_add_func("/virtio/blk/pci/basic", pci_basic); > qtest_add_func("/virtio/blk/pci/indirect", pci_indirect); > qtest_add_func("/virtio/blk/pci/config", pci_config); > - qtest_add_func("/virtio/blk/pci/msix", pci_msix); > - qtest_add_func("/virtio/blk/pci/idx", pci_idx); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qtest_add_func("/virtio/blk/pci/msix", pci_msix); > + qtest_add_func("/virtio/blk/pci/idx", pci_idx); > + } > qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug); > } else if (strcmp(arch, "arm") == 0) { > qtest_add_func("/virtio/blk/mmio/basic", mmio_basic); > diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c > index 3e83685..14f2920 100644 > --- a/tests/virtio-net-test.c > +++ b/tests/virtio-net-test.c > @@ -13,6 +13,7 @@ > #include "qemu/sockets.h" > #include "qemu/iov.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "qemu/bswap.h" > @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot) > > static QOSState *pci_test_start(int socket) > { > + const char *arch = qtest_get_arch(); > const char *cmd = "-netdev socket,fd=%d,id=hs0 -device " > "virtio-net-pci,netdev=hs0"; > > - return qtest_pc_boot(cmd, socket); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + return qtest_pc_boot(cmd, socket); > + } > + if (strcmp(arch, "ppc64") == 0) { > + return qtest_spapr_boot(cmd, socket); > + } > + return NULL; > } > > static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev) > @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data) > > static void hotplug(void) > { > + const char *arch = qtest_get_arch(); > + > qtest_start("-device virtio-net-pci"); > > qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL); > - qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); > + > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); > + } > > test_end(); > } > diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c > index e1b2640..dcecf77 100644 > --- a/tests/virtio-rng-test.c > +++ b/tests/virtio-rng-test.c > @@ -20,8 +20,13 @@ static void pci_nop(void) > > static void hotplug(void) > { > + const char *arch = qtest_get_arch(); > + > qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL); > - qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); > + > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); > + } > } > > int main(int argc, char **argv) > diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c > index 721ae1f..dd5457b 100644 > --- a/tests/virtio-scsi-test.c > +++ b/tests/virtio-scsi-test.c > @@ -12,6 +12,7 @@ > #include "libqtest.h" > #include "block/scsi.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "standard-headers/linux/virtio_ids.h" > @@ -33,11 +34,18 @@ typedef struct { > > static QOSState *qvirtio_scsi_start(const char *extra_opts) > { > + const char *arch = qtest_get_arch(); > const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw " > "-device virtio-scsi-pci,id=vs0 " > "-device scsi-hd,bus=vs0.0,drive=drv0 %s"; > > - return qtest_pc_boot(cmd, extra_opts ? : ""); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + return qtest_pc_boot(cmd, extra_opts ? : ""); > + } > + if (strcmp(arch, "ppc64") == 0) { > + return qtest_spapr_boot(cmd, extra_opts ? : ""); > + } > + return NULL; > } > > static void qvirtio_scsi_stop(QOSState *qs)
On 30/09/2016 03:30, David Gibson wrote: > On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote: >> but disable MSI-X tests on SPAPR as we can't check the result >> (the memory region used on PC is not readable on SPAPR). >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> tests/Makefile.include | 3 ++- >> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- >> tests/virtio-9p-test.c | 11 ++++++++++- >> tests/virtio-blk-test.c | 22 +++++++++++++++++----- >> tests/virtio-net-test.c | 17 +++++++++++++++-- >> tests/virtio-rng-test.c | 7 ++++++- >> tests/virtio-scsi-test.c | 10 +++++++++- >> 7 files changed, 79 insertions(+), 13 deletions(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index c46a32d..1e4a3d5 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF) >> gcov-files-ppc64-y += hw/usb/hcd-uhci.c >> check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF) >> gcov-files-ppc64-y += hw/usb/hcd-xhci.c >> +check-qtest-ppc64-y += $(check-qtest-virtio-y) >> >> check-qtest-sh4-y = tests/endianness-test$(EXESUF) >> >> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o >> libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o >> libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o >> libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o >> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o >> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o >> >> tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o >> tests/rtc-test$(EXESUF): tests/rtc-test.o >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c >> index 6e005c1..ed81df6 100644 >> --- a/tests/libqos/virtio-pci.c >> +++ b/tests/libqos/virtio-pci.c >> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) >> return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr); >> } >> >> +/* PCI is always read in little-endian order >> + * but virtio ( < 1.0) is in guest order >> + * so with a big-endian guest the order has been reversed >> + * reverse it again >> + */ >> + >> static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr) >> { >> QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; >> - return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); >> + uint16_t value; >> + >> + value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); >> + if (target_big_endian()) { >> + value = bswap16(value); >> + } > > Don't you need some sort of test to distinguish the virtio < 1.0 and > virtio-1.0 cases? yes, but for the moment we don't test virtio-1.0, we will add the test when will support it. http://wiki.qemu.org/Outreachy_2016_DecemberMarch#VIRTIO_1.0_support_in_libqos > >> + return value; >> } >> >> static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr) >> { >> QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; >> - return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); >> + uint32_t value; >> + >> + value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); >> + if (target_big_endian()) { >> + value = bswap32(value); >> + } >> + return value; >> } >> >> static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr) >> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c >> index 28d7f5b..a73bccb 100644 >> --- a/tests/virtio-9p-test.c >> +++ b/tests/virtio-9p-test.c >> @@ -11,6 +11,7 @@ >> #include "libqtest.h" >> #include "qemu-common.h" >> #include "libqos/libqos-pc.h" >> +#include "libqos/libqos-spapr.h" >> #include "libqos/virtio.h" >> #include "libqos/virtio-pci.h" >> #include "standard-headers/linux/virtio_ids.h" >> @@ -22,12 +23,20 @@ static char *test_share; >> >> static QOSState *qvirtio_9p_start(void) >> { >> + const char *arch = qtest_get_arch(); >> + QOSState *qs = NULL; >> test_share = g_strdup("/tmp/qtest.XXXXXX"); >> g_assert_nonnull(mkdtemp(test_share)); >> const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " >> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; >> >> - return qtest_pc_boot(cmd, test_share, mount_tag); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qs = qtest_pc_boot(cmd, test_share, mount_tag); >> + } else if (strcmp(arch, "ppc64") == 0) { >> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); >> + } >> + >> + return qs; >> } >> >> static void qvirtio_9p_stop(QOSState *qs) >> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c >> index a4de3e4..a032f8e 100644 >> --- a/tests/virtio-blk-test.c >> +++ b/tests/virtio-blk-test.c >> @@ -11,6 +11,7 @@ >> #include "qemu/osdep.h" >> #include "libqtest.h" >> #include "libqos/libqos-pc.h" >> +#include "libqos/libqos-spapr.h" >> #include "libqos/virtio.h" >> #include "libqos/virtio-pci.h" >> #include "libqos/virtio-mmio.h" >> @@ -58,6 +59,7 @@ static char *drive_create(void) >> >> static QOSState *pci_test_start(void) >> { >> + const char *arch = qtest_get_arch(); >> QOSState *qs = NULL; >> char *tmp_path; >> const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw " >> @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void) >> >> tmp_path = drive_create(); >> >> - qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); >> + } else if (strcmp(arch, "ppc64") == 0) { >> + qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); >> + } >> unlink(tmp_path); >> g_free(tmp_path); >> return qs; >> @@ -676,6 +682,7 @@ static void pci_hotplug(void) >> { >> QVirtioPCIDevice *dev; >> QOSState *qs; >> + const char *arch = qtest_get_arch(); >> >> qs = pci_test_start(); >> g_assert(qs); >> @@ -690,7 +697,9 @@ static void pci_hotplug(void) >> g_free(dev); >> >> /* unplug secondary disk */ >> - qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); >> + } >> qtest_shutdown(qs); >> } >> >> @@ -741,12 +750,15 @@ int main(int argc, char **argv) >> >> g_test_init(&argc, &argv, NULL); >> >> - if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 || >> + strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) { >> qtest_add_func("/virtio/blk/pci/basic", pci_basic); >> qtest_add_func("/virtio/blk/pci/indirect", pci_indirect); >> qtest_add_func("/virtio/blk/pci/config", pci_config); >> - qtest_add_func("/virtio/blk/pci/msix", pci_msix); >> - qtest_add_func("/virtio/blk/pci/idx", pci_idx); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qtest_add_func("/virtio/blk/pci/msix", pci_msix); >> + qtest_add_func("/virtio/blk/pci/idx", pci_idx); >> + } >> qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug); >> } else if (strcmp(arch, "arm") == 0) { >> qtest_add_func("/virtio/blk/mmio/basic", mmio_basic); >> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c >> index 3e83685..14f2920 100644 >> --- a/tests/virtio-net-test.c >> +++ b/tests/virtio-net-test.c >> @@ -13,6 +13,7 @@ >> #include "qemu/sockets.h" >> #include "qemu/iov.h" >> #include "libqos/libqos-pc.h" >> +#include "libqos/libqos-spapr.h" >> #include "libqos/virtio.h" >> #include "libqos/virtio-pci.h" >> #include "qemu/bswap.h" >> @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot) >> >> static QOSState *pci_test_start(int socket) >> { >> + const char *arch = qtest_get_arch(); >> const char *cmd = "-netdev socket,fd=%d,id=hs0 -device " >> "virtio-net-pci,netdev=hs0"; >> >> - return qtest_pc_boot(cmd, socket); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + return qtest_pc_boot(cmd, socket); >> + } >> + if (strcmp(arch, "ppc64") == 0) { >> + return qtest_spapr_boot(cmd, socket); >> + } >> + return NULL; >> } >> >> static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev) >> @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data) >> >> static void hotplug(void) >> { >> + const char *arch = qtest_get_arch(); >> + >> qtest_start("-device virtio-net-pci"); >> >> qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL); >> - qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); >> + >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); >> + } >> >> test_end(); >> } >> diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c >> index e1b2640..dcecf77 100644 >> --- a/tests/virtio-rng-test.c >> +++ b/tests/virtio-rng-test.c >> @@ -20,8 +20,13 @@ static void pci_nop(void) >> >> static void hotplug(void) >> { >> + const char *arch = qtest_get_arch(); >> + >> qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL); >> - qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); >> + >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); >> + } >> } >> >> int main(int argc, char **argv) >> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c >> index 721ae1f..dd5457b 100644 >> --- a/tests/virtio-scsi-test.c >> +++ b/tests/virtio-scsi-test.c >> @@ -12,6 +12,7 @@ >> #include "libqtest.h" >> #include "block/scsi.h" >> #include "libqos/libqos-pc.h" >> +#include "libqos/libqos-spapr.h" >> #include "libqos/virtio.h" >> #include "libqos/virtio-pci.h" >> #include "standard-headers/linux/virtio_ids.h" >> @@ -33,11 +34,18 @@ typedef struct { >> >> static QOSState *qvirtio_scsi_start(const char *extra_opts) >> { >> + const char *arch = qtest_get_arch(); >> const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw " >> "-device virtio-scsi-pci,id=vs0 " >> "-device scsi-hd,bus=vs0.0,drive=drv0 %s"; >> >> - return qtest_pc_boot(cmd, extra_opts ? : ""); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + return qtest_pc_boot(cmd, extra_opts ? : ""); >> + } >> + if (strcmp(arch, "ppc64") == 0) { >> + return qtest_spapr_boot(cmd, extra_opts ? : ""); >> + } >> + return NULL; >> } >> >> static void qvirtio_scsi_stop(QOSState *qs) > Thanks, Laurent
On Fri, Sep 30, 2016 at 08:59:39AM +0200, Laurent Vivier wrote: > > > On 30/09/2016 03:30, David Gibson wrote: > > On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote: > >> but disable MSI-X tests on SPAPR as we can't check the result > >> (the memory region used on PC is not readable on SPAPR). > >> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > >> --- > >> tests/Makefile.include | 3 ++- > >> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > >> tests/virtio-9p-test.c | 11 ++++++++++- > >> tests/virtio-blk-test.c | 22 +++++++++++++++++----- > >> tests/virtio-net-test.c | 17 +++++++++++++++-- > >> tests/virtio-rng-test.c | 7 ++++++- > >> tests/virtio-scsi-test.c | 10 +++++++++- > >> 7 files changed, 79 insertions(+), 13 deletions(-) > >> > >> diff --git a/tests/Makefile.include b/tests/Makefile.include > >> index c46a32d..1e4a3d5 100644 > >> --- a/tests/Makefile.include > >> +++ b/tests/Makefile.include > >> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF) > >> gcov-files-ppc64-y += hw/usb/hcd-uhci.c > >> check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF) > >> gcov-files-ppc64-y += hw/usb/hcd-xhci.c > >> +check-qtest-ppc64-y += $(check-qtest-virtio-y) > >> > >> check-qtest-sh4-y = tests/endianness-test$(EXESUF) > >> > >> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o > >> libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o > >> libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o > >> libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o > >> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > >> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > >> > >> tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o > >> tests/rtc-test$(EXESUF): tests/rtc-test.o > >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > >> index 6e005c1..ed81df6 100644 > >> --- a/tests/libqos/virtio-pci.c > >> +++ b/tests/libqos/virtio-pci.c > >> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) > >> return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr); > >> } > >> > >> +/* PCI is always read in little-endian order > >> + * but virtio ( < 1.0) is in guest order > >> + * so with a big-endian guest the order has been reversed > >> + * reverse it again > >> + */ > >> + > >> static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr) > >> { > >> QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > >> - return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > >> + uint16_t value; > >> + > >> + value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > >> + if (target_big_endian()) { > >> + value = bswap16(value); > >> + } > > > > Don't you need some sort of test to distinguish the virtio < 1.0 and > > virtio-1.0 cases? > > yes, but for the moment we don't test virtio-1.0, we will add the test > when will support it. > > http://wiki.qemu.org/Outreachy_2016_DecemberMarch#VIRTIO_1.0_support_in_libqos Ok. Please drop a comment in though, to make one less mystery for whoever does end up adding the virtio 1.0 support.
On Thu, 29 Sep 2016 19:15:07 +0200 Laurent Vivier <lvivier@redhat.com> wrote: > but disable MSI-X tests on SPAPR as we can't check the result > (the memory region used on PC is not readable on SPAPR). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > tests/Makefile.include | 3 ++- > tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > tests/virtio-9p-test.c | 11 ++++++++++- > tests/virtio-blk-test.c | 22 +++++++++++++++++----- > tests/virtio-net-test.c | 17 +++++++++++++++-- > tests/virtio-rng-test.c | 7 ++++++- > tests/virtio-scsi-test.c | 10 +++++++++- > 7 files changed, 79 insertions(+), 13 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index c46a32d..1e4a3d5 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF) > gcov-files-ppc64-y += hw/usb/hcd-uhci.c > check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF) > gcov-files-ppc64-y += hw/usb/hcd-xhci.c > +check-qtest-ppc64-y += $(check-qtest-virtio-y) > > check-qtest-sh4-y = tests/endianness-test$(EXESUF) > > @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o > libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o > libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o > libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o > -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o > > tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o > tests/rtc-test$(EXESUF): tests/rtc-test.o > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > index 6e005c1..ed81df6 100644 > --- a/tests/libqos/virtio-pci.c > +++ b/tests/libqos/virtio-pci.c > @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) > return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr); > } > > +/* PCI is always read in little-endian order > + * but virtio ( < 1.0) is in guest order > + * so with a big-endian guest the order has been reversed > + * reverse it again > + */ > + > static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr) > { > QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > - return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > + uint16_t value; > + > + value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); > + if (target_big_endian()) { > + value = bswap16(value); > + } > + return value; > } > > static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr) > { > QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > - return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); > + uint32_t value; > + > + value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); > + if (target_big_endian()) { > + value = bswap32(value); > + } > + return value; > } > > static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr) > diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > index 28d7f5b..a73bccb 100644 > --- a/tests/virtio-9p-test.c > +++ b/tests/virtio-9p-test.c > @@ -11,6 +11,7 @@ > #include "libqtest.h" > #include "qemu-common.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "standard-headers/linux/virtio_ids.h" > @@ -22,12 +23,20 @@ static char *test_share; > > static QOSState *qvirtio_9p_start(void) > { > + const char *arch = qtest_get_arch(); > + QOSState *qs = NULL; > test_share = g_strdup("/tmp/qtest.XXXXXX"); > g_assert_nonnull(mkdtemp(test_share)); > const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " > "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; > > - return qtest_pc_boot(cmd, test_share, mount_tag); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qs = qtest_pc_boot(cmd, test_share, mount_tag); > + } else if (strcmp(arch, "ppc64") == 0) { > + qs = qtest_spapr_boot(cmd, test_share, mount_tag); > + } > + What about introducing a qtest_arch_boot() helper that does ^^ and } else { g_printerr("qtest_arch_boot() not supported for arch %s\n", qtest_get_arch()); exit(EXIT_FAILURE); } > + return qs; > } > > static void qvirtio_9p_stop(QOSState *qs) > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > index a4de3e4..a032f8e 100644 > --- a/tests/virtio-blk-test.c > +++ b/tests/virtio-blk-test.c > @@ -11,6 +11,7 @@ > #include "qemu/osdep.h" > #include "libqtest.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "libqos/virtio-mmio.h" > @@ -58,6 +59,7 @@ static char *drive_create(void) > > static QOSState *pci_test_start(void) > { > + const char *arch = qtest_get_arch(); > QOSState *qs = NULL; > char *tmp_path; > const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw " > @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void) > > tmp_path = drive_create(); > > - qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + } else if (strcmp(arch, "ppc64") == 0) { > + qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); > + } > unlink(tmp_path); > g_free(tmp_path); > return qs; > @@ -676,6 +682,7 @@ static void pci_hotplug(void) > { > QVirtioPCIDevice *dev; > QOSState *qs; > + const char *arch = qtest_get_arch(); > > qs = pci_test_start(); > g_assert(qs); > @@ -690,7 +697,9 @@ static void pci_hotplug(void) > g_free(dev); > > /* unplug secondary disk */ > - qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); > + } > qtest_shutdown(qs); > } > > @@ -741,12 +750,15 @@ int main(int argc, char **argv) > > g_test_init(&argc, &argv, NULL); > > - if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 || > + strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) { > qtest_add_func("/virtio/blk/pci/basic", pci_basic); > qtest_add_func("/virtio/blk/pci/indirect", pci_indirect); > qtest_add_func("/virtio/blk/pci/config", pci_config); > - qtest_add_func("/virtio/blk/pci/msix", pci_msix); > - qtest_add_func("/virtio/blk/pci/idx", pci_idx); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qtest_add_func("/virtio/blk/pci/msix", pci_msix); > + qtest_add_func("/virtio/blk/pci/idx", pci_idx); > + } > qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug); > } else if (strcmp(arch, "arm") == 0) { > qtest_add_func("/virtio/blk/mmio/basic", mmio_basic); > diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c > index 3e83685..14f2920 100644 > --- a/tests/virtio-net-test.c > +++ b/tests/virtio-net-test.c > @@ -13,6 +13,7 @@ > #include "qemu/sockets.h" > #include "qemu/iov.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "qemu/bswap.h" > @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot) > > static QOSState *pci_test_start(int socket) > { > + const char *arch = qtest_get_arch(); > const char *cmd = "-netdev socket,fd=%d,id=hs0 -device " > "virtio-net-pci,netdev=hs0"; > > - return qtest_pc_boot(cmd, socket); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + return qtest_pc_boot(cmd, socket); > + } > + if (strcmp(arch, "ppc64") == 0) { > + return qtest_spapr_boot(cmd, socket); > + } > + return NULL; > } > > static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev) > @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data) > > static void hotplug(void) > { > + const char *arch = qtest_get_arch(); > + > qtest_start("-device virtio-net-pci"); > > qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL); > - qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); > + > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); > + } > > test_end(); > } > diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c > index e1b2640..dcecf77 100644 > --- a/tests/virtio-rng-test.c > +++ b/tests/virtio-rng-test.c > @@ -20,8 +20,13 @@ static void pci_nop(void) > > static void hotplug(void) > { > + const char *arch = qtest_get_arch(); > + > qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL); > - qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); > + > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); > + } > } > > int main(int argc, char **argv) > diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c > index 721ae1f..dd5457b 100644 > --- a/tests/virtio-scsi-test.c > +++ b/tests/virtio-scsi-test.c > @@ -12,6 +12,7 @@ > #include "libqtest.h" > #include "block/scsi.h" > #include "libqos/libqos-pc.h" > +#include "libqos/libqos-spapr.h" > #include "libqos/virtio.h" > #include "libqos/virtio-pci.h" > #include "standard-headers/linux/virtio_ids.h" > @@ -33,11 +34,18 @@ typedef struct { > > static QOSState *qvirtio_scsi_start(const char *extra_opts) > { > + const char *arch = qtest_get_arch(); > const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw " > "-device virtio-scsi-pci,id=vs0 " > "-device scsi-hd,bus=vs0.0,drive=drv0 %s"; > > - return qtest_pc_boot(cmd, extra_opts ? : ""); > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + return qtest_pc_boot(cmd, extra_opts ? : ""); > + } > + if (strcmp(arch, "ppc64") == 0) { > + return qtest_spapr_boot(cmd, extra_opts ? : ""); > + } > + return NULL; > } > > static void qvirtio_scsi_stop(QOSState *qs)
On 30/09/2016 12:18, Greg Kurz wrote: > On Thu, 29 Sep 2016 19:15:07 +0200 > Laurent Vivier <lvivier@redhat.com> wrote: > >> but disable MSI-X tests on SPAPR as we can't check the result >> (the memory region used on PC is not readable on SPAPR). >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> tests/Makefile.include | 3 ++- >> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- >> tests/virtio-9p-test.c | 11 ++++++++++- >> tests/virtio-blk-test.c | 22 +++++++++++++++++----- >> tests/virtio-net-test.c | 17 +++++++++++++++-- >> tests/virtio-rng-test.c | 7 ++++++- >> tests/virtio-scsi-test.c | 10 +++++++++- >> 7 files changed, 79 insertions(+), 13 deletions(-) ... >> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c >> index 28d7f5b..a73bccb 100644 >> --- a/tests/virtio-9p-test.c >> +++ b/tests/virtio-9p-test.c >> @@ -11,6 +11,7 @@ >> #include "libqtest.h" >> #include "qemu-common.h" >> #include "libqos/libqos-pc.h" >> +#include "libqos/libqos-spapr.h" >> #include "libqos/virtio.h" >> #include "libqos/virtio-pci.h" >> #include "standard-headers/linux/virtio_ids.h" >> @@ -22,12 +23,20 @@ static char *test_share; >> >> static QOSState *qvirtio_9p_start(void) >> { >> + const char *arch = qtest_get_arch(); >> + QOSState *qs = NULL; >> test_share = g_strdup("/tmp/qtest.XXXXXX"); >> g_assert_nonnull(mkdtemp(test_share)); >> const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " >> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; >> >> - return qtest_pc_boot(cmd, test_share, mount_tag); >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + qs = qtest_pc_boot(cmd, test_share, mount_tag); >> + } else if (strcmp(arch, "ppc64") == 0) { >> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); >> + } >> + > > What about introducing a qtest_arch_boot() helper that does ^^ and > > } else { > g_printerr("qtest_arch_boot() not supported for arch %s\n", > qtest_get_arch()); > exit(EXIT_FAILURE); > } > The problem with adding a function like that is it will pull $(libqos-pc-obj-y) and $(libqos-spapr-obj-y) for every tests using it, and for the moment we are pulling pc or spapr objects only if we need them for the given test. I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't have a generic qtest_boot() calling the architecture specific function. I cc: John Snow as he has written the initial code for this. ("90e5add libqos: add pc specific interface") Laurent
On Fri, 30 Sep 2016 12:30:08 +0200 Laurent Vivier <lvivier@redhat.com> wrote: > On 30/09/2016 12:18, Greg Kurz wrote: > > On Thu, 29 Sep 2016 19:15:07 +0200 > > Laurent Vivier <lvivier@redhat.com> wrote: > > > >> but disable MSI-X tests on SPAPR as we can't check the result > >> (the memory region used on PC is not readable on SPAPR). > >> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > >> --- > >> tests/Makefile.include | 3 ++- > >> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > >> tests/virtio-9p-test.c | 11 ++++++++++- > >> tests/virtio-blk-test.c | 22 +++++++++++++++++----- > >> tests/virtio-net-test.c | 17 +++++++++++++++-- > >> tests/virtio-rng-test.c | 7 ++++++- > >> tests/virtio-scsi-test.c | 10 +++++++++- > >> 7 files changed, 79 insertions(+), 13 deletions(-) > ... > >> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > >> index 28d7f5b..a73bccb 100644 > >> --- a/tests/virtio-9p-test.c > >> +++ b/tests/virtio-9p-test.c > >> @@ -11,6 +11,7 @@ > >> #include "libqtest.h" > >> #include "qemu-common.h" > >> #include "libqos/libqos-pc.h" > >> +#include "libqos/libqos-spapr.h" > >> #include "libqos/virtio.h" > >> #include "libqos/virtio-pci.h" > >> #include "standard-headers/linux/virtio_ids.h" > >> @@ -22,12 +23,20 @@ static char *test_share; > >> > >> static QOSState *qvirtio_9p_start(void) > >> { > >> + const char *arch = qtest_get_arch(); > >> + QOSState *qs = NULL; > >> test_share = g_strdup("/tmp/qtest.XXXXXX"); > >> g_assert_nonnull(mkdtemp(test_share)); > >> const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " > >> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; > >> > >> - return qtest_pc_boot(cmd, test_share, mount_tag); > >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > >> + qs = qtest_pc_boot(cmd, test_share, mount_tag); > >> + } else if (strcmp(arch, "ppc64") == 0) { > >> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); > >> + } > >> + > > > > What about introducing a qtest_arch_boot() helper that does ^^ and > > > > } else { > > g_printerr("qtest_arch_boot() not supported for arch %s\n", > > qtest_get_arch()); > > exit(EXIT_FAILURE); > > } > > > > The problem with adding a function like that is it will pull > $(libqos-pc-obj-y) and $(libqos-spapr-obj-y) for every tests using it, > and for the moment we are pulling pc or spapr objects only if we need > them for the given test. > > I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't > have a generic qtest_boot() calling the architecture specific function. > Ok I see the problem... this would call for a libqos-all-arch-obj-y variable to be used by specific tests maybe. And by the way, even if not adding such generic function, each test that can only run on specific archs should have an g_printerr()+exit() path, instead of calling g_assert(), for the same reason it was done for rtas-test.c. > I cc: John Snow as he has written the initial code for this. > ("90e5add libqos: add pc specific interface") > > Laurent > > Cheers. -- Greg
On 09/30/2016 06:30 AM, Laurent Vivier wrote: > > > On 30/09/2016 12:18, Greg Kurz wrote: >> On Thu, 29 Sep 2016 19:15:07 +0200 >> Laurent Vivier <lvivier@redhat.com> wrote: >> >>> but disable MSI-X tests on SPAPR as we can't check the result >>> (the memory region used on PC is not readable on SPAPR). >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> tests/Makefile.include | 3 ++- >>> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- >>> tests/virtio-9p-test.c | 11 ++++++++++- >>> tests/virtio-blk-test.c | 22 +++++++++++++++++----- >>> tests/virtio-net-test.c | 17 +++++++++++++++-- >>> tests/virtio-rng-test.c | 7 ++++++- >>> tests/virtio-scsi-test.c | 10 +++++++++- >>> 7 files changed, 79 insertions(+), 13 deletions(-) > ... >>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c >>> index 28d7f5b..a73bccb 100644 >>> --- a/tests/virtio-9p-test.c >>> +++ b/tests/virtio-9p-test.c >>> @@ -11,6 +11,7 @@ >>> #include "libqtest.h" >>> #include "qemu-common.h" >>> #include "libqos/libqos-pc.h" >>> +#include "libqos/libqos-spapr.h" >>> #include "libqos/virtio.h" >>> #include "libqos/virtio-pci.h" >>> #include "standard-headers/linux/virtio_ids.h" >>> @@ -22,12 +23,20 @@ static char *test_share; >>> >>> static QOSState *qvirtio_9p_start(void) >>> { >>> + const char *arch = qtest_get_arch(); >>> + QOSState *qs = NULL; >>> test_share = g_strdup("/tmp/qtest.XXXXXX"); >>> g_assert_nonnull(mkdtemp(test_share)); >>> const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " >>> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; >>> >>> - return qtest_pc_boot(cmd, test_share, mount_tag); >>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >>> + qs = qtest_pc_boot(cmd, test_share, mount_tag); >>> + } else if (strcmp(arch, "ppc64") == 0) { >>> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); >>> + } >>> + >> >> What about introducing a qtest_arch_boot() helper that does ^^ and >> >> } else { >> g_printerr("qtest_arch_boot() not supported for arch %s\n", >> qtest_get_arch()); >> exit(EXIT_FAILURE); >> } >> > > The problem with adding a function like that is it will pull > $(libqos-pc-obj-y) and $(libqos-spapr-obj-y) for every tests using it, > and for the moment we are pulling pc or spapr objects only if we need > them for the given test. > > I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't > have a generic qtest_boot() calling the architecture specific function. > > I cc: John Snow as he has written the initial code for this. > ("90e5add libqos: add pc specific interface") > > Laurent > > This was a while ago for me (and I was brand new to QEMU!), but that sounds about right. I wasn't able to reason about requirements for other architectures, so we made the PC-specific frontend to do the configuration for us. libqos.o does not pull in any of the PC-specific requirements as a result. Neither does the allocator. I didn't necessarily design it to be like this, just a path of least resistance type of thing. You probably could make an ArchOps callback structure if you wanted and pass that along to a generic bootup function to avoid the linking issues if you wanted a one-size-fits-all initialization function. --js
On Fri, 30 Sep 2016 11:19:39 -0400 John Snow <jsnow@redhat.com> wrote: > On 09/30/2016 06:30 AM, Laurent Vivier wrote: > > > > > > On 30/09/2016 12:18, Greg Kurz wrote: > >> On Thu, 29 Sep 2016 19:15:07 +0200 > >> Laurent Vivier <lvivier@redhat.com> wrote: > >> > >>> but disable MSI-X tests on SPAPR as we can't check the result > >>> (the memory region used on PC is not readable on SPAPR). > >>> > >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > >>> --- > >>> tests/Makefile.include | 3 ++- > >>> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > >>> tests/virtio-9p-test.c | 11 ++++++++++- > >>> tests/virtio-blk-test.c | 22 +++++++++++++++++----- > >>> tests/virtio-net-test.c | 17 +++++++++++++++-- > >>> tests/virtio-rng-test.c | 7 ++++++- > >>> tests/virtio-scsi-test.c | 10 +++++++++- > >>> 7 files changed, 79 insertions(+), 13 deletions(-) > > ... > >>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > >>> index 28d7f5b..a73bccb 100644 > >>> --- a/tests/virtio-9p-test.c > >>> +++ b/tests/virtio-9p-test.c > >>> @@ -11,6 +11,7 @@ > >>> #include "libqtest.h" > >>> #include "qemu-common.h" > >>> #include "libqos/libqos-pc.h" > >>> +#include "libqos/libqos-spapr.h" > >>> #include "libqos/virtio.h" > >>> #include "libqos/virtio-pci.h" > >>> #include "standard-headers/linux/virtio_ids.h" > >>> @@ -22,12 +23,20 @@ static char *test_share; > >>> > >>> static QOSState *qvirtio_9p_start(void) > >>> { > >>> + const char *arch = qtest_get_arch(); > >>> + QOSState *qs = NULL; > >>> test_share = g_strdup("/tmp/qtest.XXXXXX"); > >>> g_assert_nonnull(mkdtemp(test_share)); > >>> const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " > >>> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; > >>> > >>> - return qtest_pc_boot(cmd, test_share, mount_tag); > >>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > >>> + qs = qtest_pc_boot(cmd, test_share, mount_tag); > >>> + } else if (strcmp(arch, "ppc64") == 0) { > >>> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); > >>> + } > >>> + > >> > >> What about introducing a qtest_arch_boot() helper that does ^^ and > >> > >> } else { > >> g_printerr("qtest_arch_boot() not supported for arch %s\n", > >> qtest_get_arch()); > >> exit(EXIT_FAILURE); > >> } > >> > > > > The problem with adding a function like that is it will pull > > $(libqos-pc-obj-y) and $(libqos-spapr-obj-y) for every tests using it, > > and for the moment we are pulling pc or spapr objects only if we need > > them for the given test. > > > > I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't > > have a generic qtest_boot() calling the architecture specific function. > > > > I cc: John Snow as he has written the initial code for this. > > ("90e5add libqos: add pc specific interface") > > > > Laurent > > > > > > This was a while ago for me (and I was brand new to QEMU!), but that > sounds about right. I wasn't able to reason about requirements for other > architectures, so we made the PC-specific frontend to do the > configuration for us. libqos.o does not pull in any of the PC-specific > requirements as a result. Neither does the allocator. > > I didn't necessarily design it to be like this, just a path of least > resistance type of thing. > > You probably could make an ArchOps callback structure if you wanted and > pass that along to a generic bootup function to avoid the linking issues > if you wanted a one-size-fits-all initialization function. > > --js It looks like libqos may be divided in 3 families: - base libqos for tests that don't need platform specific support (basically what we currently have in libqos-y) - platfrom specific libqos for platform specific tests (libqos-pc, libqos-spapr) - one-size-fits-all libqos for tests that should run on several platforms Makes sense ? -- Greg
diff --git a/tests/Makefile.include b/tests/Makefile.include index c46a32d..1e4a3d5 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF) gcov-files-ppc64-y += hw/usb/hcd-uhci.c check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF) gcov-files-ppc64-y += hw/usb/hcd-xhci.c +check-qtest-ppc64-y += $(check-qtest-virtio-y) check-qtest-sh4-y = tests/endianness-test$(EXESUF) @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o tests/rtc-test$(EXESUF): tests/rtc-test.o diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c index 6e005c1..ed81df6 100644 --- a/tests/libqos/virtio-pci.c +++ b/tests/libqos/virtio-pci.c @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr); } +/* PCI is always read in little-endian order + * but virtio ( < 1.0) is in guest order + * so with a big-endian guest the order has been reversed + * reverse it again + */ + static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr) { QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; - return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); + uint16_t value; + + value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr); + if (target_big_endian()) { + value = bswap16(value); + } + return value; } static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr) { QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; - return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); + uint32_t value; + + value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr); + if (target_big_endian()) { + value = bswap32(value); + } + return value; } static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr) diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c index 28d7f5b..a73bccb 100644 --- a/tests/virtio-9p-test.c +++ b/tests/virtio-9p-test.c @@ -11,6 +11,7 @@ #include "libqtest.h" #include "qemu-common.h" #include "libqos/libqos-pc.h" +#include "libqos/libqos-spapr.h" #include "libqos/virtio.h" #include "libqos/virtio-pci.h" #include "standard-headers/linux/virtio_ids.h" @@ -22,12 +23,20 @@ static char *test_share; static QOSState *qvirtio_9p_start(void) { + const char *arch = qtest_get_arch(); + QOSState *qs = NULL; test_share = g_strdup("/tmp/qtest.XXXXXX"); g_assert_nonnull(mkdtemp(test_share)); const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s " "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; - return qtest_pc_boot(cmd, test_share, mount_tag); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qs = qtest_pc_boot(cmd, test_share, mount_tag); + } else if (strcmp(arch, "ppc64") == 0) { + qs = qtest_spapr_boot(cmd, test_share, mount_tag); + } + + return qs; } static void qvirtio_9p_stop(QOSState *qs) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index a4de3e4..a032f8e 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -11,6 +11,7 @@ #include "qemu/osdep.h" #include "libqtest.h" #include "libqos/libqos-pc.h" +#include "libqos/libqos-spapr.h" #include "libqos/virtio.h" #include "libqos/virtio-pci.h" #include "libqos/virtio-mmio.h" @@ -58,6 +59,7 @@ static char *drive_create(void) static QOSState *pci_test_start(void) { + const char *arch = qtest_get_arch(); QOSState *qs = NULL; char *tmp_path; const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw " @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void) tmp_path = drive_create(); - qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); + } else if (strcmp(arch, "ppc64") == 0) { + qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN); + } unlink(tmp_path); g_free(tmp_path); return qs; @@ -676,6 +682,7 @@ static void pci_hotplug(void) { QVirtioPCIDevice *dev; QOSState *qs; + const char *arch = qtest_get_arch(); qs = pci_test_start(); g_assert(qs); @@ -690,7 +697,9 @@ static void pci_hotplug(void) g_free(dev); /* unplug secondary disk */ - qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); + } qtest_shutdown(qs); } @@ -741,12 +750,15 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); - if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 || + strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) { qtest_add_func("/virtio/blk/pci/basic", pci_basic); qtest_add_func("/virtio/blk/pci/indirect", pci_indirect); qtest_add_func("/virtio/blk/pci/config", pci_config); - qtest_add_func("/virtio/blk/pci/msix", pci_msix); - qtest_add_func("/virtio/blk/pci/idx", pci_idx); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qtest_add_func("/virtio/blk/pci/msix", pci_msix); + qtest_add_func("/virtio/blk/pci/idx", pci_idx); + } qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug); } else if (strcmp(arch, "arm") == 0) { qtest_add_func("/virtio/blk/mmio/basic", mmio_basic); diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c index 3e83685..14f2920 100644 --- a/tests/virtio-net-test.c +++ b/tests/virtio-net-test.c @@ -13,6 +13,7 @@ #include "qemu/sockets.h" #include "qemu/iov.h" #include "libqos/libqos-pc.h" +#include "libqos/libqos-spapr.h" #include "libqos/virtio.h" #include "libqos/virtio-pci.h" #include "qemu/bswap.h" @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot) static QOSState *pci_test_start(int socket) { + const char *arch = qtest_get_arch(); const char *cmd = "-netdev socket,fd=%d,id=hs0 -device " "virtio-net-pci,netdev=hs0"; - return qtest_pc_boot(cmd, socket); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + return qtest_pc_boot(cmd, socket); + } + if (strcmp(arch, "ppc64") == 0) { + return qtest_spapr_boot(cmd, socket); + } + return NULL; } static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev) @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data) static void hotplug(void) { + const char *arch = qtest_get_arch(); + qtest_start("-device virtio-net-pci"); qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL); - qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); + + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); + } test_end(); } diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c index e1b2640..dcecf77 100644 --- a/tests/virtio-rng-test.c +++ b/tests/virtio-rng-test.c @@ -20,8 +20,13 @@ static void pci_nop(void) static void hotplug(void) { + const char *arch = qtest_get_arch(); + qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL); - qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); + + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); + } } int main(int argc, char **argv) diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c index 721ae1f..dd5457b 100644 --- a/tests/virtio-scsi-test.c +++ b/tests/virtio-scsi-test.c @@ -12,6 +12,7 @@ #include "libqtest.h" #include "block/scsi.h" #include "libqos/libqos-pc.h" +#include "libqos/libqos-spapr.h" #include "libqos/virtio.h" #include "libqos/virtio-pci.h" #include "standard-headers/linux/virtio_ids.h" @@ -33,11 +34,18 @@ typedef struct { static QOSState *qvirtio_scsi_start(const char *extra_opts) { + const char *arch = qtest_get_arch(); const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw " "-device virtio-scsi-pci,id=vs0 " "-device scsi-hd,bus=vs0.0,drive=drv0 %s"; - return qtest_pc_boot(cmd, extra_opts ? : ""); + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + return qtest_pc_boot(cmd, extra_opts ? : ""); + } + if (strcmp(arch, "ppc64") == 0) { + return qtest_spapr_boot(cmd, extra_opts ? : ""); + } + return NULL; } static void qvirtio_scsi_stop(QOSState *qs)
but disable MSI-X tests on SPAPR as we can't check the result (the memory region used on PC is not readable on SPAPR). Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- tests/Makefile.include | 3 ++- tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- tests/virtio-9p-test.c | 11 ++++++++++- tests/virtio-blk-test.c | 22 +++++++++++++++++----- tests/virtio-net-test.c | 17 +++++++++++++++-- tests/virtio-rng-test.c | 7 ++++++- tests/virtio-scsi-test.c | 10 +++++++++- 7 files changed, 79 insertions(+), 13 deletions(-)