On 01/10/16 17:23, Thomas Huth wrote:
> On 01.10.2016 14:29, Greg Kurz wrote:
>> On Fri, 30 Sep 2016 16:19:12 +0200
>> Laurent Vivier <lviv...@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 <lviv...@redhat.com>
>>> ---
>>> tests/Makefile.include | 3 ++-
>>> tests/libqos/virtio-pci.c | 26 ++++++++++++++++++++++++--
>>> tests/virtio-9p-test.c | 12 +++++++++++-
>>> tests/virtio-blk-test.c | 25 ++++++++++++++++++++-----
>>> tests/virtio-net-test.c | 18 ++++++++++++++++--
>>> tests/virtio-rng-test.c | 7 ++++++-
>>> tests/virtio-scsi-test.c | 12 +++++++++++-
>>> 7 files changed, 90 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..bf775fa 100644
>>> --- a/tests/libqos/virtio-pci.c
>>> +++ b/tests/libqos/virtio-pci.c
>>> @@ -68,16 +68,38 @@ 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
>>> + * virtio-1.0 is always little-endian, like PCI, but as it
>>> + * is not implemented in libqos, we don't manage this case.
>>> + */
>>> +
>>> 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);
>>> + /* FIXME: don't swap with virtio-1.0 */
>>> + 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);
>>> + /* FIXME: don't swap with virtio-1.0 */
>>> + 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 7698014..9adb58f 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,21 @@ static char *test_share;
>>>
>>> static QOSState *qvirtio_9p_start(void)
>>> {
>>> + const char *arch = qtest_get_arch();
>>> 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) {
>>> + return qtest_pc_boot(cmd, test_share, mount_tag);
>>> + }
>>> + if (strcmp(arch, "ppc64") == 0) {
>>> + return qtest_spapr_boot(cmd, test_share, mount_tag);
>>> + }
>>> +
>>> + g_printerr("virtio-9p tests are only available on x86 or ppc64\n");
>>> + exit(EXIT_FAILURE);
>>> }
>>>
>>
>> According to your remarks and John's answer, I agree that this is ok.
>> A one-size-fits-all API may be interesting, but this is definitely
>> another work and it shouldn't prevent this patchset from being applied.
>>
>>> static void qvirtio_9p_stop(QOSState *qs)
>>> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
>>> index 96d0125..b4494c9 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"
>>> @@ -59,6 +60,7 @@ static char *drive_create(void)
>>> static QOSState *pci_test_start(void)
>>> {
>>> QOSState *qs;
>>> + const char *arch = qtest_get_arch();
>>> char *tmp_path;
>>> const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
>>> "-drive if=none,id=drive1,file=/dev/null,format=raw "
>>> @@ -67,7 +69,14 @@ 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);
>>> + } else {
>>> + g_printerr("virtio-blk tests are only available on x86 or
>>> ppc64\n");
>>> + exit(EXIT_FAILURE);
>>> + }
>>> unlink(tmp_path);
>>> g_free(tmp_path);
>>> return qs;
>>> @@ -671,6 +680,7 @@ static void pci_hotplug(void)
>>> {
>>> QVirtioPCIDevice *dev;
>>> QOSState *qs;
>>> + const char *arch = qtest_get_arch();
>>>
>>> qs = pci_test_start();
>>>
>>> @@ -684,7 +694,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);
>>> }
>>>
>>> @@ -735,12 +747,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) {
>>
>> Can this test actually run on a ppc compatible platform ?
>
> Not sure, but I think openbios supports virtio-blk, doesn't it? So you
> could use it with openbios on the g3beige machine, too?
I posted a patch to add virtio-blk support to OpenBIOS a while back,
however it's based upon the legacy rather than 1.0 specification so I
wasn't sure whether it make sense to convert it to the proper standard
first.
I don't remember receiving any feedback either way, but if people don't
mind a legacy driver for the moment then I can look at getting it committed.
ATB,
Mark.
