Le 30/06/2015 18:45, Peter Maydell a écrit :
> On 30 June 2015 at 17:19, Laurent Vivier <laur...@vivier.eu> 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 <laur...@vivier.eu>
>> ---
>> 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
