From c6986f16a7022ccfb73d91bc7676c8e1d15e5342 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 1 Mar 2021 12:02:44 +0100 Subject: [PATCH 01/23] KVM: x86: do not fail if software breakpoint has already been removed If kvm_arch_remove_sw_breakpoint finds that a software breakpoint does not have an INT3 instruction, it fails. This can happen if one sets a software breakpoint in a kernel module and then reloads it. gdb then thinks the breakpoint cannot be deleted and there is no way to add it back. Suggested-by: Maxim Levitsky Reviewed-by: Maxim Levitsky Signed-off-by: Paolo Bonzini --- target/i386/kvm/kvm.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 0b5755e42b..c8d61daf68 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4352,8 +4352,13 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) { uint8_t int3; - if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0) || int3 != 0xcc || - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { + if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0)) { + return -EINVAL; + } + if (int3 != 0xcc) { + return 0; + } + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { return -EINVAL; } return 0; From 2c933ac6a883606b85f8cf271bfb40379d077e97 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 1 Mar 2021 12:14:14 +0100 Subject: [PATCH 02/23] KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc The userspace local APIC is basically untested and does not support many features such as TSC deadline timer, x2APIC or PV spinlocks. On the other hand, the PIT and IOAPIC are okay as they are not tied to the processor and are tested with -M kernel-irqchip=split. Therefore, deprecate the local APIC and, with it, limit -M kernel-irqchip=off to the ISA PC machine type, which does not have a local APIC at all. Reviewed-by: Maxim Levitsky Signed-off-by: Paolo Bonzini --- docs/system/deprecated.rst | 7 +++++++ hw/intc/apic.c | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 561c916da2..fcf0ca4068 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -153,6 +153,13 @@ The ``-writeconfig`` option is not able to serialize the entire contents of the QEMU command line. It is thus considered a failed experiment and deprecated, with no current replacement. +Userspace local APIC with KVM (x86, since 6.0) +'''''''''''''''''''''''''''''''''''''''''''''' + +Using ``-M kernel-irqchip=off`` with x86 machine types that include a local +APIC is deprecated. The ``split`` setting is supported, as is using +``-M kernel-irqchip=off`` with the ISA PC machine type. + QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 3ada22f427..f4f50f974e 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -25,6 +25,7 @@ #include "hw/intc/i8259.h" #include "hw/pci/msi.h" #include "qemu/host-utils.h" +#include "sysemu/kvm.h" #include "trace.h" #include "hw/i386/apic-msidef.h" #include "qapi/error.h" @@ -875,6 +876,11 @@ static void apic_realize(DeviceState *dev, Error **errp) return; } + if (kvm_enabled()) { + warn_report("Userspace local APIC is deprecated for KVM."); + warn_report("Do not use kernel-irqchip except for the -M isapc machine type."); + } + memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, "apic-msi", APIC_SPACE_SIZE); From 9f34101db00eabd8f424e98b481c2394e6509198 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 1 Mar 2021 13:16:07 +0100 Subject: [PATCH 03/23] qga-vss: Use dynamic linking for GLib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current GLib version implements the DllMain function. DllMain is also present in the provider.cpp code. So in the case of static linking, the DllMain redefinition error occurs. For now, just switch to dynamic linking and revert this patch when the issue will be solved. See Glib issue for more details https://gitlab.gnome.org/GNOME/glib/-/issues/692 Signed-off-by: Kostiantyn Kostiuk Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- qga/vss-win32/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build index 780c461432..90825edef3 100644 --- a/qga/vss-win32/meson.build +++ b/qga/vss-win32/meson.build @@ -1,5 +1,5 @@ if add_languages('cpp', required: false) - glib_static = dependency('glib-2.0', static: true) + glib_dynamic = dependency('glib-2.0', static: false) link_args = cc.get_supported_link_arguments(['-fstack-protector-all', '-fstack-protector-strong', '-Wl,--add-stdcall-alias', '-Wl,--enable-stdcall-fixup']) @@ -8,7 +8,7 @@ if add_languages('cpp', required: false) cpp_args: ['-Wno-unknown-pragmas', '-Wno-delete-non-virtual-dtor', '-Wno-non-virtual-dtor'], link_args: link_args, vs_module_defs: 'qga-vss.def', - dependencies: [glib_static, socket, + dependencies: [glib_dynamic, socket, cc.find_library('ole32'), cc.find_library('oleaut32'), cc.find_library('shlwapi'), From a9b1315f86d9323587b340bd3bf83b9d66a55563 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 25 Feb 2021 11:47:52 +0100 Subject: [PATCH 04/23] chardev: add nodelay option The "delay" option was introduced as a way to enable Nagle's algorithm with ",nodelay". Since the short form for boolean options has now been deprecated, introduce a more properly named "nodelay" option. The "delay" option remains as an undocumented option. "delay" and "nodelay" are mutually exclusive. Because the check is done at consumption time, the code also rejects them if one of the two is specified via -set. Based-on: <20210226080526.651705-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- chardev/char-socket.c | 13 +++++++++++-- chardev/char.c | 3 +++ gdbstub.c | 2 +- qemu-options.hx | 14 +++++++------- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 06a37c0cc8..c8bced76b7 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -1472,8 +1472,17 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, sock = backend->u.socket.data = g_new0(ChardevSocket, 1); qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock)); - sock->has_nodelay = qemu_opt_get(opts, "delay"); - sock->nodelay = !qemu_opt_get_bool(opts, "delay", true); + if (qemu_opt_get(opts, "delay") && qemu_opt_get(opts, "nodelay")) { + error_setg(errp, "'delay' and 'nodelay' are mutually exclusive"); + return; + } + sock->has_nodelay = + qemu_opt_get(opts, "delay") || + qemu_opt_get(opts, "nodelay"); + sock->nodelay = + !qemu_opt_get_bool(opts, "delay", true) || + qemu_opt_get_bool(opts, "nodelay", false); + /* * We have different default to QMP for 'server', hence * we can't just check for existence of 'server' diff --git a/chardev/char.c b/chardev/char.c index 288efebd12..97cafd6849 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -867,6 +867,9 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "delay", .type = QEMU_OPT_BOOL, + },{ + .name = "nodelay", + .type = QEMU_OPT_BOOL, },{ .name = "reconnect", .type = QEMU_OPT_NUMBER, diff --git a/gdbstub.c b/gdbstub.c index 3ee40479b6..16d7c8f534 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -3505,7 +3505,7 @@ int gdbserver_start(const char *device) if (strstart(device, "tcp:", NULL)) { /* enforce required TCP attributes */ snprintf(gdbstub_device_name, sizeof(gdbstub_device_name), - "%s,wait=off,delay=off,server=on", device); + "%s,wait=off,nodelay=on,server=on", device); device = gdbstub_device_name; } #ifndef _WIN32 diff --git a/qemu-options.hx b/qemu-options.hx index 252db9357c..90801286c6 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3033,7 +3033,7 @@ DEFHEADING(Character device options:) DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, "-chardev help\n" "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" - "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off][,reconnect=seconds]\n" + "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n" " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n" " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n" "-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n" @@ -3184,7 +3184,7 @@ The available backends are: TCP and unix socket options are given below: - ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off]`` + ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]`` ``host`` for a listening socket specifies the local address to be bound. For a connecting socket species the remote host to connect to. ``host`` is optional for listening sockets. If not @@ -3204,7 +3204,7 @@ The available backends are: or IPv6 must be used. If neither is specified the socket may use either protocol. - ``delay=on|off`` disables the Nagle algorithm. + ``nodelay=on|off`` disables the Nagle algorithm. ``unix options: path=path[,abstract=on|off][,tight=on|off]`` ``path`` specifies the local path of the unix socket. ``path`` @@ -3593,13 +3593,13 @@ SRST ``telnet options:`` localhost 5555 - ``tcp:[host]:port[,server=on|off][,wait=on|off][,delay=on|off][,reconnect=seconds]`` + ``tcp:[host]:port[,server=on|off][,wait=on|off][,nodelay=on|off][,reconnect=seconds]`` The TCP Net Console has two modes of operation. It can send the serial I/O to a location or wait for a connection from a location. By default the TCP Net Console is sent to host at the port. If you use the ``server=on`` option QEMU will wait for a client socket application to connect to the port before continuing, - unless the ``wait=on|off`` option was specified. The ``delay=on|off`` + unless the ``wait=on|off`` option was specified. The ``nodelay=on|off`` option disables the Nagle buffering algorithm. The ``reconnect=on`` option only applies if ``server=no`` is set, if the connection goes down it will attempt to reconnect at the given interval. If host @@ -3616,7 +3616,7 @@ SRST ``Example to not wait and listen on ip 192.168.0.100 port 4444`` -serial tcp:192.168.0.100:4444,server=on,wait=off - ``telnet:host:port[,server=on|off][,wait=on|off][,delay=on|off]`` + ``telnet:host:port[,server=on|off][,wait=on|off][,nodelay=on|off]`` The telnet protocol is used instead of raw tcp sockets. The options work the same as if you had specified ``-serial tcp``. The difference is that the port acts like a telnet server or @@ -3626,7 +3626,7 @@ SRST you do it with Control-] and then type "send break" followed by pressing the enter key. - ``websocket:host:port,server=on[,wait=on|off][,delay=on|off]`` + ``websocket:host:port,server=on[,wait=on|off][,nodelay=on|off]`` The WebSocket protocol is used instead of raw tcp socket. The port acts as a WebSocket server. Client mode is not supported. From 0bd5a2eb7927189c40ca5394079b1c0e88cea7cb Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 2 Mar 2021 18:16:23 +0100 Subject: [PATCH 05/23] qom: Check for wellformed id in user_creatable_add_type() Most code paths for creating a user creatable object go through QemuOpts, which ensures that the provided 'id' option is actually a valid identifier. However, there are some code paths that don't go through QemuOpts: qemu-storage-daemon --object (since commit 8db1efd3) and QMP object-add (since it was first introduced in commit cff8b2c6). We need to have the same validity check for those, too. This adds the check and makes it print the same error message as QemuOpts on failure. Signed-off-by: Kevin Wolf Message-Id: <20210302171623.49709-1-kwolf@redhat.com> Signed-off-by: Paolo Bonzini --- qom/object_interfaces.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 1e9ad6f08a..7661270b98 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -8,6 +8,7 @@ #include "qapi/qobject-input-visitor.h" #include "qom/object_interfaces.h" #include "qemu/help_option.h" +#include "qemu/id.h" #include "qemu/module.h" #include "qemu/option.h" #include "qapi/opts-visitor.h" @@ -41,11 +42,19 @@ Object *user_creatable_add_type(const char *type, const char *id, const QDict *qdict, Visitor *v, Error **errp) { + ERRP_GUARD(); Object *obj; ObjectClass *klass; const QDictEntry *e; Error *local_err = NULL; + if (id != NULL && !id_wellformed(id)) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); + error_append_hint(errp, "Identifiers consist of letters, digits, " + "'-', '.', '_', starting with a letter.\n"); + return NULL; + } + klass = object_class_by_name(type); if (!klass) { error_setg(errp, "invalid object type: %s", type); From 10b6ee1616f984c1889d0851226a9ff14b35ac58 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Tue, 5 Jan 2021 15:14:37 -0300 Subject: [PATCH 06/23] vl.c: do not execute trace_init_backends() before daemonizing Commit v5.2.0-190-g0546c0609c ("vl: split various early command line options to a separate function") moved the trace backend init code to the qemu_process_early_options(). Which is now being called before os_daemonize() via qemu_maybe_daemonize(). Turns out that this change of order causes a problem when executing QEMU in daemon mode and with CONFIG_TRACE_SIMPLE. The trace thread is now being created by the parent, and the parent is left waiting for a trace file flush that was registered via st_init(). The result is that the parent process never exits. To reproduce, fire up a QEMU process with -daemonize and with CONFIG_TRACE_SIMPLE enabled. Two QEMU process will be left in the host: $ sudo ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults \ -nographic -machine none,accel=kvm:tcg -daemonize $ ps axf | grep qemu 529710 pts/3 S+ 0:00 | \_ grep --color=auto qemu 529697 ? Ssl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize 529699 ? Sl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize The parent thread is hang in flush_trace_file: $ sudo gdb ./x86_64-softmmu/qemu-system-x86_64 529697 (..) (gdb) bt #0 0x00007f9dac6a137d in syscall () at /lib64/libc.so.6 #1 0x00007f9dacc3c4f3 in g_cond_wait () at /lib64/libglib-2.0.so.0 #2 0x0000555d12f952da in flush_trace_file (wait=true) at ../trace/simple.c:140 #3 0x0000555d12f95b4c in st_flush_trace_buffer () at ../trace/simple.c:383 #4 0x00007f9dac5e43a7 in __run_exit_handlers () at /lib64/libc.so.6 #5 0x00007f9dac5e4550 in on_exit () at /lib64/libc.so.6 #6 0x0000555d12d454de in os_daemonize () at ../os-posix.c:255 #7 0x0000555d12d0bd5c in qemu_maybe_daemonize (pid_file=0x0) at ../softmmu/vl.c:2408 #8 0x0000555d12d0e566 in qemu_init (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/vl.c:3459 #9 0x0000555d128edac1 in main (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/main.c:49 (gdb) Aside from the 'zombie' process in the host, this is directly impacting Libvirt. Libvirt waits for the parent process to exit to be sure that the QMP monitor is available in the daemonized process to fetch QEMU capabilities, and as is now Libvirt hangs at daemon start waiting for the parent thread to exit. The fix is simple: just move the trace backend related code back to be executed after daemonizing. Fixes: 0546c0609cb5a8d90c1cbac8e0d64b5a048bbb19 Reviewed-by: Paolo Bonzini Signed-off-by: Daniel Henrique Barboza Message-Id: <20210105181437.538366-2-danielhb413@gmail.com> Acked-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- softmmu/vl.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/softmmu/vl.c b/softmmu/vl.c index 10bd8a10a3..7e8110bd6e 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2361,11 +2361,6 @@ static void qemu_process_early_options(void) cleanup_add_fd, NULL, &error_fatal); #endif - if (!trace_init_backends()) { - exit(1); - } - trace_init_file(); - /* Open the logfile at this point and set the log mask if necessary. */ qemu_set_log_filename(log_file, &error_fatal); if (log_mask) { @@ -3475,6 +3470,19 @@ void qemu_init(int argc, char **argv, char **envp) qemu_process_help_options(); qemu_maybe_daemonize(pid_file); + /* + * The trace backend must be initialized after daemonizing. + * trace_init_backends() will call st_init(), which will create the + * trace thread in the parent, and also register st_flush_trace_buffer() + * in atexit(). This function will force the parent to wait for the + * writeout thread to finish, which will not occur, and the parent + * process will be left in the host. + */ + if (!trace_init_backends()) { + exit(1); + } + trace_init_file(); + qemu_init_main_loop(&error_fatal); cpu_timers_init(); From e0a8f99355c32b48c9ef867127075b5267ae23d8 Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Thu, 17 Dec 2020 09:49:40 +0800 Subject: [PATCH 07/23] accel: kvm: Fix memory waste under mismatch page size When handle dirty log, we face qemu_real_host_page_size and TARGET_PAGE_SIZE. The first one is the granule of KVM dirty bitmap, and the second one is the granule of QEMU dirty bitmap. As qemu_real_host_page_size >= TARGET_PAGE_SIZE (kvm_init() enforced it), misuse TARGET_PAGE_SIZE to init kvmslot dirty_bmap may waste memory. For example, when qemu_real_host_page_size is 64K and TARGET_PAGE_SIZE is 4K, it wastes 93.75% (15/16) memory. Signed-off-by: Keqian Zhu Reviewed-by: Andrew Jones Reviewed-by: Peter Xu Message-Id: <20201217014941.22872-2-zhukeqian1@huawei.com> Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 84c943fcdb..ffce83f1a7 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -598,8 +598,12 @@ static void kvm_memslot_init_dirty_bitmap(KVMSlot *mem) * too, in most cases). * So for now, let's align to 64 instead of HOST_LONG_BITS here, in * a hope that sizeof(long) won't become >8 any time soon. + * + * Note: the granule of kvm dirty log is qemu_real_host_page_size. + * And mem->memory_size is aligned to it (otherwise this mem can't + * be registered to KVM). */ - hwaddr bitmap_size = ALIGN(((mem->memory_size) >> TARGET_PAGE_BITS), + hwaddr bitmap_size = ALIGN(mem->memory_size / qemu_real_host_page_size, /*HOST_LONG_BITS*/ 64) / 8; mem->dirty_bmap = g_malloc0(bitmap_size); } From 3920552846e881bafa9f9aad0bb1a6eef874d7fb Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Thu, 17 Dec 2020 09:49:41 +0800 Subject: [PATCH 08/23] accel: kvm: Add aligment assert for kvm_log_clear_one_slot The parameters start and size are transfered from QEMU memory emulation layer. It can promise that they are TARGET_PAGE_SIZE aligned. However, KVM needs they are qemu_real_page_size aligned. Though no caller breaks this aligned requirement currently, we'd better add an explicit assert to avoid future breaking. Signed-off-by: Keqian Zhu Acked-by: Peter Xu Reviewed-by: Andrew Jones Message-Id: <20201217014941.22872-3-zhukeqian1@huawei.com> Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index ffce83f1a7..f88a52393f 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -673,6 +673,10 @@ out: #define KVM_CLEAR_LOG_ALIGN (qemu_real_host_page_size << KVM_CLEAR_LOG_SHIFT) #define KVM_CLEAR_LOG_MASK (-KVM_CLEAR_LOG_ALIGN) +/* + * As the granule of kvm dirty log is qemu_real_host_page_size, + * @start and @size are expected and restricted to align to it. + */ static int kvm_log_clear_one_slot(KVMSlot *mem, int as_id, uint64_t start, uint64_t size) { @@ -682,6 +686,9 @@ static int kvm_log_clear_one_slot(KVMSlot *mem, int as_id, uint64_t start, unsigned long *bmap_clear = NULL, psize = qemu_real_host_page_size; int ret; + /* Make sure start and size are qemu_real_host_page_size aligned */ + assert(QEMU_IS_ALIGNED(start | size, psize)); + /* * We need to extend either the start or the size or both to * satisfy the KVM interface requirement. Firstly, do the start From faabca42cc4ff51110116dfe44d420c668b4d8d8 Mon Sep 17 00:00:00 2001 From: Peng Liang Date: Tue, 2 Mar 2021 21:30:16 +0800 Subject: [PATCH 09/23] lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize PCI_DEVICE has overwritten DeviceState::unrealize (pci_qdev_unrealize). However, LSI53C895A, which is a subclass of PCI_DEVICE, overwrites it again and doesn't save the parent's implementation so the PCI_DEVICE's implementation of DeviceState::unrealize will never be called when unrealize a LSI53C895A device. And it will lead to memory leak and unplug failure. For a PCI device, it's better to implement PCIDevice::exit instead of DeviceState::unrealize. So let's change to use PCIDevice::exit. Fixes: a8632434c7e9 ("lsi: implement I/O memory space for Memory Move instructions") Cc: qemu-stable@nongnu.org Signed-off-by: Peng Liang Message-Id: <20210302133016.1221081-1-liangpeng10@huawei.com> Signed-off-by: Paolo Bonzini --- hw/scsi/lsi53c895a.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index a4e58580e4..e2c19180a0 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -2312,7 +2312,7 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp) scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL); } -static void lsi_scsi_unrealize(DeviceState *dev) +static void lsi_scsi_exit(PCIDevice *dev) { LSIState *s = LSI53C895A(dev); @@ -2325,11 +2325,11 @@ static void lsi_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->realize = lsi_scsi_realize; + k->exit = lsi_scsi_exit; k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC; k->device_id = PCI_DEVICE_ID_LSI_53C895A; k->class_id = PCI_CLASS_STORAGE_SCSI; k->subsystem_id = 0x1000; - dc->unrealize = lsi_scsi_unrealize; dc->reset = lsi_scsi_reset; dc->vmsd = &vmstate_lsi_scsi; set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); From 64d70277114b069579c96e6daf83922b9eacc383 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Tue, 2 Mar 2021 09:03:14 +0000 Subject: [PATCH 10/23] elf_ops: correct loading of 32 bit PVH kernel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because sizeof(struct elf64_note) == sizeof(struct elf32_note), attempting to use the size of the currently defined struct elf_note as a discriminator for whether the object being loaded is 64 bit in load_elf() fails. Instead, take advantage of the existing glue parameter SZ, which is defined as 32 or 64 in the respective variants of load_elf(). Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()") Signed-off-by: David Edmondson Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Message-Id: <20210302090315.3031492-2-david.edmondson@oracle.com> Signed-off-by: Paolo Bonzini --- include/hw/elf_ops.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 8e8436831d..78409ab34a 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -598,9 +598,7 @@ static int glue(load_elf, SZ)(const char *name, int fd, nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align, *(uint64_t *)translate_opaque); if (nhdr != NULL) { - bool is64 = - sizeof(struct elf_note) == sizeof(struct elf64_note); - elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64); + elf_note_fn((void *)nhdr, (void *)&ph->p_align, SZ == 64); } data = NULL; } From e20e182ea0ab5c16557603f457fe0db445b63726 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Tue, 2 Mar 2021 09:03:15 +0000 Subject: [PATCH 11/23] x86/pvh: extract only 4 bytes of start address for 32 bit kernels When loading the PVH start address from a 32 bit ELF note, extract only the appropriate number of bytes. Fixes: ab969087da65 ("pvh: Boot uncompressed kernel using direct boot ABI") Signed-off-by: David Edmondson Reviewed-by: Stefano Garzarella Message-Id: <20210302090315.3031492-3-david.edmondson@oracle.com> Signed-off-by: Paolo Bonzini --- hw/i386/x86.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 6329f90ef9..7865660e2c 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -690,6 +690,8 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) elf_note_data_addr = ((void *)nhdr64) + nhdr_size64 + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); + + pvh_start_addr = *elf_note_data_addr; } else { struct elf32_note *nhdr32 = (struct elf32_note *)arg1; uint32_t nhdr_size32 = sizeof(struct elf32_note); @@ -699,9 +701,9 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) elf_note_data_addr = ((void *)nhdr32) + nhdr_size32 + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); - } - pvh_start_addr = *elf_note_data_addr; + pvh_start_addr = *(uint32_t *)elf_note_data_addr; + } return pvh_start_addr; } From f7544edcd32e602af1aae86714dc7c32350d5d7c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 26 Feb 2021 12:08:16 -0500 Subject: [PATCH 12/23] qemu-config: add error propagation to qemu_config_parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This enables some simplification of vl.c via error_fatal, and improves error messages. Before: $ ./qemu-system-x86_64 -readconfig . qemu-system-x86_64: error reading file qemu-system-x86_64: -readconfig .: read config .: Invalid argument $ /usr/libexec/qemu-kvm -readconfig foo qemu-kvm: -readconfig foo: read config foo: No such file or directory After: $ ./qemu-system-x86_64 -readconfig . qemu-system-x86_64: -readconfig .: Cannot read config file: Is a directory $ ./qemu-system-x86_64 -readconfig foo qemu-system-x86_64: -readconfig foo: Could not open 'foo': No such file or directory Signed-off-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster Message-Id: <20210226170816.231173-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- block/blkdebug.c | 3 +-- include/qemu/config-file.h | 5 +++-- softmmu/vl.c | 29 +++++++++++------------------ util/qemu-config.c | 23 ++++++++++++----------- 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 5fe6172da9..7eaa8a28bf 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -279,9 +279,8 @@ static int read_config(BDRVBlkdebugState *s, const char *filename, return -errno; } - ret = qemu_config_parse(f, config_groups, filename); + ret = qemu_config_parse(f, config_groups, filename, errp); if (ret < 0) { - error_setg(errp, "Could not parse blkdebug config file"); goto fail; } } diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h index 29226107bd..8d3e53ae4d 100644 --- a/include/qemu/config-file.h +++ b/include/qemu/config-file.h @@ -11,9 +11,10 @@ void qemu_add_drive_opts(QemuOptsList *list); int qemu_global_option(const char *str); void qemu_config_write(FILE *fp); -int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname); +int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, + Error **errp); -int qemu_read_config_file(const char *filename); +int qemu_read_config_file(const char *filename, Error **errp); /* Parse QDict options as a replacement for a config file (allowing multiple enumerated (0..(n-1)) configuration "sections") */ diff --git a/softmmu/vl.c b/softmmu/vl.c index 7e8110bd6e..2f4958db2a 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2062,17 +2062,19 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) return 0; } -static int qemu_read_default_config_file(void) +static void qemu_read_default_config_file(Error **errp) { + ERRP_GUARD(); int ret; g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf"); - ret = qemu_read_config_file(file); - if (ret < 0 && ret != -ENOENT) { - return ret; + ret = qemu_read_config_file(file, errp); + if (ret < 0) { + if (ret == -ENOENT) { + error_free(*errp); + *errp = NULL; + } } - - return 0; } static int qemu_set_option(const char *str) @@ -2633,9 +2635,7 @@ void qemu_init(int argc, char **argv, char **envp) } if (userconfig) { - if (qemu_read_default_config_file() < 0) { - exit(1); - } + qemu_read_default_config_file(&error_fatal); } /* second pass of option parsing */ @@ -3323,15 +3323,8 @@ void qemu_init(int argc, char **argv, char **envp) qemu_plugin_opt_parse(optarg, &plugin_list); break; case QEMU_OPTION_readconfig: - { - int ret = qemu_read_config_file(optarg); - if (ret < 0) { - error_report("read config %s: %s", optarg, - strerror(-ret)); - exit(1); - } - break; - } + qemu_read_config_file(optarg, &error_fatal); + break; case QEMU_OPTION_spice: olist = qemu_find_opts_err("spice", NULL); if (!olist) { diff --git a/util/qemu-config.c b/util/qemu-config.c index e2a700b284..670bd6ebca 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -350,7 +350,7 @@ void qemu_config_write(FILE *fp) } /* Returns number of config groups on success, -errno on error */ -int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) +int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp) { char line[1024], group[64], id[64], arg[64], value[1024]; Location loc; @@ -375,7 +375,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) /* group with id */ list = find_list(lists, group, &local_err); if (local_err) { - error_report_err(local_err); + error_propagate(errp, local_err); goto out; } opts = qemu_opts_create(list, id, 1, NULL); @@ -386,7 +386,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) /* group without id */ list = find_list(lists, group, &local_err); if (local_err) { - error_report_err(local_err); + error_propagate(errp, local_err); goto out; } opts = qemu_opts_create(list, NULL, 0, &error_abort); @@ -398,21 +398,21 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) sscanf(line, " %63s = \"\"", arg) == 1) { /* arg = value */ if (opts == NULL) { - error_report("no group defined"); + error_setg(errp, "no group defined"); goto out; } - if (!qemu_opt_set(opts, arg, value, &local_err)) { - error_report_err(local_err); + if (!qemu_opt_set(opts, arg, value, errp)) { goto out; } continue; } - error_report("parse error"); + error_setg(errp, "parse error"); goto out; } if (ferror(fp)) { - error_report("error reading file"); - goto out; + loc_pop(&loc); + error_setg_errno(errp, errno, "Cannot read config file"); + return res; } res = count; out: @@ -420,16 +420,17 @@ out: return res; } -int qemu_read_config_file(const char *filename) +int qemu_read_config_file(const char *filename, Error **errp) { FILE *f = fopen(filename, "r"); int ret; if (f == NULL) { + error_setg_file_open(errp, errno, filename); return -errno; } - ret = qemu_config_parse(f, vm_config_groups, filename); + ret = qemu_config_parse(f, vm_config_groups, filename, errp); fclose(f); return ret; } From 41af878b96582fc8c83303ab8921e40468403702 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 16 Nov 2020 19:40:38 +0100 Subject: [PATCH 13/23] scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes We really should make a distinction between legitimate sense codes (ie if one is running against an emulated block device or for pass-through sense codes), and the intermediate errors generated during processing of the command, which really are not sense codes but refer to some specific internal status. And this internal state is not necessarily linux-specific, but rather can refer to the qemu implementation itself. So rename the linux-only SG_ERR codes to SCSI_HOST codes and make them available generally. Signed-off-by: Hannes Reinecke Message-Id: <20201116184041.60465-5-hare@suse.de> Signed-off-by: Paolo Bonzini --- include/scsi/utils.h | 23 ++++++++++++++++------- scsi/utils.c | 6 +++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/scsi/utils.h b/include/scsi/utils.h index ff7c7091b6..ddb22b56df 100644 --- a/include/scsi/utils.h +++ b/include/scsi/utils.h @@ -16,6 +16,22 @@ enum SCSIXferMode { SCSI_XFER_TO_DEV, /* WRITE, MODE_SELECT, ... */ }; +enum SCSIHostStatus { + SCSI_HOST_OK, + SCSI_HOST_NO_LUN, + SCSI_HOST_BUSY, + SCSI_HOST_TIME_OUT, + SCSI_HOST_BAD_RESPONSE, + SCSI_HOST_ABORTED, + SCSI_HOST_ERROR = 0x07, + SCSI_HOST_RESET = 0x08, + SCSI_HOST_TRANSPORT_DISRUPTED = 0xe, + SCSI_HOST_TARGET_FAILURE = 0x10, + SCSI_HOST_RESERVATION_ERROR = 0x11, + SCSI_HOST_ALLOCATION_FAILURE = 0x12, + SCSI_HOST_MEDIUM_ERROR = 0x13, +}; + typedef struct SCSICommand { uint8_t buf[SCSI_CMD_BUF_SIZE]; int len; @@ -124,13 +140,6 @@ int scsi_cdb_length(uint8_t *buf); #define SG_ERR_DRIVER_TIMEOUT 0x06 #define SG_ERR_DRIVER_SENSE 0x08 -#define SG_ERR_DID_OK 0x00 -#define SG_ERR_DID_NO_CONNECT 0x01 -#define SG_ERR_DID_BUS_BUSY 0x02 -#define SG_ERR_DID_TIME_OUT 0x03 - -#define SG_ERR_DRIVER_SENSE 0x08 - int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, SCSISense *sense); #endif diff --git a/scsi/utils.c b/scsi/utils.c index 6b56e01002..4d994b6d56 100644 --- a/scsi/utils.c +++ b/scsi/utils.c @@ -612,9 +612,9 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, if (errno_value != 0) { return scsi_sense_from_errno(errno_value, sense); } else { - if (io_hdr->host_status == SG_ERR_DID_NO_CONNECT || - io_hdr->host_status == SG_ERR_DID_BUS_BUSY || - io_hdr->host_status == SG_ERR_DID_TIME_OUT || + if (io_hdr->host_status == SCSI_HOST_NO_LUN || + io_hdr->host_status == SCSI_HOST_BUSY || + io_hdr->host_status == SCSI_HOST_TIME_OUT || (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT)) { return BUSY; } else if (io_hdr->host_status) { From db66a15cb80f09da24a5311a3f3b8f0c1835bf71 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 16 Nov 2020 19:40:39 +0100 Subject: [PATCH 14/23] scsi: Add mapping for generic SCSI_HOST status to sense codes As we don't have a driver-specific mapping (yet) we should provide for a detailed mapping from host_status to SCSI sense codes. Signed-off-by: Hannes Reinecke Message-Id: <20201116184041.60465-6-hare@suse.de> Signed-off-by: Paolo Bonzini --- include/scsi/utils.h | 1 + scsi/utils.c | 65 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/include/scsi/utils.h b/include/scsi/utils.h index ddb22b56df..9080d65e27 100644 --- a/include/scsi/utils.h +++ b/include/scsi/utils.h @@ -145,5 +145,6 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, #endif int scsi_sense_from_errno(int errno_value, SCSISense *sense); +int scsi_sense_from_host_status(uint8_t host_status, SCSISense *sense); #endif diff --git a/scsi/utils.c b/scsi/utils.c index 4d994b6d56..28eb32746e 100644 --- a/scsi/utils.c +++ b/scsi/utils.c @@ -257,6 +257,21 @@ const struct SCSISense sense_code_LUN_COMM_FAILURE = { .key = ABORTED_COMMAND, .asc = 0x08, .ascq = 0x00 }; +/* Command aborted, LUN does not respond to selection */ +const struct SCSISense sense_code_LUN_NOT_RESPONDING = { + .key = ABORTED_COMMAND, .asc = 0x05, .ascq = 0x00 +}; + +/* Command aborted, Command Timeout during processing */ +const struct SCSISense sense_code_COMMAND_TIMEOUT = { + .key = ABORTED_COMMAND, .asc = 0x2e, .ascq = 0x02 +}; + +/* Command aborted, Commands cleared by device server */ +const struct SCSISense sense_code_COMMAND_ABORTED = { + .key = ABORTED_COMMAND, .asc = 0x2f, .ascq = 0x02 +}; + /* Medium Error, Unrecovered read error */ const struct SCSISense sense_code_READ_ERROR = { .key = MEDIUM_ERROR, .asc = 0x11, .ascq = 0x00 @@ -605,6 +620,45 @@ int scsi_sense_from_errno(int errno_value, SCSISense *sense) } } +int scsi_sense_from_host_status(uint8_t host_status, + SCSISense *sense) +{ + switch (host_status) { + case SCSI_HOST_NO_LUN: + *sense = SENSE_CODE(LUN_NOT_RESPONDING); + return CHECK_CONDITION; + case SCSI_HOST_BUSY: + return BUSY; + case SCSI_HOST_TIME_OUT: + *sense = SENSE_CODE(COMMAND_TIMEOUT); + return CHECK_CONDITION; + case SCSI_HOST_BAD_RESPONSE: + *sense = SENSE_CODE(LUN_COMM_FAILURE); + return CHECK_CONDITION; + case SCSI_HOST_ABORTED: + *sense = SENSE_CODE(COMMAND_ABORTED); + return CHECK_CONDITION; + case SCSI_HOST_RESET: + *sense = SENSE_CODE(RESET); + return CHECK_CONDITION; + case SCSI_HOST_TRANSPORT_DISRUPTED: + *sense = SENSE_CODE(I_T_NEXUS_LOSS); + return CHECK_CONDITION; + case SCSI_HOST_TARGET_FAILURE: + *sense = SENSE_CODE(TARGET_FAILURE); + return CHECK_CONDITION; + case SCSI_HOST_RESERVATION_ERROR: + return RESERVATION_CONFLICT; + case SCSI_HOST_ALLOCATION_FAILURE: + *sense = SENSE_CODE(SPACE_ALLOC_FAILED); + return CHECK_CONDITION; + case SCSI_HOST_MEDIUM_ERROR: + *sense = SENSE_CODE(READ_ERROR); + return CHECK_CONDITION; + } + return GOOD; +} + #ifdef CONFIG_LINUX int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, SCSISense *sense) @@ -612,14 +666,11 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, if (errno_value != 0) { return scsi_sense_from_errno(errno_value, sense); } else { - if (io_hdr->host_status == SCSI_HOST_NO_LUN || - io_hdr->host_status == SCSI_HOST_BUSY || - io_hdr->host_status == SCSI_HOST_TIME_OUT || - (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT)) { + int status = scsi_sense_from_host_status(io_hdr->host_status, sense); + if (status) { + return status; + } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { return BUSY; - } else if (io_hdr->host_status) { - *sense = SENSE_CODE(I_T_NEXUS_LOSS); - return CHECK_CONDITION; } else if (io_hdr->status) { return io_hdr->status; } else if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) { From 9738c657208800298a7d68272b861fb2dc49fee1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sat, 6 Mar 2021 11:24:12 +0100 Subject: [PATCH 15/23] scsi-generic: do not snoop the output of failed commands If a READ CAPACITY command would fail, for example s->qdev.blocksize would be set to zero and cause a division by zero on the next use. Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-generic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index cf7e11cf44..f9fd2ccfdd 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -288,7 +288,10 @@ static void scsi_read_complete(void * opaque, int ret) } } - if (len == 0) { + if (r->io_header.host_status != SCSI_HOST_OK || + (r->io_header.driver_status & SG_ERR_DRIVER_TIMEOUT) || + r->io_header.status != GOOD || + len == 0) { scsi_command_complete_noio(r, 0); goto done; } From a108557bbff8a3f44233982f015f996426411be8 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 16 Nov 2020 19:40:40 +0100 Subject: [PATCH 16/23] scsi: inline sg_io_sense_from_errno() into the callers. Currently sg_io_sense_from_errno() converts the two input parameters 'errno' and 'io_hdr' into sense code and SCSI status. Having split the function off into scsi_sense_from_errno() and scsi_sense_from_host_status(), both of which are available generically, we now inline the logic in the callers so that scsi-disk and scsi-generic will be able to pass host_status to the HBA. Signed-off-by: Hannes Reinecke Message-Id: <20201116184041.60465-7-hare@suse.de> [Put together from "scsi-disk: Add sg_io callback to evaluate status" and what remains of "scsi: split sg_io_sense_from_errno() in two functions", with many other fixes. - Paolo] Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 47 +++++++++++++++++++++++++++++++++++++----- hw/scsi/scsi-generic.c | 22 ++++++++++++++------ include/scsi/utils.h | 3 --- scsi/qemu-pr-helper.c | 24 ++++++++++++++------- scsi/utils.c | 23 --------------------- 5 files changed, 75 insertions(+), 44 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index a5a58d7db3..ceaf78b423 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -77,7 +77,6 @@ typedef struct SCSIDiskReq { struct iovec iov; QEMUIOVector qiov; BlockAcctCookie acct; - unsigned char *status; } SCSIDiskReq; #define SCSI_DISK_F_REMOVABLE 0 @@ -261,8 +260,6 @@ static bool scsi_disk_req_check_error(SCSIDiskReq *r, int ret, bool acct_failed) if (ret < 0) { return scsi_handle_rw_error(r, ret, acct_failed); - } else if (r->status && *r->status) { - return scsi_handle_rw_error(r, *r->status, acct_failed); } return false; @@ -2697,8 +2694,47 @@ typedef struct SCSIBlockReq { /* CDB passed to SG_IO. */ uint8_t cdb[16]; + BlockCompletionFunc *cb; + void *cb_opaque; } SCSIBlockReq; +static void scsi_block_sgio_complete(void *opaque, int ret) +{ + SCSIBlockReq *req = (SCSIBlockReq *)opaque; + SCSIDiskReq *r = &req->req; + SCSIDevice *s = r->req.dev; + sg_io_hdr_t *io_hdr = &req->io_header; + SCSISense sense; + + if (ret == 0) { + if (io_hdr->host_status != SCSI_HOST_OK) { + ret = scsi_sense_from_host_status(io_hdr->host_status, &sense); + if (ret == CHECK_CONDITION) { + scsi_req_build_sense(&r->req, sense); + } + } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { + ret = BUSY; + } else { + ret = io_hdr->status; + } + + if (ret > 0) { + aio_context_acquire(blk_get_aio_context(s->conf.blk)); + if (scsi_handle_rw_error(r, ret, true)) { + aio_context_release(blk_get_aio_context(s->conf.blk)); + scsi_req_unref(&r->req); + return; + } + aio_context_release(blk_get_aio_context(s->conf.blk)); + + /* Ignore error. */ + ret = 0; + } + } + + req->cb(req->cb_opaque, ret); +} + static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req, int64_t offset, QEMUIOVector *iov, int direction, @@ -2777,9 +2813,11 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req, io_header->timeout = s->qdev.io_timeout * 1000; io_header->usr_ptr = r; io_header->flags |= SG_FLAG_DIRECT_IO; + req->cb = cb; + req->cb_opaque = opaque; trace_scsi_disk_aio_sgio_command(r->req.tag, req->cdb[0], lba, nb_logical_blocks, io_header->timeout); - aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque); + aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, scsi_block_sgio_complete, req); assert(aiocb != NULL); return aiocb; } @@ -2893,7 +2931,6 @@ static int32_t scsi_block_dma_command(SCSIRequest *req, uint8_t *buf) return 0; } - r->req.status = &r->io_header.status; return scsi_disk_dma_command(req, buf); } diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index f9fd2ccfdd..02b87819e5 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -75,6 +75,7 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret) { int status; SCSISense sense; + sg_io_hdr_t *io_hdr = &r->io_header; assert(r->req.aiocb == NULL); @@ -82,15 +83,24 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret) scsi_req_cancel_complete(&r->req); goto done; } - status = sg_io_sense_from_errno(-ret, &r->io_header, &sense); - if (status == CHECK_CONDITION) { - if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) { - r->req.sense_len = r->io_header.sb_len_wr; - } else { + if (ret < 0) { + status = scsi_sense_from_errno(-ret, &sense); + if (status == CHECK_CONDITION) { scsi_req_build_sense(&r->req, sense); } + } else if (io_hdr->host_status != SCSI_HOST_OK) { + status = scsi_sense_from_host_status(io_hdr->host_status, &sense); + if (status == CHECK_CONDITION) { + scsi_req_build_sense(&r->req, sense); + } + } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { + status = BUSY; + } else { + status = io_hdr->status; + if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) { + r->req.sense_len = io_hdr->sb_len_wr; + } } - trace_scsi_generic_command_complete_noio(r, r->req.tag, status); scsi_req_complete(&r->req, status); diff --git a/include/scsi/utils.h b/include/scsi/utils.h index 9080d65e27..d5c8efa16e 100644 --- a/include/scsi/utils.h +++ b/include/scsi/utils.h @@ -139,9 +139,6 @@ int scsi_cdb_length(uint8_t *buf); #ifdef CONFIG_LINUX #define SG_ERR_DRIVER_TIMEOUT 0x06 #define SG_ERR_DRIVER_SENSE 0x08 - -int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, - SCSISense *sense); #endif int scsi_sense_from_errno(int errno_value, SCSISense *sense); diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index 2733d92f2d..7b9389b47b 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -149,19 +149,29 @@ static int do_sgio_worker(void *opaque) io_hdr.dxferp = (char *)data->buf; io_hdr.dxfer_len = data->sz; ret = ioctl(data->fd, SG_IO, &io_hdr); - status = sg_io_sense_from_errno(ret < 0 ? errno : 0, &io_hdr, - &sense_code); + + if (ret < 0) { + status = scsi_sense_from_errno(errno, &sense_code); + if (status == CHECK_CONDITION) { + scsi_build_sense(data->sense, sense_code); + } + } else if (io_hdr.host_status != SCSI_HOST_OK) { + status = scsi_sense_from_host_status(io_hdr.host_status, &sense_code); + if (status == CHECK_CONDITION) { + scsi_build_sense(data->sense, sense_code); + } + } else if (io_hdr.driver_status & SG_ERR_DRIVER_TIMEOUT) { + status = BUSY; + } else { + status = io_hdr.status; + } + if (status == GOOD) { data->sz -= io_hdr.resid; } else { data->sz = 0; } - if (status == CHECK_CONDITION && - !(io_hdr.driver_status & SG_ERR_DRIVER_SENSE)) { - scsi_build_sense(data->sense, sense_code); - } - return status; } diff --git a/scsi/utils.c b/scsi/utils.c index 28eb32746e..873e05aeaf 100644 --- a/scsi/utils.c +++ b/scsi/utils.c @@ -658,26 +658,3 @@ int scsi_sense_from_host_status(uint8_t host_status, } return GOOD; } - -#ifdef CONFIG_LINUX -int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr, - SCSISense *sense) -{ - if (errno_value != 0) { - return scsi_sense_from_errno(errno_value, sense); - } else { - int status = scsi_sense_from_host_status(io_hdr->host_status, sense); - if (status) { - return status; - } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { - return BUSY; - } else if (io_hdr->status) { - return io_hdr->status; - } else if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) { - return CHECK_CONDITION; - } else { - return GOOD; - } - } -} -#endif From f3126d65b393c015e8f87763fdccee99bb1119af Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 24 Feb 2021 19:14:50 +0100 Subject: [PATCH 17/23] scsi: move host_status handling into SCSI drivers Some SCSI drivers like virtio have an internal mapping for the host_status. This patch moves the host_status translation into the SCSI drivers to allow those drivers to set up the correct values. Signed-off-by: Hannes Reinecke . [Added default handling to avoid touching all drivers. - Paolo] Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 33 ++++++++++++++++++++++++++++-- hw/scsi/scsi-disk.c | 12 +++++------ hw/scsi/scsi-generic.c | 6 ++---- hw/scsi/virtio-scsi.c | 46 ++++++++++++++++++++++++++++++++++++++++++ hw/scsi/vmw_pvscsi.c | 39 +++++++++++++++++++++++++++++++++++ include/hw/scsi/scsi.h | 5 ++++- 6 files changed, 128 insertions(+), 13 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index dc4141ec8d..2d674f94d7 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -692,6 +692,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d, req->lun = lun; req->hba_private = hba_private; req->status = -1; + req->host_status = -1; req->ops = reqops; object_ref(OBJECT(d)); object_ref(OBJECT(qbus->parent)); @@ -1455,10 +1456,38 @@ void scsi_req_print(SCSIRequest *req) } } +void scsi_req_complete_failed(SCSIRequest *req, int host_status) +{ + SCSISense sense; + int status; + + assert(req->status == -1 && req->host_status == -1); + assert(req->ops != &reqops_unit_attention); + + if (!req->bus->info->fail) { + status = scsi_sense_from_host_status(req->host_status, &sense); + if (status == CHECK_CONDITION) { + scsi_req_build_sense(req, sense); + } + scsi_req_complete(req, status); + return; + } + + req->host_status = host_status; + scsi_req_ref(req); + scsi_req_dequeue(req); + req->bus->info->fail(req); + + /* Cancelled requests might end up being completed instead of cancelled */ + notifier_list_notify(&req->cancel_notifiers, req); + scsi_req_unref(req); +} + void scsi_req_complete(SCSIRequest *req, int status) { - assert(req->status == -1); + assert(req->status == -1 && req->host_status == -1); req->status = status; + req->host_status = SCSI_HOST_OK; assert(req->sense_len <= sizeof(req->sense)); if (status == GOOD) { @@ -1646,7 +1675,7 @@ static int put_scsi_requests(QEMUFile *f, void *pv, size_t size, QTAILQ_FOREACH(req, &s->requests, next) { assert(!req->io_canceled); - assert(req->status == -1); + assert(req->status == -1 && req->host_status == -1); assert(req->enqueued); qemu_put_sbyte(f, req->retry ? 1 : 2); diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index ceaf78b423..bd7103cd0e 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2704,15 +2704,15 @@ static void scsi_block_sgio_complete(void *opaque, int ret) SCSIDiskReq *r = &req->req; SCSIDevice *s = r->req.dev; sg_io_hdr_t *io_hdr = &req->io_header; - SCSISense sense; if (ret == 0) { if (io_hdr->host_status != SCSI_HOST_OK) { - ret = scsi_sense_from_host_status(io_hdr->host_status, &sense); - if (ret == CHECK_CONDITION) { - scsi_req_build_sense(&r->req, sense); - } - } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { + scsi_req_complete_failed(&r->req, io_hdr->host_status); + scsi_req_unref(&r->req); + return; + } + + if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { ret = BUSY; } else { ret = io_hdr->status; diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 02b87819e5..98c30c5d5c 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -89,10 +89,8 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret) scsi_req_build_sense(&r->req, sense); } } else if (io_hdr->host_status != SCSI_HOST_OK) { - status = scsi_sense_from_host_status(io_hdr->host_status, &sense); - if (status == CHECK_CONDITION) { - scsi_req_build_sense(&r->req, sense); - } + scsi_req_complete_failed(&r->req, io_hdr->host_status); + goto done; } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) { status = BUSY; } else { diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 358c0e70b0..6d80730287 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -500,6 +500,51 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req) virtio_scsi_complete_req(req); } +static void virtio_scsi_command_failed(SCSIRequest *r) +{ + VirtIOSCSIReq *req = r->hba_private; + + if (r->io_canceled) { + return; + } + + req->resp.cmd.status = GOOD; + switch (r->host_status) { + case SCSI_HOST_NO_LUN: + req->resp.cmd.response = VIRTIO_SCSI_S_INCORRECT_LUN; + break; + case SCSI_HOST_BUSY: + req->resp.cmd.response = VIRTIO_SCSI_S_BUSY; + break; + case SCSI_HOST_TIME_OUT: + case SCSI_HOST_ABORTED: + req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED; + break; + case SCSI_HOST_BAD_RESPONSE: + req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET; + break; + case SCSI_HOST_RESET: + req->resp.cmd.response = VIRTIO_SCSI_S_RESET; + break; + case SCSI_HOST_TRANSPORT_DISRUPTED: + req->resp.cmd.response = VIRTIO_SCSI_S_TRANSPORT_FAILURE; + break; + case SCSI_HOST_TARGET_FAILURE: + req->resp.cmd.response = VIRTIO_SCSI_S_TARGET_FAILURE; + break; + case SCSI_HOST_RESERVATION_ERROR: + req->resp.cmd.response = VIRTIO_SCSI_S_NEXUS_FAILURE; + break; + case SCSI_HOST_ALLOCATION_FAILURE: + case SCSI_HOST_MEDIUM_ERROR: + case SCSI_HOST_ERROR: + default: + req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE; + break; + } + virtio_scsi_complete_cmd_req(req); +} + static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid) { VirtIOSCSIReq *req = r->hba_private; @@ -908,6 +953,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .max_lun = VIRTIO_SCSI_MAX_LUN, .complete = virtio_scsi_command_complete, + .fail = virtio_scsi_command_failed, .cancel = virtio_scsi_request_cancelled, .change = virtio_scsi_change, .parse_cdb = virtio_scsi_parse_cdb, diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c index 0da378ed50..1f30cb020a 100644 --- a/hw/scsi/vmw_pvscsi.c +++ b/hw/scsi/vmw_pvscsi.c @@ -510,6 +510,44 @@ pvscsi_write_sense(PVSCSIRequest *r, uint8_t *sense, int len) cpu_physical_memory_write(r->req.senseAddr, sense, r->cmp.senseLen); } +static void +pvscsi_command_failed(SCSIRequest *req) +{ + PVSCSIRequest *pvscsi_req = req->hba_private; + PVSCSIState *s; + + if (!pvscsi_req) { + trace_pvscsi_command_complete_not_found(req->tag); + return; + } + s = pvscsi_req->dev; + + switch (req->host_status) { + case SCSI_HOST_NO_LUN: + pvscsi_req->cmp.hostStatus = BTSTAT_LUNMISMATCH; + break; + case SCSI_HOST_BUSY: + pvscsi_req->cmp.hostStatus = BTSTAT_ABORTQUEUE; + break; + case SCSI_HOST_TIME_OUT: + case SCSI_HOST_ABORTED: + pvscsi_req->cmp.hostStatus = BTSTAT_SENTRST; + break; + case SCSI_HOST_BAD_RESPONSE: + pvscsi_req->cmp.hostStatus = BTSTAT_SELTIMEO; + break; + case SCSI_HOST_RESET: + pvscsi_req->cmp.hostStatus = BTSTAT_BUSRESET; + break; + default: + pvscsi_req->cmp.hostStatus = BTSTAT_HASOFTWARE; + break; + } + pvscsi_req->cmp.scsiStatus = GOOD; + qemu_sglist_destroy(&pvscsi_req->sgl); + pvscsi_complete_request(s, pvscsi_req); +} + static void pvscsi_command_complete(SCSIRequest *req, size_t resid) { @@ -1103,6 +1141,7 @@ static const struct SCSIBusInfo pvscsi_scsi_info = { .get_sg_list = pvscsi_get_sg_list, .complete = pvscsi_command_complete, .cancel = pvscsi_request_cancelled, + .fail = pvscsi_command_failed, }; static void diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 5d992e6e1d..0b726bc78c 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -27,7 +27,8 @@ struct SCSIRequest { uint32_t refcount; uint32_t tag; uint32_t lun; - uint32_t status; + int16_t status; + int16_t host_status; void *hba_private; size_t resid; SCSICommand cmd; @@ -123,6 +124,7 @@ struct SCSIBusInfo { int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, void *hba_private); void (*transfer_data)(SCSIRequest *req, uint32_t arg); + void (*fail)(SCSIRequest *req); void (*complete)(SCSIRequest *req, size_t resid); void (*cancel)(SCSIRequest *req); void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense); @@ -177,6 +179,7 @@ void scsi_req_print(SCSIRequest *req); void scsi_req_continue(SCSIRequest *req); void scsi_req_data(SCSIRequest *req, int len); void scsi_req_complete(SCSIRequest *req, int status); +void scsi_req_complete_failed(SCSIRequest *req, int host_status); uint8_t *scsi_req_get_buf(SCSIRequest *req); int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len); void scsi_req_cancel_complete(SCSIRequest *req); From fe636424caabb4e8b5b96d8a994f58e321bd71d9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 25 Feb 2021 11:51:30 +0100 Subject: [PATCH 18/23] qemu-option: do not suggest using the delay option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "delay" option was a hack that was introduced to allow writing "nodelay". We are adding a "nodelay" option to be used as "nodelay=on", so recommend it instead of "delay". This is quite ugly, but a proper deprecation of "delay" cannot be done if QEMU starts suggesting it. Since it's the only case I opted for this very much ad-hoc patch. Reviewed-by: Daniel P. Berrangé Signed-off-by: Paolo Bonzini --- docs/system/deprecated.rst | 6 ++++++ util/qemu-option.c | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index fcf0ca4068..cfabe69846 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -134,6 +134,12 @@ Boolean options such as ``share=on``/``share=off`` could be written in short form as ``share`` and ``noshare``. This is now deprecated and will cause a warning. +``delay`` option for socket character devices (since 6.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on`` +rather than ``delay=off``. + ``--enable-fips`` (since 6.0) ''''''''''''''''''''''''''''' diff --git a/util/qemu-option.c b/util/qemu-option.c index 40564a12eb..9678d5b682 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -785,7 +785,11 @@ static const char *get_opt_name_value(const char *params, } if (!is_help && warn_on_flag) { warn_report("short-form boolean option '%s%s' deprecated", prefix, *name); - error_printf("Please use %s=%s instead\n", *name, *value); + if (g_str_equal(*name, "delay")) { + error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off"); + } else { + error_printf("Please use %s=%s instead\n", *name, *value); + } } } } else { From ff012d9a52ea2ee9223ad5c78d19c0c6b6898690 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 15 Feb 2021 13:21:03 +0100 Subject: [PATCH 19/23] build-sys: invoke ninja with -d keepdepfile After reading the dependency file, ninja just deletes it, in the name of cleanliness I guess. However this complicates debugging unnecessarily compared to good old "-include *.d". Use the keepdepfile debugging option to make it easier to see what is going on. Signed-off-by: Paolo Bonzini Message-Id: <20210215122103.63933-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d7fb6b270e..bcbbec71a1 100644 --- a/Makefile +++ b/Makefile @@ -149,7 +149,7 @@ $(ninja-targets): run-ninja # --output-sync line. run-ninja: config-host.mak ifneq ($(filter $(ninja-targets), $(ninja-cmd-goals)),) - +$(quiet-@)$(if $(MAKE.nq),@:, $(NINJA) \ + +$(quiet-@)$(if $(MAKE.nq),@:, $(NINJA) -d keepdepfile \ $(NINJAFLAGS) $(sort $(filter $(ninja-targets), $(ninja-cmd-goals))) | cat) endif endif From dc1d91ac567c49cf07d8312c97b4a02e25047d50 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 11 Feb 2021 05:48:52 -0500 Subject: [PATCH 20/23] meson: adjust timeouts for some slower tests Adjust the timeouts for the benchmarks (Meson 0.57 allows 0 to mean infinite) and for the longest running tests. These are the times that I measured and the corresponding timeouts. For generic qtests, the target that reported the longest runtime is included. unit tests: test-crypto-tlscredsx509 13.15s 45s test-crypto-tlssession 14.12s 45s qtests: qos-test 21.26s 60s (i386) ahci-test 22.18s 60s pxe-test 26.51s 60s boot-serial-test 28.02s 60s (sparc) prom-env-test 28.86s 60s bios-tables-test 50.17s 120s (aarch64) test-hmp 57.15s 120s (aarch64) npcm7xx_pwm-test 71.27s 150s migration-test 97.09s 150s (aarch64) qom-test 139.20s 240s (aarch64) Signed-off-by: Paolo Bonzini --- tests/fp/meson.build | 2 +- tests/meson.build | 8 ++++++++ tests/qtest/meson.build | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/fp/meson.build b/tests/fp/meson.build index 8d739c4d59..1c3eee9955 100644 --- a/tests/fp/meson.build +++ b/tests/fp/meson.build @@ -624,7 +624,7 @@ test('fp-test-mulAdd', fptest, # no fptest_rounding_args args: fptest_args + ['f16_mulAdd', 'f32_mulAdd', 'f64_mulAdd', 'f128_mulAdd'], - suite: ['softfloat-slow', 'softfloat-ops-slow'], timeout: 60) + suite: ['softfloat-slow', 'softfloat-ops-slow'], timeout: 90) fpbench = executable( 'fp-bench', diff --git a/tests/meson.build b/tests/meson.build index 7d7da6a636..656d211e25 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -237,6 +237,11 @@ test_env = environment() test_env.set('G_TEST_SRCDIR', meson.current_source_dir()) test_env.set('G_TEST_BUILDDIR', meson.current_build_dir()) +slow_tests = { + 'test-crypto-tlscredsx509': 45, + 'test-crypto-tlssession': 45 +} + foreach test_name, extra: tests src = [test_name + '.c'] deps = [qemuutil] @@ -254,6 +259,8 @@ foreach test_name, extra: tests env: test_env, args: ['--tap', '-k'], protocol: 'tap', + timeout: slow_tests.get(test_name, 30), + priority: slow_tests.get(test_name, 30), suite: ['unit']) endforeach @@ -263,6 +270,7 @@ foreach bench_name, deps: benchs benchmark(bench_name, exe, args: ['--tap', '-k'], protocol: 'tap', + timeout: 0, suite: ['speed']) endforeach diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index ba6ecaed32..8c79d5cc3b 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -4,6 +4,19 @@ if not config_host.has_key('CONFIG_POSIX') subdir_done() endif +slow_qtests = { + 'ahci-test' : 60, + 'bios-tables-test' : 120, + 'boot-serial-test' : 60, + 'migration-test' : 150, + 'npcm7xx_pwm-test': 150, + 'prom-env-test' : 60, + 'pxe-test' : 60, + 'qos-test' : 60, + 'qom-test' : 300, + 'test-hmp' : 120, +} + qtests_generic = [ 'cdrom-test', 'device-introspect-test', @@ -273,6 +286,8 @@ foreach dir : target_dirs env: qtest_env, args: ['--tap', '-k'], protocol: 'tap', + timeout: slow_qtests.get(test, 30), + priority: slow_qtests.get(test, 30), suite: ['qtest', 'qtest-' + target_base]) endforeach endforeach From 9f45a641097b0a54c673fe3399c7a8ccb6f06af1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 9 Feb 2021 15:57:58 +0100 Subject: [PATCH 21/23] trace: fix "-trace file=..." Because trace_opt_parse always deletes the options it has parsed, trace_init_file's call to qemu_find_opts_singleton always creates an empty -trace option group. Therefore, the subsequent qemu_opt_get(opts, "file") always returns NULL. To fix this, save the last "-trace file=..." option in a global variable and use it later in trace_init_file. This is similar to what was done before commit 92eecfff32 ("trace: remove argument from trace_init_file", 2020-11-11), except contained within trace/control.c and without memory leaks. Fixes: 92eecfff32 ("trace: remove argument from trace_init_file", 2020-11-11) Cc: stefanha@redhat.com Reported-by: armbru@redhat.com Signed-off-by: Paolo Bonzini Message-Id: <20210209145759.141231-2-pbonzini@redhat.com> --- trace/control.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/trace/control.c b/trace/control.c index cd04dd4e0c..4be38e1af2 100644 --- a/trace/control.c +++ b/trace/control.c @@ -40,6 +40,7 @@ static size_t nevent_groups; static uint32_t next_id; static uint32_t next_vcpu_id; static bool init_trace_on_startup; +static char *trace_opts_file; QemuOptsList qemu_trace_opts = { .name = "trace", @@ -224,10 +225,8 @@ static void trace_init_events(const char *fname) void trace_init_file(void) { - QemuOpts *opts = qemu_find_opts_singleton("trace"); - const char *file = qemu_opt_get(opts, "file"); #ifdef CONFIG_TRACE_SIMPLE - st_set_trace_file(file); + st_set_trace_file(trace_opts_file); if (init_trace_on_startup) { st_set_trace_file_enabled(true); } @@ -238,11 +237,11 @@ void trace_init_file(void) * backend. However we should only override -D if we actually have * something to override it with. */ - if (file) { - qemu_set_log_filename(file, &error_fatal); + if (trace_opts_file) { + qemu_set_log_filename(trace_opts_file, &error_fatal); } #else - if (file) { + if (trace_opts_file) { fprintf(stderr, "error: --trace file=...: " "option not supported by the selected tracing backends\n"); exit(1); @@ -303,6 +302,8 @@ void trace_opt_parse(const char *optarg) } trace_init_events(qemu_opt_get(opts, "events")); init_trace_on_startup = true; + g_free(trace_opts_file); + trace_opts_file = g_strdup(qemu_opt_get(opts, "file")); qemu_opts_del(opts); } From 7520c4f0847093aefa87f23113f28d5d1d574aed Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 9 Feb 2021 15:57:59 +0100 Subject: [PATCH 22/23] trace: skip qemu_set_log_filename if no "-D" option was passed When the "simple" backend is not active but the "log" backend is, both "-trace file=" and "-D" will result in a call to qemu_set_log_filename. Unfortunately, QEMU was also calling qemu_set_log_filename if "-D" was not passed, so the "-trace file=" option had no effect and the tracepoints went back to stderr. Fortunately we can just skip qemu_set_log_filename in that case, because the log backend will initialize itself just fine as soon as qemu_set_log is called, also in qemu_process_early_options. Cc: stefanha@redhat.com Signed-off-by: Paolo Bonzini Message-Id: <20210209145759.141231-3-pbonzini@redhat.com> --- softmmu/vl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/softmmu/vl.c b/softmmu/vl.c index 2f4958db2a..ff488ea3e7 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2364,7 +2364,9 @@ static void qemu_process_early_options(void) #endif /* Open the logfile at this point and set the log mask if necessary. */ - qemu_set_log_filename(log_file, &error_fatal); + if (log_file) { + qemu_set_log_filename(log_file, &error_fatal); + } if (log_mask) { int mask; mask = qemu_str_to_log_mask(log_mask); From c715343fd96bcf93263fda38d81af815fdb5a7fa Mon Sep 17 00:00:00 2001 From: Daniele Buono Date: Wed, 3 Mar 2021 21:59:38 -0500 Subject: [PATCH 23/23] meson: Stop if cfi is enabled with system slirp For CFI, we need to compile slirp as a static library together with qemu. This is because we register slirp functions as callbacks for QEMU Timers. When using a system-wide shared libslirp, the type information for the callback is missing and the timer call produces a false positive with CFI. With this patch, meson will stop if CFI is enabled with system-wide slirp. In 6.1 we will introduce a new interface to slirp where the callback is passed as an enum rather than a function pointer. Signed-off-by: Daniele Buono Message-Id: <20210304025939.9164-1-dbuono@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini --- meson.build | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/meson.build b/meson.build index 07bc23129a..05fb125dc2 100644 --- a/meson.build +++ b/meson.build @@ -1574,6 +1574,18 @@ if have_system endif endif +# For CFI, we need to compile slirp as a static library together with qemu. +# This is because we register slirp functions as callbacks for QEMU Timers. +# When using a system-wide shared libslirp, the type information for the +# callback is missing and the timer call produces a false positive with CFI. +# +# Now that slirp_opt has been defined, check if the selected slirp is compatible +# with control-flow integrity. +if get_option('cfi') and slirp_opt == 'system' + error('Control-Flow Integrity is not compatible with system-wide slirp.' \ + + ' Please configure with --enable-slirp=git') +endif + fdt = not_found fdt_opt = get_option('fdt') if have_system