From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kpo3u-00044E-VK for qemu-devel@nongnu.org; Tue, 14 Oct 2008 13:52:03 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kpo3s-00042n-VC for qemu-devel@nongnu.org; Tue, 14 Oct 2008 13:52:02 -0400 Received: from [199.232.76.173] (port=59692 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kpo3s-00042h-Ne for qemu-devel@nongnu.org; Tue, 14 Oct 2008 13:52:00 -0400 Received: from an-out-0708.google.com ([209.85.132.245]:25198) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Kpo3s-00008Z-80 for qemu-devel@nongnu.org; Tue, 14 Oct 2008 13:52:00 -0400 Received: by an-out-0708.google.com with SMTP id d18so169526and.130 for ; Tue, 14 Oct 2008 10:51:59 -0700 (PDT) Message-ID: <5d6222a80810141051s5ebc91c9na8c7dc41a3c764f8@mail.gmail.com> Date: Tue, 14 Oct 2008 15:51:59 -0200 From: "Glauber Costa" Subject: Re: [Qemu-devel] Re: [PATCH 02/13] Refactor and enhance break/watchpoint API In-Reply-To: <48F4DAB1.4020308@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20081014091223.932366369@mchn012c.ww002.siemens.net> <20081014091224.774875732@mchn012c.ww002.siemens.net> <5d6222a80810141024x1077066ek851f96a815c0cbe4@mail.gmail.com> <48F4DAB1.4020308@web.de> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Tue, Oct 14, 2008 at 3:45 PM, Jan Kiszka wrote: > Glauber Costa wrote: >>> Index: b/exec.c >>> =================================================================== >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -537,7 +537,6 @@ void cpu_exec_init(CPUState *env) >>> cpu_index++; >>> } >>> env->cpu_index = cpu_index; >>> - env->nb_watchpoints = 0; >>> *penv = env; >>> #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) >>> register_savevm("cpu_common", cpu_index, CPU_COMMON_SAVE_VERSION, >>> @@ -1311,107 +1310,150 @@ static void breakpoint_invalidate(CPUSta >>> #endif >>> >>> /* Add a watchpoint. */ >>> -int cpu_watchpoint_insert(CPUState *env, target_ulong addr, int type) >>> +int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len, >>> + int flags, CPUWatchpoint **watchpoint) >>> { >>> - int i; >>> + CPUWatchpoint *wp; >>> >>> - for (i = 0; i < env->nb_watchpoints; i++) { >>> - if (addr == env->watchpoint[i].vaddr) >>> - return 0; >>> - } >>> - if (env->nb_watchpoints >= MAX_WATCHPOINTS) >>> - return -1; >>> + wp = qemu_malloc(sizeof(*wp)); >>> + if (!wp) >>> + return -ENOBUFS; >>> + >>> + wp->vaddr = addr; >>> + wp->len = len; >>> + wp->flags = flags; >>> + >>> + wp->next = env->watchpoints; >>> + wp->prev = NULL; >>> + if (wp->next) >>> + wp->next->prev = wp; >>> + env->watchpoints = wp; >>> >>> - i = env->nb_watchpoints++; >>> - env->watchpoint[i].vaddr = addr; >>> - env->watchpoint[i].type = type; >>> tlb_flush_page(env, addr); >>> /* FIXME: This flush is needed because of the hack to make memory ops >>> terminate the TB. It can be removed once the proper IO trap and >>> re-execute bits are in. */ >>> tb_flush(env); >> >>> Index: b/cpu-defs.h >>> +typedef struct CPUBreakpoint { >>> + target_ulong pc; >>> + int flags; /* BP_* */ >>> + struct CPUBreakpoint *prev, *next; >>> +} CPUBreakpoint; >>> + >>> +typedef struct CPUWatchpoint { >>> + target_ulong vaddr; >>> + target_ulong len; >>> + int flags; /* BP_* */ >>> + struct CPUWatchpoint *prev, *next; >>> +} CPUWatchpoint; >>> + >> >> Most of the time, you are transversing the list in a single direction. >> So any particular reason to use a double linked list? > > When looking as this patch only, one may get along with a singly-linked > list. But patch 13 adds a use case where the back-reference pays off. fair. > >> By the way, /me thinks it is about time for us to have a generic >> linked list implementation > > Probably - but $SOMEONE will have to do the time-consuming conversion > work to make QEMU really benefit from this... Actually we don't need a conversion. We just need an implementation, and the conversion happens through time, as old code gets replaced. But we still need the $SOMEONE, and it's not exactly my priority right now. > > Jan > > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act."