From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52442) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9z6M-0004rz-96 for qemu-devel@nongnu.org; Tue, 30 Jun 2015 13:13:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z9z6J-0001rI-1g for qemu-devel@nongnu.org; Tue, 30 Jun 2015 13:13:42 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:56762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9z6I-0001r6-OL for qemu-devel@nongnu.org; Tue, 30 Jun 2015 13:13:38 -0400 References: <1435681199-14392-1-git-send-email-laurent@vivier.eu> From: Laurent Vivier Message-ID: <5592CE38.2010509@vivier.eu> Date: Tue, 30 Jun 2015 19:13:28 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , QEMU Developers Le 30/06/2015 18:45, Peter Maydell a écrit : > On 30 June 2015 at 17:19, Laurent Vivier wrote: >> When guest base is disabled, RESERVED_VA is 0, and >> (__guest < RESERVED_VA) is always false as __guest is unsigned. >> >> With -Werror=type-limits, this triggers an error: >> >> include/exec/cpu_ldst.h:60:31: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits] >> (!RESERVED_VA || (__guest < RESERVED_VA)); \ >> >> This patch removes this comparison when guest base is disabled. > > Is there a useful reason to compile with --disable-guest-base > (ie why we should retain the !CONFIG_USE_GUEST_BASE code > in QEMU at all) ? It was originally optional because we > didn't support it in all our TCG hosts, but we fixed that > back in 2012... TCG generates less code, so performance is better (well, it is what I guess). I've compiled a kernel with and without guest base in a chrooted linux-user-qemu. Without guest base it is ~1 minute less for a 13 minutes build. I can do more tests if you want. > (We can certainly take a compile fix for 2.4 even if > we decide we want to rip it out for 2.5, of course.) > >> Signed-off-by: Laurent Vivier >> --- >> include/exec/cpu_ldst.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h >> index 1239c60..f278126 100644 >> --- a/include/exec/cpu_ldst.h >> +++ b/include/exec/cpu_ldst.h >> @@ -54,11 +54,16 @@ >> #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS >> #define h2g_valid(x) 1 >> #else >> +#if defined(CONFIG_USE_GUEST_BASE) >> #define h2g_valid(x) ({ \ >> unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \ >> (__guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) && \ >> (!RESERVED_VA || (__guest < RESERVED_VA)); \ >> }) >> +#else >> +#define h2g_valid(x) \ >> + ((unsigned long)(x) < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) > > "ul" as a suffix is almost always wrong, incidentally, > though obviously here you're just copying the condition > from the existing code. Consider the case when an > unsigned long is 32 bits but TARGET_VIRT_ADDR_SPACE_BITS > is 32 or more (ie almost always on a 32-bit host). I think it can't happen because of previous lines: ... #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS #define h2g_valid(x) 1 #else ... Laurent