From acedc9a660f83b362a1dec4b699e85d5dd82a067 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sat, 7 Jan 2023 14:32:41 +0100 Subject: [PATCH 01/29] configure: fix GLIB_VERSION for cross-compilation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit configure uses "pkg-config" directly so that GLIB_VERSION is always based on host glib version. To correctly handle cross-compilation it should use "$pkg_config" and take GLIB_VERSION from the cross-compiled glib. Reported-by: Валентин Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1414 Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 2281892657..6f5e77a713 100755 --- a/configure +++ b/configure @@ -2375,7 +2375,7 @@ echo "QEMU_OBJCFLAGS=$QEMU_OBJCFLAGS" >> $config_host_mak echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak echo "GLIB_LIBS=$glib_libs" >> $config_host_mak echo "GLIB_BINDIR=$glib_bindir" >> $config_host_mak -echo "GLIB_VERSION=$(pkg-config --modversion glib-2.0)" >> $config_host_mak +echo "GLIB_VERSION=$($pkg_config --modversion glib-2.0)" >> $config_host_mak echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak echo "EXESUF=$EXESUF" >> $config_host_mak From b585edca34a817fdb751dfe94fbd3cde32ffe60d Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Sat, 24 Dec 2022 16:16:04 -0800 Subject: [PATCH 02/29] i386: Emit correct error code for 64-bit IDT entry When in 64-bit mode, IDT entiries are 16 bytes, so `intno * 16` is used for base/limit/offset calculations. However, even in 64-bit mode, the exception error code still uses bits [3,16) for the invlaid interrupt index. This means the error code should still be `intno * 8 + 2` even in 64-bit mode. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1382 Signed-off-by: Joe Richey Signed-off-by: Paolo Bonzini --- target/i386/tcg/seg_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 539189b4d1..03b58e94a2 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -882,7 +882,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int, dt = &env->idt; if (intno * 16 + 15 > dt->limit) { - raise_exception_err(env, EXCP0D_GPF, intno * 16 + 2); + raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); } ptr = dt->base + intno * 16; e1 = cpu_ldl_kernel(env, ptr); @@ -895,18 +895,18 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int, case 15: /* 386 trap gate */ break; default: - raise_exception_err(env, EXCP0D_GPF, intno * 16 + 2); + raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); break; } dpl = (e2 >> DESC_DPL_SHIFT) & 3; cpl = env->hflags & HF_CPL_MASK; /* check privilege if software int */ if (is_int && dpl < cpl) { - raise_exception_err(env, EXCP0D_GPF, intno * 16 + 2); + raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); } /* check valid bit */ if (!(e2 & DESC_P_MASK)) { - raise_exception_err(env, EXCP0B_NOSEG, intno * 16 + 2); + raise_exception_err(env, EXCP0B_NOSEG, intno * 8 + 2); } selector = e1 >> 16; offset = ((target_ulong)e3 << 32) | (e2 & 0xffff0000) | (e1 & 0x0000ffff); From bd688fc93120fb3e28aa70e3dfdf567ccc1e0bc1 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Fri, 11 Nov 2022 10:47:56 -0500 Subject: [PATCH 03/29] accel: introduce accelerator blocker API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This API allows the accelerators to prevent vcpus from issuing new ioctls while execting a critical section marked with the accel_ioctl_inhibit_begin/end functions. Note that all functions submitting ioctls must mark where the ioctl is being called with accel_{cpu_}ioctl_begin/end(). This API requires the caller to always hold the BQL. API documentation is in sysemu/accel-blocker.h Internally, it uses a QemuLockCnt together with a per-CPU QemuLockCnt (to minimize cache line bouncing) to keep avoid that new ioctls run when the critical section starts, and a QemuEvent to wait that all running ioctls finish. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221111154758.1372674-2-eesposit@redhat.com> Signed-off-by: Paolo Bonzini --- accel/accel-blocker.c | 154 +++++++++++++++++++++++++++++++++ accel/meson.build | 2 +- hw/core/cpu-common.c | 2 + include/hw/core/cpu.h | 3 + include/sysemu/accel-blocker.h | 56 ++++++++++++ util/meson.build | 2 +- 6 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 accel/accel-blocker.c create mode 100644 include/sysemu/accel-blocker.h diff --git a/accel/accel-blocker.c b/accel/accel-blocker.c new file mode 100644 index 0000000000..1e7f423462 --- /dev/null +++ b/accel/accel-blocker.c @@ -0,0 +1,154 @@ +/* + * Lock to inhibit accelerator ioctls + * + * Copyright (c) 2022 Red Hat Inc. + * + * Author: Emanuele Giuseppe Esposito + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qemu/thread.h" +#include "qemu/main-loop.h" +#include "hw/core/cpu.h" +#include "sysemu/accel-blocker.h" + +static QemuLockCnt accel_in_ioctl_lock; +static QemuEvent accel_in_ioctl_event; + +void accel_blocker_init(void) +{ + qemu_lockcnt_init(&accel_in_ioctl_lock); + qemu_event_init(&accel_in_ioctl_event, false); +} + +void accel_ioctl_begin(void) +{ + if (likely(qemu_mutex_iothread_locked())) { + return; + } + + /* block if lock is taken in kvm_ioctl_inhibit_begin() */ + qemu_lockcnt_inc(&accel_in_ioctl_lock); +} + +void accel_ioctl_end(void) +{ + if (likely(qemu_mutex_iothread_locked())) { + return; + } + + qemu_lockcnt_dec(&accel_in_ioctl_lock); + /* change event to SET. If event was BUSY, wake up all waiters */ + qemu_event_set(&accel_in_ioctl_event); +} + +void accel_cpu_ioctl_begin(CPUState *cpu) +{ + if (unlikely(qemu_mutex_iothread_locked())) { + return; + } + + /* block if lock is taken in kvm_ioctl_inhibit_begin() */ + qemu_lockcnt_inc(&cpu->in_ioctl_lock); +} + +void accel_cpu_ioctl_end(CPUState *cpu) +{ + if (unlikely(qemu_mutex_iothread_locked())) { + return; + } + + qemu_lockcnt_dec(&cpu->in_ioctl_lock); + /* change event to SET. If event was BUSY, wake up all waiters */ + qemu_event_set(&accel_in_ioctl_event); +} + +static bool accel_has_to_wait(void) +{ + CPUState *cpu; + bool needs_to_wait = false; + + CPU_FOREACH(cpu) { + if (qemu_lockcnt_count(&cpu->in_ioctl_lock)) { + /* exit the ioctl, if vcpu is running it */ + qemu_cpu_kick(cpu); + needs_to_wait = true; + } + } + + return needs_to_wait || qemu_lockcnt_count(&accel_in_ioctl_lock); +} + +void accel_ioctl_inhibit_begin(void) +{ + CPUState *cpu; + + /* + * We allow to inhibit only when holding the BQL, so we can identify + * when an inhibitor wants to issue an ioctl easily. + */ + g_assert(qemu_mutex_iothread_locked()); + + /* Block further invocations of the ioctls outside the BQL. */ + CPU_FOREACH(cpu) { + qemu_lockcnt_lock(&cpu->in_ioctl_lock); + } + qemu_lockcnt_lock(&accel_in_ioctl_lock); + + /* Keep waiting until there are running ioctls */ + while (true) { + + /* Reset event to FREE. */ + qemu_event_reset(&accel_in_ioctl_event); + + if (accel_has_to_wait()) { + /* + * If event is still FREE, and there are ioctls still in progress, + * wait. + * + * If an ioctl finishes before qemu_event_wait(), it will change + * the event state to SET. This will prevent qemu_event_wait() from + * blocking, but it's not a problem because if other ioctls are + * still running the loop will iterate once more and reset the event + * status to FREE so that it can wait properly. + * + * If an ioctls finishes while qemu_event_wait() is blocking, then + * it will be waken up, but also here the while loop makes sure + * to re-enter the wait if there are other running ioctls. + */ + qemu_event_wait(&accel_in_ioctl_event); + } else { + /* No ioctl is running */ + return; + } + } +} + +void accel_ioctl_inhibit_end(void) +{ + CPUState *cpu; + + qemu_lockcnt_unlock(&accel_in_ioctl_lock); + CPU_FOREACH(cpu) { + qemu_lockcnt_unlock(&cpu->in_ioctl_lock); + } +} + diff --git a/accel/meson.build b/accel/meson.build index 3a480cc2ef..49558dd232 100644 --- a/accel/meson.build +++ b/accel/meson.build @@ -1,4 +1,4 @@ -specific_ss.add(files('accel-common.c')) +specific_ss.add(files('accel-common.c', 'accel-blocker.c')) softmmu_ss.add(files('accel-softmmu.c')) user_ss.add(files('accel-user.c')) diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index b177e761f0..5ccc3837b6 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -238,6 +238,7 @@ static void cpu_common_initfn(Object *obj) cpu->cflags_next_tb = -1; qemu_mutex_init(&cpu->work_mutex); + qemu_lockcnt_init(&cpu->in_ioctl_lock); QSIMPLEQ_INIT(&cpu->work_list); QTAILQ_INIT(&cpu->breakpoints); QTAILQ_INIT(&cpu->watchpoints); @@ -249,6 +250,7 @@ static void cpu_common_finalize(Object *obj) { CPUState *cpu = CPU(obj); + qemu_lockcnt_destroy(&cpu->in_ioctl_lock); qemu_mutex_destroy(&cpu->work_mutex); } diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 8830546121..2417597236 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -398,6 +398,9 @@ struct CPUState { uint32_t kvm_fetch_index; uint64_t dirty_pages; + /* Use by accel-block: CPU is executing an ioctl() */ + QemuLockCnt in_ioctl_lock; + /* Used for events with 'vcpu' and *without* the 'disabled' properties */ DECLARE_BITMAP(trace_dstate_delayed, CPU_TRACE_DSTATE_MAX_EVENTS); DECLARE_BITMAP(trace_dstate, CPU_TRACE_DSTATE_MAX_EVENTS); diff --git a/include/sysemu/accel-blocker.h b/include/sysemu/accel-blocker.h new file mode 100644 index 0000000000..72020529ef --- /dev/null +++ b/include/sysemu/accel-blocker.h @@ -0,0 +1,56 @@ +/* + * Accelerator blocking API, to prevent new ioctls from starting and wait the + * running ones finish. + * This mechanism differs from pause/resume_all_vcpus() in that it does not + * release the BQL. + * + * Copyright (c) 2022 Red Hat Inc. + * + * Author: Emanuele Giuseppe Esposito + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#ifndef ACCEL_BLOCKER_H +#define ACCEL_BLOCKER_H + +#include "qemu/osdep.h" +#include "sysemu/cpus.h" + +extern void accel_blocker_init(void); + +/* + * accel_{cpu_}ioctl_begin/end: + * Mark when ioctl is about to run or just finished. + * + * accel_{cpu_}ioctl_begin will block after accel_ioctl_inhibit_begin() is + * called, preventing new ioctls to run. They will continue only after + * accel_ioctl_inibith_end(). + */ +extern void accel_ioctl_begin(void); +extern void accel_ioctl_end(void); +extern void accel_cpu_ioctl_begin(CPUState *cpu); +extern void accel_cpu_ioctl_end(CPUState *cpu); + +/* + * accel_ioctl_inhibit_begin: start critical section + * + * This function makes sure that: + * 1) incoming accel_{cpu_}ioctl_begin() calls block + * 2) wait that all ioctls that were already running reach + * accel_{cpu_}ioctl_end(), kicking vcpus if necessary. + * + * This allows the caller to access shared data or perform operations without + * worrying of concurrent vcpus accesses. + */ +extern void accel_ioctl_inhibit_begin(void); + +/* + * accel_ioctl_inhibit_end: end critical section started by + * accel_ioctl_inhibit_begin() + * + * This function allows blocked accel_{cpu_}ioctl_begin() to continue. + */ +extern void accel_ioctl_inhibit_end(void); + +#endif /* ACCEL_BLOCKER_H */ diff --git a/util/meson.build b/util/meson.build index d8d109ff84..26c73e586b 100644 --- a/util/meson.build +++ b/util/meson.build @@ -58,6 +58,7 @@ util_ss.add(files('yank.c')) util_ss.add(files('int128.c')) util_ss.add(files('memalign.c')) util_ss.add(files('interval-tree.c')) +util_ss.add(files('lockcnt.c')) if have_user util_ss.add(files('selfmap.c')) @@ -72,7 +73,6 @@ endif if have_block or have_ga util_ss.add(files('aiocb.c', 'async.c')) util_ss.add(files('base64.c')) - util_ss.add(files('lockcnt.c')) util_ss.add(files('main-loop.c')) util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c')) util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND']))) From a27dd2de68f37ba96fe164a42121daa5f0750afc Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Fri, 11 Nov 2022 10:47:57 -0500 Subject: [PATCH 04/29] KVM: keep track of running ioctls Using the new accel-blocker API, mark where ioctls are being called in KVM. Next, we will implement the critical section that will take care of performing memslots modifications atomically, therefore preventing any new ioctl from running and allowing the running ones to finish. Signed-off-by: David Hildenbrand Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20221111154758.1372674-3-eesposit@redhat.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 e86c33e0e6..8760d55002 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -2310,6 +2310,7 @@ static int kvm_init(MachineState *ms) assert(TARGET_PAGE_SIZE <= qemu_real_host_page_size()); s->sigmask_len = 8; + accel_blocker_init(); #ifdef KVM_CAP_SET_GUEST_DEBUG QTAILQ_INIT(&s->kvm_sw_breakpoints); @@ -3014,7 +3015,9 @@ int kvm_vm_ioctl(KVMState *s, int type, ...) va_end(ap); trace_kvm_vm_ioctl(type, arg); + accel_ioctl_begin(); ret = ioctl(s->vmfd, type, arg); + accel_ioctl_end(); if (ret == -1) { ret = -errno; } @@ -3032,7 +3035,9 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...) va_end(ap); trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg); + accel_cpu_ioctl_begin(cpu); ret = ioctl(cpu->kvm_fd, type, arg); + accel_cpu_ioctl_end(cpu); if (ret == -1) { ret = -errno; } @@ -3050,7 +3055,9 @@ int kvm_device_ioctl(int fd, int type, ...) va_end(ap); trace_kvm_device_ioctl(fd, type, arg); + accel_ioctl_begin(); ret = ioctl(fd, type, arg); + accel_ioctl_end(); if (ret == -1) { ret = -errno; } From f39b7d2b96e3e73c01bb678cd096f7baf0b9ab39 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Fri, 11 Nov 2022 10:47:58 -0500 Subject: [PATCH 05/29] kvm: Atomic memslot updates If we update an existing memslot (e.g., resize, split), we temporarily remove the memslot to re-add it immediately afterwards. These updates are not atomic, especially not for KVM VCPU threads, such that we can get spurious faults. Let's inhibit most KVM ioctls while performing relevant updates, such that we can perform the update just as if it would happen atomically without additional kernel support. We capture the add/del changes and apply them in the notifier commit stage instead. There, we can check for overlaps and perform the ioctl inhibiting only if really required (-> overlap). To keep things simple we don't perform additional checks that wouldn't actually result in an overlap -- such as !RAM memory regions in some cases (see kvm_set_phys_mem()). To minimize cache-line bouncing, use a separate indicator (in_ioctl_lock) per CPU. Also, make sure to hold the kvm_slots_lock while performing both actions (removing+re-adding). We have to wait until all IOCTLs were exited and block new ones from getting executed. This approach cannot result in a deadlock as long as the inhibitor does not hold any locks that might hinder an IOCTL from getting finished and exited - something fairly unusual. The inhibitor will always hold the BQL. AFAIKs, one possible candidate would be userfaultfd. If a page cannot be placed (e.g., during postcopy), because we're waiting for a lock, or if the userfaultfd thread cannot process a fault, because it is waiting for a lock, there could be a deadlock. However, the BQL is not applicable here, because any other guest memory access while holding the BQL would already result in a deadlock. Nothing else in the kernel should block forever and wait for userspace intervention. Note: pause_all_vcpus()/resume_all_vcpus() or start_exclusive()/end_exclusive() cannot be used, as they either drop the BQL or require to be called without the BQL - something inhibitors cannot handle. We need a low-level locking mechanism that is deadlock-free even when not releasing the BQL. Signed-off-by: David Hildenbrand Signed-off-by: Emanuele Giuseppe Esposito Tested-by: Emanuele Giuseppe Esposito Message-Id: <20221111154758.1372674-4-eesposit@redhat.com> Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 101 ++++++++++++++++++++++++++++++++++----- include/sysemu/kvm_int.h | 8 ++++ 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 8760d55002..7e6a6076b1 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -31,6 +31,7 @@ #include "sysemu/kvm_int.h" #include "sysemu/runstate.h" #include "sysemu/cpus.h" +#include "sysemu/accel-blocker.h" #include "qemu/bswap.h" #include "exec/memory.h" #include "exec/ram_addr.h" @@ -46,6 +47,7 @@ #include "sysemu/hw_accel.h" #include "kvm-cpus.h" #include "sysemu/dirtylimit.h" +#include "qemu/range.h" #include "hw/boards.h" #include "monitor/stats.h" @@ -1292,6 +1294,7 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size) kvm_max_slot_size = max_slot_size; } +/* Called with KVMMemoryListener.slots_lock held */ static void kvm_set_phys_mem(KVMMemoryListener *kml, MemoryRegionSection *section, bool add) { @@ -1326,14 +1329,12 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml, ram = memory_region_get_ram_ptr(mr) + mr_offset; ram_start_offset = memory_region_get_ram_addr(mr) + mr_offset; - kvm_slots_lock(); - if (!add) { do { slot_size = MIN(kvm_max_slot_size, size); mem = kvm_lookup_matching_slot(kml, start_addr, slot_size); if (!mem) { - goto out; + return; } if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) { /* @@ -1371,7 +1372,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml, start_addr += slot_size; size -= slot_size; } while (size); - goto out; + return; } /* register the new slot */ @@ -1396,9 +1397,6 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml, ram += slot_size; size -= slot_size; } while (size); - -out: - kvm_slots_unlock(); } static void *kvm_dirty_ring_reaper_thread(void *data) @@ -1455,18 +1453,95 @@ static void kvm_region_add(MemoryListener *listener, MemoryRegionSection *section) { KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener); + KVMMemoryUpdate *update; - memory_region_ref(section->mr); - kvm_set_phys_mem(kml, section, true); + update = g_new0(KVMMemoryUpdate, 1); + update->section = *section; + + QSIMPLEQ_INSERT_TAIL(&kml->transaction_add, update, next); } static void kvm_region_del(MemoryListener *listener, MemoryRegionSection *section) { KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener); + KVMMemoryUpdate *update; - kvm_set_phys_mem(kml, section, false); - memory_region_unref(section->mr); + update = g_new0(KVMMemoryUpdate, 1); + update->section = *section; + + QSIMPLEQ_INSERT_TAIL(&kml->transaction_del, update, next); +} + +static void kvm_region_commit(MemoryListener *listener) +{ + KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, + listener); + KVMMemoryUpdate *u1, *u2; + bool need_inhibit = false; + + if (QSIMPLEQ_EMPTY(&kml->transaction_add) && + QSIMPLEQ_EMPTY(&kml->transaction_del)) { + return; + } + + /* + * We have to be careful when regions to add overlap with ranges to remove. + * We have to simulate atomic KVM memslot updates by making sure no ioctl() + * is currently active. + * + * The lists are order by addresses, so it's easy to find overlaps. + */ + u1 = QSIMPLEQ_FIRST(&kml->transaction_del); + u2 = QSIMPLEQ_FIRST(&kml->transaction_add); + while (u1 && u2) { + Range r1, r2; + + range_init_nofail(&r1, u1->section.offset_within_address_space, + int128_get64(u1->section.size)); + range_init_nofail(&r2, u2->section.offset_within_address_space, + int128_get64(u2->section.size)); + + if (range_overlaps_range(&r1, &r2)) { + need_inhibit = true; + break; + } + if (range_lob(&r1) < range_lob(&r2)) { + u1 = QSIMPLEQ_NEXT(u1, next); + } else { + u2 = QSIMPLEQ_NEXT(u2, next); + } + } + + kvm_slots_lock(); + if (need_inhibit) { + accel_ioctl_inhibit_begin(); + } + + /* Remove all memslots before adding the new ones. */ + while (!QSIMPLEQ_EMPTY(&kml->transaction_del)) { + u1 = QSIMPLEQ_FIRST(&kml->transaction_del); + QSIMPLEQ_REMOVE_HEAD(&kml->transaction_del, next); + + kvm_set_phys_mem(kml, &u1->section, false); + memory_region_unref(u1->section.mr); + + g_free(u1); + } + while (!QSIMPLEQ_EMPTY(&kml->transaction_add)) { + u1 = QSIMPLEQ_FIRST(&kml->transaction_add); + QSIMPLEQ_REMOVE_HEAD(&kml->transaction_add, next); + + memory_region_ref(u1->section.mr); + kvm_set_phys_mem(kml, &u1->section, true); + + g_free(u1); + } + + if (need_inhibit) { + accel_ioctl_inhibit_end(); + } + kvm_slots_unlock(); } static void kvm_log_sync(MemoryListener *listener, @@ -1610,8 +1685,12 @@ void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml, kml->slots[i].slot = i; } + QSIMPLEQ_INIT(&kml->transaction_add); + QSIMPLEQ_INIT(&kml->transaction_del); + kml->listener.region_add = kvm_region_add; kml->listener.region_del = kvm_region_del; + kml->listener.commit = kvm_region_commit; kml->listener.log_start = kvm_log_start; kml->listener.log_stop = kvm_log_stop; kml->listener.priority = 10; diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h index 3b4adcdc10..60b520a13e 100644 --- a/include/sysemu/kvm_int.h +++ b/include/sysemu/kvm_int.h @@ -12,6 +12,7 @@ #include "exec/memory.h" #include "qapi/qapi-types-common.h" #include "qemu/accel.h" +#include "qemu/queue.h" #include "sysemu/kvm.h" typedef struct KVMSlot @@ -31,10 +32,17 @@ typedef struct KVMSlot ram_addr_t ram_start_offset; } KVMSlot; +typedef struct KVMMemoryUpdate { + QSIMPLEQ_ENTRY(KVMMemoryUpdate) next; + MemoryRegionSection section; +} KVMMemoryUpdate; + typedef struct KVMMemoryListener { MemoryListener listener; KVMSlot *slots; int as_id; + QSIMPLEQ_HEAD(, KVMMemoryUpdate) transaction_add; + QSIMPLEQ_HEAD(, KVMMemoryUpdate) transaction_del; } KVMMemoryListener; #define KVM_MSI_HASHTAB_SIZE 256 From c0a6665c3c4d63b113ab31c624c53d4a32de2926 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Thu, 22 Dec 2022 15:01:58 +0100 Subject: [PATCH 06/29] target/i386: Remove compilation errors when -Werror=maybe-uninitialized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid compilation errors when -Werror=maybe-uninitialized is used, add a default case with g_assert_not_reached(). Otherwise with GCC 11.3.1 "cc (GCC) 11.3.1 20220421 (Red Hat 11.3.1-2)" we get: ../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’: ../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2495 | d->Q(3) = r3; | ~~~~~~~~^~~~ ../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2494 | d->Q(2) = r2; | ~~~~~~~~^~~~ ../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2493 | d->Q(1) = r1; | ~~~~~~~~^~~~ ../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2492 | d->Q(0) = r0; | ~~~~~~~~^~~~ Signed-off-by: Eric Auger Message-Id: <20221222140158.1260748-1-eric.auger@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/ops_sse.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h index 3cbc36a59d..0bd6bfad8a 100644 --- a/target/i386/ops_sse.h +++ b/target/i386/ops_sse.h @@ -2470,6 +2470,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t order) r0 = s->Q(2); r1 = s->Q(3); break; + default: /* default case added to help the compiler to avoid warnings */ + g_assert_not_reached(); } switch ((order >> 4) & 3) { case 0: @@ -2488,6 +2490,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t order) r2 = s->Q(2); r3 = s->Q(3); break; + default: /* default case added to help the compiler to avoid warnings */ + g_assert_not_reached(); } d->Q(0) = r0; d->Q(1) = r1; From 1baf34a1369ed323717c215d69bb6261b89cb5d7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 19 Dec 2022 10:17:09 +0100 Subject: [PATCH 07/29] chardev: clean up chardev-parallel.c Replace HAVE_CHARDEV_PARPORT with a Meson conditional, remove unnecessary defines, and close the file descriptor on FreeBSD/DragonFly. Signed-off-by: Paolo Bonzini --- chardev/char-parallel.c | 15 ++------------- chardev/meson.build | 5 ++++- include/qemu/osdep.h | 5 ----- 3 files changed, 6 insertions(+), 19 deletions(-) diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c index 05e7efbd6c..a5164f975a 100644 --- a/chardev/char-parallel.c +++ b/chardev/char-parallel.c @@ -238,7 +238,6 @@ static void qemu_chr_open_pp_fd(Chardev *chr, } #endif -#ifdef HAVE_CHARDEV_PARPORT static void qmp_chardev_open_parallel(Chardev *chr, ChardevBackend *backend, bool *be_opened, @@ -276,29 +275,21 @@ static void char_parallel_class_init(ObjectClass *oc, void *data) cc->parse = qemu_chr_parse_parallel; cc->open = qmp_chardev_open_parallel; -#if defined(__linux__) cc->chr_ioctl = pp_ioctl; -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \ - defined(__DragonFly__) - cc->chr_ioctl = pp_ioctl; -#endif } static void char_parallel_finalize(Object *obj) { -#if defined(__linux__) Chardev *chr = CHARDEV(obj); ParallelChardev *drv = PARALLEL_CHARDEV(chr); int fd = drv->fd; +#if defined(__linux__) pp_hw_mode(drv, IEEE1284_MODE_COMPAT); ioctl(fd, PPRELEASE); +#endif close(fd); qemu_chr_be_event(chr, CHR_EVENT_CLOSED); -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \ - defined(__DragonFly__) - /* FIXME: close fd? */ -#endif } static const TypeInfo char_parallel_type_info = { @@ -315,5 +306,3 @@ static void register_types(void) } type_init(register_types); - -#endif diff --git a/chardev/meson.build b/chardev/meson.build index 664f77b887..789b50056a 100644 --- a/chardev/meson.build +++ b/chardev/meson.build @@ -14,9 +14,12 @@ chardev_ss.add(files( )) chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files( 'char-fd.c', - 'char-parallel.c', 'char-pty.c', ), util]) +if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly'] + chardev_ss.add(files('char-parallel.c')) +endif + chardev_ss.add(when: 'CONFIG_WIN32', if_true: files( 'char-console.c', 'char-win-stdio.c', diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 7d059ad526..bd23a08595 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -427,11 +427,6 @@ void qemu_anon_ram_free(void *ptr, size_t size); #define HAVE_CHARDEV_SERIAL 1 #endif -#if defined(__linux__) || defined(__FreeBSD__) || \ - defined(__FreeBSD_kernel__) || defined(__DragonFly__) -#define HAVE_CHARDEV_PARPORT 1 -#endif - #if defined(__HAIKU__) #define SIGIO SIGPOLL #endif From 190973dc71b91ccb973fe2f2ce82c20a8e057a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 3 Nov 2022 13:30:43 -0400 Subject: [PATCH 08/29] gitlab: remove redundant setting of PKG_CONFIG_PATH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PKG_CONFIG_PATH variable is not defined in GitLab CI envs and even if it was, we don't need to set it to its existing value. Signed-off-by: Daniel P. Berrangé Reviewed-by: Stefan Hajnoczi Message-Id: <20221103173044.3969425-2-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- .gitlab-ci.d/crossbuild-template.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml index 5e8892fd49..6d709628f1 100644 --- a/.gitlab-ci.d/crossbuild-template.yml +++ b/.gitlab-ci.d/crossbuild-template.yml @@ -6,8 +6,7 @@ script: - mkdir build - cd build - - PKG_CONFIG_PATH=$PKG_CONFIG_PATH - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS + - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS --disable-user --target-list-exclude="arm-softmmu cris-softmmu i386-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu mips64-softmmu ppc-softmmu riscv32-softmmu sh4-softmmu @@ -32,8 +31,7 @@ script: - mkdir build - cd build - - PKG_CONFIG_PATH=$PKG_CONFIG_PATH - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS + - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS --disable-tools --enable-${ACCEL:-kvm} $EXTRA_CONFIGURE_OPTS - make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS @@ -44,8 +42,7 @@ script: - mkdir build - cd build - - PKG_CONFIG_PATH=$PKG_CONFIG_PATH - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS + - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS --disable-system --target-list-exclude="aarch64_be-linux-user alpha-linux-user cris-linux-user m68k-linux-user microblazeel-linux-user nios2-linux-user or1k-linux-user ppc-linux-user sparc-linux-user From d94e96e7cf02c8fe3e5ea75da5216b4bad79aa9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:00 -0500 Subject: [PATCH 09/29] disas: add G_GNUC_PRINTF to gstring_printf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Message-Id: <20221219130205.687815-2-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- disas.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/disas.c b/disas.c index 94d3b45042..3b31315f40 100644 --- a/disas.c +++ b/disas.c @@ -239,7 +239,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code, } } -static int gstring_printf(FILE *stream, const char *fmt, ...) +static int G_GNUC_PRINTF(2, 3) +gstring_printf(FILE *stream, const char *fmt, ...) { /* We abuse the FILE parameter to pass a GString. */ GString *s = (GString *)stream; From d62449daf237ee2a48aa61931dde6ab5df0ee282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:01 -0500 Subject: [PATCH 10/29] hw/xen: use G_GNUC_PRINTF/SCANF for various functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Acked-by: Anthony PERARD Message-Id: <20221219130205.687815-3-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- hw/xen/xen-bus.c | 1 + hw/xen/xen_pvdev.c | 1 + include/hw/xen/xen-bus-helper.h | 6 ++++-- include/hw/xen/xen-bus.h | 3 ++- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index 645a29a5a0..df3f6b9ae0 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -561,6 +561,7 @@ void xen_device_backend_printf(XenDevice *xendev, const char *key, } } +G_GNUC_SCANF(3, 4) static int xen_device_backend_scanf(XenDevice *xendev, const char *key, const char *fmt, ...) { diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c index 037152f063..1a5177b354 100644 --- a/hw/xen/xen_pvdev.c +++ b/hw/xen/xen_pvdev.c @@ -196,6 +196,7 @@ const char *xenbus_strstate(enum xenbus_state state) * 2 == noisy debug messages (logfile only). * 3 == will flood your log (logfile only). */ +G_GNUC_PRINTF(3, 0) static void xen_pv_output_msg(struct XenLegacyDevice *xendev, FILE *f, const char *fmt, va_list args) { diff --git a/include/hw/xen/xen-bus-helper.h b/include/hw/xen/xen-bus-helper.h index 629a904d1a..8782f30550 100644 --- a/include/hw/xen/xen-bus-helper.h +++ b/include/hw/xen/xen-bus-helper.h @@ -31,10 +31,12 @@ void xs_node_printf(struct xs_handle *xsh, xs_transaction_t tid, /* Read from node/key unless node is empty, in which case read from key */ int xs_node_vscanf(struct xs_handle *xsh, xs_transaction_t tid, const char *node, const char *key, Error **errp, - const char *fmt, va_list ap); + const char *fmt, va_list ap) + G_GNUC_SCANF(6, 0); int xs_node_scanf(struct xs_handle *xsh, xs_transaction_t tid, const char *node, const char *key, Error **errp, - const char *fmt, ...); + const char *fmt, ...) + G_GNUC_SCANF(6, 7); /* Watch node/key unless node is empty, in which case watch key */ void xs_node_watch(struct xs_handle *xsh, const char *node, const char *key, diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h index 713e763348..4d966a2dbb 100644 --- a/include/hw/xen/xen-bus.h +++ b/include/hw/xen/xen-bus.h @@ -94,7 +94,8 @@ void xen_device_frontend_printf(XenDevice *xendev, const char *key, G_GNUC_PRINTF(3, 4); int xen_device_frontend_scanf(XenDevice *xendev, const char *key, - const char *fmt, ...); + const char *fmt, ...) + G_GNUC_SCANF(3, 4); void xen_device_set_max_grant_refs(XenDevice *xendev, unsigned int nr_refs, Error **errp); From e4418354c0789330b57b06358d51d8ecc21eb39c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:02 -0500 Subject: [PATCH 11/29] tools/virtiofsd: add G_GNUC_PRINTF for logging functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Message-Id: <20221219130205.687815-4-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- tools/virtiofsd/fuse_log.c | 1 + tools/virtiofsd/fuse_log.h | 6 ++++-- tools/virtiofsd/passthrough_ll.c | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/virtiofsd/fuse_log.c b/tools/virtiofsd/fuse_log.c index 745d88cd2a..2de3f48ee7 100644 --- a/tools/virtiofsd/fuse_log.c +++ b/tools/virtiofsd/fuse_log.c @@ -12,6 +12,7 @@ #include "fuse_log.h" +G_GNUC_PRINTF(2, 0) static void default_log_func(__attribute__((unused)) enum fuse_log_level level, const char *fmt, va_list ap) { diff --git a/tools/virtiofsd/fuse_log.h b/tools/virtiofsd/fuse_log.h index 8d7091bd4d..e5c2967ab9 100644 --- a/tools/virtiofsd/fuse_log.h +++ b/tools/virtiofsd/fuse_log.h @@ -45,7 +45,8 @@ enum fuse_log_level { * @param ap format string arguments */ typedef void (*fuse_log_func_t)(enum fuse_log_level level, const char *fmt, - va_list ap); + va_list ap) + G_GNUC_PRINTF(2, 0); /** * Install a custom log handler function. @@ -68,6 +69,7 @@ void fuse_set_log_func(fuse_log_func_t func); * @param level severity level (FUSE_LOG_ERR, FUSE_LOG_DEBUG, etc) * @param fmt sprintf-style format string including newline */ -void fuse_log(enum fuse_log_level level, const char *fmt, ...); +void fuse_log(enum fuse_log_level level, const char *fmt, ...) + G_GNUC_PRINTF(2, 3); #endif /* FUSE_LOG_H_ */ diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 20f0f41f99..40ea2ed27f 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -4182,6 +4182,7 @@ static void setup_nofile_rlimit(unsigned long rlimit_nofile) } } +G_GNUC_PRINTF(2, 0) static void log_func(enum fuse_log_level level, const char *fmt, va_list ap) { g_autofree char *localfmt = NULL; From beede7e848cc0ca30599644c19c078bbe3cd9d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:03 -0500 Subject: [PATCH 12/29] util/error: add G_GNUC_PRINTF for various functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221219130205.687815-5-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- util/error-report.c | 1 + util/error.c | 1 + 2 files changed, 2 insertions(+) diff --git a/util/error-report.c b/util/error-report.c index 5edb2e6040..6e44a55732 100644 --- a/util/error-report.c +++ b/util/error-report.c @@ -193,6 +193,7 @@ real_time_iso8601(void) * a single phrase, with no newline or trailing punctuation. * Prepend the current location and append a newline. */ +G_GNUC_PRINTF(2, 0) static void vreport(report_type type, const char *fmt, va_list ap) { gchar *timestr; diff --git a/util/error.c b/util/error.c index b6c89d1412..1e7af665b8 100644 --- a/util/error.c +++ b/util/error.c @@ -45,6 +45,7 @@ static void error_handle_fatal(Error **errp, Error *err) } } +G_GNUC_PRINTF(6, 0) static void error_setv(Error **errp, const char *src, int line, const char *func, ErrorClass err_class, const char *fmt, va_list ap, From 0472b2e541971d162a9bbca7541c8f9299e9d78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:04 -0500 Subject: [PATCH 13/29] tests: add G_GNUC_PRINTF for various functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Message-Id: <20221219130205.687815-6-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- tests/qtest/ahci-test.c | 3 +++ tests/qtest/arm-cpu-features.c | 1 + tests/qtest/erst-test.c | 2 +- tests/qtest/ide-test.c | 3 ++- tests/qtest/ivshmem-test.c | 4 ++-- tests/qtest/libqmp.c | 2 +- tests/qtest/libqos/libqos-pc.h | 6 ++++-- tests/qtest/libqos/libqos-spapr.h | 6 ++++-- tests/qtest/libqos/libqos.h | 6 ++++-- tests/qtest/libqos/virtio-9p.c | 1 + tests/qtest/migration-helpers.h | 1 + tests/qtest/rtas-test.c | 2 +- tests/qtest/usb-hcd-uhci-test.c | 4 ++-- tests/unit/test-qmp-cmds.c | 13 +++++++++---- 14 files changed, 36 insertions(+), 18 deletions(-) diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c index 66652fed04..1967cd5898 100644 --- a/tests/qtest/ahci-test.c +++ b/tests/qtest/ahci-test.c @@ -154,6 +154,7 @@ static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri) /** * Start a Q35 machine and bookmark a handle to the AHCI device. */ +G_GNUC_PRINTF(1, 0) static AHCIQState *ahci_vboot(const char *cli, va_list ap) { AHCIQState *s; @@ -171,6 +172,7 @@ static AHCIQState *ahci_vboot(const char *cli, va_list ap) /** * Start a Q35 machine and bookmark a handle to the AHCI device. */ +G_GNUC_PRINTF(1, 2) static AHCIQState *ahci_boot(const char *cli, ...) { AHCIQState *s; @@ -209,6 +211,7 @@ static void ahci_shutdown(AHCIQState *ahci) * Boot and fully enable the HBA device. * @see ahci_boot, ahci_pci_enable and ahci_hba_enable. */ +G_GNUC_PRINTF(1, 2) static AHCIQState *ahci_boot_and_enable(const char *cli, ...) { AHCIQState *ahci; diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c index 5a14527386..8691802950 100644 --- a/tests/qtest/arm-cpu-features.c +++ b/tests/qtest/arm-cpu-features.c @@ -32,6 +32,7 @@ static QDict *do_query_no_props(QTestState *qts, const char *cpu_type) QUERY_TAIL, cpu_type); } +G_GNUC_PRINTF(3, 4) static QDict *do_query(QTestState *qts, const char *cpu_type, const char *fmt, ...) { diff --git a/tests/qtest/erst-test.c b/tests/qtest/erst-test.c index 974e8bcfe5..c45bee7f05 100644 --- a/tests/qtest/erst-test.c +++ b/tests/qtest/erst-test.c @@ -98,7 +98,7 @@ static void setup_vm_cmd(ERSTState *s, const char *cmd) const char *arch = qtest_get_arch(); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - s->qs = qtest_pc_boot(cmd); + s->qs = qtest_pc_boot("%s", cmd); } else { g_printerr("erst-test tests are only available on x86\n"); exit(EXIT_FAILURE); diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c index dbe1563b23..dcb050bf9b 100644 --- a/tests/qtest/ide-test.c +++ b/tests/qtest/ide-test.c @@ -125,6 +125,7 @@ static QGuestAllocator guest_malloc; static char *tmp_path[2]; static char *debug_path; +G_GNUC_PRINTF(1, 2) static QTestState *ide_test_start(const char *cmdline_fmt, ...) { QTestState *qts; @@ -788,7 +789,7 @@ static void test_flush_nodev(void) QPCIDevice *dev; QPCIBar bmdma_bar, ide_bar; - qts = ide_test_start(""); + qts = ide_test_start("%s", ""); dev = get_pci_device(qts, &bmdma_bar, &ide_bar); diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c index cd550c8935..9bf8e78df6 100644 --- a/tests/qtest/ivshmem-test.c +++ b/tests/qtest/ivshmem-test.c @@ -109,9 +109,9 @@ static void setup_vm_cmd(IVState *s, const char *cmd, bool msix) const char *arch = qtest_get_arch(); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - s->qs = qtest_pc_boot(cmd); + s->qs = qtest_pc_boot("%s", cmd); } else if (strcmp(arch, "ppc64") == 0) { - s->qs = qtest_spapr_boot(cmd); + s->qs = qtest_spapr_boot("%s", cmd); } else { g_printerr("ivshmem-test tests are only available on x86 or ppc64\n"); exit(EXIT_FAILURE); diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c index 2b08382e5d..a89cab03c3 100644 --- a/tests/qtest/libqmp.c +++ b/tests/qtest/libqmp.c @@ -134,7 +134,7 @@ static void socket_send_fds(int socket_fd, int *fds, size_t fds_num, * in the case that they choose to discard all replies up until * a particular EVENT is received. */ -static void +static G_GNUC_PRINTF(4, 0) void _qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num, const char *fmt, va_list ap) { diff --git a/tests/qtest/libqos/libqos-pc.h b/tests/qtest/libqos/libqos-pc.h index 1a9923ead4..a2e4209a49 100644 --- a/tests/qtest/libqos/libqos-pc.h +++ b/tests/qtest/libqos/libqos-pc.h @@ -3,8 +3,10 @@ #include "libqos.h" -QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap); -QOSState *qtest_pc_boot(const char *cmdline_fmt, ...); +QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap) + G_GNUC_PRINTF(1, 0); +QOSState *qtest_pc_boot(const char *cmdline_fmt, ...) + G_GNUC_PRINTF(1, 2); void qtest_pc_shutdown(QOSState *qs); #endif diff --git a/tests/qtest/libqos/libqos-spapr.h b/tests/qtest/libqos/libqos-spapr.h index c61338917a..e4483c14f8 100644 --- a/tests/qtest/libqos/libqos-spapr.h +++ b/tests/qtest/libqos/libqos-spapr.h @@ -3,8 +3,10 @@ #include "libqos.h" -QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap); -QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...); +QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap) + G_GNUC_PRINTF(1, 0); +QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...) + G_GNUC_PRINTF(1, 2); void qtest_spapr_shutdown(QOSState *qs); /* List of capabilities needed to silence warnings with TCG */ diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h index 9b4dd509f0..12d05b2365 100644 --- a/tests/qtest/libqos/libqos.h +++ b/tests/qtest/libqos/libqos.h @@ -21,8 +21,10 @@ struct QOSState { QOSOps *ops; }; -QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap); -QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...); +QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap) + G_GNUC_PRINTF(2, 0); +QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...) + G_GNUC_PRINTF(2, 3); void qtest_common_shutdown(QOSState *qs); void qtest_shutdown(QOSState *qs); bool have_qemu_img(void); diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c index 7f21028256..186fcc1141 100644 --- a/tests/qtest/libqos/virtio-9p.c +++ b/tests/qtest/libqos/virtio-9p.c @@ -211,6 +211,7 @@ static void *virtio_9p_pci_create(void *pci_bus, QGuestAllocator *t_alloc, * variable arguments of this function to this * replacement string */ +G_GNUC_PRINTF(3, 4) static void regex_replace(GString *haystack, const char *pattern, const char *replace_fmt, ...) { diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h index db0684de48..a188b62787 100644 --- a/tests/qtest/migration-helpers.h +++ b/tests/qtest/migration-helpers.h @@ -25,6 +25,7 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...); G_GNUC_PRINTF(2, 3) QDict *wait_command(QTestState *who, const char *command, ...); +G_GNUC_PRINTF(2, 3) QDict *qmp_command(QTestState *who, const char *command, ...); G_GNUC_PRINTF(3, 4) diff --git a/tests/qtest/rtas-test.c b/tests/qtest/rtas-test.c index 50df60e5b2..1ba42b37d2 100644 --- a/tests/qtest/rtas-test.c +++ b/tests/qtest/rtas-test.c @@ -13,7 +13,7 @@ static void run_test_rtas_get_time_of_day(const char *machine) uint64_t ret; time_t t1, t2; - qs = qtest_spapr_boot(machine); + qs = qtest_spapr_boot("%s", machine); t1 = time(NULL); ret = qrtas_get_time_of_day(qs->qts, &qs->alloc, &tm, &ns); diff --git a/tests/qtest/usb-hcd-uhci-test.c b/tests/qtest/usb-hcd-uhci-test.c index 7a117b64d9..f264d2bf73 100644 --- a/tests/qtest/usb-hcd-uhci-test.c +++ b/tests/qtest/usb-hcd-uhci-test.c @@ -72,9 +72,9 @@ int main(int argc, char **argv) qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - qs = qtest_pc_boot(cmd); + qs = qtest_pc_boot("%s", cmd); } else if (strcmp(arch, "ppc64") == 0) { - qs = qtest_spapr_boot(cmd); + qs = qtest_spapr_boot("%s", cmd); } else { g_printerr("usb-hcd-uhci-test tests are only " "available on x86 or ppc64\n"); diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c index 2373cd64cb..6d52b4e5d8 100644 --- a/tests/unit/test-qmp-cmds.c +++ b/tests/unit/test-qmp-cmds.c @@ -138,6 +138,7 @@ void qmp___org_qemu_x_command(__org_qemu_x_EnumList *a, } +G_GNUC_PRINTF(2, 3) static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...) { va_list ap; @@ -160,6 +161,7 @@ static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...) return ret; } +G_GNUC_PRINTF(3, 4) static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls, const char *template, ...) { @@ -269,7 +271,7 @@ static void test_dispatch_cmd_io(void) static void test_dispatch_cmd_deprecated(void) { - const char *cmd = "{ 'execute': 'test-command-features1' }"; + #define cmd "{ 'execute': 'test-command-features1' }" QDict *ret; memset(&compat_policy, 0, sizeof(compat_policy)); @@ -287,12 +289,13 @@ static void test_dispatch_cmd_deprecated(void) compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT; do_qmp_dispatch_error(false, ERROR_CLASS_COMMAND_NOT_FOUND, cmd); + #undef cmd } static void test_dispatch_cmd_arg_deprecated(void) { - const char *cmd = "{ 'execute': 'test-features0'," - " 'arguments': { 'fs1': { 'foo': 42 } } }"; + #define cmd "{ 'execute': 'test-features0'," \ + " 'arguments': { 'fs1': { 'foo': 42 } } }" QDict *ret; memset(&compat_policy, 0, sizeof(compat_policy)); @@ -310,11 +313,12 @@ static void test_dispatch_cmd_arg_deprecated(void) compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT; do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, cmd); + #undef cmd } static void test_dispatch_cmd_ret_deprecated(void) { - const char *cmd = "{ 'execute': 'test-features0' }"; + #define cmd "{ 'execute': 'test-features0' }" QDict *ret; memset(&compat_policy, 0, sizeof(compat_policy)); @@ -334,6 +338,7 @@ static void test_dispatch_cmd_ret_deprecated(void) ret = qobject_to(QDict, do_qmp_dispatch(false, cmd)); assert(ret && qdict_size(ret) == 0); qobject_unref(ret); + #undef cmd } /* test generated dealloc functions for generated types */ From 88a0ef00d70c61fdb46ee88ced87046dfb11eb3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 08:02:05 -0500 Subject: [PATCH 14/29] enforce use of G_GNUC_PRINTF attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We've been very gradually adding G_GNUC_PRINTF annotations to functions over years. This has been useful in detecting certain malformed printf strings, or cases where we pass user data as the printf format which is a potential security flaw. Given the inherant memory corruption danger in use of format strings vs mis-matched variadic arguments, it is worth applying G_GNUC_PRINTF to all functions using printf, even if we know they are safe. The compilers can reasonably reliably identify such places with the -Wsuggest-attribute=format / -Wmissing-format-attribute flags. Signed-off-by: Daniel P. Berrangé Message-Id: <20221219130205.687815-7-berrange@redhat.com> [-Wsuggest-attribute=format and -Wmissing-format-attribute are synonyms, only include one; disable it for testfloat. - Paolo] Signed-off-by: Paolo Bonzini --- configure | 1 + tests/fp/meson.build | 1 + 2 files changed, 2 insertions(+) diff --git a/configure b/configure index 6f5e77a713..643aed7533 100755 --- a/configure +++ b/configure @@ -1183,6 +1183,7 @@ add_to warn_flags -Wnested-externs add_to warn_flags -Wendif-labels add_to warn_flags -Wexpansion-to-defined add_to warn_flags -Wimplicit-fallthrough=2 +add_to warn_flags -Wmissing-format-attribute nowarn_flags= add_to nowarn_flags -Wno-initializer-overrides diff --git a/tests/fp/meson.build b/tests/fp/meson.build index 6258e2bd7d..312a4d301f 100644 --- a/tests/fp/meson.build +++ b/tests/fp/meson.build @@ -37,6 +37,7 @@ tfcflags = [ '-Wno-missing-prototypes', '-Wno-return-type', '-Wno-unused-function', + '-Wno-missing-format-attribute', '-Wno-error', ] From 8b8437259ca68e8e2145e0566cf67a29d6ee1816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 19 Dec 2022 12:58:30 +0000 Subject: [PATCH 15/29] hw/display: avoid creating empty loadable modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using --disable-virglrenderer, QEMU still creates hw-display-virtio-gpu-gl.so hw-display-virtio-vga-gl.so hw-display-virtio-gpu-pci-gl.so but when these are loaded, they provide no functionality as the code which registers types is not compiled in. Funtionally this is relatively harmless, because QEMU is fine loading a module with no types. This is rather confusing for users and OS distro maintainers though, as they think they have the GL functionality built, but in fact the module they are looking at provides nothing of value. The root cause is the use of 'when/if_true' rules when adding sources to the module source set. If all the rules evaluate to false, then we have declared the module, but not added anything to it. We need to put declaration of the entire module inside a condition based on existance of the 3rd party library deps that are mandatory. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1352 Signed-off-by: Daniel P. Berrangé Message-Id: <20221219125830.2369169-1-berrange@redhat.com> [Do not check for pixman. - Paolo] Signed-off-by: Paolo Bonzini --- hw/display/meson.build | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/hw/display/meson.build b/hw/display/meson.build index 7a725ed80e..f860c2c562 100644 --- a/hw/display/meson.build +++ b/hw/display/meson.build @@ -73,10 +73,12 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU') virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: files('vhost-user-gpu.c')) hw_display_modules += {'virtio-gpu': virtio_gpu_ss} - virtio_gpu_gl_ss = ss.source_set() - virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl, opengl], - if_true: [files('virtio-gpu-gl.c', 'virtio-gpu-virgl.c'), pixman, virgl]) - hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss} + if virgl.found() and opengl.found() + virtio_gpu_gl_ss = ss.source_set() + virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl, opengl], + if_true: [files('virtio-gpu-gl.c', 'virtio-gpu-virgl.c'), pixman, virgl]) + hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss} + endif endif if config_all_devices.has_key('CONFIG_VIRTIO_PCI') @@ -87,10 +89,12 @@ if config_all_devices.has_key('CONFIG_VIRTIO_PCI') if_true: files('vhost-user-gpu-pci.c')) hw_display_modules += {'virtio-gpu-pci': virtio_gpu_pci_ss} - virtio_gpu_pci_gl_ss = ss.source_set() - virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', virgl, opengl], - if_true: [files('virtio-gpu-pci-gl.c'), pixman]) - hw_display_modules += {'virtio-gpu-pci-gl': virtio_gpu_pci_gl_ss} + if virgl.found() and opengl.found() + virtio_gpu_pci_gl_ss = ss.source_set() + virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', virgl, opengl], + if_true: [files('virtio-gpu-pci-gl.c'), pixman]) + hw_display_modules += {'virtio-gpu-pci-gl': virtio_gpu_pci_gl_ss} + endif endif if config_all_devices.has_key('CONFIG_VIRTIO_VGA') From dadc3d01bcb0d8654361e5ce77f65a679f2d7a80 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:40 +0100 Subject: [PATCH 16/29] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Then the libvhost-user sources are used by another project, it can not be guaranteed that _GNU_SOURCE is set by the build system. If it is for example not set, errors like this show up. CC libvhost-user.o libvhost-user.c: In function ‘vu_panic’: libvhost-user.c:195:9: error: implicit declaration of function ‘vasprintf’; did you mean ‘vsprintf’? [-Werror=implicit-function-declaration] 195 | if (vasprintf(&buf, msg, ap) < 0) { | ^~~~~~~~~ | vsprintf The simplest way to allow external complication of libvhost-user.[ch] is by setting _GNU_SOURCE if it is not already set by the build system. Signed-off-by: Marcel Holtmann Message-Id: Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index d6ee6e7d91..b55b9e244d 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -13,6 +13,10 @@ * later. See the COPYING file in the top-level directory. */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + /* this code avoids GLib dependency */ #include #include From aa5d395ac4351d85472fa3d4266b26b8025f9356 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:41 +0100 Subject: [PATCH 17/29] libvhost-user: Replace typeof with __typeof__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strictly speaking only -std=gnu99 support the usage of typeof and for easier inclusion in external projects, it is better to use __typeof__. CC libvhost-user.o libvhost-user.c: In function ‘vu_log_queue_fill’: libvhost-user.c:86:13: error: implicit declaration of function ‘typeof’ [-Werror=implicit-function-declaration] 86 | typeof(x) _min1 = (x); \ | ^~~~~~ Changing these two users of typeof makes the compiler happy and no extra flags or pragmas need to be provided. Signed-off-by: Marcel Holtmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <981aa822bcaaa2b8d74f245339a99a85c25b346f.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index b55b9e244d..67d75ece53 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -62,8 +62,8 @@ #endif /* !__GNUC__ */ #ifndef MIN #define MIN(x, y) ({ \ - typeof(x) _min1 = (x); \ - typeof(y) _min2 = (y); \ + __typeof__(x) _min1 = (x); \ + __typeof__(y) _min2 = (y); \ (void) (&_min1 == &_min2); \ _min1 < _min2 ? _min1 : _min2; }) #endif From 18fa7f1e95e22f74f509e1b5cf759f7a19a0954d Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:42 +0100 Subject: [PATCH 18/29] libvhost-user: Cast rc variable to avoid compiler warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assert from recvmsg() return value against an uint32_t size field from a protocol struct throws a compiler warning. CC libvhost-user.o In file included from libvhost-user.c:27: libvhost-user.c: In function ‘vu_message_read_default’: libvhost-user.c:363:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare] 363 | assert(rc == vmsg->size); | ^~ This is not critical, but annoying when the libvhost-user source are used in an external project that has this compiler warning switched on. Signed-off-by: Marcel Holtmann Message-Id: <7a791e27b7bd3e0a8b8cc8fbb15090a870d226d5.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index 67d75ece53..bcdf32a24f 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -339,7 +339,7 @@ vu_message_read_default(VuDev *dev, int conn_fd, VhostUserMsg *vmsg) goto fail; } - assert(rc == vmsg->size); + assert((uint32_t)rc == vmsg->size); } return true; From 92bf246130faa85b20ca8e3b6c4eda50d5825a5a Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:43 +0100 Subject: [PATCH 19/29] libvhost-user: Use unsigned int i for some for-loop iterations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sign-compare warning also hits some of the for-loops, but it easy fixed by just making the iterator variable unsigned int. CC libvhost-user.o libvhost-user.c: In function ‘vu_gpa_to_va’: libvhost-user.c:223:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare] 223 | for (i = 0; i < dev->nregions; i++) { | ^ Signed-off-by: Marcel Holtmann Message-Id: Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index bcdf32a24f..211d31a4cc 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -192,7 +192,7 @@ vu_panic(VuDev *dev, const char *msg, ...) void * vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr) { - int i; + unsigned int i; if (*plen == 0) { return NULL; @@ -218,7 +218,7 @@ vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr) static void * qva_to_va(VuDev *dev, uint64_t qemu_addr) { - int i; + unsigned int i; /* Find matching memory region. */ for (i = 0; i < dev->nregions; i++) { @@ -621,7 +621,7 @@ map_ring(VuDev *dev, VuVirtq *vq) static bool generate_faults(VuDev *dev) { - int i; + unsigned int i; for (i = 0; i < dev->nregions; i++) { VuDevRegion *dev_region = &dev->regions[i]; int ret; @@ -829,7 +829,7 @@ static inline bool reg_equal(VuDevRegion *vudev_reg, static bool vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) { VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m; - int i; + unsigned int i; bool found = false; if (vmsg->fd_num > 1) { @@ -895,7 +895,7 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) { static bool vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg) { - int i; + unsigned int i; VhostUserMemory m = vmsg->payload.memory, *memory = &m; dev->nregions = memory->nregions; @@ -972,7 +972,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg) static bool vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) { - int i; + unsigned int i; VhostUserMemory m = vmsg->payload.memory, *memory = &m; for (i = 0; i < dev->nregions; i++) { @@ -1980,7 +1980,7 @@ end: void vu_deinit(VuDev *dev) { - int i; + unsigned int i; for (i = 0; i < dev->nregions; i++) { VuDevRegion *r = &dev->regions[i]; From d87a642403364d4704324197d75d41641287b4b1 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:44 +0100 Subject: [PATCH 20/29] libvhost-user: Declare uffdio_register early to make it C90 compliant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using libvhost-user source in an external project that wants to comply with the C90 standard, it is best to declare variables before code. CC libvhost-user.o libvhost-user.c: In function ‘generate_faults’: libvhost-user.c:683:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 683 | struct uffdio_register reg_struct; | ^~~~~~ In this case, it is also simple enough and doesn't cause any extra ifdef additions. Signed-off-by: Marcel Holtmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <556c2d00c01fa134d13c0371d4014c90694c2943.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index 211d31a4cc..bf92cc85c0 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -626,6 +626,8 @@ generate_faults(VuDev *dev) { VuDevRegion *dev_region = &dev->regions[i]; int ret; #ifdef UFFDIO_REGISTER + struct uffdio_register reg_struct; + /* * We should already have an open ufd. Mark each memory * range as ufd. @@ -659,7 +661,7 @@ generate_faults(VuDev *dev) { "%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n", __func__, i, strerror(errno)); } - struct uffdio_register reg_struct; + reg_struct.range.start = (uintptr_t)dev_region->mmap_addr; reg_struct.range.len = dev_region->size + dev_region->mmap_offset; reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING; From f1c563d209e91635974384c0690155a101dca5b4 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:45 +0100 Subject: [PATCH 21/29] libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assignment of dev->postcopy_ufd can be moved into an else clause and then the code becomes C90 compliant. CC libvhost-user.o libvhost-user.c: In function ‘vu_set_postcopy_advise’: libvhost-user.c:1625:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 1625 | struct uffdio_api api_struct; | ^~~~~~ Understandable, it might be desired to avoid else clauses, but in this case it seems clear enough and frankly the dev->postcopy_ufd is only assigned once. Signed-off-by: Marcel Holtmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <74db52afb1203c4580ffc7fa462b4b2ba260a353.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index bf92cc85c0..b28b66cdb1 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -1599,12 +1599,13 @@ vu_set_config(VuDev *dev, VhostUserMsg *vmsg) static bool vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg) { - dev->postcopy_ufd = -1; #ifdef UFFDIO_API struct uffdio_api api_struct; dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK); vmsg->size = 0; +#else + dev->postcopy_ufd = -1; #endif if (dev->postcopy_ufd == -1) { From 518ac42879560e656a62c3f4ed16569cc169202b Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:46 +0100 Subject: [PATCH 22/29] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the libvduse sources are used by another project, it can not be guaranteed that _GNU_SOURCE is set by the build system. If it is for example not set, errors like this show up. CC libvduse.o libvduse.c: In function ‘vduse_log_get’: libvduse.c:172:9: error: implicit declaration of function ‘ftruncate’; did you mean ‘strncat’? [-Werror=implicit-function-declaration] 172 | if (ftruncate(fd, size) == -1) { | ^~~~~~~~~ | strncat The simplest way to allow external complication of libvduse.[ch] by setting _GNU_SOURCE if it is not already set by the build system. Signed-off-by: Marcel Holtmann Message-Id: <407f3665f0605df936e5bfe60831d180edfb8cca.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvduse/libvduse.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index e089d4d546..c871bd331a 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -16,6 +16,10 @@ * later. See the COPYING file in the top-level directory. */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + #include #include #include From 85899f8e6b3c4dae924a550778a8f594ecc37b33 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:47 +0100 Subject: [PATCH 23/29] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It seems there is no need to keep the inuse field signed and end up with compiler warnings for sign-compare. CC libvduse.o libvduse.c: In function ‘vduse_queue_pop’: libvduse.c:789:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare] 789 | if (vq->inuse >= vq->vring.num) { | ^~ Instead of casting the comparison to unsigned int, just make the inuse field unsigned int in the fist place. Signed-off-by: Marcel Holtmann Reviewed-by: Xie Yongji Message-Id: <9fe3fd8b042e048bd04d506ca6e43d738b5c45b7.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvduse/libvduse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index c871bd331a..338ad5e352 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -101,7 +101,7 @@ struct VduseVirtq { uint16_t signalled_used; bool signalled_used_valid; int index; - int inuse; + unsigned int inuse; bool ready; int fd; VduseDev *dev; From 86e61e4233b8de44805914475669c0ca906809c0 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:48 +0100 Subject: [PATCH 24/29] libvduse: Fix assignment in vring_set_avail_event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the assignment is causing a compiler warning, fix it by using memcpy instead. CC libvduse.o libvduse.c: In function ‘vring_set_avail_event’: libvduse.c:603:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasin] 603 | *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val); | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Marcel Holtmann Suggested-by: Xie Yongji Suggested-by: Paolo Bonzini Message-Id: <4a0fe2a6436464473119fdbf0bc4076b36fbb37f.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvduse/libvduse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index 338ad5e352..377959a0b4 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -582,7 +582,8 @@ void vduse_queue_notify(VduseVirtq *vq) static inline void vring_set_avail_event(VduseVirtq *vq, uint16_t val) { - *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val); + uint16_t val_le = htole16(val); + memcpy(&vq->vring.used->ring[vq->vring.num], &val_le, sizeof(uint16_t)); } static bool vduse_queue_map_single_desc(VduseVirtq *vq, unsigned int *p_num_sg, From 950a2f2eff1aa39620ff78be4485f6290939d8dd Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:49 +0100 Subject: [PATCH 25/29] libvhost-user: Fix assignment in vring_set_avail_event Since it was proposed to change the code in libvduse.c to use memcpy instead of an assignment, the code in libvhost-user.c should also be changed to use memcpy. Signed-off-by: Marcel Holtmann Suggested-by: Paolo Bonzini Message-Id: <502b22723264db064e4b05008233a9c1f2f8aaaa.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index b28b66cdb1..fc69783d2b 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -2478,14 +2478,13 @@ vring_used_flags_unset_bit(VuVirtq *vq, int mask) static inline void vring_set_avail_event(VuVirtq *vq, uint16_t val) { - uint16_t *avail; + uint16_t val_le = htole16(val); if (!vq->notification) { return; } - avail = (uint16_t *)&vq->vring.used->ring[vq->vring.num]; - *avail = htole16(val); + memcpy(&vq->vring.used->ring[vq->vring.num], &val_le, sizeof(uint16_t)); } void From 722b62d97d75689cff1ef3f1e4eb6d77d6498811 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:50 +0100 Subject: [PATCH 26/29] libvhost-user: Add extra compiler warnings In case libvhost-user is used externally, that projects compiler warnings might be more strict. Enforce an extra set of compiler warnings to catch issues early on. Signed-off-by: Marcel Holtmann Suggested-by: Paolo Bonzini Message-Id: <737ebf2e697f8640558e6f73d96a692711f548f6.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/meson.build | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/subprojects/libvhost-user/meson.build b/subprojects/libvhost-user/meson.build index 39825d9404..a18014e7f2 100644 --- a/subprojects/libvhost-user/meson.build +++ b/subprojects/libvhost-user/meson.build @@ -1,6 +1,12 @@ project('libvhost-user', 'c', license: 'GPL-2.0-or-later', - default_options: ['c_std=gnu99']) + default_options: ['warning_level=1', 'c_std=gnu99']) + +cc = meson.get_compiler('c') +add_project_arguments(cc.get_supported_arguments('-Wsign-compare', + '-Wdeclaration-after-statement', + '-Wstrict-aliasing'), + native: false, language: 'c') threads = dependency('threads') glib = dependency('glib-2.0') From 8d5666d76b0a2b0f852e51412c79c3d3d8c90801 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Thu, 22 Dec 2022 21:36:51 +0100 Subject: [PATCH 27/29] libvduse: Add extra compiler warnings In case libvhost-user is used externally, that projects compiler warnings might be more strict. Enforce an extra set of compiler warnings to catch issues early on. Signed-off-by: Marcel Holtmann Suggested-by: Paolo Bonzini Message-Id: <08daa1896ad8824e17d57d6a970bc0b4bee73ece.1671741278.git.marcel@holtmann.org> Signed-off-by: Paolo Bonzini --- subprojects/libvduse/meson.build | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/subprojects/libvduse/meson.build b/subprojects/libvduse/meson.build index ba08f5ee1a..3e3b53da33 100644 --- a/subprojects/libvduse/meson.build +++ b/subprojects/libvduse/meson.build @@ -1,6 +1,12 @@ project('libvduse', 'c', license: 'GPL-2.0-or-later', - default_options: ['c_std=gnu99']) + default_options: ['warning_level=1', 'c_std=gnu99']) + +cc = meson.get_compiler('c') +add_project_arguments(cc.get_supported_arguments('-Wsign-compare', + '-Wdeclaration-after-statement', + '-Wstrict-aliasing'), + native: false, language: 'c') libvduse = static_library('vduse', files('libvduse.c'), From 3d304620ec6c95f31db17acc132f42f243369299 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sat, 7 Jan 2023 18:14:20 +0100 Subject: [PATCH 28/29] target/i386: fix operand size of unary SSE operations VRCPSS, VRSQRTSS and VCVTSx2Sx have a 32-bit or 64-bit memory operand, which is represented in the decoding tables by X86_VEX_REPScalar. Add it to the tables, and make validate_vex() handle the case of an instruction that is in exception type 4 without the REP prefix and exception type 5 with it; this is the cas of VRCP and VRSQRT. Reported-by: yongwoo Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1377 Signed-off-by: Paolo Bonzini --- target/i386/tcg/decode-new.c.inc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc index 80c579164f..d5fd8d965c 100644 --- a/target/i386/tcg/decode-new.c.inc +++ b/target/i386/tcg/decode-new.c.inc @@ -105,6 +105,7 @@ #define vex3 .vex_class = 3, #define vex4 .vex_class = 4, #define vex4_unal .vex_class = 4, .vex_special = X86_VEX_SSEUnaligned, +#define vex4_rep5 .vex_class = 4, .vex_special = X86_VEX_REPScalar, #define vex5 .vex_class = 5, #define vex6 .vex_class = 6, #define vex7 .vex_class = 7, @@ -839,8 +840,8 @@ static const X86OpEntry opcodes_0F[256] = { [0x50] = X86_OP_ENTRY3(MOVMSK, G,y, None,None, U,x, vex7 p_00_66), [0x51] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), - [0x52] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex5 p_00_f3), - [0x53] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex5 p_00_f3), + [0x52] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex4_rep5 p_00_f3), + [0x53] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex4_rep5 p_00_f3), [0x54] = X86_OP_ENTRY3(PAND, V,x, H,x, W,x, vex4 p_00_66), /* vand */ [0x55] = X86_OP_ENTRY3(PANDN, V,x, H,x, W,x, vex4 p_00_66), /* vandn */ [0x56] = X86_OP_ENTRY3(POR, V,x, H,x, W,x, vex4 p_00_66), /* vor */ @@ -878,7 +879,7 @@ static const X86OpEntry opcodes_0F[256] = { [0x58] = X86_OP_ENTRY3(VADD, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), [0x59] = X86_OP_ENTRY3(VMUL, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), - [0x5a] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex3 p_00_66_f3_f2), + [0x5a] = X86_OP_GROUP3(sse_unary, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), [0x5b] = X86_OP_GROUP0(0F5B), [0x5c] = X86_OP_ENTRY3(VSUB, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), [0x5d] = X86_OP_ENTRY3(VMIN, V,x, H,x, W,x, vex2_rep3 p_00_66_f3_f2), @@ -1447,9 +1448,9 @@ static bool validate_vex(DisasContext *s, X86DecodedInsn *decode) * Instructions which differ between 00/66 and F2/F3 in the * exception classification and the size of the memory operand. */ - assert(e->vex_class == 1 || e->vex_class == 2); + assert(e->vex_class == 1 || e->vex_class == 2 || e->vex_class == 4); if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) { - e->vex_class = 3; + e->vex_class = e->vex_class < 4 ? 3 : 5; if (s->vex_l) { goto illegal; } From 75cc286485742feeb00f4b446f5682765792323e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 14 Jul 2022 14:58:55 +0200 Subject: [PATCH 29/29] configure: remove backwards-compatibility code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cmd_line.txt mangling is only needed when rebuilding from very old trees and is kept mostly as an example of how to extend it. However, Meson 0.63 introduces a deprecation mechanism for meson_options.txt that can be used instead, so get rid of our home-grown hack. Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- configure | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/configure b/configure index 643aed7533..9e407ce2e3 100755 --- a/configure +++ b/configure @@ -2568,16 +2568,6 @@ if test "$skip_meson" = no; then if test "$?" -ne 0 ; then error_exit "meson setup failed" fi -else - if test -f meson-private/cmd_line.txt; then - # Adjust old command line options whose type was changed - # Avoids having to use "setup --wipe" when Meson is upgraded - perl -i -ne ' - s/^gettext = true$/gettext = auto/; - s/^gettext = false$/gettext = disabled/; - /^b_staticpic/ && next; - print;' meson-private/cmd_line.txt - fi fi # Save the configure command line for later reuse.