From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKlba-0006rb-Th for qemu-devel@nongnu.org; Mon, 31 Oct 2011 02:44:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RKlbZ-0004af-PX for qemu-devel@nongnu.org; Mon, 31 Oct 2011 02:44:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52119) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKlbZ-0004aY-DV for qemu-devel@nongnu.org; Mon, 31 Oct 2011 02:44:21 -0400 From: Markus Armbruster References: <1319976446-12602-1-git-send-email-sw@weilnetz.de> <4EAD5F72.4080805@weilnetz.de> <62BEC008-676E-4A6C-90BD-28159908B008@suse.de> Date: Mon, 31 Oct 2011 07:44:18 +0100 In-Reply-To: <62BEC008-676E-4A6C-90BD-28159908B008@suse.de> (Alexander Graf's message of "Sun, 30 Oct 2011 15:45:10 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Stefan Weil , qemu-devel@nongnu.org, Avi Kivity Alexander Graf writes: > On 30.10.2011, at 15:30, Stefan Weil wrote: > >> Am 30.10.2011 14:41, schrieb Alexander Graf: >>> On 30.10.2011, at 13:07, Stefan Weil wrote: >>>> Valgrind is a tool which can automatically detect many kinds of bugs. >>>> >>>> Running QEMU on Valgrind with x86_64 hosts was not possible because >>>> Valgrind aborts when memalign is called with an alignment larger than >>>> 1 MiB. QEMU normally uses 2 MiB on Linux x86_64. >>>> >>>> Now the alignment is reduced to the page size when QEMU is running on >>>> Valgrind. >>>> >>>> valgrind.h is a copy from Valgrind svn trunk r12226 with trailing >>>> whitespace stripped but otherwise unmodified, so it still raises lots >>>> of errors when checked with scripts/checkpatch.pl. >>> >>> Can't we just require valgrind header files to be around when kvm is enabled? I would rather not copy code from other projects. Alternatively you could take the header and shrink it down to maybe 5 lines of inline asm code that would be a lot more readable :). You're #ifdef'ing on x86_64 already anyways. >> >> The patch is currently required for x86_64 hosts running Linux. >> I estimate that this is one of the most frequently used QEMU host platforms, >> and in most cases, KVM will be configured because this is the default >> and also because it is reasonable for this platform. >> >> How many of these hosts will have the Valgrind header around? >> I estimate less than 20 %, so configure would have to test whether >> valgrind.h is available or not. I think providing valgrind.h is >> a much better (and simpler) solution. > > Hrm. I see your point. > >> Stripping valgrind.h is not a good idea: the file is specially designed >> to be included in other projects like QEMU. As soon as the 2 MiB alignment >> is used for other hosts (ppc64, ...), you would have to take more and more >> from the original code. The file was not designed to be readable. >> Although it contains lots of comments which improve readability, >> there remains code which is less easy to read. I cite one of those >> comments: >> >> /* The following defines the magic code sequences which the JITter >> spots and handles magically. Don't look too closely at them as >> they will rot your brain. >> >> Instead of rotting my brain, I prefer using a copy of the original code. Yes, copies are evil. > Could we maybe use a git submodule to point to the valgrind repo and fetch it from there? Anyone sophisticated enough to make use of valgrind should be able to install valgrind.h just fine. It's not rocket science: # yum provides \*/valgrind.h [...] # yum install valgrind-devel [...]