From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Add memory clobber to hypercalls (v2) Date: Sat, 28 Jun 2008 06:43:04 +0300 Message-ID: <4865B348.1050207@qumranet.com> References: <1214504729-16880-1-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Marcelo Tosatti , Hollis Blanchard , Alexandre Oliva , Christian Borntraeger To: Anthony Liguori Return-path: Received: from il.qumranet.com ([212.179.150.194]:50963 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752455AbYF1DnH (ORCPT ); Fri, 27 Jun 2008 23:43:07 -0400 In-Reply-To: <1214504729-16880-1-git-send-email-aliguori@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: Anthony Liguori wrote: > Hypercalls can modify arbitrary regions of memory. Make sure to indicate this > in the clobber list. This fixes a hang when using KVM_GUEST kernel built with > GCC 4.3.0. > > This was originally spotted and analyzed by Marcelo. > > Since v1, I've also added a "m" constraint for the inputs to the hypercall. > This was suggested by Christian since it's not entirely clear whether a memory > clobber will force the data to be in memory before the asm statement. In the > very least, it helps to be more conservative. > > Signed-off-by: Anthony Liguori > > @@ -80,7 +81,9 @@ static inline long kvm_hypercall1(unsigned int nr, unsigned long p1) > long ret; > asm volatile(KVM_HYPERCALL > : "=a"(ret) > - : "a"(nr), "b"(p1)); > + : "a"(nr), "b"(p1), > + "m"(*(char *)p1) > + : "memory"); > return ret; > } > > Those are physical addresses, not virtual, and on i386 the addresses are split across multiple registers. However a small test program shows that the memory clobber does work with gcc 4.3, so I'll pick the earlier patch. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.