From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Add memory clobber to hypercalls (v2) Date: Mon, 30 Jun 2008 18:59:08 +0300 Message-ID: <486902CC.9020404@qumranet.com> References: <1214504729-16880-1-git-send-email-aliguori@us.ibm.com> <4865B348.1050207@qumranet.com> <1214837660.7257.3.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , kvm@vger.kernel.org, Marcelo Tosatti , Alexandre Oliva , Christian Borntraeger To: Hollis Blanchard Return-path: Received: from il.qumranet.com ([212.179.150.194]:27856 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755278AbYF3P7J (ORCPT ); Mon, 30 Jun 2008 11:59:09 -0400 In-Reply-To: <1214837660.7257.3.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: Hollis Blanchard wrote: > On Sat, 2008-06-28 at 06:43 +0300, Avi Kivity wrote: > >> 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. >> > > What about gcc 4.4? 4.5? 5.0? > > Alexandre Oliva's idea of adding a constraint in __pa() to tell gcc that the memory there must be written seems to be the best option here. Though perhaps gcc should consider all memory pointed to by a pointer that is cast to an integer as escaped. -- error compiling committee.c: too many arguments to function