From d9bcd6f7f23a13ea627d8edb85c0706525da0b75 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 5 Dec 2017 23:15:53 +0800 Subject: [PATCH 01/51] scsi-generic: Add share-rw option Add the property to the device model, then parse it by calling blkconf_apply_backend_options(). In addition to blk_set_perm(), the called function also handles error options and wce. For error options we've already checked that the default values are used, for wce we don't have the option either so it is always the default (true). In other words there is no change of behavior in these regards. Signed-off-by: Fam Zheng Message-Id: <20171205151553.7834-1-famz@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-generic.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index bd0d9ff355..ba70c0dc19 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -482,6 +482,7 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp) int rc; int sg_version; struct sg_scsi_id scsiid; + Error *local_err = NULL; if (!s->conf.blk) { error_setg(errp, "drive property not set"); @@ -515,6 +516,13 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp) error_setg(errp, "SG_GET_SCSI_ID ioctl failed"); return; } + blkconf_apply_backend_options(&s->conf, + blk_is_read_only(s->conf.blk), + true, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } /* define device state */ s->type = scsiid.scsi_type; @@ -565,6 +573,7 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun, static Property scsi_generic_properties[] = { DEFINE_PROP_DRIVE("drive", SCSIDevice, conf.blk), + DEFINE_PROP_BOOL("share-rw", SCSIDevice, conf.share_rw, false), DEFINE_PROP_END_OF_LIST(), }; From 2770c90d432b571cab718e28f838097f0b2201ec Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 22 Dec 2017 16:30:34 +0100 Subject: [PATCH 02/51] scsi: fix scsi_convert_sense crash when in_buf == NULL && in_len == 0 scsi_disk_emulate_command passes in_buf == NULL when sent a REQUEST SENSE command. Check for in_len == 0 before dereferencing in_buf. Fixes: f68d98b21fa74155dc7c1fd212474379ac3c7531 Reported-by: Roman Kagan Tested-by: Roman Kagan Signed-off-by: Paolo Bonzini --- scsi/utils.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scsi/utils.c b/scsi/utils.c index ddae650a99..8738522955 100644 --- a/scsi/utils.c +++ b/scsi/utils.c @@ -322,18 +322,18 @@ int scsi_convert_sense(uint8_t *in_buf, int in_len, SCSISense sense; bool fixed_in; - fixed_in = (in_buf[0] & 2) == 0; - if (in_len && fixed == fixed_in) { - memcpy(buf, in_buf, MIN(len, in_len)); - return MIN(len, in_len); + if (in_len == 0) { + return scsi_build_sense_buf(buf, len, SENSE_CODE(NO_SENSE), fixed); } - if (in_len == 0) { - sense = SENSE_CODE(NO_SENSE); + fixed_in = (in_buf[0] & 2) == 0; + if (fixed == fixed_in) { + memcpy(buf, in_buf, MIN(len, in_len)); + return MIN(len, in_len); } else { sense = scsi_parse_sense_buf(in_buf, in_len); + return scsi_build_sense_buf(buf, len, sense, fixed); } - return scsi_build_sense_buf(buf, len, sense, fixed); } int scsi_sense_to_errno(int key, int asc, int ascq) From 8cd91acec8dfea6065272ca828405333f564a612 Mon Sep 17 00:00:00 2001 From: Haozhong Zhang Date: Fri, 22 Dec 2017 09:51:20 +0800 Subject: [PATCH 03/51] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type When -no-acpi option is used with Q35 machine type, no guest ACPI is built, but the ACPI device is still created, so only checking the presence of ACPI device before memory plug/unplug is not enough in such cases. Check whether ACPI is disabled globally in addition and fail memory plug/unplug if it's disabled. Signed-off-by: Haozhong Zhang Message-Id: <20171222015120.31730-1-haozhong.zhang@intel.com> Reviewed-by: Igor Mammedov Signed-off-by: Paolo Bonzini --- hw/i386/pc.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 3fcf318a95..55686bf5d8 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1695,9 +1695,14 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, align = memory_region_get_alignment(mr); } - if (!pcms->acpi_dev) { + /* + * When -no-acpi is used with Q35 machine type, no ACPI is built, + * but pcms->acpi_dev is still created. Check !acpi_enabled in + * addition to cover this case. + */ + if (!pcms->acpi_dev || !acpi_enabled) { error_setg(&local_err, - "memory hotplug is not enabled: missing acpi device"); + "memory hotplug is not enabled: missing acpi device or acpi disabled"); goto out; } @@ -1729,9 +1734,14 @@ static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev, Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); - if (!pcms->acpi_dev) { + /* + * When -no-acpi is used with Q35 machine type, no ACPI is built, + * but pcms->acpi_dev is still created. Check !acpi_enabled in + * addition to cover this case. + */ + if (!pcms->acpi_dev || !acpi_enabled) { error_setg(&local_err, - "memory hotplug is not enabled: missing acpi device"); + "memory hotplug is not enabled: missing acpi device or acpi disabled"); goto out; } From 829600a519386c7b188d5d813e78ba69bf0bd323 Mon Sep 17 00:00:00 2001 From: Pavel Dovgalyuk Date: Thu, 11 Jan 2018 11:24:58 +0300 Subject: [PATCH 04/51] hpet: recover timer offset correctly HPET saves its state by calculating the current time and recovers timer offset using this calculated value. But these calculations include divisions and multiplications. Therefore the timer state cannot be recovered precise enough. This patch introduces saving of the original value of the offset to preserve the determinism of the timer. Signed-off-by: Pavel Dovgalyuk Signed-off-by: Maria Klimushenkova Reviewed-by: Juan Quintela -- v3: Added compat property for correct migration. Signed-off-by: Paolo Bonzini --- hw/timer/hpet.c | 30 ++++++++++++++++++++++++++++-- include/hw/compat.h | 6 +++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 577371bc6d..d97436bc7b 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -70,6 +70,7 @@ typedef struct HPETState { MemoryRegion iomem; uint64_t hpet_offset; + bool hpet_offset_saved; qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; uint32_t flags; uint8_t rtc_irq_level; @@ -221,7 +222,9 @@ static int hpet_pre_save(void *opaque) HPETState *s = opaque; /* save current counter value */ - s->hpet_counter = hpet_get_ticks(s); + if (hpet_enabled(s)) { + s->hpet_counter = hpet_get_ticks(s); + } return 0; } @@ -252,7 +255,10 @@ static int hpet_post_load(void *opaque, int version_id) HPETState *s = opaque; /* Recalculate the offset between the main counter and guest time */ - s->hpet_offset = ticks_to_ns(s->hpet_counter) - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + if (!s->hpet_offset_saved) { + s->hpet_offset = ticks_to_ns(s->hpet_counter) + - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + } /* Push number of timers into capability returned via HPET_ID */ s->capability &= ~HPET_ID_NUM_TIM_MASK; @@ -267,6 +273,13 @@ static int hpet_post_load(void *opaque, int version_id) return 0; } +static bool hpet_offset_needed(void *opaque) +{ + HPETState *s = opaque; + + return hpet_enabled(s) && s->hpet_offset_saved; +} + static bool hpet_rtc_irq_level_needed(void *opaque) { HPETState *s = opaque; @@ -285,6 +298,17 @@ static const VMStateDescription vmstate_hpet_rtc_irq_level = { } }; +static const VMStateDescription vmstate_hpet_offset = { + .name = "hpet/offset", + .version_id = 1, + .minimum_version_id = 1, + .needed = hpet_offset_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT64(hpet_offset, HPETState), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_hpet_timer = { .name = "hpet_timer", .version_id = 1, @@ -320,6 +344,7 @@ static const VMStateDescription vmstate_hpet = { }, .subsections = (const VMStateDescription*[]) { &vmstate_hpet_rtc_irq_level, + &vmstate_hpet_offset, NULL } }; @@ -762,6 +787,7 @@ static Property hpet_device_properties[] = { DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS), DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false), DEFINE_PROP_UINT32(HPET_INTCAP, HPETState, intcap, 0), + DEFINE_PROP_BOOL("hpet-offset-saved", HPETState, hpet_offset_saved, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/compat.h b/include/hw/compat.h index 263de973a7..7f31850dfa 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -2,7 +2,11 @@ #define HW_COMPAT_H #define HW_COMPAT_2_11 \ - /* empty */ + {\ + .driver = "hpet",\ + .property = "hpet-offset-saved",\ + .value = "false",\ + }, #define HW_COMPAT_2_10 \ {\ From 0b368a10c71af96f6cf93b0ba5c2ee3bdbd50e96 Mon Sep 17 00:00:00 2001 From: Jan Dakinevich Date: Wed, 27 Dec 2017 17:04:26 +0300 Subject: [PATCH 05/51] i386/cpu/kvm: look at PMU's CPUID before setting MSRs Certain PMU-related MSRs are not supported for CPUs with PMU architecture below version 2. KVM rejects any access to them (see intel_is_valid_msr_idx routine in KVM), and QEMU fails on the following assertion: kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed. QEMU also could fail if KVM exposes less fixed counters then 3. It could happen if host system run inside another hypervisor, which is tweaking PMU-related CPUID. To prevent possible fail, number of fixed counters now is obtained in the same way as number of GP counters. Reviewed-by: Roman Kagan Signed-off-by: Jan Dakinevich Message-Id: <1514383466-7257-1-git-send-email-jan.dakinevich@virtuozzo.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm.c | 78 ++++++++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 6f69e2fcfd..d23127cf83 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -92,8 +92,9 @@ static bool has_msr_hv_stimer; static bool has_msr_hv_frequencies; static bool has_msr_xss; -static bool has_msr_architectural_pmu; -static uint32_t num_architectural_pmu_counters; +static uint32_t has_architectural_pmu_version; +static uint32_t num_architectural_pmu_gp_counters; +static uint32_t num_architectural_pmu_fixed_counters; static int has_xsave; static int has_xcrs; @@ -872,19 +873,28 @@ int kvm_arch_init_vcpu(CPUState *cs) } if (limit >= 0x0a) { - uint32_t ver; + uint32_t eax, edx; - cpu_x86_cpuid(env, 0x0a, 0, &ver, &unused, &unused, &unused); - if ((ver & 0xff) > 0) { - has_msr_architectural_pmu = true; - num_architectural_pmu_counters = (ver & 0xff00) >> 8; + cpu_x86_cpuid(env, 0x0a, 0, &eax, &unused, &unused, &edx); + + has_architectural_pmu_version = eax & 0xff; + if (has_architectural_pmu_version > 0) { + num_architectural_pmu_gp_counters = (eax & 0xff00) >> 8; /* Shouldn't be more than 32, since that's the number of bits * available in EBX to tell us _which_ counters are available. * Play it safe. */ - if (num_architectural_pmu_counters > MAX_GP_COUNTERS) { - num_architectural_pmu_counters = MAX_GP_COUNTERS; + if (num_architectural_pmu_gp_counters > MAX_GP_COUNTERS) { + num_architectural_pmu_gp_counters = MAX_GP_COUNTERS; + } + + if (has_architectural_pmu_version > 1) { + num_architectural_pmu_fixed_counters = edx & 0x1f; + + if (num_architectural_pmu_fixed_counters > MAX_FIXED_COUNTERS) { + num_architectural_pmu_fixed_counters = MAX_FIXED_COUNTERS; + } } } } @@ -1650,32 +1660,36 @@ static int kvm_put_msrs(X86CPU *cpu, int level) if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) { kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr); } - if (has_msr_architectural_pmu) { - /* Stop the counter. */ - kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0); + if (has_architectural_pmu_version > 0) { + if (has_architectural_pmu_version > 1) { + /* Stop the counter. */ + kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0); + } /* Set the counter values. */ - for (i = 0; i < MAX_FIXED_COUNTERS; i++) { + for (i = 0; i < num_architectural_pmu_fixed_counters; i++) { kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR0 + i, env->msr_fixed_counters[i]); } - for (i = 0; i < num_architectural_pmu_counters; i++) { + for (i = 0; i < num_architectural_pmu_gp_counters; i++) { kvm_msr_entry_add(cpu, MSR_P6_PERFCTR0 + i, env->msr_gp_counters[i]); kvm_msr_entry_add(cpu, MSR_P6_EVNTSEL0 + i, env->msr_gp_evtsel[i]); } - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS, - env->msr_global_status); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL, - env->msr_global_ovf_ctrl); + if (has_architectural_pmu_version > 1) { + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS, + env->msr_global_status); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL, + env->msr_global_ovf_ctrl); - /* Now start the PMU. */ - kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, - env->msr_fixed_ctr_ctrl); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, - env->msr_global_ctrl); + /* Now start the PMU. */ + kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, + env->msr_fixed_ctr_ctrl); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, + env->msr_global_ctrl); + } } /* * Hyper-V partition-wide MSRs: to avoid clearing them on cpu hot-add, @@ -2030,15 +2044,17 @@ static int kvm_get_msrs(X86CPU *cpu) if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) { kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0); } - if (has_msr_architectural_pmu) { - kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS, 0); - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL, 0); - for (i = 0; i < MAX_FIXED_COUNTERS; i++) { + if (has_architectural_pmu_version > 0) { + if (has_architectural_pmu_version > 1) { + kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS, 0); + kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL, 0); + } + for (i = 0; i < num_architectural_pmu_fixed_counters; i++) { kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR0 + i, 0); } - for (i = 0; i < num_architectural_pmu_counters; i++) { + for (i = 0; i < num_architectural_pmu_gp_counters; i++) { kvm_msr_entry_add(cpu, MSR_P6_PERFCTR0 + i, 0); kvm_msr_entry_add(cpu, MSR_P6_EVNTSEL0 + i, 0); } From 91e14fb8e9ee351f01ffe391d7a45cccc3b8da65 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 4 Jan 2018 22:18:33 +0800 Subject: [PATCH 06/51] chardev: use backend chr context when watch for fe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit 6bbb6c0644 ("chardev: use per-dev context for io_add_watch_poll", 2017-09-22) all the chardev watches are converted to use per-chardev gcontext to support chardev to be run outside default main thread. However that's still missing one call from the frontend code. Touch that up. Reviewed-by: Stefan Hajnoczi Reviewed-by: Marc-André Lureau Signed-off-by: Peter Xu Message-Id: <20180104141835.17987-2-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- chardev/char-fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/char-fe.c b/chardev/char-fe.c index ee6d596100..c611b3fa3e 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -356,7 +356,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond, } g_source_set_callback(src, (GSourceFunc)func, user_data, NULL); - tag = g_source_attach(src, NULL); + tag = g_source_attach(src, s->gcontext); g_source_unref(src); return tag; From 938eb9e9c83d34d90cac6ec5c5388e7998840e4e Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 4 Jan 2018 22:18:34 +0800 Subject: [PATCH 07/51] chardev: let g_idle_add() be with chardev gcontext The idle task will be attached to main gcontext even if the chardev backend is running in another gcontext. Fix the only caller by extending the g_idle_add() logic into the more powerful g_source_attach(). It's basically g_idle_add_full() implementation, but with the chardev's gcontext passed in. Signed-off-by: Peter Xu Message-Id: <20180104141835.17987-3-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- chardev/char-pty.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/chardev/char-pty.c b/chardev/char-pty.c index 761ae6dec1..8248e36ab9 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -43,7 +43,7 @@ typedef struct { /* Protected by the Chardev chr_write_lock. */ int connected; guint timer_tag; - guint open_tag; + GSource *open_source; } PtyChardev; #define PTY_CHARDEV(obj) OBJECT_CHECK(PtyChardev, (obj), TYPE_CHARDEV_PTY) @@ -58,7 +58,7 @@ static gboolean pty_chr_timer(gpointer opaque) qemu_mutex_lock(&chr->chr_write_lock); s->timer_tag = 0; - s->open_tag = 0; + s->open_source = NULL; if (!s->connected) { /* Next poll ... */ pty_chr_update_read_handler_locked(chr); @@ -183,7 +183,7 @@ static gboolean qemu_chr_be_generic_open_func(gpointer opaque) Chardev *chr = CHARDEV(opaque); PtyChardev *s = PTY_CHARDEV(opaque); - s->open_tag = 0; + s->open_source = NULL; qemu_chr_be_event(chr, CHR_EVENT_OPENED); return FALSE; } @@ -194,9 +194,10 @@ static void pty_chr_state(Chardev *chr, int connected) PtyChardev *s = PTY_CHARDEV(chr); if (!connected) { - if (s->open_tag) { - g_source_remove(s->open_tag); - s->open_tag = 0; + if (s->open_source) { + g_source_destroy(s->open_source); + g_source_unref(s->open_source); + s->open_source = NULL; } remove_fd_in_watch(chr); s->connected = 0; @@ -210,9 +211,13 @@ static void pty_chr_state(Chardev *chr, int connected) s->timer_tag = 0; } if (!s->connected) { - g_assert(s->open_tag == 0); + g_assert(s->open_source == NULL); + s->open_source = g_idle_source_new(); s->connected = 1; - s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr); + g_source_set_callback(s->open_source, + qemu_chr_be_generic_open_func, + chr, NULL); + g_source_attach(s->open_source, chr->gcontext); } if (!chr->gsource) { chr->gsource = io_add_watch_poll(chr, s->ioc, From 2c716ba1506769c9be2caa02f0f6d6e7c00f4304 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 4 Jan 2018 22:18:35 +0800 Subject: [PATCH 08/51] chardev: introduce qemu_chr_timeout_add_ms() It's a replacement of g_timeout_add[_seconds]() for chardevs. Chardevs now can have dedicated gcontext, we should always bind chardev tasks onto those gcontext rather than the default main context. Since there are quite a few of g_timeout_add[_seconds]() callers, a new function qemu_chr_timeout_add_ms() is introduced. One thing to mention is that, terminal3270 is still always running on main gcontext. However let's convert that as well since it's still part of chardev codes and in case one day we'll miss that when we move it out of main gcontext too. Also, convert all the timers from GSource tags into GSource pointers. Gsource tag IDs and g_source_remove()s can only work with default gcontext, while now these GSources can logically be attached to other contexts. So let's use explicit g_source_destroy() plus another g_source_unref() to remove a timer. Note: when in the timer handler, we don't need the g_source_destroy() any more since that'll be done automatically if the timer handler returns false (and that's what all the current handlers do). Yet another note: in pty_chr_rearm_timer() we take special care for ms=1000. This patch merged the two cases into one. Signed-off-by: Peter Xu Message-Id: <20180104141835.17987-4-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- chardev/char-pty.c | 43 +++++++++++++++++++----------------------- chardev/char-socket.c | 28 +++++++++++++++++---------- chardev/char.c | 18 ++++++++++++++++++ hw/char/terminal3270.c | 28 +++++++++++++++------------ include/chardev/char.h | 3 +++ 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/chardev/char-pty.c b/chardev/char-pty.c index 8248e36ab9..89315e6807 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -42,7 +42,7 @@ typedef struct { /* Protected by the Chardev chr_write_lock. */ int connected; - guint timer_tag; + GSource *timer_src; GSource *open_source; } PtyChardev; @@ -57,7 +57,8 @@ static gboolean pty_chr_timer(gpointer opaque) PtyChardev *s = PTY_CHARDEV(opaque); qemu_mutex_lock(&chr->chr_write_lock); - s->timer_tag = 0; + s->timer_src = NULL; + g_source_unref(s->open_source); s->open_source = NULL; if (!s->connected) { /* Next poll ... */ @@ -67,25 +68,25 @@ static gboolean pty_chr_timer(gpointer opaque) return FALSE; } +static void pty_chr_timer_cancel(PtyChardev *s) +{ + if (s->timer_src) { + g_source_destroy(s->timer_src); + g_source_unref(s->timer_src); + s->timer_src = NULL; + } +} + /* Called with chr_write_lock held. */ static void pty_chr_rearm_timer(Chardev *chr, int ms) { PtyChardev *s = PTY_CHARDEV(chr); char *name; - if (s->timer_tag) { - g_source_remove(s->timer_tag); - s->timer_tag = 0; - } - - if (ms == 1000) { - name = g_strdup_printf("pty-timer-secs-%s", chr->label); - s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr); - } else { - name = g_strdup_printf("pty-timer-ms-%s", chr->label); - s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr); - } - g_source_set_name_by_id(s->timer_tag, name); + pty_chr_timer_cancel(s); + name = g_strdup_printf("pty-timer-%s", chr->label); + s->timer_src = qemu_chr_timeout_add_ms(chr, ms, pty_chr_timer, chr); + g_source_set_name(s->timer_src, name); g_free(name); } @@ -206,10 +207,7 @@ static void pty_chr_state(Chardev *chr, int connected) * the virtual device linked to our pty. */ pty_chr_rearm_timer(chr, 1000); } else { - if (s->timer_tag) { - g_source_remove(s->timer_tag); - s->timer_tag = 0; - } + pty_chr_timer_cancel(s); if (!s->connected) { g_assert(s->open_source == NULL); s->open_source = g_idle_source_new(); @@ -236,10 +234,7 @@ static void char_pty_finalize(Object *obj) qemu_mutex_lock(&chr->chr_write_lock); pty_chr_state(chr, 0); object_unref(OBJECT(s->ioc)); - if (s->timer_tag) { - g_source_remove(s->timer_tag); - s->timer_tag = 0; - } + pty_chr_timer_cancel(s); qemu_mutex_unlock(&chr->chr_write_lock); qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } @@ -272,7 +267,7 @@ static void char_pty_open(Chardev *chr, name = g_strdup_printf("chardev-pty-%s", chr->label); qio_channel_set_name(QIO_CHANNEL(s->ioc), name); g_free(name); - s->timer_tag = 0; + s->timer_src = NULL; *be_opened = false; } diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 630a7f2995..77cdf487eb 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -57,7 +57,7 @@ typedef struct { bool is_telnet; bool is_tn3270; - guint reconnect_timer; + GSource *reconnect_timer; int64_t reconnect_time; bool connect_err_reported; } SocketChardev; @@ -67,16 +67,27 @@ typedef struct { static gboolean socket_reconnect_timeout(gpointer opaque); +static void tcp_chr_reconn_timer_cancel(SocketChardev *s) +{ + if (s->reconnect_timer) { + g_source_destroy(s->reconnect_timer); + g_source_unref(s->reconnect_timer); + s->reconnect_timer = NULL; + } +} + static void qemu_chr_socket_restart_timer(Chardev *chr) { SocketChardev *s = SOCKET_CHARDEV(chr); char *name; assert(s->connected == 0); - s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time, - socket_reconnect_timeout, chr); name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label); - g_source_set_name_by_id(s->reconnect_timer, name); + s->reconnect_timer = qemu_chr_timeout_add_ms(chr, + s->reconnect_time * 1000, + socket_reconnect_timeout, + chr); + g_source_set_name(s->reconnect_timer, name); g_free(name); } @@ -781,11 +792,7 @@ static void char_socket_finalize(Object *obj) SocketChardev *s = SOCKET_CHARDEV(obj); tcp_chr_free_connection(chr); - - if (s->reconnect_timer) { - g_source_remove(s->reconnect_timer); - s->reconnect_timer = 0; - } + tcp_chr_reconn_timer_cancel(s); qapi_free_SocketAddress(s->addr); if (s->listener) { qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL); @@ -824,7 +831,8 @@ static gboolean socket_reconnect_timeout(gpointer opaque) SocketChardev *s = SOCKET_CHARDEV(opaque); QIOChannelSocket *sioc; - s->reconnect_timer = 0; + g_source_unref(s->reconnect_timer); + s->reconnect_timer = NULL; if (chr->be_open) { return false; diff --git a/chardev/char.c b/chardev/char.c index 8c3765ee99..3e14de1920 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -1084,6 +1084,24 @@ void qmp_chardev_send_break(const char *id, Error **errp) qemu_chr_be_event(chr, CHR_EVENT_BREAK); } +/* + * Add a timeout callback for the chardev (in milliseconds), return + * the GSource object created. Please use this to add timeout hook for + * chardev instead of g_timeout_add() and g_timeout_add_seconds(), to + * make sure the gcontext that the task bound to is correct. + */ +GSource *qemu_chr_timeout_add_ms(Chardev *chr, guint ms, + GSourceFunc func, void *private) +{ + GSource *source = g_timeout_source_new(ms); + + assert(func); + g_source_set_callback(source, func, private, NULL); + g_source_attach(source, chr->gcontext); + + return source; +} + void qemu_chr_cleanup(void) { object_unparent(get_chardevs_root()); diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c index a109ce5987..e9c45e55b1 100644 --- a/hw/char/terminal3270.c +++ b/hw/char/terminal3270.c @@ -31,7 +31,7 @@ typedef struct Terminal3270 { uint8_t outv[OUTPUT_BUFFER_SIZE]; int in_len; bool handshake_done; - guint timer_tag; + GSource *timer_src; } Terminal3270; #define TYPE_TERMINAL_3270 "x-terminal3270" @@ -45,6 +45,15 @@ static int terminal_can_read(void *opaque) return INPUT_BUFFER_SIZE - t->in_len; } +static void terminal_timer_cancel(Terminal3270 *t) +{ + if (t->timer_src) { + g_source_destroy(t->timer_src); + g_source_unref(t->timer_src); + t->timer_src = NULL; + } +} + /* * Protocol handshake done, * signal guest by an unsolicited DE irq. @@ -90,12 +99,9 @@ static void terminal_read(void *opaque, const uint8_t *buf, int size) assert(size <= (INPUT_BUFFER_SIZE - t->in_len)); - if (t->timer_tag) { - g_source_remove(t->timer_tag); - t->timer_tag = 0; - } - t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t); - + terminal_timer_cancel(t); + t->timer_src = qemu_chr_timeout_add_ms(t->chr.chr, 600 * 1000, + send_timing_mark_cb, t); memcpy(&t->inv[t->in_len], buf, size); t->in_len += size; if (t->in_len < 2) { @@ -145,10 +151,7 @@ static void chr_event(void *opaque, int event) /* Ensure the initial status correct, always reset them. */ t->in_len = 0; t->handshake_done = false; - if (t->timer_tag) { - g_source_remove(t->timer_tag); - t->timer_tag = 0; - } + terminal_timer_cancel(t); switch (event) { case CHR_EVENT_OPENED: @@ -157,7 +160,8 @@ static void chr_event(void *opaque, int event) * char-socket.c. Once qemu receives the terminal-type of the * client, mark handshake done and trigger everything rolling again. */ - t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t); + t->timer_src = qemu_chr_timeout_add_ms(t->chr.chr, 600 * 1000, + send_timing_mark_cb, t); break; case CHR_EVENT_CLOSED: sch->curr_status.scsw.dstat = SCSW_DSTAT_DEVICE_END; diff --git a/include/chardev/char.h b/include/chardev/char.h index 778d610295..d8941fcbb1 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -256,6 +256,9 @@ Chardev *qemu_chardev_new(const char *id, const char *typename, extern int term_escape_char; +GSource *qemu_chr_timeout_add_ms(Chardev *chr, guint ms, + GSourceFunc func, void *private); + /* console.c */ void qemu_chr_parse_vc(QemuOpts *opts, ChardevBackend *backend, Error **errp); From 14ab3aa7dc43b2eebbdf8b04a3b351b5ca5e13fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:06 +0100 Subject: [PATCH 09/51] build-sys: fix qemu-ga -pthread linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When linking qemu-ga under some configuration (when gthread-2.0.pc doesn't have -pthread, as happening atm with meson build), you may have this linking issue: /usr/bin/ld: libqemuutil.a(qemu-thread-posix.o): undefined reference to symbol 'pthread_setname_np@@GLIBC_2.12' /usr/lib64/libpthread.so.0: error adding symbols: DSO missing from command line Make sure qemu-ga links with the pthread library, by adding correct flags to libs_qga. This is really a QEMU bug, because it's QEMU code that's using pthread functions, and so we must explicitly link against pthreads. The bug was just masked by the fact that often some pkg-config or another for one of our dependencies will add -pthread to the link line anyway. Signed-off-by: Marc-André Lureau Reviewed-by: Peter Maydell Message-Id: <20180104160523.22995-2-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 89bd662a6a..ac392d2fda 100755 --- a/configure +++ b/configure @@ -3464,6 +3464,7 @@ else done if test "$found" = "no"; then LIBS="$pthread_lib $LIBS" + libs_qga="$pthread_lib $libs_qga" fi PTHREAD_LIB="$pthread_lib" break From 42a77f1ce4934b243df003f95bda88530631387a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:07 +0100 Subject: [PATCH 10/51] build-sys: silence make by default or V=0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move generic make flags in MAKEFLAGS (SUBDIR_MAKEFLAGS is more qemu specific). Use --quiet to silence make 'is up to date' message. Signed-off-by: Marc-André Lureau Tested-by: Eric Blake Reviewed-by: Paolo Bonzini Message-Id: <20180104160523.22995-3-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- Makefile | 2 +- rules.mak | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d86ecd2dd4..1671db3bdd 100644 --- a/Makefile +++ b/Makefile @@ -277,7 +277,7 @@ else DOCS= endif -SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR) +SUBDIR_MAKEFLAGS=BUILD_DIR=$(BUILD_DIR) SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS)) SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS)) diff --git a/rules.mak b/rules.mak index 6e943335f3..5fb4951561 100644 --- a/rules.mak +++ b/rules.mak @@ -131,6 +131,8 @@ modules: # If called with only a single argument, will print nothing in quiet mode. quiet-command = $(if $(V),$1,$(if $(2),@printf " %-7s %s\n" $2 $3 && $1, @$1)) +MAKEFLAGS += $(if $(V),,--no-print-directory --quiet) + # cc-option # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0) From de1da442ea38b7f4627072df0cd80f3537cf8390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:08 +0100 Subject: [PATCH 11/51] build-sys: add a rule to print a variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $ make print-CFLAGS CFLAGS=-fsanitize=address -Og -g Trick from various sources: https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile https://www.cmcrossroads.com/article/printing-value-makefile-variable Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20180104160523.22995-4-marcandre.lureau@redhat.com> Tested-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- Makefile | 5 ++++- docs/devel/build-system.txt | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1671db3bdd..f26ef1b1df 100644 --- a/Makefile +++ b/Makefile @@ -8,9 +8,12 @@ SRC_PATH=. UNCHECKED_GOALS := %clean TAGS cscope ctags dist \ html info pdf txt \ - help check-help \ + help check-help print-% \ docker docker-% vm-test vm-build-% +print-%: + @echo '$*=$($*)' + # All following code might depend on configuration variables ifneq ($(wildcard config-host.mak),) # Put the all: rule here so that config-host.mak can contain dependencies. diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt index 386ef36ee3..52501f2ad9 100644 --- a/docs/devel/build-system.txt +++ b/docs/devel/build-system.txt @@ -510,3 +510,16 @@ default-configs/$TARGET-NAME file as input. This is the entrypoint used when make recurses to build a single system or userspace emulator target. It is merely a symlink back to the Makefile.target in the top level. + + +Useful make targets +=================== + +- help + + Print a help message for the most common build targets. + +- print-VAR + + Print the value of the variable VAR. Useful for debugging the build + system. From 906548689e37ab6cca1e93b3f8d9327a4e17e8af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:09 +0100 Subject: [PATCH 12/51] build-sys: compile with -Og or -O1 when --enable-debug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When --enable-debug is turned on, configure doesn't set -O level, and uses default compiler -O0 level, which is slow. Instead, use -Og if supported by the compiler (optimize debugging experience), or -O1 (keeps code somewhat debuggable and works around compiler bugs). Unfortunately, gcc has many false-positive maybe-uninitialized errors with Og and O1 (f27 gcc 7.2.1 20170915): /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c: In function ‘ipmi_kcs_ioport_read’: /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c:279:12: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] return ret; ^~~ cc1: all warnings being treated as errors make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ipmi/isa_ipmi_kcs.o] Error 1 make: *** Waiting for unfinished jobs.... /home/elmarco/src/qemu/hw/ide/ahci.c: In function ‘ahci_populate_sglist’: /home/elmarco/src/qemu/hw/ide/ahci.c:903:58: error: ‘tbl_entry_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) { ~~~~~~~~~^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ide/ahci.o] Error 1 /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_add_memslot’: /home/elmarco/src/qemu/hw/display/qxl.c:1397:52: error: ‘pci_start’ may be used uninitialized in this function [-Werror=maybe-uninitialized] memslot.virt_end = virt_start + (guest_end - pci_start); ~~~~~~~~~~~~~^~~~~~~~~~~~ /home/elmarco/src/qemu/hw/display/qxl.c:1389:9: error: ‘pci_region’ may be used uninitialized in this function [-Werror=maybe-uninitialized] qxl_set_guest_bug(d, "%s: pci_region = %d", __func__, pci_region); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors There seems to be a long list of related bugs in upstream GCC, some of them are being fixed very recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639 For now, let's workaround it by using Wno-maybe-uninitialized (gcc-only). Suggested-by: Paolo Bonzini Signed-off-by: Marc-André Lureau Message-Id: <20180104160523.22995-5-marcandre.lureau@redhat.com> Tested-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- configure | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/configure b/configure index ac392d2fda..6f1b7cd83d 100755 --- a/configure +++ b/configure @@ -5194,8 +5194,19 @@ if test "$gcov" = "yes" ; then LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS" elif test "$fortify_source" = "yes" ; then CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" -elif test "$debug" = "no"; then - CFLAGS="-O2 $CFLAGS" +elif test "$debug" = "yes"; then + if compile_prog "-Og" ""; then + CFLAGS="-Og $CFLAGS" + elif compile_prog "-O1" ""; then + CFLAGS="-O1 $CFLAGS" + fi + # Workaround GCC false-positive Wuninitialized bugs with Og or O1: + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639 + if cc_has_warning_flag "-Wno-maybe-uninitialized"; then + CFLAGS="-Wno-maybe-uninitialized $CFLAGS" + fi +else + CFLAGS="-O2 $CFLAGS" fi ########################################## From c08d08b27cadb0809eea25b49d8128e9335b5dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:10 +0100 Subject: [PATCH 13/51] tests/docker: add some sanitizers to fedora dockerfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build fedora image with ASAN/UBSan support. Signed-off-by: Marc-André Lureau Message-Id: <20180104160523.22995-6-marcandre.lureau@redhat.com> Tested-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- tests/docker/dockerfiles/fedora.docker | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 4b26c3aded..32de731675 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -3,7 +3,7 @@ ENV PACKAGES \ ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \ glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \ gcc gcc-c++ clang make perl which bc findutils libaio-devel \ - nettle-devel \ + nettle-devel libasan libubsan \ mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \ mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \ mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \ @@ -15,4 +15,4 @@ ENV PACKAGES \ RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt -ENV FEATURES mingw clang pyyaml +ENV FEATURES mingw clang pyyaml asan From 11545663d1277c5aecb6ccbcbf6cce78340df7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:11 +0100 Subject: [PATCH 14/51] tests/docker: add test-debug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new test with --enable-debug using clang/asan/ubsan, remove --enable-debug from test-clang & test-mingw. Signed-off-by: Marc-André Lureau Message-Id: <20180104160523.22995-7-marcandre.lureau@redhat.com> Tested-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- tests/docker/test-clang | 2 +- tests/docker/test-debug | 26 ++++++++++++++++++++++++++ tests/docker/test-mingw | 2 -- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100755 tests/docker/test-debug diff --git a/tests/docker/test-clang b/tests/docker/test-clang index 1eb61a3af7..e90a793178 100755 --- a/tests/docker/test-clang +++ b/tests/docker/test-clang @@ -17,7 +17,7 @@ requires clang cd "$BUILD_DIR" -OPTS="--enable-debug --cxx=clang++ --cc=clang --host-cc=clang" +OPTS="--cxx=clang++ --cc=clang --host-cc=clang" # -fsanitize=undefined is broken on Fedora 23, skip it for now # See also: https://bugzilla.redhat.com/show_bug.cgi?id=1263834 #OPTS="$OPTS --extra-cflags=-fsanitize=undefined \ diff --git a/tests/docker/test-debug b/tests/docker/test-debug new file mode 100755 index 0000000000..d020b06917 --- /dev/null +++ b/tests/docker/test-debug @@ -0,0 +1,26 @@ +#!/bin/bash -e +# +# Compile and check with clang & --enable-debug. +# +# Copyright (c) 2016-2018 Red Hat Inc. +# +# Authors: +# Fam Zheng +# Marc-André Lureau +# +# This work is licensed under the terms of the GNU GPL, version 2 +# or (at your option) any later version. See the COPYING file in +# the top-level directory. + +. common.rc + +requires clang asan + +cd "$BUILD_DIR" + +OPTS="--cxx=clang++ --cc=clang --host-cc=clang" +OPTS="--enable-debug $OPTS" + +build_qemu $OPTS +make $MAKEFLAGS check +install_qemu diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw index 39a1da448e..503a6bc6f7 100755 --- a/tests/docker/test-mingw +++ b/tests/docker/test-mingw @@ -22,7 +22,6 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \ build_qemu --cross-prefix=$prefix \ --enable-trace-backends=simple \ - --enable-debug \ --enable-gnutls \ --enable-nettle \ --enable-curl \ @@ -35,4 +34,3 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do make clean done - From 87c258cd1e1c10faaeee8016ab6c67de97d6b996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:13 +0100 Subject: [PATCH 15/51] tests: fix check-qobject leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /public/qobject_is_equal_conversion: OK ================================================================= ==14396==ERROR: LeakSanitizer: detected memory leaks Direct leak of 56 byte(s) in 1 object(s) allocated from: #0 0x7f07682c5850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7f0767d12f0c in g_malloc ../glib/gmem.c:94 #2 0x7f0767d131cf in g_malloc_n ../glib/gmem.c:331 #3 0x562bd767371f in do_test_equality /home/elmarco/src/qq/tests/check-qobject.c:49 #4 0x562bd7674a35 in qobject_is_equal_dict_test /home/elmarco/src/qq/tests/check-qobject.c:267 #5 0x7f0767d37b04 in test_case_run ../glib/gtestutils.c:2237 #6 0x7f0767d37ec4 in g_test_run_suite_internal ../glib/gtestutils.c:2321 #7 0x7f0767d37f6d in g_test_run_suite_internal ../glib/gtestutils.c:2333 #8 0x7f0767d38184 in g_test_run_suite ../glib/gtestutils.c:2408 #9 0x7f0767d36e0d in g_test_run ../glib/gtestutils.c:1674 #10 0x562bd7674e75 in main /home/elmarco/src/qq/tests/check-qobject.c:327 #11 0x7f0766009039 in __libc_start_main (/lib64/libc.so.6+0x21039) Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20180104160523.22995-9-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- tests/check-qobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/check-qobject.c b/tests/check-qobject.c index 03e9175113..710f9e6b0a 100644 --- a/tests/check-qobject.c +++ b/tests/check-qobject.c @@ -59,6 +59,8 @@ static void do_test_equality(bool expected, int _, ...) g_assert(qobject_is_equal(args[i], args[j]) == expected); } } + + g_free(args); } #define check_equal(...) \ From 354711279fcc532cee310ed8098f51403dfef5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:14 +0100 Subject: [PATCH 16/51] vl: fix direct firmware directories leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that data_dir[] will now point to allocated strings. Fixes: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x7f1448181850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7f1446ed8f0c in g_malloc ../glib/gmem.c:94 #2 0x7f1446ed91cf in g_malloc_n ../glib/gmem.c:331 #3 0x7f1446ef739a in g_strsplit ../glib/gstrfuncs.c:2364 #4 0x55cf276439d7 in main /home/elmarco/src/qq/vl.c:4311 #5 0x7f143dfad039 in __libc_start_main (/lib64/libc.so.6+0x21039) Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20180104160523.22995-10-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- vl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vl.c b/vl.c index 444b7507da..3599485226 100644 --- a/vl.c +++ b/vl.c @@ -2318,7 +2318,7 @@ static void qemu_add_data_dir(const char *path) return; /* duplicate */ } } - data_dir[data_dir_idx++] = path; + data_dir[data_dir_idx++] = g_strdup(path); } static inline bool nonempty_str(const char *str) @@ -3078,7 +3078,7 @@ int main(int argc, char **argv, char **envp) Error *main_loop_err = NULL; Error *err = NULL; bool list_data_dirs = false; - char **dirs; + char *dir, **dirs; typedef struct BlockdevOptions_queue { BlockdevOptions *bdo; Location loc; @@ -4181,9 +4181,12 @@ int main(int argc, char **argv, char **envp) for (i = 0; dirs[i] != NULL; i++) { qemu_add_data_dir(dirs[i]); } + g_strfreev(dirs); /* try to find datadir relative to the executable path */ - qemu_add_data_dir(os_find_datadir()); + dir = os_find_datadir(); + qemu_add_data_dir(dir); + g_free(dir); /* add the datadir specified when building */ qemu_add_data_dir(CONFIG_QEMU_DATADIR); From e5dc1a6c6c4359cd783810f63eb68e9e09350708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:15 +0100 Subject: [PATCH 17/51] readline: add a free function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes leaks such as: Direct leak of 2 byte(s) in 1 object(s) allocated from: #0 0x7eff58beb850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7eff57942f0c in g_malloc ../glib/gmem.c:94 #2 0x7eff579431cf in g_malloc_n ../glib/gmem.c:331 #3 0x7eff5795f6eb in g_strdup ../glib/gstrfuncs.c:363 #4 0x55db720f1d46 in readline_hist_add /home/elmarco/src/qq/util/readline.c:258 #5 0x55db720f2d34 in readline_handle_byte /home/elmarco/src/qq/util/readline.c:387 #6 0x55db71539d00 in monitor_read /home/elmarco/src/qq/monitor.c:3896 #7 0x55db71f9be35 in qemu_chr_be_write_impl /home/elmarco/src/qq/chardev/char.c:167 #8 0x55db71f9bed3 in qemu_chr_be_write /home/elmarco/src/qq/chardev/char.c:179 #9 0x55db71fa013c in fd_chr_read /home/elmarco/src/qq/chardev/char-fd.c:66 #10 0x55db71fe18a8 in qio_channel_fd_source_dispatch /home/elmarco/src/qq/io/channel-watch.c:84 #11 0x7eff5793a90b in g_main_dispatch ../glib/gmain.c:3182 #12 0x7eff5793b7ac in g_main_context_dispatch ../glib/gmain.c:3847 #13 0x55db720af3bd in glib_pollfds_poll /home/elmarco/src/qq/util/main-loop.c:214 #14 0x55db720af505 in os_host_main_loop_wait /home/elmarco/src/qq/util/main-loop.c:261 #15 0x55db720af6d6 in main_loop_wait /home/elmarco/src/qq/util/main-loop.c:515 #16 0x55db7184e0de in main_loop /home/elmarco/src/qq/vl.c:1995 #17 0x55db7185e956 in main /home/elmarco/src/qq/vl.c:4914 #18 0x7eff4ea17039 in __libc_start_main (/lib64/libc.so.6+0x21039) (while at it, use g_new0(ReadLineState), it's a bit easier to read) Signed-off-by: Marc-André Lureau Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20180104160523.22995-11-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- include/qemu/readline.h | 1 + monitor.c | 2 +- util/readline.c | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/include/qemu/readline.h b/include/qemu/readline.h index c08cf7400e..e81258322b 100644 --- a/include/qemu/readline.h +++ b/include/qemu/readline.h @@ -59,5 +59,6 @@ ReadLineState *readline_init(ReadLinePrintfFunc *printf_func, ReadLineFlushFunc *flush_func, void *opaque, ReadLineCompletionFunc *completion_finder); +void readline_free(ReadLineState *rs); #endif /* READLINE_H */ diff --git a/monitor.c b/monitor.c index d682eee2d8..b9da5e20d1 100644 --- a/monitor.c +++ b/monitor.c @@ -583,7 +583,7 @@ static void monitor_data_destroy(Monitor *mon) if (monitor_is_qmp(mon)) { json_message_parser_destroy(&mon->qmp.parser); } - g_free(mon->rs); + readline_free(mon->rs); QDECREF(mon->outbuf); qemu_mutex_destroy(&mon->out_lock); } diff --git a/util/readline.c b/util/readline.c index bbdee790b0..24ec839854 100644 --- a/util/readline.c +++ b/util/readline.c @@ -500,12 +500,28 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index) return rs->history[index]; } +void readline_free(ReadLineState *rs) +{ + int i; + + if (!rs) { + return; + } + for (i = 0; i < READLINE_MAX_CMDS; i++) { + g_free(rs->history[i]); + } + for (i = 0; i < READLINE_MAX_COMPLETIONS; i++) { + g_free(rs->completions[i]); + } + g_free(rs); +} + ReadLineState *readline_init(ReadLinePrintfFunc *printf_func, ReadLineFlushFunc *flush_func, void *opaque, ReadLineCompletionFunc *completion_finder) { - ReadLineState *rs = g_malloc0(sizeof(*rs)); + ReadLineState *rs = g_new0(ReadLineState, 1); rs->hist_entry = -1; rs->opaque = opaque; From 890241ab6942a0186eaf485dabf266a5a7aac428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:16 +0100 Subject: [PATCH 18/51] tests: fix migration-test leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct leak of 12 byte(s) in 2 object(s) allocated from: #0 0x7f50d403c850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7f50d1ddf98f in vasprintf (/lib64/libc.so.6+0x8098f) Signed-off-by: Marc-André Lureau Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20180104160523.22995-12-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- tests/migration-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index be598d3257..799e24ebc6 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who, const char *parameter, const char *value) { QDict *rsp, *rsp_return; - const char *result; + char *result; rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }"); rsp_return = qdict_get_qdict(rsp, "return"); result = g_strdup_printf("%" PRId64, qdict_get_try_int(rsp_return, parameter, -1)); g_assert_cmpstr(result, ==, value); + g_free(result); QDECREF(rsp); } From 83e33300a2342c5d0bf48474fdf8da22c22b4973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:17 +0100 Subject: [PATCH 19/51] crypto: fix stack-buffer-overflow error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ASAN complains about: ==8856==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd8a1fe168 at pc 0x561136cb4451 bp 0x7ffd8a1fe130 sp 0x7ffd8a1fd8e0 READ of size 16 at 0x7ffd8a1fe168 thread T0 #0 0x561136cb4450 in __asan_memcpy (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) #1 0x561136d2a6a7 in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:83:5 #2 0x561136d29af8 in qcrypto_ivgen_calculate /home/elmarco/src/qq/crypto/ivgen.c:72:12 #3 0x561136d07c8e in test_ivgen /home/elmarco/src/qq/tests/test-crypto-ivgen.c:148:5 #4 0x7f77772c3b04 in test_case_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2237 #5 0x7f77772c3ec4 in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2321 #6 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333 #7 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333 #8 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333 #9 0x7f77772c4184 in g_test_run_suite /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2408 #10 0x7f77772c2e0d in g_test_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:1674 #11 0x561136d0799b in main /home/elmarco/src/qq/tests/test-crypto-ivgen.c:173:12 #12 0x7f77756e6039 in __libc_start_main (/lib64/libc.so.6+0x21039) #13 0x561136c13d89 in _start (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x6fd89) Address 0x7ffd8a1fe168 is located in stack of thread T0 at offset 40 in frame #0 0x561136d2a40f in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:76 This frame has 1 object(s): [32, 40) 'sector.addr' <== Memory access at offset 40 overflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) in __asan_memcpy Shadow bytes around the buggy address: 0x100031437bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x100031437c20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00[f3]f3 f3 0x100031437c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100031437c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb It looks like the rest of the code copes with ndata being larger than sizeof(sector), so limit the memcpy() range. Signed-off-by: Marc-André Lureau Reviewed-by: Daniel P. Berrange Message-Id: <20180104160523.22995-13-marcandre.lureau@redhat.com> Tested-by: Thomas Huth Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- crypto/ivgen-essiv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c index cba20bde6c..ad4d926c19 100644 --- a/crypto/ivgen-essiv.c +++ b/crypto/ivgen-essiv.c @@ -79,7 +79,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen, uint8_t *data = g_new(uint8_t, ndata); sector = cpu_to_le64(sector); - memcpy(data, (uint8_t *)§or, ndata); + memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata)); if (sizeof(sector) < ndata) { memset(data + sizeof(sector), 0, ndata - sizeof(sector)); } From b11e20fb6c658bc13b2e4dfc1b86c2eb8731e374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:18 +0100 Subject: [PATCH 20/51] qemu-config: fix leak in query-command-line-options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct leak of 160 byte(s) in 4 object(s) allocated from: #0 0x55ed7678cda8 in calloc (/home/elmarco/src/qq/build/x86_64-softmmu/qemu-system-x86_64+0x797da8) #1 0x7f3f5e725f75 in g_malloc0 /home/elmarco/src/gnome/glib/builddir/../glib/gmem.c:124 #2 0x55ed778aa3a7 in query_option_descs /home/elmarco/src/qq/util/qemu-config.c:60:16 #3 0x55ed778aa307 in get_drive_infolist /home/elmarco/src/qq/util/qemu-config.c:140:19 #4 0x55ed778a9f40 in qmp_query_command_line_options /home/elmarco/src/qq/util/qemu-config.c:254:36 #5 0x55ed76d4868c in qmp_marshal_query_command_line_options /home/elmarco/src/qq/build/qmp-marshal.c:3078:14 #6 0x55ed77855dd5 in do_qmp_dispatch /home/elmarco/src/qq/qapi/qmp-dispatch.c:104:5 #7 0x55ed778558cc in qmp_dispatch /home/elmarco/src/qq/qapi/qmp-dispatch.c:131:11 #8 0x55ed768b592f in handle_qmp_command /home/elmarco/src/qq/monitor.c:3840:11 #9 0x55ed7786ccfe in json_message_process_token /home/elmarco/src/qq/qobject/json-streamer.c:105:5 #10 0x55ed778fe37c in json_lexer_feed_char /home/elmarco/src/qq/qobject/json-lexer.c:323:13 #11 0x55ed778fdde6 in json_lexer_feed /home/elmarco/src/qq/qobject/json-lexer.c:373:15 #12 0x55ed7786cd83 in json_message_parser_feed /home/elmarco/src/qq/qobject/json-streamer.c:124:12 #13 0x55ed768b559e in monitor_qmp_read /home/elmarco/src/qq/monitor.c:3882:5 #14 0x55ed77714f29 in qemu_chr_be_write_impl /home/elmarco/src/qq/chardev/char.c:167:9 #15 0x55ed77714fde in qemu_chr_be_write /home/elmarco/src/qq/chardev/char.c:179:9 #16 0x55ed7772ffad in tcp_chr_read /home/elmarco/src/qq/chardev/char-socket.c:440:13 #17 0x55ed7777113b in qio_channel_fd_source_dispatch /home/elmarco/src/qq/io/channel-watch.c:84:12 #18 0x7f3f5e71d90b in g_main_dispatch /home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3182 #19 0x7f3f5e71e7ac in g_main_context_dispatch /home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3847 #20 0x55ed77886ffc in glib_pollfds_poll /home/elmarco/src/qq/util/main-loop.c:214:9 #21 0x55ed778865fd in os_host_main_loop_wait /home/elmarco/src/qq/util/main-loop.c:261:5 #22 0x55ed77886222 in main_loop_wait /home/elmarco/src/qq/util/main-loop.c:515:11 #23 0x55ed76d2a4df in main_loop /home/elmarco/src/qq/vl.c:1995:9 #24 0x55ed76d1cb4a in main /home/elmarco/src/qq/vl.c:4914:5 #25 0x7f3f555f6039 in __libc_start_main (/lib64/libc.so.6+0x21039) Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20180104160523.22995-14-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- util/qemu-config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/qemu-config.c b/util/qemu-config.c index 99b0e46fa3..029fec53a9 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -105,7 +105,8 @@ static void cleanup_infolist(CommandLineParameterInfoList *head) if (!strcmp(pre_entry->value->name, cur->next->value->name)) { del_entry = cur->next; cur->next = cur->next->next; - g_free(del_entry); + del_entry->next = NULL; + qapi_free_CommandLineParameterInfoList(del_entry); break; } pre_entry = pre_entry->next; From e313d5cec564a9b708bad1bb44c291530a5a4935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:19 +0100 Subject: [PATCH 21/51] tests: fix qmp-test leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct leak of 913 byte(s) in 43 object(s) allocated from: #0 0x55880a15df60 in __interceptor_malloc (/home/elmarco/src/qq/build/tests/qmp-test+0x110f60) #1 0x7f3f20fd098f in _IO_vasprintf (/lib64/libc.so.6+0x8098f) Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20180104160523.22995-15-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- tests/qmp-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qmp-test.c b/tests/qmp-test.c index c5a5c10b41..36feb2204b 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -271,7 +271,7 @@ static void add_query_tests(QmpSchema *schema) { SchemaInfoList *tail; SchemaInfo *si, *arg_type, *ret_type; - const char *test_name; + char *test_name; /* Test the query-like commands */ for (tail = schema->list; tail; tail = tail->next) { @@ -297,6 +297,7 @@ static void add_query_tests(QmpSchema *schema) test_name = g_strdup_printf("qmp/%s", si->name); qtest_add_data_func(test_name, si->name, test_query); + g_free(test_name); } } From 6b2fef739127ee6135d5ccc2da0bf1f3bebf66b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:21 +0100 Subject: [PATCH 22/51] tests: fix coroutine leak in /basic/entered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coroutine is not finished by the time the test ends, resulting in ASAN warning: ==7005==ERROR: LeakSanitizer: detected memory leaks Direct leak of 312 byte(s) in 1 object(s) allocated from: #0 0x7fd35290fa38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38) #1 0x7fd3506c5f75 in g_malloc0 ../glib/gmem.c:124 #2 0x55994af03e47 in qemu_coroutine_new /home/elmarco/src/qemu/util/coroutine-ucontext.c:144 #3 0x55994aefed99 in qemu_coroutine_create /home/elmarco/src/qemu/util/qemu-coroutine.c:76 #4 0x55994ac1eb50 in verify_entered_step_1 /home/elmarco/src/qemu/tests/test-coroutine.c:80 #5 0x55994af03c75 in coroutine_trampoline /home/elmarco/src/qemu/util/coroutine-ucontext.c:119 #6 0x7fd34ec02bef (/lib64/libc.so.6+0x50bef) Do not yield() to let the coroutine terminate. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi Message-Id: <20180104160523.22995-17-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- tests/test-coroutine.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c index abd97c23c1..76c646107e 100644 --- a/tests/test-coroutine.c +++ b/tests/test-coroutine.c @@ -67,7 +67,6 @@ static void coroutine_fn verify_entered_step_2(void *opaque) /* Once more to check it still works after yielding */ g_assert(qemu_coroutine_entered(caller)); g_assert(qemu_coroutine_entered(qemu_coroutine_self())); - qemu_coroutine_yield(); } static void coroutine_fn verify_entered_step_1(void *opaque) From b7438458a1f801096afe984e959855e77d22dc2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:22 +0100 Subject: [PATCH 23/51] mips: fix potential fopen(NULL,...) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spotted thanks to ASAN. Signed-off-by: Marc-André Lureau Message-Id: <20180104160523.22995-18-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- hw/nvram/ds1225y.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/nvram/ds1225y.c b/hw/nvram/ds1225y.c index 57d5ab2154..ad7345f288 100644 --- a/hw/nvram/ds1225y.c +++ b/hw/nvram/ds1225y.c @@ -80,7 +80,7 @@ static int nvram_post_load(void *opaque, int version_id) } /* Write back nvram contents */ - s->file = fopen(s->filename, "wb"); + s->file = s->filename ? fopen(s->filename, "wb") : NULL; if (s->file) { /* Write back contents, as 'wb' mode cleaned the file */ if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) { @@ -126,7 +126,7 @@ static int nvram_sysbus_initfn(SysBusDevice *dev) sysbus_init_mmio(dev, &s->iomem); /* Read current file */ - file = fopen(s->filename, "rb"); + file = s->filename ? fopen(s->filename, "rb") : NULL; if (file) { /* Read nvram contents */ if (fread(s->contents, s->chip_size, 1, file) != 1) { From 02a2ad217b0de16cc6c4f36fe325907cdbe7766b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jan 2018 17:05:23 +0100 Subject: [PATCH 24/51] disas/s390: fix global-buffer-overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spotted thanks to ASAN: ==25226==ERROR: AddressSanitizer: global-buffer-overflow on address 0x556715a1f120 at pc 0x556714b6f6b1 bp 0x7ffcdfac1360 sp 0x7ffcdfac1350 READ of size 1 at 0x556715a1f120 thread T0 #0 0x556714b6f6b0 in init_disasm /home/elmarco/src/qemu/disas/s390.c:219 #1 0x556714b6fa6a in print_insn_s390 /home/elmarco/src/qemu/disas/s390.c:294 #2 0x55671484d031 in monitor_disas /home/elmarco/src/qemu/disas.c:635 #3 0x556714862ec0 in memory_dump /home/elmarco/src/qemu/monitor.c:1324 #4 0x55671486342a in hmp_memory_dump /home/elmarco/src/qemu/monitor.c:1418 #5 0x5567148670be in handle_hmp_command /home/elmarco/src/qemu/monitor.c:3109 #6 0x5567148674ed in qmp_human_monitor_command /home/elmarco/src/qemu/monitor.c:613 #7 0x556714b00918 in qmp_marshal_human_monitor_command /home/elmarco/src/qemu/build/qmp-marshal.c:1704 #8 0x556715138a3e in do_qmp_dispatch /home/elmarco/src/qemu/qapi/qmp-dispatch.c:104 #9 0x556715138f83 in qmp_dispatch /home/elmarco/src/qemu/qapi/qmp-dispatch.c:131 #10 0x55671485cf88 in handle_qmp_command /home/elmarco/src/qemu/monitor.c:3839 #11 0x55671514e80b in json_message_process_token /home/elmarco/src/qemu/qobject/json-streamer.c:105 #12 0x5567151bf2dc in json_lexer_feed_char /home/elmarco/src/qemu/qobject/json-lexer.c:323 #13 0x5567151bf827 in json_lexer_feed /home/elmarco/src/qemu/qobject/json-lexer.c:373 #14 0x55671514ee62 in json_message_parser_feed /home/elmarco/src/qemu/qobject/json-streamer.c:124 #15 0x556714854b1f in monitor_qmp_read /home/elmarco/src/qemu/monitor.c:3881 #16 0x556715045440 in qemu_chr_be_write_impl /home/elmarco/src/qemu/chardev/char.c:172 #17 0x556715047184 in qemu_chr_be_write /home/elmarco/src/qemu/chardev/char.c:184 #18 0x55671505a8e6 in tcp_chr_read /home/elmarco/src/qemu/chardev/char-socket.c:440 #19 0x5567150943c3 in qio_channel_fd_source_dispatch /home/elmarco/src/qemu/io/channel-watch.c:84 #20 0x7fb90292b90b in g_main_dispatch ../glib/gmain.c:3182 #21 0x7fb90292c7ac in g_main_context_dispatch ../glib/gmain.c:3847 #22 0x556715162eca in glib_pollfds_poll /home/elmarco/src/qemu/util/main-loop.c:214 #23 0x556715163001 in os_host_main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:261 #24 0x5567151631fa in main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:515 #25 0x556714ad6d3b in main_loop /home/elmarco/src/qemu/vl.c:1950 #26 0x556714ade329 in main /home/elmarco/src/qemu/vl.c:4865 #27 0x7fb8fe5c9009 in __libc_start_main (/lib64/libc.so.6+0x21009) #28 0x5567147af4d9 in _start (/home/elmarco/src/qemu/build/s390x-softmmu/qemu-system-s390x+0xf674d9) 0x556715a1f120 is located 32 bytes to the left of global variable 'char_hci_type_info' defined in '/home/elmarco/src/qemu/hw/bt/hci-csr.c:493:23' (0x556715a1f140) of size 104 0x556715a1f120 is located 8 bytes to the right of global variable 's390_opcodes' defined in '/home/elmarco/src/qemu/disas/s390.c:860:33' (0x556715a15280) of size 40600 This fix is based on Andreas Arnez upstream commit: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=9ace48f3d7d80ce09c5df60cccb433470410b11b 2014-08-19 Andreas Arnez * s390-dis.c (init_disasm): Simplify initialization of opc_index[]. This also fixes an access after the last element of s390_opcodes[]. Signed-off-by: Marc-André Lureau Message-Id: <20180104160523.22995-19-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- disas/s390.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/disas/s390.c b/disas/s390.c index 1f167d2eaa..6393860239 100644 --- a/disas/s390.c +++ b/disas/s390.c @@ -207,18 +207,14 @@ static int opc_index[256]; static void init_disasm (struct disassemble_info *info) { - const struct s390_opcode *opcode; - const struct s390_opcode *opcode_end; + int i; memset (opc_index, 0, sizeof (opc_index)); - opcode_end = s390_opcodes + s390_num_opcodes; - for (opcode = s390_opcodes; opcode < opcode_end; opcode++) - { - opc_index[(int) opcode->opcode[0]] = opcode - s390_opcodes; - while ((opcode < opcode_end) && - (opcode[1].opcode[0] == opcode->opcode[0])) - opcode++; - } + + /* Reverse order, such that each opc_index ends up pointing to the + first matching entry instead of the last. */ + for (i = s390_num_opcodes; i--; ) + opc_index[s390_opcodes[i].opcode[0]] = i; #ifdef QEMU_DISABLE switch (info->mach) From 24355b79bdaf6ab12f7c610b032fc35ec045cd55 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 4 Jan 2018 14:25:02 +0000 Subject: [PATCH 25/51] scsi-disk: release AioContext in unaligned WRITE SAME case scsi_write_same_complete() can retry the write if the request was unaligned. Make sure to release the AioContext when that code path is taken! This patch fixes a hang when QEMU terminates after an unaligned WRITE SAME request has been processed with dataplane. The hang occurs because iothread_stop_all() cannot acquire the AioContext lock that was leaked by the IOThread in scsi_write_same_complete(). Fixes: b9e413dd37 ("block: explicitly acquire aiocontext in aio callbacks that need it"). Cc: Paolo Bonzini Cc: qemu-stable@nongnu.org Reported-by: Cong Li Signed-off-by: Stefan Hajnoczi Message-Id: <20180104142502.15175-1-stefanha@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index e58833a087..49d2559d93 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -1755,6 +1755,7 @@ static void scsi_write_same_complete(void *opaque, int ret) data->sector << BDRV_SECTOR_BITS, &data->qiov, 0, scsi_write_same_complete, data); + aio_context_release(blk_get_aio_context(s->qdev.conf.blk)); return; } From acf53766fc26fd90a0b06bae81638502d249f7e7 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 30 Nov 2017 09:53:05 +0100 Subject: [PATCH 26/51] tests/boot-serial-test: Add tests for microblaze boards This adds two simple TCG + UART tests for the microblaze boards, one in big endian mode, and one in little endian mode. Signed-off-by: Thomas Huth Message-Id: <1512031988-32490-5-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- tests/Makefile.include | 2 ++ tests/boot-serial-test.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/Makefile.include b/tests/Makefile.include index 39a4b5359d..561e14ba3d 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -300,6 +300,8 @@ check-qtest-alpha-y = tests/boot-serial-test$(EXESUF) check-qtest-m68k-y = tests/boot-serial-test$(EXESUF) +check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF) + check-qtest-mips-y = tests/endianness-test$(EXESUF) check-qtest-mips64-y = tests/endianness-test$(EXESUF) diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c index dd3828c49b..a39273a83d 100644 --- a/tests/boot-serial-test.c +++ b/tests/boot-serial-test.c @@ -24,6 +24,22 @@ static const uint8_t kernel_mcf5208[] = { 0x60, 0xfa /* bra.s loop */ }; +static const uint8_t kernel_pls3adsp1800[] = { + 0xb0, 0x00, 0x84, 0x00, /* imm 0x8400 */ + 0x30, 0x60, 0x00, 0x04, /* addik r3,r0,4 */ + 0x30, 0x80, 0x00, 0x54, /* addik r4,r0,'T' */ + 0xf0, 0x83, 0x00, 0x00, /* sbi r4,r3,0 */ + 0xb8, 0x00, 0xff, 0xfc /* bri -4 loop */ +}; + +static const uint8_t kernel_plml605[] = { + 0xe0, 0x83, 0x00, 0xb0, /* imm 0x83e0 */ + 0x00, 0x10, 0x60, 0x30, /* addik r3,r0,0x1000 */ + 0x54, 0x00, 0x80, 0x30, /* addik r4,r0,'T' */ + 0x00, 0x00, 0x83, 0xf0, /* sbi r4,r3,0 */ + 0xfc, 0xff, 0x00, 0xb8 /* bri -4 loop */ +}; + typedef struct testdef { const char *arch; /* Target architecture */ const char *machine; /* Name of the machine */ @@ -50,6 +66,10 @@ static testdef_t tests[] = { { "s390x", "s390-ccw-virtio", "-nodefaults -device sclpconsole,chardev=serial0", "virtio device" }, { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 }, + { "microblaze", "petalogix-s3adsp1800", "", "TT", + sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 }, + { "microblazeel", "petalogix-ml605", "", "TT", + sizeof(kernel_plml605), kernel_plml605 }, { NULL } }; From 7244edf22e6c158036e21567c4cf6e4ddce84092 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 30 Nov 2017 09:53:07 +0100 Subject: [PATCH 27/51] tests/boot-serial-test: Add a test for the moxiesim machine Now that moxiesim supports the -bios parameter, we can check this machine in the boot-serial tester, too, by supplying a mini bios that only writes 'T' characters to the UART. Signed-off-by: Thomas Huth Message-Id: <1512031988-32490-7-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- tests/Makefile.include | 2 ++ tests/boot-serial-test.c | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/tests/Makefile.include b/tests/Makefile.include index 561e14ba3d..13d6684c0e 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -308,6 +308,8 @@ check-qtest-mips64-y = tests/endianness-test$(EXESUF) check-qtest-mips64el-y = tests/endianness-test$(EXESUF) +check-qtest-moxie-y = tests/boot-serial-test$(EXESUF) + check-qtest-ppc-y = tests/endianness-test$(EXESUF) check-qtest-ppc-y += tests/boot-order-test$(EXESUF) check-qtest-ppc-y += tests/prom-env-test$(EXESUF) diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c index a39273a83d..1deddb8f80 100644 --- a/tests/boot-serial-test.c +++ b/tests/boot-serial-test.c @@ -40,6 +40,13 @@ static const uint8_t kernel_plml605[] = { 0xfc, 0xff, 0x00, 0xb8 /* bri -4 loop */ }; +static const uint8_t bios_moxiesim[] = { + 0x20, 0x10, 0x00, 0x00, 0x03, 0xf8, /* ldi.s r1,0x3f8 */ + 0x1b, 0x20, 0x00, 0x00, 0x00, 0x54, /* ldi.b r2,'T' */ + 0x1e, 0x12, /* st.b r1,r2 */ + 0x1a, 0x00, 0x00, 0x00, 0x10, 0x00 /* jmpa 0x1000 */ +}; + typedef struct testdef { const char *arch; /* Target architecture */ const char *machine; /* Name of the machine */ @@ -70,6 +77,7 @@ static testdef_t tests[] = { sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 }, { "microblazeel", "petalogix-ml605", "", "TT", sizeof(kernel_plml605), kernel_plml605 }, + { "moxie", "moxiesim", "", "TT", sizeof(bios_moxiesim), 0, bios_moxiesim }, { NULL } }; From 52cb6817a715dde6bc0ba531b23c2525f298925d Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 30 Nov 2017 09:53:08 +0100 Subject: [PATCH 28/51] tests/boot-serial-test: Add support for the raspi2 machine The raspi2 machine supports loading firmware images, so we can easily load a small test sequence as raw binary blob here to test the UART. Signed-off-by: Thomas Huth Message-Id: <1512031988-32490-8-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- tests/Makefile.include | 1 + tests/boot-serial-test.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/tests/Makefile.include b/tests/Makefile.include index 13d6684c0e..8883274ae1 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -362,6 +362,7 @@ check-qtest-arm-y += tests/virtio-blk-test$(EXESUF) gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF) gcov-files-arm-y += hw/timer/arm_mptimer.c +check-qtest-arm-y += tests/boot-serial-test$(EXESUF) check-qtest-aarch64-y = tests/numa-test$(EXESUF) diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c index 1deddb8f80..663b78b950 100644 --- a/tests/boot-serial-test.c +++ b/tests/boot-serial-test.c @@ -47,6 +47,14 @@ static const uint8_t bios_moxiesim[] = { 0x1a, 0x00, 0x00, 0x00, 0x10, 0x00 /* jmpa 0x1000 */ }; +static const uint8_t bios_raspi2[] = { + 0x08, 0x30, 0x9f, 0xe5, /* ldr r3,[pc,#8] Get base */ + 0x54, 0x20, 0xa0, 0xe3, /* mov r2,#'T' */ + 0x00, 0x20, 0xc3, 0xe5, /* strb r2,[r3] */ + 0xfb, 0xff, 0xff, 0xea, /* b loop */ + 0x00, 0x10, 0x20, 0x3f, /* 0x3f201000 = UART0 base addr */ +}; + typedef struct testdef { const char *arch; /* Target architecture */ const char *machine; /* Name of the machine */ @@ -78,6 +86,7 @@ static testdef_t tests[] = { { "microblazeel", "petalogix-ml605", "", "TT", sizeof(kernel_plml605), kernel_plml605 }, { "moxie", "moxiesim", "", "TT", sizeof(bios_moxiesim), 0, bios_moxiesim }, + { "arm", "raspi2", "", "TT", sizeof(bios_raspi2), 0, bios_raspi2 }, { NULL } }; From 35b1b927514d34862e352ff83ab0cbb1439b5303 Mon Sep 17 00:00:00 2001 From: Tao Wu Date: Wed, 10 Jan 2018 11:50:54 -0800 Subject: [PATCH 29/51] target/i386: move hflags update code to a function We will share the same code for hax/kvm. Signed-off-by: Tao Wu Message-Id: <20180110195056.85403-1-lepton@google.com> Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++ target/i386/cpu.h | 2 ++ target/i386/kvm.c | 40 +--------------------------------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 3818d72831..ad8196b166 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4147,6 +4147,48 @@ static void x86_disas_set_info(CPUState *cs, disassemble_info *info) info->cap_insn_split = 8; } +void x86_update_hflags(CPUX86State *env) +{ + uint32_t hflags; +#define HFLAG_COPY_MASK \ + ~( HF_CPL_MASK | HF_PE_MASK | HF_MP_MASK | HF_EM_MASK | \ + HF_TS_MASK | HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK | \ + HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \ + HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK) + + hflags = env->hflags & HFLAG_COPY_MASK; + hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; + hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT); + hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) & + (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK); + hflags |= (env->eflags & (HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK)); + + if (env->cr[4] & CR4_OSFXSR_MASK) { + hflags |= HF_OSFXSR_MASK; + } + + if (env->efer & MSR_EFER_LMA) { + hflags |= HF_LMA_MASK; + } + + if ((hflags & HF_LMA_MASK) && (env->segs[R_CS].flags & DESC_L_MASK)) { + hflags |= HF_CS32_MASK | HF_SS32_MASK | HF_CS64_MASK; + } else { + hflags |= (env->segs[R_CS].flags & DESC_B_MASK) >> + (DESC_B_SHIFT - HF_CS32_SHIFT); + hflags |= (env->segs[R_SS].flags & DESC_B_MASK) >> + (DESC_B_SHIFT - HF_SS32_SHIFT); + if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK) || + !(hflags & HF_CS32_MASK)) { + hflags |= HF_ADDSEG_MASK; + } else { + hflags |= ((env->segs[R_DS].base | env->segs[R_ES].base | + env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT; + } + } + env->hflags = hflags; +} + static Property x86_cpu_properties[] = { #ifdef CONFIG_USER_ONLY /* apic_id = 0 by default for *-user, see commit 9886e834 */ diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 62c4742703..f64e5ed827 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1778,4 +1778,6 @@ bool cpu_is_bsp(X86CPU *cpu); void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf); void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf); +void x86_update_hflags(CPUX86State* env); + #endif /* I386_CPU_H */ diff --git a/target/i386/kvm.c b/target/i386/kvm.c index d23127cf83..825aea5bd5 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -1891,7 +1891,6 @@ static int kvm_get_sregs(X86CPU *cpu) { CPUX86State *env = &cpu->env; struct kvm_sregs sregs; - uint32_t hflags; int bit, i, ret; ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS, &sregs); @@ -1933,44 +1932,7 @@ static int kvm_get_sregs(X86CPU *cpu) env->efer = sregs.efer; /* changes to apic base and cr8/tpr are read back via kvm_arch_post_run */ - -#define HFLAG_COPY_MASK \ - ~( HF_CPL_MASK | HF_PE_MASK | HF_MP_MASK | HF_EM_MASK | \ - HF_TS_MASK | HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK | \ - HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \ - HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK) - - hflags = env->hflags & HFLAG_COPY_MASK; - hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; - hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT); - hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) & - (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK); - hflags |= (env->eflags & (HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK)); - - if (env->cr[4] & CR4_OSFXSR_MASK) { - hflags |= HF_OSFXSR_MASK; - } - - if (env->efer & MSR_EFER_LMA) { - hflags |= HF_LMA_MASK; - } - - if ((hflags & HF_LMA_MASK) && (env->segs[R_CS].flags & DESC_L_MASK)) { - hflags |= HF_CS32_MASK | HF_SS32_MASK | HF_CS64_MASK; - } else { - hflags |= (env->segs[R_CS].flags & DESC_B_MASK) >> - (DESC_B_SHIFT - HF_CS32_SHIFT); - hflags |= (env->segs[R_SS].flags & DESC_B_MASK) >> - (DESC_B_SHIFT - HF_SS32_SHIFT); - if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK) || - !(hflags & HF_CS32_MASK)) { - hflags |= HF_ADDSEG_MASK; - } else { - hflags |= ((env->segs[R_DS].base | env->segs[R_ES].base | - env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT; - } - } - env->hflags = hflags; + x86_update_hflags(env); return 0; } From e527f86e3eb5a973d2e11f8ea08937bcc166d17a Mon Sep 17 00:00:00 2001 From: Tao Wu Date: Wed, 10 Jan 2018 11:50:55 -0800 Subject: [PATCH 30/51] target/i386: hax: change to use x86_update_hflags Change to use x86_update_hflags instead of keeping another copy at hax side. This also fix bug like HF_CPL_MASK should be SS.DPL, not CS.DPL. Signed-off-by: Tao Wu Message-Id: <20180110195056.85403-2-lepton@google.com> Signed-off-by: Paolo Bonzini --- target/i386/hax-all.c | 52 +------------------------------------------ 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index 3ce6950296..07df73ef47 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -782,56 +782,6 @@ static int hax_set_segments(CPUArchState *env, struct vcpu_state_t *sregs) return 0; } -/* - * After get the state from the kernel module, some - * qemu emulator state need be updated also - */ -static int hax_setup_qemu_emulator(CPUArchState *env) -{ - -#define HFLAG_COPY_MASK (~( \ - HF_CPL_MASK | HF_PE_MASK | HF_MP_MASK | HF_EM_MASK | \ - HF_TS_MASK | HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK | \ - HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \ - HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK)) - - uint32_t hflags; - - hflags = (env->segs[R_CS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; - hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT); - hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) & - (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK); - hflags |= (env->eflags & (HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK)); - hflags |= (env->cr[4] & CR4_OSFXSR_MASK) << - (HF_OSFXSR_SHIFT - CR4_OSFXSR_SHIFT); - - if (env->efer & MSR_EFER_LMA) { - hflags |= HF_LMA_MASK; - } - - if ((hflags & HF_LMA_MASK) && (env->segs[R_CS].flags & DESC_L_MASK)) { - hflags |= HF_CS32_MASK | HF_SS32_MASK | HF_CS64_MASK; - } else { - hflags |= (env->segs[R_CS].flags & DESC_B_MASK) >> - (DESC_B_SHIFT - HF_CS32_SHIFT); - hflags |= (env->segs[R_SS].flags & DESC_B_MASK) >> - (DESC_B_SHIFT - HF_SS32_SHIFT); - if (!(env->cr[0] & CR0_PE_MASK) || - (env->eflags & VM_MASK) || !(hflags & HF_CS32_MASK)) { - hflags |= HF_ADDSEG_MASK; - } else { - hflags |= ((env->segs[R_DS].base | - env->segs[R_ES].base | - env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT; - } - } - - hflags &= ~HF_SMM_MASK; - - env->hflags = (env->hflags & HFLAG_COPY_MASK) | hflags; - return 0; -} - static int hax_sync_vcpu_register(CPUArchState *env, int set) { struct vcpu_state_t regs; @@ -888,7 +838,7 @@ static int hax_sync_vcpu_register(CPUArchState *env, int set) } } if (!set) { - hax_setup_qemu_emulator(env); + x86_update_hflags(env); } return 0; } From df16af8741398fee4f8bd5112a00730b1ec6a0f6 Mon Sep 17 00:00:00 2001 From: Tao Wu Date: Wed, 10 Jan 2018 11:50:56 -0800 Subject: [PATCH 31/51] target/i386: hax: Move x86_update_hflags. x86_update_hflags reference env->efer which is updated in hax_get_msrs, so it has to be called after hax_get_msrs. This fix the bug that sometimes dump_state show 32 bits regs even in 64 bits mode. Signed-off-by: Tao Wu Message-Id: <20180110195056.85403-3-lepton@google.com> Signed-off-by: Paolo Bonzini --- target/i386/hax-all.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index 07df73ef47..934ec4afd1 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -837,9 +837,6 @@ static int hax_sync_vcpu_register(CPUArchState *env, int set) return -1; } } - if (!set) { - x86_update_hflags(env); - } return 0; } @@ -1020,6 +1017,7 @@ static int hax_arch_get_registers(CPUArchState *env) return ret; } + x86_update_hflags(env); return 0; } From 809092f313a7430afcd8cf53d1392624915a90e0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 11 Jan 2018 14:18:12 +0100 Subject: [PATCH 32/51] target-i386: update hflags on Hypervisor.framework This ensures that x86_cpu_dump_state shows registers with the correct size. Signed-off-by: Paolo Bonzini --- target/i386/hvf/x86hvf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c index 71c0515073..7803e09a28 100644 --- a/target/i386/hvf/x86hvf.c +++ b/target/i386/hvf/x86hvf.c @@ -297,7 +297,6 @@ int hvf_get_registers(CPUState *cpu_state) X86CPU *x86cpu = X86_CPU(cpu_state); CPUX86State *env = &x86cpu->env; - env->regs[R_EAX] = rreg(cpu_state->hvf_fd, HV_X86_RAX); env->regs[R_EBX] = rreg(cpu_state->hvf_fd, HV_X86_RBX); env->regs[R_ECX] = rreg(cpu_state->hvf_fd, HV_X86_RCX); @@ -333,6 +332,7 @@ int hvf_get_registers(CPUState *cpu_state) env->dr[6] = rreg(cpu_state->hvf_fd, HV_X86_DR6); env->dr[7] = rreg(cpu_state->hvf_fd, HV_X86_DR7); + x86_update_hflags(env); return 0; } From f1cd52d8912f124cc513a00143fe3baeb44772d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 11 Jan 2018 11:27:11 +0300 Subject: [PATCH 33/51] scripts/qemu-gdb: add simple tcg lock status helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a simple helper to dump lock state. Signed-off-by: Alex Bennée Signed-off-by: Paolo Bonzini --- scripts/qemu-gdb.py | 3 ++- scripts/qemugdb/tcg.py | 46 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 scripts/qemugdb/tcg.py diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py index b3f8e04f77..d58213e549 100644 --- a/scripts/qemu-gdb.py +++ b/scripts/qemu-gdb.py @@ -26,7 +26,7 @@ sys.path.append(os.path.dirname(__file__)) -from qemugdb import aio, mtree, coroutine +from qemugdb import aio, mtree, coroutine, tcg class QemuCommand(gdb.Command): '''Prefix for QEMU debug support commands''' @@ -38,6 +38,7 @@ def __init__(self): coroutine.CoroutineCommand() mtree.MtreeCommand() aio.HandlersCommand() +tcg.TCGLockStatusCommand() coroutine.CoroutineSPFunction() coroutine.CoroutinePCFunction() diff --git a/scripts/qemugdb/tcg.py b/scripts/qemugdb/tcg.py new file mode 100644 index 0000000000..8c7f1d7454 --- /dev/null +++ b/scripts/qemugdb/tcg.py @@ -0,0 +1,46 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# +# GDB debugging support, TCG status +# +# Copyright 2016 Linaro Ltd +# +# Authors: +# Alex Bennée +# +# This work is licensed under the terms of the GNU GPL, version 2. See +# the COPYING file in the top-level directory. +# +# Contributions after 2012-01-13 are licensed under the terms of the +# GNU GPL, version 2 or (at your option) any later version. + +# 'qemu tcg-lock-status' -- display the TCG lock status across threads + +import gdb + +class TCGLockStatusCommand(gdb.Command): + '''Display TCG Execution Status''' + def __init__(self): + gdb.Command.__init__(self, 'qemu tcg-lock-status', gdb.COMMAND_DATA, + gdb.COMPLETE_NONE) + + def invoke(self, arg, from_tty): + gdb.write("Thread, BQL (iothread_mutex), Replay, Blocked?\n") + for thread in gdb.inferiors()[0].threads(): + thread.switch() + + iothread = gdb.parse_and_eval("iothread_locked") + replay = gdb.parse_and_eval("replay_locked") + + frame = gdb.selected_frame() + if frame.name() == "__lll_lock_wait": + frame.older().select() + mutex = gdb.parse_and_eval("mutex") + owner = gdb.parse_and_eval("mutex->__data.__owner") + blocked = ("__lll_lock_wait waiting on %s from %d" % + (mutex, owner)) + else: + blocked = "not blocked" + + gdb.write("%d/%d, %s, %s, %s\n" % (thread.num, thread.ptid[1], + iothread, replay, blocked)) From c24999fa53e581335413c33f68748c1fc7b3d84b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 11 Jan 2018 11:27:33 +0300 Subject: [PATCH 34/51] scripts/qemu-gdb/timers.py: new helper to dump timer state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces the qemu-gdb command "qemu timers" which will dump the state of the main timers in the system. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- scripts/qemu-gdb.py | 3 ++- scripts/qemugdb/timers.py | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 scripts/qemugdb/timers.py diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py index d58213e549..690827e6fc 100644 --- a/scripts/qemu-gdb.py +++ b/scripts/qemu-gdb.py @@ -26,7 +26,7 @@ sys.path.append(os.path.dirname(__file__)) -from qemugdb import aio, mtree, coroutine, tcg +from qemugdb import aio, mtree, coroutine, tcg, timers class QemuCommand(gdb.Command): '''Prefix for QEMU debug support commands''' @@ -39,6 +39,7 @@ def __init__(self): mtree.MtreeCommand() aio.HandlersCommand() tcg.TCGLockStatusCommand() +timers.TimersCommand() coroutine.CoroutineSPFunction() coroutine.CoroutinePCFunction() diff --git a/scripts/qemugdb/timers.py b/scripts/qemugdb/timers.py new file mode 100644 index 0000000000..be71a001e3 --- /dev/null +++ b/scripts/qemugdb/timers.py @@ -0,0 +1,54 @@ +#!/usr/bin/python +# GDB debugging support +# +# Copyright 2017 Linaro Ltd +# +# Author: Alex Bennée +# +# This work is licensed under the terms of the GNU GPL, version 2. See +# the COPYING file in the top-level directory. + +# 'qemu timers' -- display the current timerlists + +import gdb + +class TimersCommand(gdb.Command): + '''Display the current QEMU timers''' + + def __init__(self): + 'Register the class as a gdb command' + gdb.Command.__init__(self, 'qemu timers', gdb.COMMAND_DATA, + gdb.COMPLETE_NONE) + + def dump_timers(self, timer): + "Follow a timer and recursively dump each one in the list." + # timer should be of type QemuTimer + gdb.write(" timer %s/%s (cb:%s,opq:%s)\n" % ( + timer['expire_time'], + timer['scale'], + timer['cb'], + timer['opaque'])) + + if int(timer['next']) > 0: + self.dump_timers(timer['next']) + + + def process_timerlist(self, tlist, ttype): + gdb.write("Processing %s timers\n" % (ttype)) + gdb.write(" clock %s is enabled:%s, last:%s\n" % ( + tlist['clock']['type'], + tlist['clock']['enabled'], + tlist['clock']['last'])) + if int(tlist['active_timers']) > 0: + self.dump_timers(tlist['active_timers']) + + + def invoke(self, arg, from_tty): + 'Run the command' + main_timers = gdb.parse_and_eval("main_loop_tlg") + + # This will break if QEMUClockType in timer.h is redfined + self.process_timerlist(main_timers['tl'][0], "Realtime") + self.process_timerlist(main_timers['tl'][1], "Virtual") + self.process_timerlist(main_timers['tl'][2], "Host") + self.process_timerlist(main_timers['tl'][3], "Virtual RT") From b39e3f34c9de7ead6a11a74aa2de78baf41d81a7 Mon Sep 17 00:00:00 2001 From: Pavel Dovgalyuk Date: Thu, 11 Jan 2018 11:26:10 +0300 Subject: [PATCH 35/51] icount: fixed saving/restoring of icount warp timers This patch adds saving and restoring of the icount warp timers in the vmstate. It is needed because there timers affect the virtual clock value. Therefore determinism of the execution in icount record/replay mode depends on determinism of the timers. Signed-off-by: Pavel Dovgalyuk Acked-by: Paolo Bonzini Signed-off-by: Paolo Bonzini Signed-off-by: Pavel Dovgalyuk --- cpus.c | 85 +++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/cpus.c b/cpus.c index e8139de534..f99253746a 100644 --- a/cpus.c +++ b/cpus.c @@ -120,16 +120,11 @@ static bool all_cpu_threads_idle(void) /* Protected by TimersState seqlock */ static bool icount_sleep = true; -static int64_t vm_clock_warp_start = -1; /* Conversion factor from emulated instructions to virtual clock ticks. */ static int icount_time_shift; /* Arbitrarily pick 1MIPS as the minimum allowable speed. */ #define MAX_ICOUNT_SHIFT 10 -static QEMUTimer *icount_rt_timer; -static QEMUTimer *icount_vm_timer; -static QEMUTimer *icount_warp_timer; - typedef struct TimersState { /* Protected by BQL. */ int64_t cpu_ticks_prev; @@ -147,6 +142,11 @@ typedef struct TimersState { int64_t qemu_icount_bias; /* Only written by TCG thread */ int64_t qemu_icount; + /* for adjusting icount */ + int64_t vm_clock_warp_start; + QEMUTimer *icount_rt_timer; + QEMUTimer *icount_vm_timer; + QEMUTimer *icount_warp_timer; } TimersState; static TimersState timers_state; @@ -432,14 +432,14 @@ static void icount_adjust(void) static void icount_adjust_rt(void *opaque) { - timer_mod(icount_rt_timer, + timer_mod(timers_state.icount_rt_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_RT) + 1000); icount_adjust(); } static void icount_adjust_vm(void *opaque) { - timer_mod(icount_vm_timer, + timer_mod(timers_state.icount_vm_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + NANOSECONDS_PER_SECOND / 10); icount_adjust(); @@ -460,7 +460,7 @@ static void icount_warp_rt(void) */ do { seq = seqlock_read_begin(&timers_state.vm_clock_seqlock); - warp_start = vm_clock_warp_start; + warp_start = timers_state.vm_clock_warp_start; } while (seqlock_read_retry(&timers_state.vm_clock_seqlock, seq)); if (warp_start == -1) { @@ -473,7 +473,7 @@ static void icount_warp_rt(void) cpu_get_clock_locked()); int64_t warp_delta; - warp_delta = clock - vm_clock_warp_start; + warp_delta = clock - timers_state.vm_clock_warp_start; if (use_icount == 2) { /* * In adaptive mode, do not let QEMU_CLOCK_VIRTUAL run too @@ -485,7 +485,7 @@ static void icount_warp_rt(void) } timers_state.qemu_icount_bias += warp_delta; } - vm_clock_warp_start = -1; + timers_state.vm_clock_warp_start = -1; seqlock_write_end(&timers_state.vm_clock_seqlock); if (qemu_clock_expired(QEMU_CLOCK_VIRTUAL)) { @@ -594,11 +594,13 @@ void qemu_start_warp_timer(void) * every 100ms. */ seqlock_write_begin(&timers_state.vm_clock_seqlock); - if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) { - vm_clock_warp_start = clock; + if (timers_state.vm_clock_warp_start == -1 + || timers_state.vm_clock_warp_start > clock) { + timers_state.vm_clock_warp_start = clock; } seqlock_write_end(&timers_state.vm_clock_seqlock); - timer_mod_anticipate(icount_warp_timer, clock + deadline); + timer_mod_anticipate(timers_state.icount_warp_timer, + clock + deadline); } } else if (deadline == 0) { qemu_clock_notify(QEMU_CLOCK_VIRTUAL); @@ -623,7 +625,7 @@ static void qemu_account_warp_timer(void) return; } - timer_del(icount_warp_timer); + timer_del(timers_state.icount_warp_timer); icount_warp_rt(); } @@ -632,6 +634,45 @@ static bool icount_state_needed(void *opaque) return use_icount; } +static bool warp_timer_state_needed(void *opaque) +{ + TimersState *s = opaque; + return s->icount_warp_timer != NULL; +} + +static bool adjust_timers_state_needed(void *opaque) +{ + TimersState *s = opaque; + return s->icount_rt_timer != NULL; +} + +/* + * Subsection for warp timer migration is optional, because may not be created + */ +static const VMStateDescription icount_vmstate_warp_timer = { + .name = "timer/icount/warp_timer", + .version_id = 1, + .minimum_version_id = 1, + .needed = warp_timer_state_needed, + .fields = (VMStateField[]) { + VMSTATE_INT64(vm_clock_warp_start, TimersState), + VMSTATE_TIMER_PTR(icount_warp_timer, TimersState), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription icount_vmstate_adjust_timers = { + .name = "timer/icount/timers", + .version_id = 1, + .minimum_version_id = 1, + .needed = adjust_timers_state_needed, + .fields = (VMStateField[]) { + VMSTATE_TIMER_PTR(icount_rt_timer, TimersState), + VMSTATE_TIMER_PTR(icount_vm_timer, TimersState), + VMSTATE_END_OF_LIST() + } +}; + /* * This is a subsection for icount migration. */ @@ -644,6 +685,11 @@ static const VMStateDescription icount_vmstate_timers = { VMSTATE_INT64(qemu_icount_bias, TimersState), VMSTATE_INT64(qemu_icount, TimersState), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription*[]) { + &icount_vmstate_warp_timer, + &icount_vmstate_adjust_timers, + NULL } }; @@ -754,7 +800,7 @@ void configure_icount(QemuOpts *opts, Error **errp) icount_sleep = qemu_opt_get_bool(opts, "sleep", true); if (icount_sleep) { - icount_warp_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT, + timers_state.icount_warp_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT, icount_timer_cb, NULL); } @@ -788,13 +834,14 @@ void configure_icount(QemuOpts *opts, Error **errp) the virtual time trigger catches emulated time passing too fast. Realtime triggers occur even when idle, so use them less frequently than VM triggers. */ - icount_rt_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL_RT, + timers_state.vm_clock_warp_start = -1; + timers_state.icount_rt_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL_RT, icount_adjust_rt, NULL); - timer_mod(icount_rt_timer, + timer_mod(timers_state.icount_rt_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_RT) + 1000); - icount_vm_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, + timers_state.icount_vm_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, icount_adjust_vm, NULL); - timer_mod(icount_vm_timer, + timer_mod(timers_state.icount_vm_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + NANOSECONDS_PER_SECOND / 10); } From db08b687cdd5319286665aabd34f82665630416f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 11 Jan 2018 13:53:12 +0100 Subject: [PATCH 36/51] cpus: unify qemu_*_wait_io_event Except for round-robin TCG, every other accelerator is using more or less the same code around qemu_wait_io_event_common. The exception is HAX, which also has to eat the dummy APC that is queued by qemu_cpu_kick_thread. We can add the SleepEx call to qemu_wait_io_event under "if (!tcg_enabled())", since that is the condition that is used in qemu_cpu_kick_thread, and unify the function for KVM, HAX, HVF and multi-threaded TCG. Single-threaded TCG code can also be simplified since it is only used in the round-robin, sleep-if-all-CPUs-idle case. Signed-off-by: Paolo Bonzini --- cpus.c | 49 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/cpus.c b/cpus.c index f99253746a..2cb0af9b22 100644 --- a/cpus.c +++ b/cpus.c @@ -909,7 +909,8 @@ static void kick_tcg_thread(void *opaque) static void start_tcg_kick_timer(void) { - if (!mttcg_enabled && !tcg_kick_vcpu_timer && CPU_NEXT(first_cpu)) { + assert(!mttcg_enabled); + if (!tcg_kick_vcpu_timer && CPU_NEXT(first_cpu)) { tcg_kick_vcpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kick_tcg_thread, NULL); timer_mod(tcg_kick_vcpu_timer, qemu_tcg_next_kick()); @@ -918,6 +919,7 @@ static void start_tcg_kick_timer(void) static void stop_tcg_kick_timer(void) { + assert(!mttcg_enabled); if (tcg_kick_vcpu_timer) { timer_del(tcg_kick_vcpu_timer); tcg_kick_vcpu_timer = NULL; @@ -1137,18 +1139,9 @@ static void qemu_wait_io_event_common(CPUState *cpu) process_queued_cpu_work(cpu); } -static bool qemu_tcg_should_sleep(CPUState *cpu) +static void qemu_tcg_rr_wait_io_event(CPUState *cpu) { - if (mttcg_enabled) { - return cpu_thread_is_idle(cpu); - } else { - return all_cpu_threads_idle(); - } -} - -static void qemu_tcg_wait_io_event(CPUState *cpu) -{ - while (qemu_tcg_should_sleep(cpu)) { + while (all_cpu_threads_idle()) { stop_tcg_kick_timer(); qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex); } @@ -1158,20 +1151,18 @@ static void qemu_tcg_wait_io_event(CPUState *cpu) qemu_wait_io_event_common(cpu); } -static void qemu_kvm_wait_io_event(CPUState *cpu) +static void qemu_wait_io_event(CPUState *cpu) { while (cpu_thread_is_idle(cpu)) { qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex); } - qemu_wait_io_event_common(cpu); -} - -static void qemu_hvf_wait_io_event(CPUState *cpu) -{ - while (cpu_thread_is_idle(cpu)) { - qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex); +#ifdef _WIN32 + /* Eat dummy APC queued by qemu_cpu_kick_thread. */ + if (!tcg_enabled()) { + SleepEx(0, TRUE); } +#endif qemu_wait_io_event_common(cpu); } @@ -1207,7 +1198,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) cpu_handle_guest_debug(cpu); } } - qemu_kvm_wait_io_event(cpu); + qemu_wait_io_event(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); qemu_kvm_destroy_vcpu(cpu); @@ -1253,7 +1244,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg) exit(1); } qemu_mutex_lock_iothread(); - qemu_wait_io_event_common(cpu); + qemu_wait_io_event(cpu); } return NULL; @@ -1470,7 +1461,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg) atomic_mb_set(&cpu->exit_request, 0); } - qemu_tcg_wait_io_event(cpu ? cpu : QTAILQ_FIRST(&cpus)); + qemu_tcg_rr_wait_io_event(cpu ? cpu : QTAILQ_FIRST(&cpus)); deal_with_unplugged_cpus(); } @@ -1501,13 +1492,7 @@ static void *qemu_hax_cpu_thread_fn(void *arg) } } - while (cpu_thread_is_idle(cpu)) { - qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex); - } -#ifdef _WIN32 - SleepEx(0, TRUE); -#endif - qemu_wait_io_event_common(cpu); + qemu_wait_io_event(cpu); } return NULL; } @@ -1544,7 +1529,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg) cpu_handle_guest_debug(cpu); } } - qemu_hvf_wait_io_event(cpu); + qemu_wait_io_event(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); hvf_vcpu_destroy(cpu); @@ -1623,7 +1608,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) } atomic_mb_set(&cpu->exit_request, 0); - qemu_tcg_wait_io_event(cpu); + qemu_wait_io_event(cpu); } return NULL; From 01960e6d21dcfbfc8a03d8fd6284c448cf75865b Mon Sep 17 00:00:00 2001 From: linzhecheng Date: Mon, 25 Dec 2017 10:47:04 +0800 Subject: [PATCH 37/51] irq: fix memory leak entry is moved from list but is not freed. Signed-off-by: linzhecheng Message-Id: <20171225024704.19540-1-linzhecheng@huawei.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 825aea5bd5..4912f4d538 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -3470,6 +3470,7 @@ int kvm_arch_release_virq_post(int virq) if (entry->virq == virq) { trace_kvm_x86_remove_msi_route(virq); QLIST_REMOVE(entry, list); + g_free(entry); break; } } From 1b4c0a0436f8741ad19e03a5822d1c546dfceecc Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:27 -0600 Subject: [PATCH 38/51] net: Drop unusual use of do { } while (0); For a couple of macros in pcnet.c, we have to provide a new scope to avoid compiler warnings about declarations in the middle of a switch statement that aren't in a sub-scope. But use of 'do { ... } while (0);' merely to provide that new scope is arcane overkill, compared to just using '{ ... }'. Signed-off-by: Eric Blake Reviewed-by: Thomas Huth Message-Id: <20171201232433.25193-2-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- hw/net/pcnet.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 39d5d93525..606b05c09f 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -456,32 +456,32 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd, #define CHECK_RMD(ADDR,RES) do { \ switch (BCR_SWSTYLE(s)) { \ case 0x00: \ - do { \ + { \ uint16_t rda[4]; \ s->phys_mem_read(s->dma_opaque, (ADDR), \ (void *)&rda[0], sizeof(rda), 0); \ (RES) |= (rda[2] & 0xf000)!=0xf000; \ (RES) |= (rda[3] & 0xf000)!=0x0000; \ - } while (0); \ + } \ break; \ case 0x01: \ case 0x02: \ - do { \ + { \ uint32_t rda[4]; \ s->phys_mem_read(s->dma_opaque, (ADDR), \ (void *)&rda[0], sizeof(rda), 0); \ (RES) |= (rda[1] & 0x0000f000L)!=0x0000f000L; \ (RES) |= (rda[2] & 0x0000f000L)!=0x00000000L; \ - } while (0); \ + } \ break; \ case 0x03: \ - do { \ + { \ uint32_t rda[4]; \ s->phys_mem_read(s->dma_opaque, (ADDR), \ (void *)&rda[0], sizeof(rda), 0); \ (RES) |= (rda[0] & 0x0000f000L)!=0x00000000L; \ (RES) |= (rda[1] & 0x0000f000L)!=0x0000f000L; \ - } while (0); \ + } \ break; \ } \ } while (0) @@ -489,22 +489,22 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd, #define CHECK_TMD(ADDR,RES) do { \ switch (BCR_SWSTYLE(s)) { \ case 0x00: \ - do { \ + { \ uint16_t xda[4]; \ s->phys_mem_read(s->dma_opaque, (ADDR), \ (void *)&xda[0], sizeof(xda), 0); \ (RES) |= (xda[2] & 0xf000)!=0xf000; \ - } while (0); \ + } \ break; \ case 0x01: \ case 0x02: \ case 0x03: \ - do { \ + { \ uint32_t xda[4]; \ s->phys_mem_read(s->dma_opaque, (ADDR), \ (void *)&xda[0], sizeof(xda), 0); \ (RES) |= (xda[1] & 0x0000f000L)!=0x0000f000L; \ - } while (0); \ + } \ break; \ } \ } while (0) From 94f5c480e9b5ce95394026b3f025816470e23eaf Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:28 -0600 Subject: [PATCH 39/51] mips: Tweak location of ';' in macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is more typical to provide the ';' by the caller of a macro than to embed it in the macro itself; this is because syntax highlight engines can get confused if a macro is called without a semicolon before the closing '}'. Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20171201232433.25193-3-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- target/mips/msa_helper.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c index f167a42655..8fb7a369ca 100644 --- a/target/mips/msa_helper.c +++ b/target/mips/msa_helper.c @@ -682,13 +682,13 @@ static inline int64_t msa_mod_u_df(uint32_t df, int64_t arg1, int64_t arg2) do { \ e = SIGNED_EVEN(a, df); \ o = SIGNED_ODD(a, df); \ - } while (0); + } while (0) #define UNSIGNED_EXTRACT(e, o, a, df) \ do { \ e = UNSIGNED_EVEN(a, df); \ o = UNSIGNED_ODD(a, df); \ - } while (0); + } while (0) static inline int64_t msa_dotp_s_df(uint32_t df, int64_t arg1, int64_t arg2) { @@ -1120,9 +1120,11 @@ void helper_msa_splat_df(CPUMIPSState *env, uint32_t df, uint32_t wd, #define MSA_LOOP_COND_D MSA_LOOP_COND(DF_DOUBLE) #define MSA_LOOP(DF) \ + do { \ for (i = 0; i < (MSA_LOOP_COND_ ## DF) ; i++) { \ - MSA_DO_ ## DF \ - } + MSA_DO_ ## DF; \ + } \ + } while (0) #define MSA_FN_DF(FUNC) \ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \ @@ -1135,17 +1137,17 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \ uint32_t i; \ switch (df) { \ case DF_BYTE: \ - MSA_LOOP_B \ + MSA_LOOP_B; \ break; \ case DF_HALF: \ - MSA_LOOP_H \ + MSA_LOOP_H; \ break; \ case DF_WORD: \ - MSA_LOOP_W \ + MSA_LOOP_W; \ break; \ case DF_DOUBLE: \ - MSA_LOOP_D \ - break; \ + MSA_LOOP_D; \ + break; \ default: \ assert(0); \ } \ @@ -1168,7 +1170,7 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \ do { \ R##DF(pwx, i) = pwt->DF[2*i]; \ L##DF(pwx, i) = pws->DF[2*i]; \ - } while (0); + } while (0) MSA_FN_DF(pckev_df) #undef MSA_DO @@ -1176,7 +1178,7 @@ MSA_FN_DF(pckev_df) do { \ R##DF(pwx, i) = pwt->DF[2*i+1]; \ L##DF(pwx, i) = pws->DF[2*i+1]; \ - } while (0); + } while (0) MSA_FN_DF(pckod_df) #undef MSA_DO @@ -1184,7 +1186,7 @@ MSA_FN_DF(pckod_df) do { \ pwx->DF[2*i] = L##DF(pwt, i); \ pwx->DF[2*i+1] = L##DF(pws, i); \ - } while (0); + } while (0) MSA_FN_DF(ilvl_df) #undef MSA_DO @@ -1192,7 +1194,7 @@ MSA_FN_DF(ilvl_df) do { \ pwx->DF[2*i] = R##DF(pwt, i); \ pwx->DF[2*i+1] = R##DF(pws, i); \ - } while (0); + } while (0) MSA_FN_DF(ilvr_df) #undef MSA_DO @@ -1200,7 +1202,7 @@ MSA_FN_DF(ilvr_df) do { \ pwx->DF[2*i] = pwt->DF[2*i]; \ pwx->DF[2*i+1] = pws->DF[2*i]; \ - } while (0); + } while (0) MSA_FN_DF(ilvev_df) #undef MSA_DO @@ -1208,7 +1210,7 @@ MSA_FN_DF(ilvev_df) do { \ pwx->DF[2*i] = pwt->DF[2*i+1]; \ pwx->DF[2*i+1] = pws->DF[2*i+1]; \ - } while (0); + } while (0) MSA_FN_DF(ilvod_df) #undef MSA_DO #undef MSA_LOOP_COND @@ -1222,7 +1224,7 @@ MSA_FN_DF(ilvod_df) uint32_t k = (pwd->DF[i] & 0x3f) % (2 * n); \ pwx->DF[i] = \ (pwd->DF[i] & 0xc0) ? 0 : k < n ? pwt->DF[k] : pws->DF[k - n]; \ - } while (0); + } while (0) MSA_FN_DF(vshf_df) #undef MSA_DO #undef MSA_LOOP_COND From 19a4d43ef05e323f811cf944980639449dbb39ac Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:29 -0600 Subject: [PATCH 40/51] chardev: Use goto/label instead of do/break/while(0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use of a do/while(0) control flow in order to permit an early break is an unusual paradigm, and triggers a false positive with a planned future syntax check against 'while (0);'. Rewrite the code to use a goto instead. This patch temporarily keeps an extra level of indentation to highlight the change; the next patch cleans it up. Signed-off-by: Eric Blake Message-Id: <20171201232433.25193-4-eblake@redhat.com> Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- chardev/char-serial.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/chardev/char-serial.c b/chardev/char-serial.c index 2f8f83821d..10162f9fa3 100644 --- a/chardev/char-serial.c +++ b/chardev/char-serial.c @@ -64,9 +64,14 @@ static void tty_serial_init(int fd, int speed, #endif tcgetattr(fd, &tty); -#define check_speed(val) if (speed <= val) { spd = B##val; break; } +#define check_speed(val) \ + if (speed <= val) { \ + spd = B##val; \ + goto done; \ + } + speed = speed * 10 / 11; - do { + { check_speed(50); check_speed(75); check_speed(110); @@ -125,8 +130,10 @@ static void tty_serial_init(int fd, int speed, check_speed(4000000); #endif spd = B115200; - } while (0); + } +#undef check_speed + done: cfsetispeed(&tty, spd); cfsetospeed(&tty, spd); From 539022dd6089cdef36589d608ed63cbdaacfd71f Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:30 -0600 Subject: [PATCH 41/51] chardev: Clean up previous patch indentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous patch left in an extra scope layer for ease of review; time to remove it. No semantic change. Signed-off-by: Eric Blake Message-Id: <20171201232433.25193-5-eblake@redhat.com> Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- chardev/char-serial.c | 66 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/chardev/char-serial.c b/chardev/char-serial.c index 10162f9fa3..93392c528c 100644 --- a/chardev/char-serial.c +++ b/chardev/char-serial.c @@ -71,66 +71,64 @@ static void tty_serial_init(int fd, int speed, } speed = speed * 10 / 11; - { - check_speed(50); - check_speed(75); - check_speed(110); - check_speed(134); - check_speed(150); - check_speed(200); - check_speed(300); - check_speed(600); - check_speed(1200); - check_speed(1800); - check_speed(2400); - check_speed(4800); - check_speed(9600); - check_speed(19200); - check_speed(38400); - /* Non-Posix values follow. They may be unsupported on some systems. */ - check_speed(57600); - check_speed(115200); + check_speed(50); + check_speed(75); + check_speed(110); + check_speed(134); + check_speed(150); + check_speed(200); + check_speed(300); + check_speed(600); + check_speed(1200); + check_speed(1800); + check_speed(2400); + check_speed(4800); + check_speed(9600); + check_speed(19200); + check_speed(38400); + /* Non-Posix values follow. They may be unsupported on some systems. */ + check_speed(57600); + check_speed(115200); #ifdef B230400 - check_speed(230400); + check_speed(230400); #endif #ifdef B460800 - check_speed(460800); + check_speed(460800); #endif #ifdef B500000 - check_speed(500000); + check_speed(500000); #endif #ifdef B576000 - check_speed(576000); + check_speed(576000); #endif #ifdef B921600 - check_speed(921600); + check_speed(921600); #endif #ifdef B1000000 - check_speed(1000000); + check_speed(1000000); #endif #ifdef B1152000 - check_speed(1152000); + check_speed(1152000); #endif #ifdef B1500000 - check_speed(1500000); + check_speed(1500000); #endif #ifdef B2000000 - check_speed(2000000); + check_speed(2000000); #endif #ifdef B2500000 - check_speed(2500000); + check_speed(2500000); #endif #ifdef B3000000 - check_speed(3000000); + check_speed(3000000); #endif #ifdef B3500000 - check_speed(3500000); + check_speed(3500000); #endif #ifdef B4000000 - check_speed(4000000); + check_speed(4000000); #endif - spd = B115200; - } + spd = B115200; #undef check_speed done: From 241187c11818e5223c4bdfac79f28fdf63731733 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:31 -0600 Subject: [PATCH 42/51] tests: Avoid 'do/while(false); ' in vhost-user-bridge Use of a do/while(0) loop as a way to allow break statements in the middle of execute-once code is unusual. More typical is the use of goto for early exits, with a label at the end of the execute-once code, rather than nesting code in a scope; however, the comment at the end of the existing code makes this alternative a bit unpractical. So, to avoid false positives from a future syntax check about 'while (false);', and to keep the loop form (in case someone ever does add DONTWAIT support, where they can just as easily manipulate the initial loop condition or add an if around the final 'break'), I opted to use the form of a while(1) loop (the break as an early exit is more idiomatic there), coupled with a final break preserving the original comment. Signed-off-by: Eric Blake Message-Id: <20171201232433.25193-6-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- tests/vhost-user-bridge.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index d820033a72..e0605a529e 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -283,7 +283,7 @@ vubr_backend_recv_cb(int sock, void *ctx) return; } - do { + while (1) { struct iovec *sg; ssize_t ret, total = 0; unsigned int num; @@ -343,7 +343,9 @@ vubr_backend_recv_cb(int sock, void *ctx) free(elem); elem = NULL; - } while (false); /* could loop if DONTWAIT worked? */ + + break; /* could loop if DONTWAIT worked? */ + } if (mhdr_cnt) { mhdr.num_buffers = i; From 2562755ee78983930d0662fa4d3bc5e2ac166350 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:32 -0600 Subject: [PATCH 43/51] maint: Fix macros with broken 'do/while(0); ' usage The point of writing a macro embedded in a 'do { ... } while (0)' loop (particularly if the macro has multiple statements or would otherwise end with an 'if' statement) is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if': if (cond) statement; else something else; that is the classic case where failure to use do/while(0) wrapping would cause the 'else' to pair with any embedded 'if' in the macro rather than the intended outer 'if'. But conversely, if the macro includes an embedded ';', then the same brace-less coding style would now have two statements, making the 'else' a syntax error rather than pairing with the outer 'if'. Thus, even though our coding style with required braces is not impacted, ending a macro with ';' makes our code harder to port to projects that use brace-less styles. The change should have no semantic impact. I was not able to fully compile-test all of the changes (as some of them are examples of the ugly bit-rotting debug print statements that are completely elided by default, and I didn't want to recompile with the necessary -D witnesses - cleaning those up is left as a bite-sized task for another day); I did, however, audit that for all files touched, all callers of the changed macros DID supply a trailing ';' at the callsite, and did not appear to be used as part of a brace-less conditional. Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\ Signed-off-by: Eric Blake Acked-by: Cornelia Huck Reviewed-by: Michael S. Tsirkin Acked-by: Dr. David Alan Gilbert Message-Id: <20171201232433.25193-7-eblake@redhat.com> Reviewed-by: Juan Quintela Signed-off-by: Paolo Bonzini --- audio/paaudio.c | 4 ++-- hw/adc/stm32f2xx_adc.c | 2 +- hw/block/m25p80.c | 2 +- hw/char/cadence_uart.c | 2 +- hw/char/stm32f2xx_usart.c | 2 +- hw/display/cg3.c | 2 +- hw/display/dpcd.c | 2 +- hw/display/xlnx_dp.c | 2 +- hw/dma/pl330.c | 2 +- hw/dma/xlnx-zynq-devcfg.c | 2 +- hw/dma/xlnx_dpdma.c | 2 +- hw/i2c/i2c-ddc.c | 2 +- hw/misc/auxbus.c | 2 +- hw/misc/macio/mac_dbdma.c | 4 ++-- hw/misc/mmio_interface.c | 2 +- hw/misc/stm32f2xx_syscfg.c | 2 +- hw/misc/zynq_slcr.c | 2 +- hw/net/cadence_gem.c | 2 +- hw/ssi/mss-spi.c | 2 +- hw/ssi/stm32f2xx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- hw/ssi/xilinx_spips.c | 2 +- hw/timer/a9gtimer.c | 2 +- hw/timer/cadence_ttc.c | 2 +- hw/timer/mss-timer.c | 2 +- hw/timer/stm32f2xx_timer.c | 2 +- hw/tpm/tpm_passthrough.c | 2 +- hw/tpm/tpm_tis.c | 2 +- migration/rdma.c | 2 +- target/arm/translate-a64.c | 2 +- target/s390x/kvm.c | 2 +- tests/acpi-utils.h | 8 ++++---- tests/tcg/test-mmap.c | 2 +- ui/sdl_zoom_template.h | 8 ++++---- 34 files changed, 42 insertions(+), 42 deletions(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index 65beb6f010..2a35e6f82c 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -89,7 +89,7 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x) } \ goto label; \ } \ - } while (0); + } while (0) #define CHECK_DEAD_GOTO(c, stream, rerror, label) \ do { \ @@ -107,7 +107,7 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x) } \ goto label; \ } \ - } while (0); + } while (0) static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror) { diff --git a/hw/adc/stm32f2xx_adc.c b/hw/adc/stm32f2xx_adc.c index 90fe9de299..13f31ad2f7 100644 --- a/hw/adc/stm32f2xx_adc.c +++ b/hw/adc/stm32f2xx_adc.c @@ -37,7 +37,7 @@ if (STM_ADC_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index ea142160b3..b49c8e9caa 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -40,7 +40,7 @@ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* Fields for FlashPartInfo->flags */ diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c index 6143494060..fbdbd463bb 100644 --- a/hw/char/cadence_uart.c +++ b/hw/char/cadence_uart.c @@ -33,7 +33,7 @@ #define DB_PRINT(...) do { \ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ - } while (0); + } while (0) #else #define DB_PRINT(...) #endif diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c index 268e435338..07b462d4b6 100644 --- a/hw/char/stm32f2xx_usart.c +++ b/hw/char/stm32f2xx_usart.c @@ -34,7 +34,7 @@ if (STM_USART_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/display/cg3.c b/hw/display/cg3.c index e069c4484c..cafd9f47ef 100644 --- a/hw/display/cg3.c +++ b/hw/display/cg3.c @@ -63,7 +63,7 @@ if (DEBUG_CG3) { \ printf("CG3: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define TYPE_CG3 "cgthree" #define CG3(obj) OBJECT_CHECK(CG3State, (obj), TYPE_CG3) diff --git a/hw/display/dpcd.c b/hw/display/dpcd.c index ce92ff6e2a..943002bee5 100644 --- a/hw/display/dpcd.c +++ b/hw/display/dpcd.c @@ -39,7 +39,7 @@ if (DEBUG_DPCD) { \ qemu_log("dpcd: " fmt, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define DPCD_READABLE_AREA 0x600 diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index 561f828e7a..ead4e1a0e4 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -34,7 +34,7 @@ if (DEBUG_DP) { \ qemu_log("xlnx_dp: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* * Register offset for DP. diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index 32cf8399b8..d071049233 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -29,7 +29,7 @@ if (PL330_ERR_DEBUG >= lvl) {\ fprintf(stderr, "PL330: %s:" fmt, __func__, ## args);\ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c index 3b10523430..12bb2e3716 100644 --- a/hw/dma/xlnx-zynq-devcfg.c +++ b/hw/dma/xlnx-zynq-devcfg.c @@ -43,7 +43,7 @@ if (XLNX_ZYNQ_DEVCFG_ERR_DEBUG) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) REG32(CTRL, 0x00) FIELD(CTRL, FORCE_RST, 31, 1) /* Not supported, wr ignored */ diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c index 8ceb21ddb3..077c7da9cc 100644 --- a/hw/dma/xlnx_dpdma.c +++ b/hw/dma/xlnx_dpdma.c @@ -34,7 +34,7 @@ if (DEBUG_DPDMA) { \ qemu_log("xlnx_dpdma: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* * Registers offset for DPDMA. diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c index 6b92e95c73..199dac9e41 100644 --- a/hw/i2c/i2c-ddc.c +++ b/hw/i2c/i2c-ddc.c @@ -30,7 +30,7 @@ if (DEBUG_I2CDDC) { \ qemu_log("i2c-ddc: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* Structure defining a monitor's characteristics in a * readable format: this should be passed to build_edid_blob() diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c index 1182745044..b4cacd664b 100644 --- a/hw/misc/auxbus.c +++ b/hw/misc/auxbus.c @@ -40,7 +40,7 @@ if (DEBUG_AUX) { \ qemu_log("aux: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define TYPE_AUXTOI2C "aux-to-i2c-bridge" #define AUXTOI2C(obj) OBJECT_CHECK(AUXTOI2CState, (obj), TYPE_AUXTOI2C) diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c index 0eddf2e700..1b2a69b3ef 100644 --- a/hw/misc/macio/mac_dbdma.c +++ b/hw/misc/macio/mac_dbdma.c @@ -52,7 +52,7 @@ if (DEBUG_DBDMA) { \ printf("DBDMA: " fmt , ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define DBDMA_DPRINTFCH(ch, fmt, ...) do { \ if (DEBUG_DBDMA) { \ @@ -60,7 +60,7 @@ printf("DBDMA[%02x]: " fmt , (ch)->channel, ## __VA_ARGS__); \ } \ } \ -} while (0); +} while (0) /* */ diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c index 894e9801cb..3b0e2039a3 100644 --- a/hw/misc/mmio_interface.c +++ b/hw/misc/mmio_interface.c @@ -39,7 +39,7 @@ static uint64_t mmio_interface_counter; if (DEBUG_MMIO_INTERFACE) { \ qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## __VA_ARGS__);\ } \ -} while (0); +} while (0) static void mmio_interface_init(Object *obj) { diff --git a/hw/misc/stm32f2xx_syscfg.c b/hw/misc/stm32f2xx_syscfg.c index 7c45833d09..7f10195862 100644 --- a/hw/misc/stm32f2xx_syscfg.c +++ b/hw/misc/stm32f2xx_syscfg.c @@ -34,7 +34,7 @@ if (STM_SYSCFG_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c index 44304d48be..d6bdd027ef 100644 --- a/hw/misc/zynq_slcr.c +++ b/hw/misc/zynq_slcr.c @@ -30,7 +30,7 @@ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ } \ - } while (0); + } while (0) #define XILINX_LOCK_KEY 0x767b #define XILINX_UNLOCK_KEY 0xdf0d diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index 3943187572..0fa4b0dc44 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -34,7 +34,7 @@ #define DB_PRINT(...) do { \ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ - } while (0); + } while (0) #else #define DB_PRINT(...) #endif diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c index d60daba882..185e1a3920 100644 --- a/hw/ssi/mss-spi.c +++ b/hw/ssi/mss-spi.c @@ -35,7 +35,7 @@ if (MSS_SPI_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt "\n", __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c index 26a1b4ddf5..69514da9fb 100644 --- a/hw/ssi/stm32f2xx_spi.c +++ b/hw/ssi/stm32f2xx_spi.c @@ -35,7 +35,7 @@ if (STM_SPI_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c index 33482f04de..83585bc8b2 100644 --- a/hw/ssi/xilinx_spi.c +++ b/hw/ssi/xilinx_spi.c @@ -36,7 +36,7 @@ #define DB_PRINT(...) do { \ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ - } while (0); + } while (0) #else #define DB_PRINT(...) #endif diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index d8187fadd1..85c5d0cb92 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -43,7 +43,7 @@ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* config register */ #define R_CONFIG (0x00 / 4) diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c index ce1dc63911..96d534d8a8 100644 --- a/hw/timer/a9gtimer.c +++ b/hw/timer/a9gtimer.c @@ -37,7 +37,7 @@ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__) diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c index 5e65fdb5a0..10056407ab 100644 --- a/hw/timer/cadence_ttc.c +++ b/hw/timer/cadence_ttc.c @@ -24,7 +24,7 @@ #define DB_PRINT(...) do { \ fprintf(stderr, ": %s: ", __func__); \ fprintf(stderr, ## __VA_ARGS__); \ - } while (0); + } while (0) #else #define DB_PRINT(...) #endif diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c index 60f1213a3b..4f814572e2 100644 --- a/hw/timer/mss-timer.c +++ b/hw/timer/mss-timer.c @@ -36,7 +36,7 @@ if (MSS_TIMER_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt "\n", __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c index e5f5e14a90..58fc7b1188 100644 --- a/hw/timer/stm32f2xx_timer.c +++ b/hw/timer/stm32f2xx_timer.c @@ -34,7 +34,7 @@ if (STM_TIMER_ERR_DEBUG >= lvl) { \ qemu_log("%s: " fmt, __func__, ## args); \ } \ -} while (0); +} while (0) #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args) diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 149fae63e6..29142f38bb 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -38,7 +38,7 @@ if (DEBUG_TPM) { \ fprintf(stderr, fmt, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define TYPE_TPM_PASSTHROUGH "tpm-passthrough" #define TPM_PASSTHROUGH(obj) \ diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index 561384cd86..8b5eb01a2c 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -90,7 +90,7 @@ typedef struct TPMState { if (DEBUG_TIS) { \ printf(fmt, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) /* tis registers */ #define TPM_TIS_REG_ACCESS 0x00 diff --git a/migration/rdma.c b/migration/rdma.c index ca56594328..9d5a424011 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -88,7 +88,7 @@ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; } \ return rdma->error_state; \ } \ - } while (0); + } while (0) /* * A work request ID is 64-bits and we split up these bits diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index ba94f7d045..cba5587812 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -400,7 +400,7 @@ static void unallocated_encoding(DisasContext *s) "at pc=%016" PRIx64 "\n", \ __FILE__, __LINE__, insn, s->pc - 4); \ unallocated_encoding(s); \ - } while (0); + } while (0) static void init_tmp_a64_array(DisasContext *s) { diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 9b8b59f2a2..6a18a413b4 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -58,7 +58,7 @@ if (DEBUG_KVM) { \ fprintf(stderr, fmt, ## __VA_ARGS__); \ } \ -} while (0); +} while (0) #define kvm_vm_check_mem_attr(s, attr) \ kvm_vm_check_attr(s, KVM_S390_VM_MEM_CTRL, attr) diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h index d5ca5b6238..ac52abd0dd 100644 --- a/tests/acpi-utils.h +++ b/tests/acpi-utils.h @@ -32,7 +32,7 @@ typedef struct { do { \ memread(addr, &field, sizeof(field)); \ addr += sizeof(field); \ - } while (0); + } while (0) #define ACPI_READ_ARRAY_PTR(arr, length, addr) \ do { \ @@ -40,7 +40,7 @@ typedef struct { for (idx = 0; idx < length; ++idx) { \ ACPI_READ_FIELD(arr[idx], addr); \ } \ - } while (0); + } while (0) #define ACPI_READ_ARRAY(arr, addr) \ ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr) @@ -56,7 +56,7 @@ typedef struct { ACPI_READ_FIELD((table)->oem_revision, addr); \ ACPI_READ_ARRAY((table)->asl_compiler_id, addr); \ ACPI_READ_FIELD((table)->asl_compiler_revision, addr); \ - } while (0); + } while (0) #define ACPI_ASSERT_CMP(actual, expected) do { \ char ACPI_ASSERT_CMP_str[5] = {}; \ @@ -77,7 +77,7 @@ typedef struct { ACPI_READ_FIELD((field).bit_offset, addr); \ ACPI_READ_FIELD((field).access_width, addr); \ ACPI_READ_FIELD((field).address, addr); \ - } while (0); + } while (0) uint8_t acpi_calc_checksum(const uint8_t *data, int len); diff --git a/tests/tcg/test-mmap.c b/tests/tcg/test-mmap.c index 3982fa2c72..cdefadfa4c 100644 --- a/tests/tcg/test-mmap.c +++ b/tests/tcg/test-mmap.c @@ -39,7 +39,7 @@ do \ fprintf (stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \ exit (EXIT_FAILURE); \ } \ -} while (0); +} while (0) unsigned char *dummybuf; static unsigned int pagesize; diff --git a/ui/sdl_zoom_template.h b/ui/sdl_zoom_template.h index 3bb508b51e..6a424adfb4 100644 --- a/ui/sdl_zoom_template.h +++ b/ui/sdl_zoom_template.h @@ -34,22 +34,22 @@ #define setRed(r, pcolor) do { \ *pcolor = ((*pcolor) & (~(dpf->Rmask))) + \ (((r) & (dpf->Rmask >> dpf->Rshift)) << dpf->Rshift); \ -} while (0); +} while (0) #define setGreen(g, pcolor) do { \ *pcolor = ((*pcolor) & (~(dpf->Gmask))) + \ (((g) & (dpf->Gmask >> dpf->Gshift)) << dpf->Gshift); \ -} while (0); +} while (0) #define setBlue(b, pcolor) do { \ *pcolor = ((*pcolor) & (~(dpf->Bmask))) + \ (((b) & (dpf->Bmask >> dpf->Bshift)) << dpf->Bshift); \ -} while (0); +} while (0) #define setAlpha(a, pcolor) do { \ *pcolor = ((*pcolor) & (~(dpf->Amask))) + \ (((a) & (dpf->Amask >> dpf->Ashift)) << dpf->Ashift); \ -} while (0); +} while (0) static void glue(sdl_zoom_rgb, BPP)(SDL_Surface *src, SDL_Surface *dst, int smooth, SDL_Rect *dst_rect) From f4bdc13e492208f4f9cad0ff1c14247dea1cd197 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Dec 2017 17:24:33 -0600 Subject: [PATCH 44/51] checkpatch: Enforce proper do/while (0) style Use of a loop construct for code that is not intended to repeat does not make much idiomatic sense, except in one place: it is a common usage in macros in order to wrap arbitrary code with single-statement semantics. But when used in a macro, it is more typical for the caller to supply the trailing ';' when calling the macro. Although qemu coding style frowns on bare: if (cond) statement1; else statement2; where extra semicolons actually cause syntax errors, we still want our macro styles to be easily copied to other projects. Thus, declare it an error if we encounter any form of 'while (0)' with a semicolon in the same line. Signed-off-by: Eric Blake Message-Id: <20171201232433.25193-8-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3dc27d9656..accba24a31 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1622,6 +1622,11 @@ sub process { } } +# 'do ... while (0/false)' only makes sense in macros, without trailing ';' + if ($line =~ /while\s*\((0|false)\);/) { + ERROR("suspicious ; after while (0)\n" . $herecurr); + } + # Check relative indent for conditionals and blocks. if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { my ($s, $c) = ($stat, $cond); From aa777e297c8408ee5bebc4f6a2e00071224e4a64 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Wed, 3 Jan 2018 18:33:36 +0000 Subject: [PATCH 45/51] cpu_physical_memory_sync_dirty_bitmap: Another alignment fix This code has an optimised, word aligned version, and a boring unaligned version. My commit f70d345 fixed one alignment issue, but there's another. The optimised version operates on 'longs' dealing with (typically) 64 pages at a time, replacing the whole long by a 0 and counting the bits. If the Ramblock is less than 64bits in length that long can contain bits representing two different RAMBlocks, but the code will update the bmap belinging to the 1st RAMBlock only while having updated the total dirty page count for both. This probably didn't matter prior to 6b6712ef which split the dirty bitmap by RAMBlock, but now they're separate RAMBlocks we end up with a count that doesn't match the state in the bitmaps. Symptom: Migration showing a few dirty pages left to be sent constantly Seen on aarch64 and x86 with x86+ovmf Signed-off-by: Dr. David Alan Gilbert Reported-by: Wei Huang Fixes: 6b6712efccd383b48a909bee0b29e079a57601ec Signed-off-by: Paolo Bonzini --- include/exec/ram_addr.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 6cbc02aa0f..7633ef6342 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -391,9 +391,10 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, uint64_t num_dirty = 0; unsigned long *dest = rb->bmap; - /* start address is aligned at the start of a word? */ + /* start address and length is aligned at the start of a word? */ if (((word * BITS_PER_LONG) << TARGET_PAGE_BITS) == - (start + rb->offset)) { + (start + rb->offset) && + !(length & ((BITS_PER_LONG << TARGET_PAGE_BITS) - 1))) { int k; int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS); unsigned long * const *src; From 154cc9ea3bc840199714ee7b95277c572c5d1533 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Fri, 5 Jan 2018 17:01:37 +0000 Subject: [PATCH 46/51] find_ram_offset: Add comments and tracing Add some comments so I can understand the various nested loops. Add some tracing so I can see what they're doing. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20180105170138.23357-2-dgilbert@redhat.com> Signed-off-by: Paolo Bonzini --- exec.c | 29 ++++++++++++++++++++++------- trace-events | 4 ++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/exec.c b/exec.c index 4722e521d4..5e2fb55080 100644 --- a/exec.c +++ b/exec.c @@ -1660,7 +1660,10 @@ static void *file_ram_alloc(RAMBlock *block, } #endif -/* Called with the ramlist lock held. */ +/* Allocate space within the ram_addr_t space that governs the + * dirty bitmaps. + * Called with the ramlist lock held. + */ static ram_addr_t find_ram_offset(ram_addr_t size) { RAMBlock *block, *next_block; @@ -1673,19 +1676,29 @@ static ram_addr_t find_ram_offset(ram_addr_t size) } RAMBLOCK_FOREACH(block) { - ram_addr_t end, next = RAM_ADDR_MAX; + ram_addr_t candidate, next = RAM_ADDR_MAX; - end = block->offset + block->max_length; + candidate = block->offset + block->max_length; + /* Search for the closest following block + * and find the gap. + */ RAMBLOCK_FOREACH(next_block) { - if (next_block->offset >= end) { + if (next_block->offset >= candidate) { next = MIN(next, next_block->offset); } } - if (next - end >= size && next - end < mingap) { - offset = end; - mingap = next - end; + + /* If it fits remember our place and remember the size + * of gap, but keep going so that we might find a smaller + * gap to fill so avoiding fragmentation. + */ + if (next - candidate >= size && next - candidate < mingap) { + offset = candidate; + mingap = next - candidate; } + + trace_find_ram_offset_loop(size, candidate, offset, next, mingap); } if (offset == RAM_ADDR_MAX) { @@ -1694,6 +1707,8 @@ static ram_addr_t find_ram_offset(ram_addr_t size) abort(); } + trace_find_ram_offset(size, offset); + return offset; } diff --git a/trace-events b/trace-events index 3695959d0a..ec95e67089 100644 --- a/trace-events +++ b/trace-events @@ -55,6 +55,10 @@ dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d cb=%p" dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d" dma_map_wait(void *dbs) "dbs=%p" +# # exec.c +find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" PRIx64 +find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ", offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64 + # memory.c memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" From 801110ab22be1ef258338345fc0b645af074d5bf Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Fri, 5 Jan 2018 17:01:38 +0000 Subject: [PATCH 47/51] find_ram_offset: Align ram_addr_t allocation on long boundaries The dirty bitmaps are built from 'long's and there is fast-path code for synchronising the case where the RAMBlock is aligned to the start of a long boundary. Align the allocation to this boundary to cause the fast path to be used. Offsets before change: 11398@1515169675.018566:find_ram_offset size: 0x1e0000 @ 0x8000000 11398@1515169675.020064:find_ram_offset size: 0x20000 @ 0x81e0000 11398@1515169675.020244:find_ram_offset size: 0x20000 @ 0x8200000 11398@1515169675.024343:find_ram_offset size: 0x1000000 @ 0x8220000 11398@1515169675.025154:find_ram_offset size: 0x10000 @ 0x9220000 11398@1515169675.027682:find_ram_offset size: 0x40000 @ 0x9230000 11398@1515169675.032921:find_ram_offset size: 0x200000 @ 0x9270000 11398@1515169675.033307:find_ram_offset size: 0x1000 @ 0x9470000 11398@1515169675.033601:find_ram_offset size: 0x1000 @ 0x9471000 after change: 10923@1515169108.818245:find_ram_offset size: 0x1e0000 @ 0x8000000 10923@1515169108.819410:find_ram_offset size: 0x20000 @ 0x8200000 10923@1515169108.819587:find_ram_offset size: 0x20000 @ 0x8240000 10923@1515169108.823708:find_ram_offset size: 0x1000000 @ 0x8280000 10923@1515169108.824503:find_ram_offset size: 0x10000 @ 0x9280000 10923@1515169108.827093:find_ram_offset size: 0x40000 @ 0x92c0000 10923@1515169108.833045:find_ram_offset size: 0x200000 @ 0x9300000 10923@1515169108.833504:find_ram_offset size: 0x1000 @ 0x9500000 10923@1515169108.833787:find_ram_offset size: 0x1000 @ 0x9540000 Suggested-by: Paolo Bonzini Signed-off-by: Dr. David Alan Gilbert Message-Id: <20180105170138.23357-3-dgilbert@redhat.com> Signed-off-by: Paolo Bonzini --- exec.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/exec.c b/exec.c index 5e2fb55080..9f4f4501a8 100644 --- a/exec.c +++ b/exec.c @@ -1678,7 +1678,11 @@ static ram_addr_t find_ram_offset(ram_addr_t size) RAMBLOCK_FOREACH(block) { ram_addr_t candidate, next = RAM_ADDR_MAX; + /* Align blocks to start on a 'long' in the bitmap + * which makes the bitmap sync'ing take the fast path. + */ candidate = block->offset + block->max_length; + candidate = ROUND_UP(candidate, BITS_PER_LONG << TARGET_PAGE_BITS); /* Search for the closest following block * and find the gap. From 79f9c75e1707082e56723787e6b3610a46843e20 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 8 Jan 2018 16:27:27 +0100 Subject: [PATCH 48/51] block/iscsi: fix initialization of iTask in iscsi_co_get_block_status in case of unaligned requests or on a target that does not support block provisioning we leave iTask uninitialized and check iTask.task for NULL later. Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Message-Id: <1515425247-21730-1-git-send-email-pl@kamp.de> Signed-off-by: Paolo Bonzini --- block/iscsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index 5c0a9e55b6..6a1c53711a 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -658,6 +658,8 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, uint64_t lba; int64_t ret; + iscsi_co_init_iscsitask(iscsilun, &iTask); + if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { ret = -EINVAL; goto out; @@ -675,7 +677,6 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, lba = sector_qemu2lun(sector_num, iscsilun); - iscsi_co_init_iscsitask(iscsilun, &iTask); qemu_mutex_lock(&iscsilun->mutex); retry: if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, From 15a356c49ac747a7202ed703949a178a054f2a55 Mon Sep 17 00:00:00 2001 From: Pavel Dovgalyuk Date: Wed, 10 Jan 2018 16:48:46 +0300 Subject: [PATCH 49/51] cpu: flush TB cache when loading VMState Flushing TB cache is required because TBs key in the cache may match different code which existed in the previous state. Signed-off-by: Pavel Dovgalyuk Signed-off-by: Maria Klimushenkova Message-Id: <20180110134846.12940.99993.stgit@pasha-VirtualBox> [Add comment suggested by Peter Maydell. - Paolo] Signed-off-by: Paolo Bonzini Signed-off-by: Pavel Dovgalyuk --- exec.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/exec.c b/exec.c index 9f4f4501a8..d28fc0cd3d 100644 --- a/exec.c +++ b/exec.c @@ -623,6 +623,13 @@ static int cpu_common_post_load(void *opaque, int version_id) cpu->interrupt_request &= ~0x01; tlb_flush(cpu); + /* loadvm has just updated the content of RAM, bypassing the + * usual mechanisms that ensure we flush TBs for writes to + * memory we've translated code from. So we must flush all TBs, + * which will now be stale. + */ + tb_flush(cpu); + return 0; } From 6c27a0ded992c2daddf12a225b71e42c965c4c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 11 Jan 2018 11:27:16 +0300 Subject: [PATCH 50/51] util/qemu-thread-*: add qemu_lock, locked and unlock trace events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alex Bennée Signed-off-by: Paolo Bonzini --- include/qemu/thread.h | 39 +++++++++++++++++++++++++++++++++++---- util/qemu-thread-posix.c | 21 ++++++++++++--------- util/qemu-thread-win32.c | 20 +++++++++++--------- util/trace-events | 7 ++++--- 4 files changed, 62 insertions(+), 25 deletions(-) diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 9910f49b3a..9af4e945aa 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -22,9 +22,31 @@ typedef struct QemuThread QemuThread; void qemu_mutex_init(QemuMutex *mutex); void qemu_mutex_destroy(QemuMutex *mutex); -void qemu_mutex_lock(QemuMutex *mutex); -int qemu_mutex_trylock(QemuMutex *mutex); -void qemu_mutex_unlock(QemuMutex *mutex); +int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line); +void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line); +void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int line); + +#define qemu_mutex_lock(mutex) \ + qemu_mutex_lock_impl(mutex, __FILE__, __LINE__) +#define qemu_mutex_trylock(mutex) \ + qemu_mutex_trylock_impl(mutex, __FILE__, __LINE__) +#define qemu_mutex_unlock(mutex) \ + qemu_mutex_unlock_impl(mutex, __FILE__, __LINE__) + +static inline void (qemu_mutex_lock)(QemuMutex *mutex) +{ + qemu_mutex_lock(mutex); +} + +static inline int (qemu_mutex_trylock)(QemuMutex *mutex) +{ + return qemu_mutex_trylock(mutex); +} + +static inline void (qemu_mutex_unlock)(QemuMutex *mutex) +{ + qemu_mutex_unlock(mutex); +} /* Prototypes for other functions are in thread-posix.h/thread-win32.h. */ void qemu_rec_mutex_init(QemuRecMutex *mutex); @@ -39,7 +61,16 @@ void qemu_cond_destroy(QemuCond *cond); */ void qemu_cond_signal(QemuCond *cond); void qemu_cond_broadcast(QemuCond *cond); -void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex); +void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, + const char *file, const int line); + +#define qemu_cond_wait(cond, mutex) \ + qemu_cond_wait_impl(cond, mutex, __FILE__, __LINE__) + +static inline void (qemu_cond_wait)(QemuCond *cond, QemuMutex *mutex) +{ + qemu_cond_wait(cond, mutex); +} void qemu_sem_init(QemuSemaphore *sem, int init); void qemu_sem_post(QemuSemaphore *sem); diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 959a57079f..b789cf32e9 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -57,26 +57,28 @@ void qemu_mutex_destroy(QemuMutex *mutex) error_exit(err, __func__); } -void qemu_mutex_lock(QemuMutex *mutex) +void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line) { int err; assert(mutex->initialized); + trace_qemu_mutex_lock(mutex, file, line); + err = pthread_mutex_lock(&mutex->lock); if (err) error_exit(err, __func__); - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); } -int qemu_mutex_trylock(QemuMutex *mutex) +int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line) { int err; assert(mutex->initialized); err = pthread_mutex_trylock(&mutex->lock); if (err == 0) { - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); return 0; } if (err != EBUSY) { @@ -85,15 +87,16 @@ int qemu_mutex_trylock(QemuMutex *mutex) return -EBUSY; } -void qemu_mutex_unlock(QemuMutex *mutex) +void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int line) { int err; assert(mutex->initialized); - trace_qemu_mutex_unlocked(mutex); err = pthread_mutex_unlock(&mutex->lock); if (err) error_exit(err, __func__); + + trace_qemu_mutex_unlock(mutex, file, line); } void qemu_rec_mutex_init(QemuRecMutex *mutex) @@ -152,14 +155,14 @@ void qemu_cond_broadcast(QemuCond *cond) error_exit(err, __func__); } -void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) +void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const char *file, const int line) { int err; assert(cond->initialized); - trace_qemu_mutex_unlocked(mutex); + trace_qemu_mutex_unlock(mutex, file, line); err = pthread_cond_wait(&cond->cond, &mutex->lock); - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); if (err) error_exit(err, __func__); } diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 94f3491a87..ab60c0d557 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -56,30 +56,32 @@ void qemu_mutex_destroy(QemuMutex *mutex) InitializeSRWLock(&mutex->lock); } -void qemu_mutex_lock(QemuMutex *mutex) +void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line) { assert(mutex->initialized); + trace_qemu_mutex_lock(mutex, file, line); + AcquireSRWLockExclusive(&mutex->lock); - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); } -int qemu_mutex_trylock(QemuMutex *mutex) +int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line) { int owned; assert(mutex->initialized); owned = TryAcquireSRWLockExclusive(&mutex->lock); if (owned) { - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); return 0; } return -EBUSY; } -void qemu_mutex_unlock(QemuMutex *mutex) +void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int line) { assert(mutex->initialized); - trace_qemu_mutex_unlocked(mutex); + trace_qemu_mutex_unlock(mutex, file, line); ReleaseSRWLockExclusive(&mutex->lock); } @@ -140,12 +142,12 @@ void qemu_cond_broadcast(QemuCond *cond) WakeAllConditionVariable(&cond->var); } -void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) +void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const char *file, const int line) { assert(cond->initialized); - trace_qemu_mutex_unlocked(mutex); + trace_qemu_mutex_unlock(mutex, file, line); SleepConditionVariableSRW(&cond->var, &mutex->lock, INFINITE, 0); - trace_qemu_mutex_locked(mutex); + trace_qemu_mutex_locked(mutex, file, line); } void qemu_sem_init(QemuSemaphore *sem, int init) diff --git a/util/trace-events b/util/trace-events index 025499f83f..515e6257fb 100644 --- a/util/trace-events +++ b/util/trace-events @@ -56,6 +56,7 @@ lockcnt_futex_wait(const void *lockcnt, int val) "lockcnt %p waiting on %d" lockcnt_futex_wait_resume(const void *lockcnt, int new) "lockcnt %p after wait: %d" lockcnt_futex_wake(const void *lockcnt) "lockcnt %p waking up one waiter" -# util/qemu-thread-posix.c -qemu_mutex_locked(void *lock) "locked mutex %p" -qemu_mutex_unlocked(void *lock) "unlocked mutex %p" +# util/qemu-thread.c +qemu_mutex_lock(void *mutex, const char *file, const int line) "waiting on mutex %p (%s:%d)" +qemu_mutex_locked(void *mutex, const char *file, const int line) "taken mutex %p (%s:%d)" +qemu_mutex_unlock(void *mutex, const char *file, const int line) "released mutex %p (%s:%d)" From b5976c2e46e86b36b01d8ac380a182e22209a7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 11 Jan 2018 11:27:22 +0300 Subject: [PATCH 51/51] scripts/analyse-locks-simpletrace.py: script to analyse lock times MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This script allows analysis of mutex acquisition and hold times based on a trace file. Given a trace control file of: qemu_mutex_lock qemu_mutex_locked qemu_mutex_unlock And running with: $QEMU $QEMU_ARGS -trace events=./lock-trace You can analyse the results with: ./scripts/analyse-locks-simpletrace.py trace-events-all ./trace-21812 Signed-off-by: Alex Bennée Signed-off-by: Paolo Bonzini --- scripts/analyse-locks-simpletrace.py | 99 ++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100755 scripts/analyse-locks-simpletrace.py diff --git a/scripts/analyse-locks-simpletrace.py b/scripts/analyse-locks-simpletrace.py new file mode 100755 index 0000000000..101e84dea5 --- /dev/null +++ b/scripts/analyse-locks-simpletrace.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# Analyse lock events and compute statistics +# +# Author: Alex Bennée +# + +import os +import simpletrace +import argparse +import numpy as np + +class MutexAnalyser(simpletrace.Analyzer): + "A simpletrace Analyser for checking locks." + + def __init__(self): + self.locks = 0 + self.locked = 0 + self.unlocks = 0 + self.mutex_records = {} + + def _get_mutex(self, mutex): + if not mutex in self.mutex_records: + self.mutex_records[mutex] = {"locks": 0, + "lock_time": 0, + "acquire_times": [], + "locked": 0, + "locked_time": 0, + "held_times": [], + "unlocked": 0} + + return self.mutex_records[mutex] + + def qemu_mutex_lock(self, timestamp, mutex, filename, line): + self.locks += 1 + rec = self._get_mutex(mutex) + rec["locks"] += 1 + rec["lock_time"] = timestamp[0] + rec["lock_loc"] = (filename, line) + + def qemu_mutex_locked(self, timestamp, mutex, filename, line): + self.locked += 1 + rec = self._get_mutex(mutex) + rec["locked"] += 1 + rec["locked_time"] = timestamp[0] + acquire_time = rec["locked_time"] - rec["lock_time"] + rec["locked_loc"] = (filename, line) + rec["acquire_times"].append(acquire_time) + + def qemu_mutex_unlock(self, timestamp, mutex, filename, line): + self.unlocks += 1 + rec = self._get_mutex(mutex) + rec["unlocked"] += 1 + held_time = timestamp[0] - rec["locked_time"] + rec["held_times"].append(held_time) + rec["unlock_loc"] = (filename, line) + + +def get_args(): + "Grab options" + parser = argparse.ArgumentParser() + parser.add_argument("--output", "-o", type=str, help="Render plot to file") + parser.add_argument("events", type=str, help='trace file read from') + parser.add_argument("tracefile", type=str, help='trace file read from') + return parser.parse_args() + +if __name__ == '__main__': + args = get_args() + + # Gather data from the trace + analyser = MutexAnalyser() + simpletrace.process(args.events, args.tracefile, analyser) + + print ("Total locks: %d, locked: %d, unlocked: %d" % + (analyser.locks, analyser.locked, analyser.unlocks)) + + # Now dump the individual lock stats + for key, val in sorted(analyser.mutex_records.iteritems(), + key=lambda (k,v): v["locks"]): + print ("Lock: %#x locks: %d, locked: %d, unlocked: %d" % + (key, val["locks"], val["locked"], val["unlocked"])) + + acquire_times = np.array(val["acquire_times"]) + if len(acquire_times) > 0: + print (" Acquire Time: min:%d median:%d avg:%.2f max:%d" % + (acquire_times.min(), np.median(acquire_times), + acquire_times.mean(), acquire_times.max())) + + held_times = np.array(val["held_times"]) + if len(held_times) > 0: + print (" Held Time: min:%d median:%d avg:%.2f max:%d" % + (held_times.min(), np.median(held_times), + held_times.mean(), held_times.max())) + + # Check if any locks still held + if val["locks"] > val["locked"]: + print (" LOCK HELD (%s:%s)" % (val["locked_loc"])) + print (" BLOCKED (%s:%s)" % (val["lock_loc"]))