On 13/12/2016 08:55, Markus Armbruster wrote:
> Laurent Vivier <lviv...@redhat.com> writes:
>
>> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
>> ---
>> tests/Makefile.include | 3 ++-
>> tests/ivshmem-test.c | 46 +++++++++++++++++++++++++++++-----------------
>> 2 files changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index b574964..09fe5b6 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -287,6 +287,7 @@ check-qtest-ppc64-y += tests/test-netfilter$(EXESUF)
>> check-qtest-ppc64-y += tests/test-filter-mirror$(EXESUF)
>> check-qtest-ppc64-y += tests/test-filter-redirector$(EXESUF)
>> check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/ivshmem-test$(EXESUF)
>>
>> check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>
>> @@ -690,7 +691,7 @@ tests/test-netfilter$(EXESUF): tests/test-netfilter.o
>> $(qtest-obj-y)
>> tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
>> tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o
>> $(qtest-obj-y)
>> tests/test-x86-cpuid-compat$(EXESUF): tests/test-x86-cpuid-compat.o
>> $(qtest-obj-y)
>> -tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o
>> contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
>> +tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o
>> contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
>> $(libqos-spapr-obj-y)
>> tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
>> tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
>> tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
>> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
>> index 04a5c5d..5c6dc38 100644
>> --- a/tests/ivshmem-test.c
>> +++ b/tests/ivshmem-test.c
>> @@ -11,8 +11,9 @@
>> #include "qemu/osdep.h"
>> #include <glib/gstdio.h>
>> #include "contrib/ivshmem-server/ivshmem-server.h"
>> -#include "libqos/pci-pc.h"
>> #include "libqtest.h"
>> +#include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>> #include "qemu-common.h"
>>
>> #define TMPSHMSIZE (1 << 20)
>> @@ -40,9 +41,8 @@ static QPCIDevice *get_device(QPCIBus *pcibus)
>> }
>>
>> typedef struct _IVState {
>> - QTestState *qtest;
>> + QOSState *qs;
>> QPCIBar reg_bar, mem_bar;
>> - QPCIBus *pcibus;
>> QPCIDevice *dev;
>> } IVState;
>>
>> @@ -74,7 +74,7 @@ static inline unsigned in_reg(IVState *s, enum Reg reg)
>> QTestState *qtest = global_qtest;
>> unsigned res;
>>
>> - global_qtest = s->qtest;
>> + global_qtest = s->qs->qts;
>> res = qpci_io_readl(s->dev, s->reg_bar, reg);
>> g_test_message("*%s -> %x\n", name, res);
>> global_qtest = qtest;
>> @@ -87,7 +87,7 @@ static inline void out_reg(IVState *s, enum Reg reg,
>> unsigned v)
>> const char *name = reg2str(reg);
>> QTestState *qtest = global_qtest;
>>
>> - global_qtest = s->qtest;
>> + global_qtest = s->qs->qts;
>> g_test_message("%x -> *%s\n", v, name);
>> qpci_io_writel(s->dev, s->reg_bar, reg, v);
>> global_qtest = qtest;
>> @@ -97,7 +97,7 @@ static inline void read_mem(IVState *s, uint64_t off, void
>> *buf, size_t len)
>> {
>> QTestState *qtest = global_qtest;
>>
>> - global_qtest = s->qtest;
>> + global_qtest = s->qs->qts;
>> qpci_memread(s->dev, s->mem_bar, off, buf, len);
>> global_qtest = qtest;
>> }
>> @@ -107,7 +107,7 @@ static inline void write_mem(IVState *s, uint64_t off,
>> {
>> QTestState *qtest = global_qtest;
>>
>> - global_qtest = s->qtest;
>> + global_qtest = s->qs->qts;
>> qpci_memwrite(s->dev, s->mem_bar, off, buf, len);
>> global_qtest = qtest;
>> }
>> @@ -115,17 +115,23 @@ static inline void write_mem(IVState *s, uint64_t off,
>> static void cleanup_vm(IVState *s)
>> {
>> g_free(s->dev);
>> - qpci_free_pc(s->pcibus);
>> - qtest_quit(s->qtest);
>> + qtest_shutdown(s->qs);
>> }
>>
>> static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
>> {
>> uint64_t barsize;
>> + const char *arch = qtest_get_arch();
>>
>> - s->qtest = qtest_start(cmd);
>> - s->pcibus = qpci_init_pc(NULL);
>> - s->dev = get_device(s->pcibus);
>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> + s->qs = qtest_pc_boot(cmd);
>> + } else if (strcmp(arch, "ppc64") == 0) {
>> + s->qs = qtest_spapr_boot(cmd);
>> + } else {
>> + g_printerr("vshmem-test tests are only available on x86 or
>> ppc64\n");
>
> Typo: s/vshmem/ivshmem
>
> To get here, you need to build and run this manually, as make check only
> builds and runs this for targets that work. But the check doesn't hurt,
> and documents expectations.
>
>> + exit(EXIT_FAILURE);
>> + }
>> + s->dev = get_device(s->qs->pcibus);
>>
>> s->reg_bar = qpci_iomap(s->dev, 0, &barsize);
>> g_assert_cmpuint(barsize, ==, 256);
>> @@ -347,7 +353,7 @@ static void test_ivshmem_server(bool msi)
>> g_assert_cmpint(vm1, !=, vm2);
>>
>> /* check number of MSI-X vectors */
>> - global_qtest = s1->qtest;
>> + global_qtest = s1->qs->qts;
>> if (msi) {
>> ret = qpci_msix_table_size(s1->dev);
>> g_assert_cmpuint(ret, ==, nvectors);
>> @@ -370,7 +376,7 @@ static void test_ivshmem_server(bool msi)
>> g_assert_cmpuint(ret, !=, 0);
>>
>> /* ping vm1 -> vm2 on vector 1 */
>> - global_qtest = s2->qtest;
>> + global_qtest = s2->qs->qts;
>> if (msi) {
>> ret = qpci_msix_pending(s2->dev, 1);
>> g_assert_cmpuint(ret, ==, 0);
>> @@ -412,6 +418,7 @@ static void test_ivshmem_server_irq(void)
>>
>> static void test_ivshmem_hotplug(void)
>> {
>> + const char *arch = qtest_get_arch();
>> gchar *opts;
>>
>> qtest_start("");
>
> Unlike the other tests, this one doesn't refuse to run on unexpected
> targets. Intentional?
Yes, because there is no dependencies on libqos PCI implementation, so
it should always work.
>> @@ -419,7 +426,9 @@ static void test_ivshmem_hotplug(void)
>> opts = g_strdup_printf("'shm': '%s', 'size': '1M'", tmpshm);
>>
>> qpci_plug_device_test("ivshmem", "iv1", PCI_SLOT_HP, opts);
>> - qpci_unplug_acpi_device_test("iv1", PCI_SLOT_HP);
>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> + qpci_unplug_acpi_device_test("iv1", PCI_SLOT_HP);
>> + }
>
> Disables unplug test for ppc64.
>
>>
>> qtest_end();
>> g_free(opts);
>> @@ -491,6 +500,7 @@ static gchar *mktempshm(int size, int *fd)
>> int main(int argc, char **argv)
>> {
>> int ret, fd;
>> + const char *arch = qtest_get_arch();
>> gchar dir[] = "/tmp/ivshmem-test.XXXXXX";
>>
>> #if !GLIB_CHECK_VERSION(2, 31, 0)
>> @@ -521,8 +531,10 @@ int main(int argc, char **argv)
>> qtest_add_func("/ivshmem/memdev", test_ivshmem_memdev);
>> if (g_test_slow()) {
>> qtest_add_func("/ivshmem/pair", test_ivshmem_pair);
>> - qtest_add_func("/ivshmem/server-msi", test_ivshmem_server_msi);
>> - qtest_add_func("/ivshmem/server-irq", test_ivshmem_server_irq);
>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> + qtest_add_func("/ivshmem/server-msi", test_ivshmem_server_msi);
>> + qtest_add_func("/ivshmem/server-irq", test_ivshmem_server_irq);
>> + }
>> }
>>
>> ret = g_test_run();
>
> The commit message should explain things in a bit more detail: the test
> works only for x86, and is only compiled and executed for x86. The
> patch converts it to libqos, adds ppc64, and makes the test fail for
> targets that don't work. It also disables the unplug part of test
> /ivshmem/hotplug for ppc64; please explain why.
>
> Please make the conversion to libqos a separate patch, for easier
> review.
>
Thank you for the review.
I'm going to change this patch according to your comments.
Laurent
