From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L0kL8-0003h1-FW for qemu-devel@nongnu.org; Thu, 13 Nov 2008 17:07:02 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L0kL7-0003gp-2x for qemu-devel@nongnu.org; Thu, 13 Nov 2008 17:07:02 -0500 Received: from [199.232.76.173] (port=53591 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L0kL6-0003gm-SI for qemu-devel@nongnu.org; Thu, 13 Nov 2008 17:07:00 -0500 Received: from relay4-v.mail.gandi.net ([217.70.178.78]:43830) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1L0kL6-0004s2-GD for qemu-devel@nongnu.org; Thu, 13 Nov 2008 17:07:00 -0500 Message-ID: <491CA4FD.8000202@bellard.org> Date: Thu, 13 Nov 2008 23:06:53 +0100 From: Fabrice Bellard MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 00/12] Enhance debugging support - 4th take References: <20081103103558.213902776@mchn012c.ww002.siemens.net> In-Reply-To: <20081103103558.213902776@mchn012c.ww002.siemens.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jan.kiszka@siemens.com Cc: qemu-devel@nongnu.org I had a quick look at the patch serie (I don't have the time to look at it carefully). I find the patch globally acceptable, but I have two remarks: - Patch 01/12 may introduce a performance regression due to the change in tb_find_fast(). If gcc does not optimizes the code correctly, your change will introduce many unneeded memory accesses and a call to memcmp() in the fast path, which is not acceptable. - Patch 12/12 needs improvements (load/save VM) and possibly more analysis to see if it complies with the x86 spec, so it could be applied later. Regards, Fabrice. Jan Kiszka wrote: > Yet another round for the gdbstub and debug regs patches. > > This version addresses the concerns about the next_cflags approach > brought up by Fabrice and Glauber. Instead of this, the first patch in > this series takes an alternative path to obtain the required parameters > for TB generation, see its description for details. > > Besides rebasing the series on top of this patch and the latest changes > in SVN, I addressed Glauber's review comments on take 3 (proper > description for patch 4, introduce CPUWatchpoint.len_mask directly). > > Jan > > -- > Siemens AG, Corporate Technology, CT SE 2 > Corporate Competence Center Embedded Linux > > > >