From patchwork Fri Jun 17 16:33:47 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: 70370 Delivered-To: patch@linaro.org Received: by 10.140.28.4 with SMTP id 4csp403303qgy; Fri, 17 Jun 2016 10:55:33 -0700 (PDT) X-Received: by 10.140.84.11 with SMTP id k11mr3620262qgd.17.1466186133255; Fri, 17 Jun 2016 10:55:33 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id j133si32338820qke.155.2016.06.17.10.55.33 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 17 Jun 2016 10:55:33 -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]:59472 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDxzQ-0001KU-PR for patch@linaro.org; Fri, 17 Jun 2016 13:55:32 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDwie-0006I1-3s for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:34:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDwib-0002vR-AK for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:34:07 -0400 Received: from mail-wm0-x235.google.com ([2a00:1450:400c:c09::235]:35372) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDwia-0002vJ-Vx for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:34:05 -0400 Received: by mail-wm0-x235.google.com with SMTP id v199so4791457wmv.0 for ; Fri, 17 Jun 2016 09:34:04 -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=Irhbyu/8O9v9W5PMWS+3E6UZELEfzIU4iYCgKRuNhYs=; b=dIjJrz4bO13ilO1tlee5CYe2BG1CJql59x1du/Hevivgb4OntZFWnQt/oKfSUj6C1G ypGfmsG2C2HrO8DqThofKfkjIADaUCniYESd8XSAqMhLD3KqCYQwAPO+V+hSkJQSVN+Y x89+ZzrycWQDNYRRdk9uHRjKwm19PEgFbiovc= 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=Irhbyu/8O9v9W5PMWS+3E6UZELEfzIU4iYCgKRuNhYs=; b=FQ0tiJ3YvNQwAZ+I4i+GvCBcP6sRH2aNiqW9sRD3Q7WjNcBV3spkVqDi5wvrFLzxa7 cpQ8XrlX8Dh30YbZGxc7qX9HjbqNFYLt4gE5G3ICte+Mc5hfSPjFHTIGr1jdkDyZs4JF Q0WLwlGsRLGegS1f7MeMW7yhn/Osl8ejRL3OoTfgi0nXRTWHt5ruvW+qbHWnJpGwPQxv guKq2oqX0OiYSstG42eLOXzTyW78SZTxrcq0ZGPuINkPveTRvTWc3JZOisqanWbe3ge6 5YsqLnM+ujISR5sRebzEtHCoAP7unObXkVsU5mh64pcH6XImfYLuBZP2GnK+mNkli+px jQug== X-Gm-Message-State: ALyK8tJDC7KBBMt65TUgnCn80dw1eI7CCRlyxDjMfiGZ+6e5RHrFJHoNL8J7EMj7O0WmQj09 X-Received: by 10.28.158.132 with SMTP id h126mr509370wme.43.1466181244296; Fri, 17 Jun 2016 09:34:04 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id b200sm426008wmb.9.2016.06.17.09.33.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Jun 2016 09:34:02 -0700 (PDT) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id 343C63E0BBF; Fri, 17 Jun 2016 17:33:59 +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, 17 Jun 2016 17:33:47 +0100 Message-Id: <1466181227-14934-8-git-send-email-alex.bennee@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1466181227-14934-1-git-send-email-alex.bennee@linaro.org> References: <1466181227-14934-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 7/7] watchpoints: put watchpoints under RCU control 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" Each time watchpoints are added/removed from the array it's done using an read-copy-update cycle. Simultaneous writes are protected by the debug_update_lock. Signed-off-by: Alex Bennée --- cpu-exec.c | 7 ++- exec.c | 193 ++++++++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 144 insertions(+), 56 deletions(-) -- 2.7.4 diff --git a/cpu-exec.c b/cpu-exec.c index 2736a27..7fcb500 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -387,12 +387,13 @@ static inline bool cpu_handle_halt(CPUState *cpu) static inline void cpu_handle_debug_exception(CPUState *cpu) { CPUClass *cc = CPU_GET_CLASS(cpu); + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); CPUWatchpoint *wp; int i; - if (!cpu->watchpoint_hit && cpu->watchpoints) { - for (i = 0; i < cpu->watchpoints->len; i++) { - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i); + if (!cpu->watchpoint_hit && wpts) { + for (i = 0; i < wpts->len; i++) { + wp = &g_array_index(wpts, CPUWatchpoint, i); wp->flags &= ~BP_WATCHPOINT_HIT; } } diff --git a/exec.c b/exec.c index d1d14c1..b9167ec 100644 --- a/exec.c +++ b/exec.c @@ -735,6 +735,18 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) } #endif +struct FreeDebugRCU { + struct rcu_head rcu; + GArray *debug; +}; + +/* RCU reclaim step */ +static void rcu_debug_free(struct FreeDebugRCU *rcu_free) +{ + g_array_free(rcu_free->debug, false); + g_free(rcu_free); +} + #if defined(CONFIG_USER_ONLY) void cpu_watchpoint_remove_all(CPUState *cpu, int mask) @@ -766,22 +778,72 @@ CPUWatchpoint *cpu_watchpoint_get_by_ref(CPUState *cpu, int ref) return NULL; } #else -/* Find watchpoint with external reference */ -CPUWatchpoint *cpu_watchpoint_get_by_ref(CPUState *cpu, int ref) +/* Create a working copy of the watchpoints. + * + * The rcu_read_lock() may already be held depending on where this + * update has been triggered from. However it is safe to nest + * rcu_read_locks() so we do the copy under the lock here. + */ + +static GArray *rcu_copy_watchpoints(CPUState *cpu) { - CPUWatchpoint *wp = NULL; + GArray *old, *new; + + rcu_read_lock(); + old = atomic_rcu_read(&cpu->watchpoints); + if (old) { + new = g_array_sized_new(false, false, sizeof(CPUWatchpoint), old->len); + memcpy(new->data, old->data, old->len * sizeof(CPUWatchpoint)); + new->len = old->len; + } else { + new = g_array_new(false, false, sizeof(CPUWatchpoint)); + } + rcu_read_unlock(); + + return new; +} + +/* Called with update lock held */ +static void rcu_update_watchpoints(CPUState *cpu, GArray *new_watchpts) +{ + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); + atomic_rcu_set(&cpu->watchpoints, new_watchpts); + if (wpts) { + struct FreeDebugRCU *rcu_free = g_malloc(sizeof(*rcu_free)); + rcu_free->debug = wpts; + call_rcu(rcu_free, rcu_debug_free, rcu); + } +} + +/* Find watchpoint with external reference, only valid for duration of + * rcu_read_lock */ +static CPUWatchpoint *find_watch_with_ref(GArray *wpts, int ref) +{ + CPUWatchpoint *wp; int i = 0; + do { - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i++); - } while (i < cpu->watchpoints->len && wp && wp->ref != ref); + wp = &g_array_index(wpts, CPUWatchpoint, i); + if (wp->ref == ref) { + return wp; + } + } while (i++ < wpts->len); + + return NULL; +} - return wp; +/* Find watchpoint with external reference */ +CPUWatchpoint *cpu_watchpoint_get_by_ref(CPUState *cpu, int ref) +{ + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); + return find_watch_with_ref(wpts, ref); } /* Add a watchpoint. */ int cpu_watchpoint_insert_with_ref(CPUState *cpu, vaddr addr, vaddr len, int flags, int ref) { + GArray *wpts; CPUWatchpoint *wp = NULL; /* forbid ranges which are empty or run off the end of the address space */ @@ -791,14 +853,14 @@ int cpu_watchpoint_insert_with_ref(CPUState *cpu, vaddr addr, vaddr len, return -EINVAL; } - /* Allocate if no previous watchpoints */ - if (!cpu->watchpoints) { - cpu->watchpoints = g_array_new(false, true, sizeof(CPUWatchpoint)); - } + qemu_mutex_lock(&cpu->update_debug_lock); + + /* This will allocate if no previous breakpoints */ + wpts = rcu_copy_watchpoints(cpu); /* Find old watchpoint */ if (ref != BPWP_NOREF) { - wp = cpu_watchpoint_get_by_ref(cpu, ref); + wp = find_watch_with_ref(wpts, ref); } if (wp) { @@ -815,15 +877,18 @@ int cpu_watchpoint_insert_with_ref(CPUState *cpu, vaddr addr, vaddr len, /* keep all GDB-injected watchpoints in front */ if (flags & BP_GDB) { - g_array_prepend_val(cpu->watchpoints, watch); + g_array_prepend_val(wpts, watch); } else { - g_array_append_val(cpu->watchpoints, watch); + g_array_append_val(wpts, watch); } } tlb_flush_page(cpu, addr); + rcu_update_watchpoints(cpu, wpts); + qemu_mutex_unlock(&cpu->update_debug_lock); + return 0; } @@ -832,69 +897,101 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int flags) return cpu_watchpoint_insert_with_ref(cpu, addr, len, flags, BPWP_NOREF); } -static void cpu_watchpoint_delete(CPUState *cpu, int index) +static void cpu_watchpoint_delete(CPUState *cpu, GArray *wpts, int index) { CPUWatchpoint *wp; - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, index); + wp = &g_array_index(wpts, CPUWatchpoint, index); tlb_flush_page(cpu, wp->vaddr); - g_array_remove_index(cpu->watchpoints, index); + g_array_remove_index(wpts, index); } /* Remove a specific watchpoint. */ int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len, int flags) { + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); CPUWatchpoint *wp; - int i = 0; + int retval = -ENOENT; + + if (unlikely(wpts) && unlikely(wpts->len)) { + int i = 0; + + qemu_mutex_lock(&cpu->update_debug_lock); + wpts = rcu_copy_watchpoints(cpu); - if (unlikely(cpu->watchpoints) && unlikely(cpu->watchpoints->len)) { do { wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i); if (wp && addr == wp->vaddr && len == wp->len && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) { - cpu_watchpoint_delete(cpu, i); - return 0; + cpu_watchpoint_delete(cpu, wpts, i); + retval = 0; + } else { + i++; } - } while (i++ < cpu->watchpoints->len); + } while (i < wpts->len); + + rcu_update_watchpoints(cpu, wpts); + qemu_mutex_unlock(&cpu->update_debug_lock); + } - return -ENOENT; + return retval; } /* Remove a specific watchpoint by external reference. */ int cpu_watchpoint_remove_by_ref(CPUState *cpu, int ref) { + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); CPUWatchpoint *wp; - int i = 0; + int retval = -ENOENT; + + if (unlikely(wpts) && unlikely(wpts->len)) { + int i = 0; + + qemu_mutex_lock(&cpu->update_debug_lock); + wpts = rcu_copy_watchpoints(cpu); - if (unlikely(cpu->watchpoints) && unlikely(cpu->watchpoints->len)) { do { - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i); + wp = &g_array_index(wpts, CPUWatchpoint, i); if (wp && wp->ref == ref) { - cpu_watchpoint_delete(cpu, i); - return 0; + cpu_watchpoint_delete(cpu, wpts, i); + retval = 0; + } else { + i++; } - } while (wp && i++ < cpu->watchpoints->len); + } while (i < wpts->len); + + rcu_update_watchpoints(cpu, wpts); + qemu_mutex_unlock(&cpu->update_debug_lock); + } - return -ENOENT; + return retval; } /* Remove all matching watchpoints. */ void cpu_watchpoint_remove_all(CPUState *cpu, int mask) { + GArray *wpts = atomic_rcu_read(&cpu->watchpoints); CPUWatchpoint *wp; - if (unlikely(cpu->watchpoints) && unlikely(cpu->watchpoints->len)) { - int i = cpu->watchpoints->len; + if (unlikely(wpts) && unlikely(wpts->len)) { + int i = 0; + + qemu_mutex_lock(&cpu->update_debug_lock); + wpts = rcu_copy_watchpoints(cpu); + do { - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i); + wp = &g_array_index(wpts, CPUWatchpoint, i); if (wp->flags & mask) { - cpu_watchpoint_delete(cpu, i); + cpu_watchpoint_delete(cpu, wpts, i); } else { - i--; + i++; } - } while (cpu->watchpoints->len && i >= 0); + } while (i < wpts->len); + + rcu_update_watchpoints(cpu, wpts); + qemu_mutex_unlock(&cpu->update_debug_lock); } } @@ -945,18 +1042,6 @@ static GArray *rcu_copy_breakpoints(CPUState *cpu) return new; } -struct BreakRCU { - struct rcu_head rcu; - GArray *bkpts; -}; - -/* RCU reclaim step */ -static void rcu_free_breakpoints(struct BreakRCU *rcu_free) -{ - g_array_free(rcu_free->bkpts, false); - g_free(rcu_free); -} - /* Called with update lock held */ static void rcu_update_breakpoints(CPUState *cpu, GArray *new_bkpts) { @@ -964,9 +1049,9 @@ static void rcu_update_breakpoints(CPUState *cpu, GArray *new_bkpts) atomic_rcu_set(&cpu->breakpoints, new_bkpts); if (bpts) { - struct BreakRCU *rcu_free = g_malloc(sizeof(*rcu_free)); - rcu_free->bkpts = bpts; - call_rcu(rcu_free, rcu_free_breakpoints, rcu); + struct FreeDebugRCU *rcu_free = g_malloc(sizeof(*rcu_free)); + rcu_free->debug = bpts; + call_rcu(rcu_free, rcu_debug_free, rcu); } } @@ -2296,6 +2381,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) target_ulong pc, cs_base; target_ulong vaddr; uint32_t cpu_flags; + GArray *wpts; if (cpu->watchpoint_hit) { /* We re-entered the check after replacing the TB. Now raise @@ -2306,11 +2392,12 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) } vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; - if (unlikely(cpu->watchpoints) && unlikely(cpu->watchpoints->len)) { + wpts = atomic_rcu_read(&cpu->watchpoints); + if (unlikely(wpts) && unlikely(wpts->len)) { CPUWatchpoint *wp; int i; - for (i = 0; i < cpu->watchpoints->len; i++) { - wp = &g_array_index(cpu->watchpoints, CPUWatchpoint, i); + for (i = 0; i < wpts->len; i++) { + wp = &g_array_index(wpts, CPUWatchpoint, i); if (cpu_watchpoint_address_matches(wp, vaddr, len) && (wp->flags & flags)) { if (flags == BP_MEM_READ) {