From nobody Wed Nov 5 14:58:26 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1534778002609303.6721324941335; Mon, 20 Aug 2018 08:13:22 -0700 (PDT) Received: from localhost ([::1]:47659 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1frlrt-000296-8d for importer@patchew.org; Mon, 20 Aug 2018 11:13:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38169) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1frlnw-0006vh-RV for qemu-devel@nongnu.org; Mon, 20 Aug 2018 11:09:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1frlnp-00068l-Aw for qemu-devel@nongnu.org; Mon, 20 Aug 2018 11:09:13 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:34014 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1frlnn-000666-5w for qemu-devel@nongnu.org; Mon, 20 Aug 2018 11:09:07 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BE861B4DF; Mon, 20 Aug 2018 15:09:05 +0000 (UTC) Received: from donizetti.mxp.redhat.com (unknown [10.32.181.140]) by smtp.corp.redhat.com (Postfix) with ESMTP id 42A881017CD2; Mon, 20 Aug 2018 15:09:05 +0000 (UTC) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 20 Aug 2018 17:09:00 +0200 Message-Id: <20180820150903.1224-2-pbonzini@redhat.com> In-Reply-To: <20180820150903.1224-1-pbonzini@redhat.com> References: <20180820150903.1224-1-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 20 Aug 2018 15:09:05 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 20 Aug 2018 15:09:05 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'pbonzini@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 1/4] cpus: protect all icount computation with seqlock X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Emilio G . Cota" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Using the seqlock makes the atomic_read__nocheck safe, because it now happens always inside a seqlock and any torn reads will be retried. qemu_icount_bias and icount_time_shift also need to be accessed with atomics. At the same time, however, you don't need atomic_read within the writer, because no concurrent writes are possible. The fix to vmstate lets us keep the struct nicely packed. Signed-off-by: Paolo Bonzini --- cpus.c | 69 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/cpus.c b/cpus.c index 91613491b7..680706aefa 100644 --- a/cpus.c +++ b/cpus.c @@ -121,8 +121,6 @@ static bool all_cpu_threads_idle(void) /* Protected by TimersState seqlock */ =20 static bool icount_sleep =3D true; -/* 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 =20 @@ -137,8 +135,9 @@ typedef struct TimersState { QemuSeqLock vm_clock_seqlock; int64_t cpu_clock_offset; int32_t cpu_ticks_enabled; - int64_t dummy; =20 + /* Conversion factor from emulated instructions to virtual clock ticks= . */ + int icount_time_shift; /* Compensate for varying guest execution speed. */ int64_t qemu_icount_bias; /* Only written by TCG thread */ @@ -247,14 +246,13 @@ void cpu_update_icount(CPUState *cpu) =20 #ifdef CONFIG_ATOMIC64 atomic_set__nocheck(&timers_state.qemu_icount, - atomic_read__nocheck(&timers_state.qemu_icount) + - executed); + timers_state.qemu_icount + executed); #else /* FIXME: we need 64bit atomics to do this safely */ timers_state.qemu_icount +=3D executed; #endif } =20 -int64_t cpu_get_icount_raw(void) +static int64_t cpu_get_icount_raw_locked(void) { CPUState *cpu =3D current_cpu; =20 @@ -266,20 +264,30 @@ int64_t cpu_get_icount_raw(void) /* Take into account what has run */ cpu_update_icount(cpu); } -#ifdef CONFIG_ATOMIC64 + /* The read is protected by the seqlock, so __nocheck is okay. */ return atomic_read__nocheck(&timers_state.qemu_icount); -#else /* FIXME: we need 64bit atomics to do this safely */ - return timers_state.qemu_icount; -#endif } =20 -/* Return the virtual CPU time, based on the instruction counter. */ static int64_t cpu_get_icount_locked(void) { - int64_t icount =3D cpu_get_icount_raw(); - return timers_state.qemu_icount_bias + cpu_icount_to_ns(icount); + int64_t icount =3D cpu_get_icount_raw_locked(); + return atomic_read(&timers_state.qemu_icount_bias) + cpu_icount_to_ns(= icount); +} + +int64_t cpu_get_icount_raw(void) +{ + int64_t icount; + unsigned start; + + do { + start =3D seqlock_read_begin(&timers_state.vm_clock_seqlock); + icount =3D cpu_get_icount_raw_locked(); + } while (seqlock_read_retry(&timers_state.vm_clock_seqlock, start)); + + return icount; } =20 +/* Return the virtual CPU time, based on the instruction counter. */ int64_t cpu_get_icount(void) { int64_t icount; @@ -295,7 +303,7 @@ int64_t cpu_get_icount(void) =20 int64_t cpu_icount_to_ns(int64_t icount) { - return icount << icount_time_shift; + return icount << atomic_read(&timers_state.icount_time_shift); } =20 /* return the time elapsed in VM between vm_start and vm_stop. Unless @@ -415,19 +423,22 @@ static void icount_adjust(void) /* FIXME: This is a very crude algorithm, somewhat prone to oscillatio= n. */ if (delta > 0 && last_delta + ICOUNT_WOBBLE < delta * 2 - && icount_time_shift > 0) { + && timers_state.icount_time_shift > 0) { /* The guest is getting too far ahead. Slow time down. */ - icount_time_shift--; + atomic_set(&timers_state.icount_time_shift, + timers_state.icount_time_shift - 1); } if (delta < 0 && last_delta - ICOUNT_WOBBLE > delta * 2 - && icount_time_shift < MAX_ICOUNT_SHIFT) { + && timers_state.icount_time_shift < MAX_ICOUNT_SHIFT) { /* The guest is getting too far behind. Speed time up. */ - icount_time_shift++; + atomic_set(&timers_state.icount_time_shift, + timers_state.icount_time_shift + 1); } last_delta =3D delta; - timers_state.qemu_icount_bias =3D cur_icount - - (timers_state.qemu_icount << icount_time_s= hift); + atomic_set__nocheck(&timers_state.qemu_icount_bias, + cur_icount - (timers_state.qemu_icount + << timers_state.icount_time_shift)); seqlock_write_end(&timers_state.vm_clock_seqlock); } =20 @@ -448,7 +459,8 @@ static void icount_adjust_vm(void *opaque) =20 static int64_t qemu_icount_round(int64_t count) { - return (count + (1 << icount_time_shift) - 1) >> icount_time_shift; + int shift =3D atomic_read(&timers_state.icount_time_shift); + return (count + (1 << shift) - 1) >> shift; } =20 static void icount_warp_rt(void) @@ -484,7 +496,8 @@ static void icount_warp_rt(void) int64_t delta =3D clock - cur_icount; warp_delta =3D MIN(warp_delta, delta); } - timers_state.qemu_icount_bias +=3D warp_delta; + atomic_set__nocheck(&timers_state.qemu_icount_bias, + timers_state.qemu_icount_bias + warp_delta); } timers_state.vm_clock_warp_start =3D -1; seqlock_write_end(&timers_state.vm_clock_seqlock); @@ -513,7 +526,8 @@ void qtest_clock_warp(int64_t dest) int64_t warp =3D qemu_soonest_timeout(dest - clock, deadline); =20 seqlock_write_begin(&timers_state.vm_clock_seqlock); - timers_state.qemu_icount_bias +=3D warp; + atomic_set__nocheck(&timers_state.qemu_icount_bias, + timers_state.qemu_icount_bias + warp); seqlock_write_end(&timers_state.vm_clock_seqlock); =20 qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL); @@ -582,7 +596,8 @@ void qemu_start_warp_timer(void) * isolated from host latencies. */ seqlock_write_begin(&timers_state.vm_clock_seqlock); - timers_state.qemu_icount_bias +=3D deadline; + atomic_set__nocheck(&timers_state.qemu_icount_bias, + timers_state.qemu_icount_bias + deadline); seqlock_write_end(&timers_state.vm_clock_seqlock); qemu_clock_notify(QEMU_CLOCK_VIRTUAL); } else { @@ -700,7 +715,7 @@ static const VMStateDescription vmstate_timers =3D { .minimum_version_id =3D 1, .fields =3D (VMStateField[]) { VMSTATE_INT64(cpu_ticks_offset, TimersState), - VMSTATE_INT64(dummy, TimersState), + VMSTATE_UNUSED(8), VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2), VMSTATE_END_OF_LIST() }, @@ -812,7 +827,7 @@ void configure_icount(QemuOpts *opts, Error **errp) } if (strcmp(option, "auto") !=3D 0) { errno =3D 0; - icount_time_shift =3D strtol(option, &rem_str, 0); + timers_state.icount_time_shift =3D strtol(option, &rem_str, 0); if (errno !=3D 0 || *rem_str !=3D '\0' || !strlen(option)) { error_setg(errp, "icount: Invalid shift value"); } @@ -828,7 +843,7 @@ void configure_icount(QemuOpts *opts, Error **errp) =20 /* 125MIPS seems a reasonable initial guess at the guest speed. It will be corrected fairly quickly anyway. */ - icount_time_shift =3D 3; + timers_state.icount_time_shift =3D 3; =20 /* Have both realtime and virtual time triggers for speed adjustment. The realtime trigger catches emulated time passing too slowly, --=20 2.17.1