* [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
@ 2015-06-30 16:19 Laurent Vivier
2015-06-30 16:45 ` Peter Maydell
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2015-06-30 16:19 UTC (permalink / raw)
To: qemu-devel; +Cc: riku.voipio, Laurent Vivier
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.
Signed-off-by: Laurent Vivier <laurent@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))
+#endif
#endif
#define h2g_nocheck(x) ({ \
--
2.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 16:19 [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base Laurent Vivier
@ 2015-06-30 16:45 ` Peter Maydell
2015-06-30 17:13 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2015-06-30 16:45 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Riku Voipio, QEMU Developers
On 30 June 2015 at 17:19, Laurent Vivier <laurent@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...
(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 <laurent@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).
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 16:45 ` Peter Maydell
@ 2015-06-30 17:13 ` Laurent Vivier
2015-06-30 17:20 ` Peter Maydell
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2015-06-30 17:13 UTC (permalink / raw)
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 <laurent@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 <laurent@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 17:13 ` Laurent Vivier
@ 2015-06-30 17:20 ` Peter Maydell
2015-06-30 23:58 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2015-06-30 17:20 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Riku Voipio, QEMU Developers
On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 30/06/2015 18:45, Peter Maydell a écrit :
>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
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 <peter.maydell@linaro.org>
(it's a bit sad the compiler doesn't notice that
it will never evaluate the x < 0 expression.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 17:20 ` Peter Maydell
@ 2015-06-30 23:58 ` Laurent Vivier
2015-07-01 11:12 ` Peter Maydell
2015-07-01 13:15 ` Aurelien Jarno
0 siblings, 2 replies; 10+ messages in thread
From: Laurent Vivier @ 2015-06-30 23:58 UTC (permalink / raw)
To: Peter Maydell; +Cc: Riku Voipio, QEMU Developers
Le 30/06/2015 19:20, Peter Maydell a écrit :
> On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>>
>> Le 30/06/2015 18:45, Peter Maydell a écrit :
>>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
>
> 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...
In fact, I have made more measurements, it saves only ~10 seconds on a
13 minutes build.
my test is: "make -j 4 vmlinux"
(target: m68k, host: x86_64, 4 cores x 2 threads)
--enable-guest-base
real 13m26.134s 13m28.712s 13m28.053s 13m28.875s
user 52m44.882s 52m56.075s 52m49.223s 52m55.366s
sys 0m33.452s 0m33.613s 0m33.013s 0m33.336s
--disable-guest-base
real 13m20.412s 13m17.773s 13m15.836s 13m13.278s
user 52m23.165s 52m7.184s 52m1.547s 51m50.277s
sys 0m33.427s 0m33.392s 0m32.954s 0m33.430s
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 23:58 ` Laurent Vivier
@ 2015-07-01 11:12 ` Peter Maydell
2015-07-01 13:15 ` Aurelien Jarno
1 sibling, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2015-07-01 11:12 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Riku Voipio, QEMU Developers
On 1 July 2015 at 00:58, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 30/06/2015 19:20, Peter Maydell a écrit :
>> On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
>>>
>>>
>>> Le 30/06/2015 18:45, Peter Maydell a écrit :
>>>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
>>
>> 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...
>
> In fact, I have made more measurements, it saves only ~10 seconds on a
> 13 minutes build.
...that seems like it swings the argument pretty strongly
towards dropping the no-guest-base config entirely.
I think my suggestion would be:
1) apply this patch for 2.4
2) note in the 2.4 changelog that we're planning to drop
--disable-guest-base for 2.5
3) remove the config option and the code after 2.4 releases
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-06-30 23:58 ` Laurent Vivier
2015-07-01 11:12 ` Peter Maydell
@ 2015-07-01 13:15 ` Aurelien Jarno
2015-07-01 18:21 ` Laurent Vivier
1 sibling, 1 reply; 10+ messages in thread
From: Aurelien Jarno @ 2015-07-01 13:15 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Peter Maydell, Riku Voipio, QEMU Developers
On 2015-07-01 01:58, Laurent Vivier wrote:
>
>
> Le 30/06/2015 19:20, Peter Maydell a écrit :
> > On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >>
> >> Le 30/06/2015 18:45, Peter Maydell a écrit :
> >>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
> >
> > 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...
>
> In fact, I have made more measurements, it saves only ~10 seconds on a
> 13 minutes build.
>
> my test is: "make -j 4 vmlinux"
> (target: m68k, host: x86_64, 4 cores x 2 threads)
Note that on x86_64, guest base is implemented by using the gs segment
register. That explains why the impact should be relatively low, as your
test shows.
> --enable-guest-base
>
> real 13m26.134s 13m28.712s 13m28.053s 13m28.875s
> user 52m44.882s 52m56.075s 52m49.223s 52m55.366s
> sys 0m33.452s 0m33.613s 0m33.013s 0m33.336s
>
> --disable-guest-base
>
> real 13m20.412s 13m17.773s 13m15.836s 13m13.278s
> user 52m23.165s 52m7.184s 52m1.547s 51m50.277s
> sys 0m33.427s 0m33.392s 0m32.954s 0m33.430s
>
> Laurent
>
>
>
>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-07-01 13:15 ` Aurelien Jarno
@ 2015-07-01 18:21 ` Laurent Vivier
2015-07-01 19:46 ` Aurelien Jarno
2015-07-21 6:51 ` Richard Henderson
0 siblings, 2 replies; 10+ messages in thread
From: Laurent Vivier @ 2015-07-01 18:21 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Peter Maydell, Riku Voipio, QEMU Developers
Le 01/07/2015 15:15, Aurelien Jarno a écrit :
> On 2015-07-01 01:58, Laurent Vivier wrote:
>>
>>
>> Le 30/06/2015 19:20, Peter Maydell a écrit :
>>> On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
>>>>
>>>>
>>>> Le 30/06/2015 18:45, Peter Maydell a écrit :
>>>>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
>>>
>>> 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...
>>
>> In fact, I have made more measurements, it saves only ~10 seconds on a
>> 13 minutes build.
>>
>> my test is: "make -j 4 vmlinux"
>> (target: m68k, host: x86_64, 4 cores x 2 threads)
>
> Note that on x86_64, guest base is implemented by using the gs segment
> register. That explains why the impact should be relatively low, as your
> test shows.
I did a similar test on a PowerPC host, It is 2 seconds MORE on an 1m27s
build WITH --disable-guest-base.
So, definitively, I think the option can be dropped.
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-07-01 18:21 ` Laurent Vivier
@ 2015-07-01 19:46 ` Aurelien Jarno
2015-07-21 6:51 ` Richard Henderson
1 sibling, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2015-07-01 19:46 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Peter Maydell, Riku Voipio, QEMU Developers
On 2015-07-01 20:21, Laurent Vivier wrote:
>
>
> Le 01/07/2015 15:15, Aurelien Jarno a écrit :
> > On 2015-07-01 01:58, Laurent Vivier wrote:
> >>
> >>
> >> Le 30/06/2015 19:20, Peter Maydell a écrit :
> >>> On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
> >>>>
> >>>>
> >>>> Le 30/06/2015 18:45, Peter Maydell a écrit :
> >>>>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
> >>>
> >>> 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...
> >>
> >> In fact, I have made more measurements, it saves only ~10 seconds on a
> >> 13 minutes build.
> >>
> >> my test is: "make -j 4 vmlinux"
> >> (target: m68k, host: x86_64, 4 cores x 2 threads)
> >
> > Note that on x86_64, guest base is implemented by using the gs segment
> > register. That explains why the impact should be relatively low, as your
> > test shows.
>
> I did a similar test on a PowerPC host, It is 2 seconds MORE on an 1m27s
> build WITH --disable-guest-base.
>
> So, definitively, I think the option can be dropped.
I fully agree.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base
2015-07-01 18:21 ` Laurent Vivier
2015-07-01 19:46 ` Aurelien Jarno
@ 2015-07-21 6:51 ` Richard Henderson
1 sibling, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2015-07-21 6:51 UTC (permalink / raw)
To: Laurent Vivier, Aurelien Jarno
Cc: Peter Maydell, Riku Voipio, QEMU Developers
On 07/01/2015 07:21 PM, Laurent Vivier wrote:
>
>
> Le 01/07/2015 15:15, Aurelien Jarno a écrit :
>> On 2015-07-01 01:58, Laurent Vivier wrote:
>>>
>>>
>>> Le 30/06/2015 19:20, Peter Maydell a écrit :
>>>> On 30 June 2015 at 18:13, Laurent Vivier <laurent@vivier.eu> wrote:
>>>>>
>>>>>
>>>>> Le 30/06/2015 18:45, Peter Maydell a écrit :
>>>>>> On 30 June 2015 at 17:19, Laurent Vivier <laurent@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.
>>>>
>>>> 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...
>>>
>>> In fact, I have made more measurements, it saves only ~10 seconds on a
>>> 13 minutes build.
>>>
>>> my test is: "make -j 4 vmlinux"
>>> (target: m68k, host: x86_64, 4 cores x 2 threads)
>>
>> Note that on x86_64, guest base is implemented by using the gs segment
>> register. That explains why the impact should be relatively low, as your
>> test shows.
>
> I did a similar test on a PowerPC host, It is 2 seconds MORE on an 1m27s
> build WITH --disable-guest-base.
That should be measurement noise, since a PPC host changes nothing except the
register number for the indexed memory address. Same for aarch64, s390, and
sparc. We do reserve a register on ia64, but that does require an extra insn.
The implementation is less friendly for arm and mips. The former because it's
pressed for registers, and the later because it hasn't seen much tlc recently.
> So, definitively, I think the option can be dropped.
Also agreed. One can still force the same effect at runtime with -B.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-21 6:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-30 16:19 [Qemu-devel] [PATCH] linux-user: Avoid compilation error with --disable-guest-base Laurent Vivier
2015-06-30 16:45 ` Peter Maydell
2015-06-30 17:13 ` Laurent Vivier
2015-06-30 17:20 ` Peter Maydell
2015-06-30 23:58 ` Laurent Vivier
2015-07-01 11:12 ` Peter Maydell
2015-07-01 13:15 ` Aurelien Jarno
2015-07-01 18:21 ` Laurent Vivier
2015-07-01 19:46 ` Aurelien Jarno
2015-07-21 6:51 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).