From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44869) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T3kNn-0008GP-N7 for qemu-devel@nongnu.org; Tue, 21 Aug 2012 05:04:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T3kNh-0002hF-QO for qemu-devel@nongnu.org; Tue, 21 Aug 2012 05:04:19 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:56596) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T3kNh-0002h2-HN for qemu-devel@nongnu.org; Tue, 21 Aug 2012 05:04:13 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 21 Aug 2012 10:04:10 +0100 Received: from d06av11.portsmouth.uk.ibm.com (d06av11.portsmouth.uk.ibm.com [9.149.37.252]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7L93Zco6815872 for ; Tue, 21 Aug 2012 09:03:35 GMT Received: from d06av11.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av11.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7L93fYh020917 for ; Tue, 21 Aug 2012 03:03:41 -0600 Message-ID: <50334EEC.20501@de.ibm.com> Date: Tue, 21 Aug 2012 11:03:40 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1344604305-25312-1-git-send-email-borntraeger@de.ibm.com> In-Reply-To: <1344604305-25312-1-git-send-email-borntraeger@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC/PATCH] qemu: Use valgrind annotations to mark kvm guest memory as defined List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Christian Borntraeger , Anthony Liguori , Marcelo Tosatti , Avi Kivity On 10/08/12 15:11, Christian Borntraeger wrote: > valgrind with kvm produces a big amount of false positives regarding > "Conditional jump or move depends on uninitialised value(s)". This > happens because the guest memory is allocated with qemu_vmalloc which > boils down posix_memalign etc. This function is (correctly) considered > by valgrind as returning undefined memory. > > Since valgrind is based on jitting code, it will not be able to see > changes made by the guest to guest memory if this is done by KVM_RUN, > thus keeping most of the guest memory undefined. > > Now lots of places in qemu will then use guest memory to change behaviour. > To avoid the flood of these messages, lets declare the whole guest > memory as defined. This will reduce the noise and allows us to see real > problems. > > In the future we might want to make this conditional, since there > is actually something that we can use those false positives for: > These messages will point to code that depends on guest memory, so > we can use these backtraces to actually make an audit that is focussed > only at those code places. For normal development we dont want to > see those messages, though. Any opinion about this patch? > Signed-off-by: Christian Borntraeger > --- > configure | 3 ++- > kvm-all.c | 7 +++++++ > 2 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/configure b/configure > index 280726c..90df0d8 100755 > --- a/configure > +++ b/configure > @@ -2900,11 +2900,12 @@ if compile_prog "-Werror" "" ; then > fi > > ######################################## > -# check if we have valgrind/valgrind.h > +# check if we have valgrind/valgrind.h and valgrind/memcheck.h > > valgrind_h=no > cat > $TMPC << EOF > #include > +#include > int main(void) { > return 0; > } > diff --git a/kvm-all.c b/kvm-all.c > index 2148b20..33d16cf 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -39,6 +39,10 @@ > #include > #endif > > +#ifdef CONFIG_VALGRIND_H > +#include > +#endif > + > /* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ > #define PAGE_SIZE TARGET_PAGE_SIZE > > @@ -1687,6 +1691,9 @@ void *kvm_vmalloc(ram_addr_t size) > > void kvm_setup_guest_memory(void *start, size_t size) > { > +#ifdef CONFIG_VALGRIND_H > + VALGRIND_MAKE_MEM_DEFINED(start, size); > +#endif > if (!kvm_has_sync_mmu()) { > int ret = qemu_madvise(start, size, QEMU_MADV_DONTFORK); >