[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disa
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base |
Date: |
Tue, 30 Jun 2015 18:20:00 +0100 |
On 30 June 2015 at 18:13, Laurent Vivier <address@hidden> wrote:
>
>
> Le 30/06/2015 18:45, Peter Maydell a écrit :
>> On 30 June 2015 at 17:19, Laurent Vivier <address@hidden> 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.
Hmm. That's a fair chunk of speedup. On the downside:
* you only get this if you're willing to build QEMU from
source with funny options
* it won't work for all guest/host combinations (sometimes
the guest really wants to be able to map at low addresses
the host won't permit)
* it's an extra configuration to maintain which we're
clearly not testing at all upstream
I'd still favour removing it completely, personally...
>> "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
> ...
Oops, yes, I didn't read back far enough.
As a patch to fix a compile warning for 2.4,
Reviewed-by: Peter Maydell <address@hidden>
(it's a bit sad the compiler doesn't notice that
it will never evaluate the x < 0 expression.)
thanks
-- PMM