From a1dcdda8275e15ab919bcec012e9e662283b6a7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:28 +0200 Subject: tests/fuzz/Makefile: Do not link code using unavailable devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some devices availability depends on CONFIG options. Use these options to only link tests when requested device is available. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-2-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qtest/fuzz/Makefile.include | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include index cde3e9636c..f259d866c9 100644 --- a/tests/qtest/fuzz/Makefile.include +++ b/tests/qtest/fuzz/Makefile.include @@ -7,9 +7,9 @@ fuzz-obj-y += tests/qtest/fuzz/fork_fuzz.o fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o # Targets -fuzz-obj-y += tests/qtest/fuzz/i440fx_fuzz.o -fuzz-obj-y += tests/qtest/fuzz/virtio_net_fuzz.o -fuzz-obj-y += tests/qtest/fuzz/virtio_scsi_fuzz.o +fuzz-obj-$(CONFIG_PCI_I440FX) += tests/qtest/fuzz/i440fx_fuzz.o +fuzz-obj-$(CONFIG_VIRTIO_NET) += tests/qtest/fuzz/virtio_net_fuzz.o +fuzz-obj-$(CONFIG_SCSI) += tests/qtest/fuzz/virtio_scsi_fuzz.o FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest -- cgit v1.2.3 From 763815a83744fe7b9c5d88bd54370f25742fd10c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:29 +0200 Subject: Makefile: List fuzz targets in 'make help' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List softmmu fuzz targets in 'make help' output: $ make help ... Architecture specific targets: aarch64-softmmu/all - Build for aarch64-softmmu aarch64-softmmu/fuzz - Build fuzzer for aarch64-softmmu alpha-softmmu/all - Build for alpha-softmmu alpha-softmmu/fuzz - Build fuzzer for alpha-softmmu arm-softmmu/all - Build for arm-softmmu arm-softmmu/fuzz - Build fuzzer for arm-softmmu ... Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-3-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 34275f57c9..40e4f7677b 100644 --- a/Makefile +++ b/Makefile @@ -1252,7 +1252,11 @@ endif @$(if $(TARGET_DIRS), \ echo 'Architecture specific targets:'; \ $(foreach t, $(TARGET_DIRS), \ - $(call print-help-run,$(t)/all,Build for $(t));) \ + $(call print-help-run,$(t)/all,Build for $(t)); \ + $(if $(CONFIG_FUZZ), \ + $(if $(findstring softmmu,$(t)), \ + $(call print-help-run,$(t)/fuzz,Build fuzzer for $(t)); \ + ))) \ echo '') @$(if $(TOOLS), \ echo 'Tools targets:'; \ -- cgit v1.2.3 From 73ee6da45d22c82275042f7e84426e647246a56d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:30 +0200 Subject: tests/fuzz: Add missing space in test description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-4-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qtest/fuzz/i440fx_fuzz.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c index ab5f112584..96fed9ff12 100644 --- a/tests/qtest/fuzz/i440fx_fuzz.c +++ b/tests/qtest/fuzz/i440fx_fuzz.c @@ -159,7 +159,7 @@ static void register_pci_fuzz_targets(void) /* Uses simple qtest commands and reboots to reset state */ fuzz_add_target(&(FuzzTarget){ .name = "i440fx-qtest-reboot-fuzz", - .description = "Fuzz the i440fx using raw qtest commands and" + .description = "Fuzz the i440fx using raw qtest commands and " "rebooting after each run", .get_init_cmdline = i440fx_argv, .fuzz = i440fx_fuzz_qtest}); @@ -167,7 +167,7 @@ static void register_pci_fuzz_targets(void) /* Uses libqos and forks to prevent state leakage */ fuzz_add_qos_target(&(FuzzTarget){ .name = "i440fx-qos-fork-fuzz", - .description = "Fuzz the i440fx using raw qtest commands and" + .description = "Fuzz the i440fx using raw qtest commands and " "rebooting after each run", .pre_vm_init = &fork_init, .fuzz = i440fx_fuzz_qos_fork,}, @@ -182,7 +182,7 @@ static void register_pci_fuzz_targets(void) */ fuzz_add_qos_target(&(FuzzTarget){ .name = "i440fx-qos-noreset-fuzz", - .description = "Fuzz the i440fx using raw qtest commands and" + .description = "Fuzz the i440fx using raw qtest commands and " "rebooting after each run", .fuzz = i440fx_fuzz_qos,}, "i440FX-pcihost", -- cgit v1.2.3 From 79e18a60ab456764eaa974b67b9c54281a5156db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:31 +0200 Subject: tests/fuzz: Remove unuseful/unused typedefs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These typedefs are not used. Use a simple structure, remote the typedefs. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-5-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qtest/fuzz/i440fx_fuzz.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c index 96fed9ff12..c197b026db 100644 --- a/tests/qtest/fuzz/i440fx_fuzz.c +++ b/tests/qtest/fuzz/i440fx_fuzz.c @@ -45,12 +45,11 @@ static void i440fx_fuzz_qtest(QTestState *s, * loop over the Data, breaking it up into actions. each action has an * opcode, address offset and value */ - typedef struct QTestFuzzAction { + struct { uint8_t opcode; uint8_t addr; uint32_t value; - } QTestFuzzAction; - QTestFuzzAction a; + } a; while (Size >= sizeof(a)) { /* make a copy of the action so we can normalize the values in-place */ @@ -91,19 +90,18 @@ static void i440fx_fuzz_qos(QTestState *s, * Same as i440fx_fuzz_qtest, but using QOS. devfn is incorporated into the * value written over Port IO */ - typedef struct QOSFuzzAction { + struct { uint8_t opcode; uint8_t offset; int devfn; uint32_t value; - } QOSFuzzAction; + } a; static QPCIBus *bus; if (!bus) { bus = qpci_new_pc(s, fuzz_qos_alloc); } - QOSFuzzAction a; while (Size >= sizeof(a)) { memcpy(&a, Data, sizeof(a)); switch (a.opcode % ACTION_MAX) { -- cgit v1.2.3 From 84cb0a6d20c94abab6bc868e0e92044fcebdb1d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:32 +0200 Subject: tests/fuzz: Extract pciconfig_fuzz_qos() method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the generic pciconfig_fuzz_qos() method from i440fx_fuzz_qos(). This will help to write tests not specific to the i440FX controller. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-6-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qtest/fuzz/i440fx_fuzz.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c index c197b026db..558fa17c93 100644 --- a/tests/qtest/fuzz/i440fx_fuzz.c +++ b/tests/qtest/fuzz/i440fx_fuzz.c @@ -84,7 +84,7 @@ static void i440fx_fuzz_qtest(QTestState *s, flush_events(s); } -static void i440fx_fuzz_qos(QTestState *s, +static void pciconfig_fuzz_qos(QTestState *s, QPCIBus *bus, const unsigned char *Data, size_t Size) { /* * Same as i440fx_fuzz_qtest, but using QOS. devfn is incorporated into the @@ -97,11 +97,6 @@ static void i440fx_fuzz_qos(QTestState *s, uint32_t value; } a; - static QPCIBus *bus; - if (!bus) { - bus = qpci_new_pc(s, fuzz_qos_alloc); - } - while (Size >= sizeof(a)) { memcpy(&a, Data, sizeof(a)); switch (a.opcode % ACTION_MAX) { @@ -130,6 +125,19 @@ static void i440fx_fuzz_qos(QTestState *s, flush_events(s); } +static void i440fx_fuzz_qos(QTestState *s, + const unsigned char *Data, + size_t Size) +{ + static QPCIBus *bus; + + if (!bus) { + bus = qpci_new_pc(s, fuzz_qos_alloc); + } + + pciconfig_fuzz_qos(s, bus, Data, Size); +} + static void i440fx_fuzz_qos_fork(QTestState *s, const unsigned char *Data, size_t Size) { if (fork() == 0) { -- cgit v1.2.3 From 6fb5f0842aab0f744eebfabefb447b3a3a1af7cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 14 May 2020 16:34:33 +0200 Subject: tests/fuzz: Extract ioport_fuzz_qtest() method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract generic ioport_fuzz_qtest() method from i440fx_fuzz_qtest(). This will help to write tests not specific to the i440FX controller. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20200514143433.18569-7-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qtest/fuzz/i440fx_fuzz.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c index 558fa17c93..bcd6769b4c 100644 --- a/tests/qtest/fuzz/i440fx_fuzz.c +++ b/tests/qtest/fuzz/i440fx_fuzz.c @@ -39,7 +39,7 @@ enum action_id { ACTION_MAX }; -static void i440fx_fuzz_qtest(QTestState *s, +static void ioport_fuzz_qtest(QTestState *s, const unsigned char *Data, size_t Size) { /* * loop over the Data, breaking it up into actions. each action has an @@ -84,10 +84,17 @@ static void i440fx_fuzz_qtest(QTestState *s, flush_events(s); } +static void i440fx_fuzz_qtest(QTestState *s, + const unsigned char *Data, + size_t Size) +{ + ioport_fuzz_qtest(s, Data, Size); +} + static void pciconfig_fuzz_qos(QTestState *s, QPCIBus *bus, const unsigned char *Data, size_t Size) { /* - * Same as i440fx_fuzz_qtest, but using QOS. devfn is incorporated into the + * Same as ioport_fuzz_qtest, but using QOS. devfn is incorporated into the * value written over Port IO */ struct { -- cgit v1.2.3 From de137e44f75d9868f5b548638081850f6ac771f2 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 11 May 2020 19:36:29 +0100 Subject: aio-posix: don't duplicate fd handler deletion in fdmon_io_uring_destroy() The io_uring file descriptor monitoring implementation has an internal list of fd handlers that are pending submission to io_uring. fdmon_io_uring_destroy() deletes all fd handlers on the list. Don't delete fd handlers directly in fdmon_io_uring_destroy() for two reasons: 1. This duplicates the aio-posix.c AioHandler deletion code and could become outdated if the struct changes. 2. Only handlers with the FDMON_IO_URING_REMOVE flag set are safe to remove. If the flag is not set then something still has a pointer to the fd handler. Let aio-posix.c and its user worry about that. In practice this isn't an issue because fdmon_io_uring_destroy() is only called when shutting down so all users have removed their fd handlers, but the next patch will need this! Signed-off-by: Stefan Hajnoczi Tested-by: Oleksandr Natalenko Message-id: 20200511183630.279750-2-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- util/aio-posix.c | 1 + util/fdmon-io_uring.c | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/util/aio-posix.c b/util/aio-posix.c index c3613d299e..8af334ab19 100644 --- a/util/aio-posix.c +++ b/util/aio-posix.c @@ -679,6 +679,7 @@ void aio_context_destroy(AioContext *ctx) { fdmon_io_uring_destroy(ctx); fdmon_epoll_disable(ctx); + aio_free_deleted_handlers(ctx); } void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns, diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c index d5a80ed6fb..1d14177df0 100644 --- a/util/fdmon-io_uring.c +++ b/util/fdmon-io_uring.c @@ -342,11 +342,18 @@ void fdmon_io_uring_destroy(AioContext *ctx) io_uring_queue_exit(&ctx->fdmon_io_uring); - /* No need to submit these anymore, just free them. */ + /* Move handlers due to be removed onto the deleted list */ while ((node = QSLIST_FIRST_RCU(&ctx->submit_list))) { + unsigned flags = atomic_fetch_and(&node->flags, + ~(FDMON_IO_URING_PENDING | + FDMON_IO_URING_ADD | + FDMON_IO_URING_REMOVE)); + + if (flags & FDMON_IO_URING_REMOVE) { + QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted); + } + QSLIST_REMOVE_HEAD_RCU(&ctx->submit_list, node_submitted); - QLIST_REMOVE(node, node); - g_free(node); } ctx->fdmon_ops = &fdmon_poll_ops; -- cgit v1.2.3 From ba607ca8bff4d2c2062902f8355657c865ac7c29 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 11 May 2020 19:36:30 +0100 Subject: aio-posix: disable fdmon-io_uring when GSource is used The glib event loop does not call fdmon_io_uring_wait() so fd handlers waiting to be submitted build up in the list. There is no benefit is using io_uring when the glib GSource is being used, so disable it instead of implementing a more complex fix. This fixes a memory leak where AioHandlers would build up and increasing amounts of CPU time were spent iterating them in aio_pending(). The symptom is that guests become slow when QEMU is built with io_uring support. Buglink: https://bugs.launchpad.net/qemu/+bug/1877716 Fixes: 73fd282e7b6dd4e4ea1c3bbb3d302c8db51e4ccf ("aio-posix: add io_uring fd monitoring implementation") Signed-off-by: Stefan Hajnoczi Tested-by: Oleksandr Natalenko Message-id: 20200511183630.279750-3-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- include/block/aio.h | 3 +++ util/aio-posix.c | 12 ++++++++++++ util/aio-win32.c | 4 ++++ util/async.c | 1 + 4 files changed, 20 insertions(+) diff --git a/include/block/aio.h b/include/block/aio.h index 62ed954344..b2f703fa3f 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -701,6 +701,9 @@ void aio_context_setup(AioContext *ctx); */ void aio_context_destroy(AioContext *ctx); +/* Used internally, do not call outside AioContext code */ +void aio_context_use_g_source(AioContext *ctx); + /** * aio_context_set_poll_params: * @ctx: the aio context diff --git a/util/aio-posix.c b/util/aio-posix.c index 8af334ab19..1b2a3af65b 100644 --- a/util/aio-posix.c +++ b/util/aio-posix.c @@ -682,6 +682,18 @@ void aio_context_destroy(AioContext *ctx) aio_free_deleted_handlers(ctx); } +void aio_context_use_g_source(AioContext *ctx) +{ + /* + * Disable io_uring when the glib main loop is used because it doesn't + * support mixed glib/aio_poll() usage. It relies on aio_poll() being + * called regularly so that changes to the monitored file descriptors are + * submitted, otherwise a list of pending fd handlers builds up. + */ + fdmon_io_uring_destroy(ctx); + aio_free_deleted_handlers(ctx); +} + void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns, int64_t grow, int64_t shrink, Error **errp) { diff --git a/util/aio-win32.c b/util/aio-win32.c index 729d533faf..953c56ab48 100644 --- a/util/aio-win32.c +++ b/util/aio-win32.c @@ -414,6 +414,10 @@ void aio_context_destroy(AioContext *ctx) { } +void aio_context_use_g_source(AioContext *ctx) +{ +} + void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns, int64_t grow, int64_t shrink, Error **errp) { diff --git a/util/async.c b/util/async.c index 3165a28f2f..1319eee3bc 100644 --- a/util/async.c +++ b/util/async.c @@ -362,6 +362,7 @@ static GSourceFuncs aio_source_funcs = { GSource *aio_get_g_source(AioContext *ctx) { + aio_context_use_g_source(ctx); g_source_ref(&ctx->source); return &ctx->source; } -- cgit v1.2.3