From patchwork Fri Jun 3 20:40:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 69278 Delivered-To: patch@linaro.org Received: by 10.140.106.246 with SMTP id e109csp449084qgf; Fri, 3 Jun 2016 13:47:12 -0700 (PDT) X-Received: by 10.200.47.156 with SMTP id l28mr774561qta.5.1464986832056; Fri, 03 Jun 2016 13:47:12 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id h57si4582206qte.81.2016.06.03.13.47.11 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 03 Jun 2016 13:47:12 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:57651 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vzr-0005IZ-HF for patch@linaro.org; Fri, 03 Jun 2016 16:47:11 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37372) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vtV-0008Px-CM for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8vtT-0000Qp-Qs for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:37 -0400 Received: from mail-wm0-x235.google.com ([2a00:1450:400c:c09::235]:35750) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vtT-0000QL-7R for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:35 -0400 Received: by mail-wm0-x235.google.com with SMTP id a136so12045941wme.0 for ; Fri, 03 Jun 2016 13:40:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=PV3LgISn+EKBpFXdNCeFaLOzYh+jpmJe2mqILe7tiuc=; b=WrGCDhHCUF4lryV9i5nRF7ur63eOimlfb8ZJNDWmwe8wHORqMKlOAEQ68zxsfm6yAk AFX+j9y3hV0AdHAJFRxDRogZ++wZiuQzXHmqpdDCbXw+7M3kLvIBlqwFOdNbMUFM0MfH 27HgK46GlMb2a3erhdi98fzZIA81jo6JhVC1Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=PV3LgISn+EKBpFXdNCeFaLOzYh+jpmJe2mqILe7tiuc=; b=AxUWbNfaO+8VwuV8gnfzhIgZ8eA76dS+3oic6PhHZukAgy6scWQ0eqzSy23Ts2UL7b bEt+v1VBVn/YbyQIoe+64CUbpLAUkALDx9j96rK4YkHOj9EuJY8xIJ0OzycYGY6e9Ry3 2VFJwwELtfG6cuFhhKhH4Ruf4fy4uYRU3q5Hl90FpUxxAhVoZ6qbYWNbAOv66yNlaLy2 TdAK8RrsgYl+EpVJs55cZvdkF68tHmYJr0dJi73OTNz+er7ykx5MKYBUAcf3UFNeZYJR OlD4vk/7TuV/RQpLfcx2ZnUeGjSKa9va1/vaAmPEFAmnHmdLU8qcXce3soV/HqUBoDvF ETHA== X-Gm-Message-State: ALyK8tJZ3z/7hQ94eX46fkTdbuohYJp2HO9nOv9SZAgTGY1pVEF2J5hTg/xrQAfAfhxUf0qM X-Received: by 10.28.109.205 with SMTP id b74mr1186568wmi.84.1464986434479; Fri, 03 Jun 2016 13:40:34 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id c191sm1242943wmh.5.2016.06.03.13.40.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jun 2016 13:40:31 -0700 (PDT) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id 9E8FC3E2D19; Fri, 3 Jun 2016 21:40:39 +0100 (BST) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= To: mttcg@listserver.greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, serge.fdrv@gmail.com, cota@braap.org, bobby.prani@gmail.com Date: Fri, 3 Jun 2016 21:40:14 +0100 Message-Id: <1464986428-6739-6-git-send-email-alex.bennee@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1464986428-6739-1-git-send-email-alex.bennee@linaro.org> References: <1464986428-6739-1-git-send-email-alex.bennee@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::235 Subject: [Qemu-devel] [RFC v3 05/19] exec: add assert_debug_safe and notes on debug structures 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: peter.maydell@linaro.org, Peter Crosthwaite , claudio.fontana@huawei.com, mark.burton@greensocs.com, jan.kiszka@siemens.com, pbonzini@redhat.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= , rth@twiddle.net Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" The debug structures are different from many of the other shared ones as they are modified from two places: - architectural debug registers - the gdb stub The first is usually in the context of vCPU currently running so modifications are safe. The second is generally only changed while the vCPUs are halted and therefore not going to clash. I fixed the commenting on DEBUG_SUBPAGE when I added the new DEBUG_DEBUG flag. KNOWN ISSUES: arm powerctl resets will trigger the asserts in multi-thread mode with smp > 1 as the reset operation is cross-cpu and clears all watchpoints. Signed-off-by: Alex Bennée --- exec.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) -- 2.7.4 diff --git a/exec.c b/exec.c index a3a93ae..b225282 100644 --- a/exec.c +++ b/exec.c @@ -25,6 +25,7 @@ #include "qemu/cutils.h" #include "cpu.h" #include "exec/exec-all.h" +#include "qom/cpu.h" #include "tcg.h" #include "hw/qdev-core.h" #if !defined(CONFIG_USER_ONLY) @@ -62,7 +63,46 @@ #include "qemu/mmap-alloc.h" #endif -//#define DEBUG_SUBPAGE +/* #define DEBUG_SUBPAGE */ +/* #define DEBUG_DEBUG */ + +#ifdef DEBUG_DEBUG +#define CHECK_DEBUG_SAFE 1 +#else +#define CHECK_DEBUG_SAFE 0 +#endif + +/* + * Safe access to debugging structures. + * + * Breakpoints and Watchpoints are kept in the vCPU structures. There + * are two ways they are manipulated: + * + * - Outside of the context of the vCPU thread (e.g. gdbstub) + * - Inside the context of the vCPU (architectural debug registers) + * + * In system emulation mode the chance of corruption is usually + * mitigated by the fact the vCPUs is usually suspended whenever these + * changes are made. + * + * In user emulation mode it is less clear (XXX: work this out) + */ + +#ifdef CONFIG_SOFTMMU +#define assert_debug_safe(cpu) do { \ + if (CHECK_DEBUG_SAFE) { \ + g_assert(!cpu->created || \ + (cpu_is_stopped(cpu) || cpu == current_cpu)); \ + } \ + } while (0) +#else +#define assert_debug_safe(cpu) do { \ + if (CHECK_DEBUG_SAFE) { \ + g_assert(false); \ + } \ + } while (0) +#endif + #if !defined(CONFIG_USER_ONLY) /* ram_list is read under rcu_read_lock()/rcu_read_unlock(). Writes @@ -737,6 +777,8 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, { CPUWatchpoint *wp; + assert_debug_safe(cpu); + /* forbid ranges which are empty or run off the end of the address space */ if (len == 0 || (addr + len - 1) < addr) { error_report("tried to set invalid watchpoint at %" @@ -769,6 +811,8 @@ int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len, { CPUWatchpoint *wp; + assert_debug_safe(cpu); + QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { if (addr == wp->vaddr && len == wp->len && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) { @@ -782,6 +826,8 @@ int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len, /* Remove a specific watchpoint by reference. */ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint) { + assert_debug_safe(cpu); + QTAILQ_REMOVE(&cpu->watchpoints, watchpoint, entry); tlb_flush_page(cpu, watchpoint->vaddr); @@ -794,6 +840,8 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask) { CPUWatchpoint *wp, *next; + assert_debug_safe(cpu); + QTAILQ_FOREACH_SAFE(wp, &cpu->watchpoints, entry, next) { if (wp->flags & mask) { cpu_watchpoint_remove_by_ref(cpu, wp); @@ -829,6 +877,8 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, { CPUBreakpoint *bp; + assert_debug_safe(cpu); + bp = g_malloc(sizeof(*bp)); bp->pc = pc; @@ -849,11 +899,16 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, return 0; } -/* Remove a specific breakpoint. */ +/* Remove a specific breakpoint. + * + * See cpu_breakpoint_insert notes for information about locking. + */ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags) { CPUBreakpoint *bp; + assert_debug_safe(cpu); + QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) { if (bp->pc == pc && bp->flags == flags) { cpu_breakpoint_remove_by_ref(cpu, bp);