From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hollis Blanchard Subject: Re: [PATCH] Add memory clobber to hypercalls (v2) Date: Mon, 30 Jun 2008 09:54:20 -0500 Message-ID: <1214837660.7257.3.camel@localhost.localdomain> References: <1214504729-16880-1-git-send-email-aliguori@us.ibm.com> <4865B348.1050207@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , kvm@vger.kernel.org, Marcelo Tosatti , Alexandre Oliva , Christian Borntraeger To: Avi Kivity Return-path: Received: from e3.ny.us.ibm.com ([32.97.182.143]:42313 "EHLO e3.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757347AbYF3Oyc (ORCPT ); Mon, 30 Jun 2008 10:54:32 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e3.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id m5UEsPSv011887 for ; Mon, 30 Jun 2008 10:54:25 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v9.0) with ESMTP id m5UEsE65211648 for ; Mon, 30 Jun 2008 10:54:15 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m5UEsEWq018361 for ; Mon, 30 Jun 2008 10:54:14 -0400 In-Reply-To: <4865B348.1050207@qumranet.com> Sender: kvm-owner@vger.kernel.org List-ID: 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? -- Hollis Blanchard IBM Linux Technology Center