All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
@ 2021-03-06 21:41 Julien Grall
  2021-03-07 10:57 ` Roger Pau Monné
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Julien Grall @ 2021-03-06 21:41 UTC (permalink / raw)
  To: xen-devel
  Cc: julien, Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Stefano Stabellini, Wei Liu

From: Julien Grall <jgrall@amazon.com>

Compilers older than 4.8 have known codegen issues which can lead to
silent miscompilation:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

Furthermore, pre-4.9 GCC have known bugs (including things like
internal compiler errors on Arm) which would require workaround (I
haven't checked if we have any in Xen).

The minimum version of GCC to build the hypervisor is now raised to 4.9.

In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
shown to emit memory references beyond the stack pointer, resulting in
memory corruption if an interrupt is taken after the stack pointer has
been adjusted but before the reference has been executed.

Therefore, the minimum for arm64 is raised to 5.1.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---

I don't have a strong opinion on the minimum version for GCC on x86.
So this is following Andrew's suggestion and the minimum from Linux.

This patch is candidate to 4.15 and backport.

This is only a build change and will be low-risk for anyone using newer
compiler (5.1+ for arm64 and 4.9 for everyone else). Xen will stop
building for anyone using older compiler. But it is better than fighting
with codegen issues.
---
 README                     |  9 ++++++---
 xen/include/xen/compiler.h | 13 +++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 8c99c30986c1..5c32c03f2ea1 100644
--- a/README
+++ b/README
@@ -38,12 +38,15 @@ provided by your OS distributor:
     * GNU Make v3.80 or later
     * C compiler and linker:
       - For x86:
-        - GCC 4.1.2_20070115 or later
+        - GCC 4.9 or later
         - GNU Binutils 2.16.91.0.5 or later
         or
         - Clang/LLVM 3.5 or later
-      - For ARM:
-        - GCC 4.8 or later
+      - For ARM 32-bit:
+        - GCC 4.9 or later
+        - GNU Binutils 2.24 or later
+      - For ARM 64-bit:
+        - GCC 5.1 or later
         - GNU Binutils 2.24 or later
     * Development install of zlib (e.g., zlib-dev)
     * Development install of Python 2.6 or later (e.g., python-dev)
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index 0ec0b4698ea7..46779660cc8f 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -5,6 +5,19 @@
 #error Sorry, your compiler is too old/not recognized.
 #endif
 
+#if CONFIG_CC_IS_GCC
+# if CONFIG_GCC_VERSION < 40900
+/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
+#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
+# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
+/*
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
+ * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
+ */
+#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
+# endif
+#endif
+
 #define barrier()     __asm__ __volatile__("": : :"memory")
 
 #define likely(x)     __builtin_expect(!!(x),1)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-06 21:41 [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64) Julien Grall
@ 2021-03-07 10:57 ` Roger Pau Monné
  2021-03-07 12:37   ` Julien Grall
  2021-03-08  8:09 ` Jan Beulich
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: Roger Pau Monné @ 2021-03-07 10:57 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Stefano Stabellini, Wei Liu

On Sat, Mar 06, 2021 at 09:41:48PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Compilers older than 4.8 have known codegen issues which can lead to
> silent miscompilation:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> 
> Furthermore, pre-4.9 GCC have known bugs (including things like
> internal compiler errors on Arm) which would require workaround (I
> haven't checked if we have any in Xen).
> 
> The minimum version of GCC to build the hypervisor is now raised to 4.9.

The README entry doesn't differentiate between tools and hypervisor
GCC version, can the bug above also manifest in toolstack code?

I don't think it makes much sense to have diverging requirements for
tools vs hypervisor builds, but if the bug(s) can also manifest in
toolstack code we might want to add a GCC version check to
tools/configure.ac.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-07 10:57 ` Roger Pau Monné
@ 2021-03-07 12:37   ` Julien Grall
  0 siblings, 0 replies; 22+ messages in thread
From: Julien Grall @ 2021-03-07 12:37 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel, Julien Grall, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Stefano Stabellini, Wei Liu

Hi Roger,

On 07/03/2021 10:57, Roger Pau Monné wrote:
> On Sat, Mar 06, 2021 at 09:41:48PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Compilers older than 4.8 have known codegen issues which can lead to
>> silent miscompilation:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>>
>> Furthermore, pre-4.9 GCC have known bugs (including things like
>> internal compiler errors on Arm) which would require workaround (I
>> haven't checked if we have any in Xen).
>>
>> The minimum version of GCC to build the hypervisor is now raised to 4.9.
> 
> The README entry doesn't differentiate between tools and hypervisor
> GCC version, can the bug above also manifest in toolstack code?

I guess it can manifest in the firmware (e.g. hvmloader, rombios). It 
looks rather unlikely in the userspace tools as we seem to barely use 
volatile.

For the Arm64 bug [1], I think it can also show up when receiving a signal.

> 
> I don't think it makes much sense to have diverging requirements for
> tools vs hypervisor builds, but if the bug(s) can also manifest in
> toolstack code we might want to add a GCC version check to
> tools/configure.ac.

Agree. I can add check in ./configure for the toolstack.

Cheers,

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-06 21:41 [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64) Julien Grall
  2021-03-07 10:57 ` Roger Pau Monné
@ 2021-03-08  8:09 ` Jan Beulich
  2021-03-08 10:51   ` Julien Grall
  2021-03-08  9:49 ` Ian Jackson
  2021-03-10 20:48 ` Stefano Stabellini
  3 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2021-03-08  8:09 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

On 06.03.2021 22:41, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Compilers older than 4.8 have known codegen issues which can lead to
> silent miscompilation:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> 
> Furthermore, pre-4.9 GCC have known bugs (including things like
> internal compiler errors on Arm) which would require workaround (I
> haven't checked if we have any in Xen).
> 
> The minimum version of GCC to build the hypervisor is now raised to 4.9.
> 
> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
> shown to emit memory references beyond the stack pointer, resulting in
> memory corruption if an interrupt is taken after the stack pointer has
> been adjusted but before the reference has been executed.
> 
> Therefore, the minimum for arm64 is raised to 5.1.

I'm sure newer compiler versions also have bugs. Therefore I'm not
convinced using this as the primary reason for a bump is enough.
Plus what if critical to us bugs get found in, say, 5.x? Are we
going to bump to 6.x then (and so on, until we allow only the most
recent major version to be used)?

Additionally - partly related to your own reply regarding the CI
failures - imo there needs to be an analysis of what older distros
will no longer build (at all or by default). We've been discussing
to bump minimum tool chain versions for a long time. At least as
far as I'm concerned, I didn't take on this job precisely because
the code changes needed are relatively simple, but justification
can (and apparently will) be rather complicated.

Fundamentally, whatever kind of criteria we use to justify the
bump now ought to be usable down the road by people justifying
further bumps.

Also - what about clang? Linux requires 10.0.1 as a minimum.

> ---
> 
> I don't have a strong opinion on the minimum version for GCC on x86.
> So this is following Andrew's suggestion and the minimum from Linux.
> 
> This patch is candidate to 4.15 and backport.

I think such a change needs to be proposed much earlier in a release
cycle.

> --- a/README
> +++ b/README
> @@ -38,12 +38,15 @@ provided by your OS distributor:
>      * GNU Make v3.80 or later
>      * C compiler and linker:
>        - For x86:
> -        - GCC 4.1.2_20070115 or later
> +        - GCC 4.9 or later
>          - GNU Binutils 2.16.91.0.5 or later

I don't think it makes much sense to keep the binutils version this
low, the more that I don't think we can really build (correctly)
with this old a version anymore. Whatever the gcc version chosen, I
think we want to pick a binutils version from about the same time
frame.

> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -5,6 +5,19 @@
>  #error Sorry, your compiler is too old/not recognized.
>  #endif
>  
> +#if CONFIG_CC_IS_GCC
> +# if CONFIG_GCC_VERSION < 40900
> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */

As per the bug report, the issue was determined to not be present
in e.g. 4.3. Hence while such a bug may influence our choice of
minimum version, I don't think it can reasonably be named here as
the apparent only reason for the choice. Personally I don't think
any justification should be put here.

> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
> +/*
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> + */
> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.

From the bug entry the fix looks to have been backported to 4.9,
or at least some (important?) branches thereof.

> +# endif
> +#endif

Instead of a completely new conditional, I think this wants to be
combined with the existing one (the tail of which is visible in
context above).

Jan


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-06 21:41 [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64) Julien Grall
  2021-03-07 10:57 ` Roger Pau Monné
  2021-03-08  8:09 ` Jan Beulich
@ 2021-03-08  9:49 ` Ian Jackson
  2021-03-10 20:48 ` Stefano Stabellini
  3 siblings, 0 replies; 22+ messages in thread
From: Ian Jackson @ 2021-03-08  9:49 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Andrew Cooper, George Dunlap,
	Jan Beulich, Stefano Stabellini, Wei Liu

Julien Grall writes ("[PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)"):
> From: Julien Grall <jgrall@amazon.com>
> 
> Compilers older than 4.8 have known codegen issues which can lead to
> silent miscompilation:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> 
> Furthermore, pre-4.9 GCC have known bugs (including things like
> internal compiler errors on Arm) which would require workaround (I
> haven't checked if we have any in Xen).
> 
> The minimum version of GCC to build the hypervisor is now raised to 4.9.
> 
> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
> shown to emit memory references beyond the stack pointer, resulting in
> memory corruption if an interrupt is taken after the stack pointer has
> been adjusted but before the reference has been executed.
> 
> Therefore, the minimum for arm64 is raised to 5.1.

How sad.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

I don't currently have an opinion about the merits of this change.
I'm hoping that the disagreement can be resolved without me having to
have one :-).

Thanks,
Ian.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-08  8:09 ` Jan Beulich
@ 2021-03-08 10:51   ` Julien Grall
  2021-03-08 11:20     ` Jan Beulich
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-08 10:51 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel



On 08/03/2021 08:09, Jan Beulich wrote:
> On 06.03.2021 22:41, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Compilers older than 4.8 have known codegen issues which can lead to
>> silent miscompilation:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>>
>> Furthermore, pre-4.9 GCC have known bugs (including things like
>> internal compiler errors on Arm) which would require workaround (I
>> haven't checked if we have any in Xen).
>>
>> The minimum version of GCC to build the hypervisor is now raised to 4.9.
>>
>> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
>> shown to emit memory references beyond the stack pointer, resulting in
>> memory corruption if an interrupt is taken after the stack pointer has
>> been adjusted but before the reference has been executed.
>>
>> Therefore, the minimum for arm64 is raised to 5.1.
> 
> I'm sure newer compiler versions also have bugs.

I don't doubt that...

> Therefore I'm not
> convinced using this as the primary reason for a bump is enough.
You also have to take into account the severity of the bug and possible 
workaround. The bug in 5.1 is severe *and* has no easy (to avoid saying 
possible) workaround.

> Plus what if critical to us bugs get found in, say, 5.x? Are we
> going to bump to 6.x then (and so on, until we allow only the most
> recent major version to be used)?

In the current situation we are claiming that all GCC versions from 
~2014 are supported.

However, in reality, there are an high number of chance that some of the 
version will not build Xen or worse miscompile it.

The former kind of bug is not a big deal because the user will notice it 
directly. However, the latter is highly critical because 1) They may 
only happen sporadically 2) compiler related bug is difficult to root cause.

So I think a smaller set of version is going to be better for the users 
and for us long term.

> 
> Additionally - partly related to your own reply regarding the CI
> failures - imo there needs to be an analysis of what older distros
> will no longer build (at all or by default).

Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)

Do you have any other in mind?

> We've been discussing
> to bump minimum tool chain versions for a long time. At least as
> far as I'm concerned, I didn't take on this job precisely because
> the code changes needed are relatively simple, but justification
> can (and apparently will) be rather complicated.
>
> Fundamentally, whatever kind of criteria we use to justify the
> bump now ought to be usable down the road by people justifying
> further bumps.
I see you mentioned about distros. Can you outline other criteria you 
have in mind?

> 
> Also - what about clang? Linux requires 10.0.1 as a minimum.

I don't know and I would rather focus on GCC first so we figure out a 
set of criterias...

> 
>> ---
>>
>> I don't have a strong opinion on the minimum version for GCC on x86.
>> So this is following Andrew's suggestion and the minimum from Linux.
>>
>> This patch is candidate to 4.15 and backport.
> 
> I think such a change needs to be proposed much earlier in a release
> cycle.

Possibly... At the same time, we don't want to release 4.15 and continue 
to say Xen on Arm64 can build fine with 4.9 and 5.0.

> 
>> --- a/README
>> +++ b/README
>> @@ -38,12 +38,15 @@ provided by your OS distributor:
>>       * GNU Make v3.80 or later
>>       * C compiler and linker:
>>         - For x86:
>> -        - GCC 4.1.2_20070115 or later
>> +        - GCC 4.9 or later
>>           - GNU Binutils 2.16.91.0.5 or later
> 
> I don't think it makes much sense to keep the binutils version this
> low, the more that I don't think we can really build (correctly)
> with this old a version anymore. Whatever the gcc version chosen, I
> think we want to pick a binutils version from about the same time
> frame.

Ok. Let's first agree on a GCC version and then we can decide on a 
binutils version.

> 
>> --- a/xen/include/xen/compiler.h
>> +++ b/xen/include/xen/compiler.h
>> @@ -5,6 +5,19 @@
>>   #error Sorry, your compiler is too old/not recognized.
>>   #endif
>>   
>> +#if CONFIG_CC_IS_GCC
>> +# if CONFIG_GCC_VERSION < 40900
>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
> 
> As per the bug report, the issue was determined to not be present
> in e.g. 4.3. Hence while such a bug may influence our choice of
> minimum version, I don't think it can reasonably be named here as
> the apparent only reason for the choice. Personally I don't think
> any justification should be put here.

Ok.

> 
>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>> +/*
>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>> + */
>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
> 
>  From the bug entry the fix looks to have been backported to 4.9,
> or at least some (important?) branches thereof.

It is not clear what's you are trying to point out. Mind clarifying?

> 
>> +# endif
>> +#endif
> 
> Instead of a completely new conditional, I think this wants to be
> combined with the existing one (the tail of which is visible in
> context above).

OK.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-08 10:51   ` Julien Grall
@ 2021-03-08 11:20     ` Jan Beulich
  2021-03-08 20:22       ` Julien Grall
  0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2021-03-08 11:20 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

On 08.03.2021 11:51, Julien Grall wrote:
> On 08/03/2021 08:09, Jan Beulich wrote:
>> On 06.03.2021 22:41, Julien Grall wrote:
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> Compilers older than 4.8 have known codegen issues which can lead to
>>> silent miscompilation:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>>>
>>> Furthermore, pre-4.9 GCC have known bugs (including things like
>>> internal compiler errors on Arm) which would require workaround (I
>>> haven't checked if we have any in Xen).
>>>
>>> The minimum version of GCC to build the hypervisor is now raised to 4.9.
>>>
>>> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
>>> shown to emit memory references beyond the stack pointer, resulting in
>>> memory corruption if an interrupt is taken after the stack pointer has
>>> been adjusted but before the reference has been executed.
>>>
>>> Therefore, the minimum for arm64 is raised to 5.1.
>>
>> I'm sure newer compiler versions also have bugs.
> 
> I don't doubt that...
> 
>> Therefore I'm not
>> convinced using this as the primary reason for a bump is enough.
> You also have to take into account the severity of the bug and possible 
> workaround. The bug in 5.1 is severe *and* has no easy (to avoid saying 
> possible) workaround.
> 
>> Plus what if critical to us bugs get found in, say, 5.x? Are we
>> going to bump to 6.x then (and so on, until we allow only the most
>> recent major version to be used)?
> 
> In the current situation we are claiming that all GCC versions from 
> ~2014 are supported.
> 
> However, in reality, there are an high number of chance that some of the 
> version will not build Xen or worse miscompile it.
> 
> The former kind of bug is not a big deal because the user will notice it 
> directly. However, the latter is highly critical because 1) They may 
> only happen sporadically 2) compiler related bug is difficult to root cause.

Since I realize it may not have been clear from my initial reply:
What you want to do for Arm64 is largely up to you. I agree that
the bug in question is really bad.

I understand it was Andrew who asked you to cover x86 at the same
time, so perhaps it should be more him than you to justify the
choice. Yet you've submitted the patch, so (I'm sorry) there you
go ...

>> Additionally - partly related to your own reply regarding the CI
>> failures - imo there needs to be an analysis of what older distros
>> will no longer build (at all or by default).
> 
> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
> 
> Do you have any other in mind?

Our SLE12 (latest service pack is SP5 and still has a while to go
to at least reach LTSS state) comes with gcc 4.8 as the default
compiler.

>>> ---
>>>
>>> I don't have a strong opinion on the minimum version for GCC on x86.
>>> So this is following Andrew's suggestion and the minimum from Linux.
>>>
>>> This patch is candidate to 4.15 and backport.
>>
>> I think such a change needs to be proposed much earlier in a release
>> cycle.
> 
> Possibly... At the same time, we don't want to release 4.15 and continue 
> to say Xen on Arm64 can build fine with 4.9 and 5.0.

4.9 could be excluded if need be. 4.8 is fine? Also I don't think
there's much point thinking about 5.0 - that's where their version
numbering scheme changed and the first released version is 5.1.

>>> --- a/xen/include/xen/compiler.h
>>> +++ b/xen/include/xen/compiler.h
>>> @@ -5,6 +5,19 @@
>>>   #error Sorry, your compiler is too old/not recognized.
>>>   #endif
>>>   
>>> +#if CONFIG_CC_IS_GCC
>>> +# if CONFIG_GCC_VERSION < 40900
>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>
>> As per the bug report, the issue was determined to not be present
>> in e.g. 4.3. Hence while such a bug may influence our choice of
>> minimum version, I don't think it can reasonably be named here as
>> the apparent only reason for the choice. Personally I don't think
>> any justification should be put here.
> 
> Ok.
> 
>>
>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>> +/*
>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>> + */
>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>
>>  From the bug entry the fix looks to have been backported to 4.9,
>> or at least some (important?) branches thereof.
> 
> It is not clear what's you are trying to point out. Mind clarifying?

Some 4.9 compilers (perhaps widely used ones) may not have the bad
issue, which puts under question their ruling out when the main
reason for doing so is that bug.

Jan


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-08 11:20     ` Jan Beulich
@ 2021-03-08 20:22       ` Julien Grall
  2021-03-09 11:20         ` Jan Beulich
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-08 20:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

Hi Jan,

On 08/03/2021 11:20, Jan Beulich wrote:
> On 08.03.2021 11:51, Julien Grall wrote:
>> On 08/03/2021 08:09, Jan Beulich wrote:
>>> On 06.03.2021 22:41, Julien Grall wrote:
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> Compilers older than 4.8 have known codegen issues which can lead to
>>>> silent miscompilation:
>>>>
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>>>>
>>>> Furthermore, pre-4.9 GCC have known bugs (including things like
>>>> internal compiler errors on Arm) which would require workaround (I
>>>> haven't checked if we have any in Xen).
>>>>
>>>> The minimum version of GCC to build the hypervisor is now raised to 4.9.
>>>>
>>>> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
>>>> shown to emit memory references beyond the stack pointer, resulting in
>>>> memory corruption if an interrupt is taken after the stack pointer has
>>>> been adjusted but before the reference has been executed.
>>>>
>>>> Therefore, the minimum for arm64 is raised to 5.1.
>>>
>>> I'm sure newer compiler versions also have bugs.
>>
>> I don't doubt that...
>>
>>> Therefore I'm not
>>> convinced using this as the primary reason for a bump is enough.
>> You also have to take into account the severity of the bug and possible
>> workaround. The bug in 5.1 is severe *and* has no easy (to avoid saying
>> possible) workaround.
>>
>>> Plus what if critical to us bugs get found in, say, 5.x? Are we
>>> going to bump to 6.x then (and so on, until we allow only the most
>>> recent major version to be used)?
>>
>> In the current situation we are claiming that all GCC versions from
>> ~2014 are supported.
>>
>> However, in reality, there are an high number of chance that some of the
>> version will not build Xen or worse miscompile it.
>>
>> The former kind of bug is not a big deal because the user will notice it
>> directly. However, the latter is highly critical because 1) They may
>> only happen sporadically 2) compiler related bug is difficult to root cause.
> 
> Since I realize it may not have been clear from my initial reply:
> What you want to do for Arm64 is largely up to you. I agree that
> the bug in question is really bad.

Ah, thanks for the clarification.

> I understand it was Andrew who asked you to cover x86 at the same
> time, so perhaps it should be more him than you to justify the
> choice. Yet you've submitted the patch, so (I'm sorry) there you
> go ...

As I initially wrote, I don't particularly mind the version for x86.

I can split the patch in two to avoid mixing the discussions.

> 
>>> Additionally - partly related to your own reply regarding the CI
>>> failures - imo there needs to be an analysis of what older distros
>>> will no longer build (at all or by default).
>>
>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>
>> Do you have any other in mind?
> 
> Our SLE12 (latest service pack is SP5 and still has a while to go
> to at least reach LTSS state) comes with gcc 4.8 as the default
> compiler.

Thanks! That's good to know. Is it the old GCC Suse supports?

> 
>>>> ---
>>>>
>>>> I don't have a strong opinion on the minimum version for GCC on x86.
>>>> So this is following Andrew's suggestion and the minimum from Linux.
>>>>
>>>> This patch is candidate to 4.15 and backport.
>>>
>>> I think such a change needs to be proposed much earlier in a release
>>> cycle.
>>
>> Possibly... At the same time, we don't want to release 4.15 and continue
>> to say Xen on Arm64 can build fine with 4.9 and 5.0.
> 
> 4.9 could be excluded if need be. 4.8 is fine?

I am not entirely sure. The bug I pointed out in the commit message 
([1]) seems to affect any GCC version until 4.8.

> Also I don't think
> there's much point thinking about 5.0 - that's where their version
> numbering scheme changed and the first released version is 5.1.

Good point. I didn't released that 5.0 never existed.

>>>> --- a/xen/include/xen/compiler.h
>>>> +++ b/xen/include/xen/compiler.h
>>>> @@ -5,6 +5,19 @@
>>>>    #error Sorry, your compiler is too old/not recognized.
>>>>    #endif
>>>>    
>>>> +#if CONFIG_CC_IS_GCC
>>>> +# if CONFIG_GCC_VERSION < 40900
>>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>>
>>> As per the bug report, the issue was determined to not be present
>>> in e.g. 4.3. Hence while such a bug may influence our choice of
>>> minimum version, I don't think it can reasonably be named here as
>>> the apparent only reason for the choice. Personally I don't think
>>> any justification should be put here.
>>
>> Ok.
>>
>>>
>>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>>> +/*
>>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>>> + */
>>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>>
>>>   From the bug entry the fix looks to have been backported to 4.9,
>>> or at least some (important?) branches thereof.
>>
>> It is not clear what's you are trying to point out. Mind clarifying?
> 
> Some 4.9 compilers (perhaps widely used ones) may not have the bad
> issue, which puts under question their ruling out when the main
> reason for doing so is that bug.

Well... We could surely try to hunt which GCC 4.9 has been fixed. But I 
am not convinced this is useful, we would need to have an allowlist of 
GCC compiler.

 From my experience, the best way to know if someone care about a 
specific version is to bump it and see who shout. If no one shout, then 
you know that it wasn't important.

If someone shout, then we can decide how to re-enable support for GCC X.Y.

The fact Linux bumped to 5.1 on Arm64 give me some confidence that the 
move is not risky.

I know you may not be happy with my problem, so one possible mitigation 
is to allow a developer to override the check using Kconfig (maybe gated 
with UNSUPPORTED/EXPERT).


[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-08 20:22       ` Julien Grall
@ 2021-03-09 11:20         ` Jan Beulich
  2021-03-10 18:05           ` Julien Grall
  0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2021-03-09 11:20 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

On 08.03.2021 21:22, Julien Grall wrote:
> On 08/03/2021 11:20, Jan Beulich wrote:
>> On 08.03.2021 11:51, Julien Grall wrote:
>>> On 08/03/2021 08:09, Jan Beulich wrote:
>>>> Additionally - partly related to your own reply regarding the CI
>>>> failures - imo there needs to be an analysis of what older distros
>>>> will no longer build (at all or by default).
>>>
>>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>>
>>> Do you have any other in mind?
>>
>> Our SLE12 (latest service pack is SP5 and still has a while to go
>> to at least reach LTSS state) comes with gcc 4.8 as the default
>> compiler.
> 
> Thanks! That's good to know. Is it the old GCC Suse supports?

Not sure I understand the question: The default compiler of this or
any distro is of course (expected to be) supported by the vendor
for the lifetime of the OS.

>>>>> --- a/xen/include/xen/compiler.h
>>>>> +++ b/xen/include/xen/compiler.h
>>>>> @@ -5,6 +5,19 @@
>>>>>    #error Sorry, your compiler is too old/not recognized.
>>>>>    #endif
>>>>>    
>>>>> +#if CONFIG_CC_IS_GCC
>>>>> +# if CONFIG_GCC_VERSION < 40900
>>>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>>>
>>>> As per the bug report, the issue was determined to not be present
>>>> in e.g. 4.3. Hence while such a bug may influence our choice of
>>>> minimum version, I don't think it can reasonably be named here as
>>>> the apparent only reason for the choice. Personally I don't think
>>>> any justification should be put here.
>>>
>>> Ok.
>>>
>>>>
>>>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>>>> +/*
>>>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>>>> + */
>>>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>>>
>>>>   From the bug entry the fix looks to have been backported to 4.9,
>>>> or at least some (important?) branches thereof.
>>>
>>> It is not clear what's you are trying to point out. Mind clarifying?
>>
>> Some 4.9 compilers (perhaps widely used ones) may not have the bad
>> issue, which puts under question their ruling out when the main
>> reason for doing so is that bug.
> 
> Well... We could surely try to hunt which GCC 4.9 has been fixed. But I 
> am not convinced this is useful, we would need to have an allowlist of 
> GCC compiler.

Or probe the compiler for the bug in question.

Jan


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09 11:20         ` Jan Beulich
@ 2021-03-10 18:05           ` Julien Grall
  2021-03-11  8:17             ` Jan Beulich
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-10 18:05 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

Hi Jan,

On 09/03/2021 11:20, Jan Beulich wrote:
> On 08.03.2021 21:22, Julien Grall wrote:
>> On 08/03/2021 11:20, Jan Beulich wrote:
>>> On 08.03.2021 11:51, Julien Grall wrote:
>>>> On 08/03/2021 08:09, Jan Beulich wrote:
>>>>> Additionally - partly related to your own reply regarding the CI
>>>>> failures - imo there needs to be an analysis of what older distros
>>>>> will no longer build (at all or by default).
>>>>
>>>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>>>
>>>> Do you have any other in mind?
>>>
>>> Our SLE12 (latest service pack is SP5 and still has a while to go
>>> to at least reach LTSS state) comes with gcc 4.8 as the default
>>> compiler.
>>
>> Thanks! That's good to know. Is it the old GCC Suse supports?
> 
> Not sure I understand the question: The default compiler of this or
> any distro is of course (expected to be) supported by the vendor
> for the lifetime of the OS.

Sorry for the wording. I was asking whether Suse also supports compiler 
older than GCC 4.8.

> 
>>>>>> --- a/xen/include/xen/compiler.h
>>>>>> +++ b/xen/include/xen/compiler.h
>>>>>> @@ -5,6 +5,19 @@
>>>>>>     #error Sorry, your compiler is too old/not recognized.
>>>>>>     #endif
>>>>>>     
>>>>>> +#if CONFIG_CC_IS_GCC
>>>>>> +# if CONFIG_GCC_VERSION < 40900
>>>>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>>>>
>>>>> As per the bug report, the issue was determined to not be present
>>>>> in e.g. 4.3. Hence while such a bug may influence our choice of
>>>>> minimum version, I don't think it can reasonably be named here as
>>>>> the apparent only reason for the choice. Personally I don't think
>>>>> any justification should be put here.
>>>>
>>>> Ok.
>>>>
>>>>>
>>>>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>>>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>>>>> +/*
>>>>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>>>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>>>>> + */
>>>>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>>>>
>>>>>    From the bug entry the fix looks to have been backported to 4.9,
>>>>> or at least some (important?) branches thereof.
>>>>
>>>> It is not clear what's you are trying to point out. Mind clarifying?
>>>
>>> Some 4.9 compilers (perhaps widely used ones) may not have the bad
>>> issue, which puts under question their ruling out when the main
>>> reason for doing so is that bug.
>>
>> Well... We could surely try to hunt which GCC 4.9 has been fixed. But I
>> am not convinced this is useful, we would need to have an allowlist of
>> GCC compiler.
> 
> Or probe the compiler for the bug in question.

I thought about it but it is not clear to me whether the reproducer 
would work on every GCC version and how to detect that this was miscompiled.

Do you have any suggestion?

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-06 21:41 [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64) Julien Grall
                   ` (2 preceding siblings ...)
  2021-03-08  9:49 ` Ian Jackson
@ 2021-03-10 20:48 ` Stefano Stabellini
  3 siblings, 0 replies; 22+ messages in thread
From: Stefano Stabellini @ 2021-03-10 20:48 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Stefano Stabellini, Wei Liu

On Sat, 6 Mar 2021, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Compilers older than 4.8 have known codegen issues which can lead to
> silent miscompilation:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> 
> Furthermore, pre-4.9 GCC have known bugs (including things like
> internal compiler errors on Arm) which would require workaround (I
> haven't checked if we have any in Xen).
> 
> The minimum version of GCC to build the hypervisor is now raised to 4.9.
> 
> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
> shown to emit memory references beyond the stack pointer, resulting in
> memory corruption if an interrupt is taken after the stack pointer has
> been adjusted but before the reference has been executed.
> 
> Therefore, the minimum for arm64 is raised to 5.1.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

For the ARM part:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 
> I don't have a strong opinion on the minimum version for GCC on x86.
> So this is following Andrew's suggestion and the minimum from Linux.
> 
> This patch is candidate to 4.15 and backport.
> 
> This is only a build change and will be low-risk for anyone using newer
> compiler (5.1+ for arm64 and 4.9 for everyone else). Xen will stop
> building for anyone using older compiler. But it is better than fighting
> with codegen issues.
> ---
>  README                     |  9 ++++++---
>  xen/include/xen/compiler.h | 13 +++++++++++++
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/README b/README
> index 8c99c30986c1..5c32c03f2ea1 100644
> --- a/README
> +++ b/README
> @@ -38,12 +38,15 @@ provided by your OS distributor:
>      * GNU Make v3.80 or later
>      * C compiler and linker:
>        - For x86:
> -        - GCC 4.1.2_20070115 or later
> +        - GCC 4.9 or later
>          - GNU Binutils 2.16.91.0.5 or later
>          or
>          - Clang/LLVM 3.5 or later
> -      - For ARM:
> -        - GCC 4.8 or later
> +      - For ARM 32-bit:
> +        - GCC 4.9 or later
> +        - GNU Binutils 2.24 or later
> +      - For ARM 64-bit:
> +        - GCC 5.1 or later
>          - GNU Binutils 2.24 or later
>      * Development install of zlib (e.g., zlib-dev)
>      * Development install of Python 2.6 or later (e.g., python-dev)
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 0ec0b4698ea7..46779660cc8f 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -5,6 +5,19 @@
>  #error Sorry, your compiler is too old/not recognized.
>  #endif
>  
> +#if CONFIG_CC_IS_GCC
> +# if CONFIG_GCC_VERSION < 40900
> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
> +/*
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> + */
> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
> +# endif
> +#endif
> +
>  #define barrier()     __asm__ __volatile__("": : :"memory")
>  
>  #define likely(x)     __builtin_expect(!!(x),1)
 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-10 18:05           ` Julien Grall
@ 2021-03-11  8:17             ` Jan Beulich
  2021-03-11  9:31               ` Julien Grall
  0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2021-03-11  8:17 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

On 10.03.2021 19:05, Julien Grall wrote:
> On 09/03/2021 11:20, Jan Beulich wrote:
>> On 08.03.2021 21:22, Julien Grall wrote:
>>> On 08/03/2021 11:20, Jan Beulich wrote:
>>>> On 08.03.2021 11:51, Julien Grall wrote:
>>>>> On 08/03/2021 08:09, Jan Beulich wrote:
>>>>>> Additionally - partly related to your own reply regarding the CI
>>>>>> failures - imo there needs to be an analysis of what older distros
>>>>>> will no longer build (at all or by default).
>>>>>
>>>>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>>>>
>>>>> Do you have any other in mind?
>>>>
>>>> Our SLE12 (latest service pack is SP5 and still has a while to go
>>>> to at least reach LTSS state) comes with gcc 4.8 as the default
>>>> compiler.
>>>
>>> Thanks! That's good to know. Is it the old GCC Suse supports?
>>
>> Not sure I understand the question: The default compiler of this or
>> any distro is of course (expected to be) supported by the vendor
>> for the lifetime of the OS.
> 
> Sorry for the wording. I was asking whether Suse also supports compiler 
> older than GCC 4.8.

Oh, I see. SLES11 has another year to go for LTSS to end, and it's 4.3
which is used there.

>>>>>>> --- a/xen/include/xen/compiler.h
>>>>>>> +++ b/xen/include/xen/compiler.h
>>>>>>> @@ -5,6 +5,19 @@
>>>>>>>     #error Sorry, your compiler is too old/not recognized.
>>>>>>>     #endif
>>>>>>>     
>>>>>>> +#if CONFIG_CC_IS_GCC
>>>>>>> +# if CONFIG_GCC_VERSION < 40900
>>>>>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>>>>>
>>>>>> As per the bug report, the issue was determined to not be present
>>>>>> in e.g. 4.3. Hence while such a bug may influence our choice of
>>>>>> minimum version, I don't think it can reasonably be named here as
>>>>>> the apparent only reason for the choice. Personally I don't think
>>>>>> any justification should be put here.
>>>>>
>>>>> Ok.
>>>>>
>>>>>>
>>>>>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>>>>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>>>>>> +/*
>>>>>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>>>>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>>>>>> + */
>>>>>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>>>>>
>>>>>>    From the bug entry the fix looks to have been backported to 4.9,
>>>>>> or at least some (important?) branches thereof.
>>>>>
>>>>> It is not clear what's you are trying to point out. Mind clarifying?
>>>>
>>>> Some 4.9 compilers (perhaps widely used ones) may not have the bad
>>>> issue, which puts under question their ruling out when the main
>>>> reason for doing so is that bug.
>>>
>>> Well... We could surely try to hunt which GCC 4.9 has been fixed. But I
>>> am not convinced this is useful, we would need to have an allowlist of
>>> GCC compiler.
>>
>> Or probe the compiler for the bug in question.
> 
> I thought about it but it is not clear to me whether the reproducer 
> would work on every GCC version and how to detect that this was miscompiled.
> 
> Do you have any suggestion?

To have one I'd have to study the problem in quite a bit more detail.
But I did say that what you do for Arm is largely up to you (and
Stefano did meanwhile ack the Arm side), so I'm not sure I want to
invest the time that would be needed.

Jan


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-11  8:17             ` Jan Beulich
@ 2021-03-11  9:31               ` Julien Grall
  2021-03-11 11:43                 ` Jan Beulich
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-11  9:31 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

Hi Jan,

On 11/03/2021 08:17, Jan Beulich wrote:
> On 10.03.2021 19:05, Julien Grall wrote:
>> On 09/03/2021 11:20, Jan Beulich wrote:
>>> On 08.03.2021 21:22, Julien Grall wrote:
>>>> On 08/03/2021 11:20, Jan Beulich wrote:
>>>>> On 08.03.2021 11:51, Julien Grall wrote:
>>>>>> On 08/03/2021 08:09, Jan Beulich wrote:
>>>>>>> Additionally - partly related to your own reply regarding the CI
>>>>>>> failures - imo there needs to be an analysis of what older distros
>>>>>>> will no longer build (at all or by default).
>>>>>>
>>>>>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>>>>>
>>>>>> Do you have any other in mind?
>>>>>
>>>>> Our SLE12 (latest service pack is SP5 and still has a while to go
>>>>> to at least reach LTSS state) comes with gcc 4.8 as the default
>>>>> compiler.
>>>>
>>>> Thanks! That's good to know. Is it the old GCC Suse supports?
>>>
>>> Not sure I understand the question: The default compiler of this or
>>> any distro is of course (expected to be) supported by the vendor
>>> for the lifetime of the OS.
>>
>> Sorry for the wording. I was asking whether Suse also supports compiler
>> older than GCC 4.8.
> 
> Oh, I see. SLES11 has another year to go for LTSS to end, and it's 4.3
> which is used there.

Thanks for the info! Are you planning to build Xen 4.15 there too?

> 
>>>>>>>> --- a/xen/include/xen/compiler.h
>>>>>>>> +++ b/xen/include/xen/compiler.h
>>>>>>>> @@ -5,6 +5,19 @@
>>>>>>>>      #error Sorry, your compiler is too old/not recognized.
>>>>>>>>      #endif
>>>>>>>>      
>>>>>>>> +#if CONFIG_CC_IS_GCC
>>>>>>>> +# if CONFIG_GCC_VERSION < 40900
>>>>>>>> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>>>>>>>
>>>>>>> As per the bug report, the issue was determined to not be present
>>>>>>> in e.g. 4.3. Hence while such a bug may influence our choice of
>>>>>>> minimum version, I don't think it can reasonably be named here as
>>>>>>> the apparent only reason for the choice. Personally I don't think
>>>>>>> any justification should be put here.
>>>>>>
>>>>>> Ok.
>>>>>>
>>>>>>>
>>>>>>>> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
>>>>>>>> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
>>>>>>>> +/*
>>>>>>>> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
>>>>>>>> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
>>>>>>>> + */
>>>>>>>> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.
>>>>>>>
>>>>>>>     From the bug entry the fix looks to have been backported to 4.9,
>>>>>>> or at least some (important?) branches thereof.
>>>>>>
>>>>>> It is not clear what's you are trying to point out. Mind clarifying?
>>>>>
>>>>> Some 4.9 compilers (perhaps widely used ones) may not have the bad
>>>>> issue, which puts under question their ruling out when the main
>>>>> reason for doing so is that bug.
>>>>
>>>> Well... We could surely try to hunt which GCC 4.9 has been fixed. But I
>>>> am not convinced this is useful, we would need to have an allowlist of
>>>> GCC compiler.
>>>
>>> Or probe the compiler for the bug in question.
>>
>> I thought about it but it is not clear to me whether the reproducer
>> would work on every GCC version and how to detect that this was miscompiled.
>>
>> Do you have any suggestion?
> 
> To have one I'd have to study the problem in quite a bit more detail.
> But I did say that what you do for Arm is largely up to you (and
> Stefano did meanwhile ack the Arm side), so I'm not sure I want to
> invest the time that would be needed.

Well, there are still miscompilation problem on x86... So it would be 
good to have a way to address it.

Anyway, I will split the Arm change in a separate patch so it can go in 
4.15.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-11  9:31               ` Julien Grall
@ 2021-03-11 11:43                 ` Jan Beulich
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2021-03-11 11:43 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, Andrew Cooper, George Dunlap, Ian Jackson,
	Stefano Stabellini, Wei Liu, xen-devel

On 11.03.2021 10:31, Julien Grall wrote:
> On 11/03/2021 08:17, Jan Beulich wrote:
>> On 10.03.2021 19:05, Julien Grall wrote:
>>> On 09/03/2021 11:20, Jan Beulich wrote:
>>>> On 08.03.2021 21:22, Julien Grall wrote:
>>>>> On 08/03/2021 11:20, Jan Beulich wrote:
>>>>>> On 08.03.2021 11:51, Julien Grall wrote:
>>>>>>> On 08/03/2021 08:09, Jan Beulich wrote:
>>>>>>>> Additionally - partly related to your own reply regarding the CI
>>>>>>>> failures - imo there needs to be an analysis of what older distros
>>>>>>>> will no longer build (at all or by default).
>>>>>>>
>>>>>>> Per the CI, this would be Ubuntu Trusty (and older), Centos 7 (and older)
>>>>>>>
>>>>>>> Do you have any other in mind?
>>>>>>
>>>>>> Our SLE12 (latest service pack is SP5 and still has a while to go
>>>>>> to at least reach LTSS state) comes with gcc 4.8 as the default
>>>>>> compiler.
>>>>>
>>>>> Thanks! That's good to know. Is it the old GCC Suse supports?
>>>>
>>>> Not sure I understand the question: The default compiler of this or
>>>> any distro is of course (expected to be) supported by the vendor
>>>> for the lifetime of the OS.
>>>
>>> Sorry for the wording. I was asking whether Suse also supports compiler
>>> older than GCC 4.8.
>>
>> Oh, I see. SLES11 has another year to go for LTSS to end, and it's 4.3
>> which is used there.
> 
> Thanks for the info! Are you planning to build Xen 4.15 there too?

If "you" is the company - there are no plans to provide 4.15 there. But
I do build Xen myself on such systems. That's how I'm noticing
compatibility issues like the one in grant table code where you don't
like the workaround.

Jan


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-10  7:52           ` Roger Pau Monné
@ 2021-03-10 10:24             ` Zheng, Fam
  0 siblings, 0 replies; 22+ messages in thread
From: Zheng, Fam @ 2021-03-10 10:24 UTC (permalink / raw)
  To: roger.pau, sstabellini
  Cc: george.dunlap, anthony.perard, andrew.cooper3, cardoe,
	Bertrand.Marquis, julien, wl, xen-devel

On Wed, 2021-03-10 at 08:52 +0100, Roger Pau Monné wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> On Tue, Mar 09, 2021 at 02:24:05PM -0800, Stefano Stabellini wrote:
> > On Tue, 9 Mar 2021, Roger Pau Monné wrote:
> > > On Tue, Mar 09, 2021 at 10:05:18AM +0000, Julien Grall wrote:
> > > > Hi Stefano,
> > > > 
> > > > On 09/03/2021 00:59, Stefano Stabellini wrote:
> > > > > On Sun, 7 Mar 2021, Julien Grall wrote:
> > > > > > On 06/03/2021 23:04, no-reply@patchew.org wrote:
> > > 
> > > Where does one has to register to get the patchew build failures?
> > 
> > By asking Fam (CC'ed) very nicely :-)
> 
> Thanks! Could we maybe setup a mailing list for those emails? ie:
> patchew-tests@... or some such, so that we don't have to go adding
> people manually?
> 
> Roger.

The idea (I think) is to group reply all once we feel comfortable with
the false positive rate. But a separate mailing list is also a good
approach.

I'll add you by hand for now, Roger.

Fam

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09 22:24         ` Stefano Stabellini
@ 2021-03-10  7:52           ` Roger Pau Monné
  2021-03-10 10:24             ` Zheng, Fam
  0 siblings, 1 reply; 22+ messages in thread
From: Roger Pau Monné @ 2021-03-10  7:52 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Julien Grall, xen-devel, famzheng, cardoe, wl, Bertrand.Marquis,
	andrew.cooper3, George Dunlap, Anthony PERARD

On Tue, Mar 09, 2021 at 02:24:05PM -0800, Stefano Stabellini wrote:
> On Tue, 9 Mar 2021, Roger Pau Monné wrote:
> > On Tue, Mar 09, 2021 at 10:05:18AM +0000, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 09/03/2021 00:59, Stefano Stabellini wrote:
> > > > On Sun, 7 Mar 2021, Julien Grall wrote:
> > > > > On 06/03/2021 23:04, no-reply@patchew.org wrote:
> > 
> > Where does one has to register to get the patchew build failures?
> 
> By asking Fam (CC'ed) very nicely :-)

Thanks! Could we maybe setup a mailing list for those emails? ie:
patchew-tests@... or some such, so that we don't have to go adding
people manually?

Roger.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09 10:32       ` Roger Pau Monné
  2021-03-09 11:11         ` Anthony PERARD
@ 2021-03-09 22:24         ` Stefano Stabellini
  2021-03-10  7:52           ` Roger Pau Monné
  1 sibling, 1 reply; 22+ messages in thread
From: Stefano Stabellini @ 2021-03-09 22:24 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Julien Grall, Stefano Stabellini, xen-devel, famzheng, cardoe,
	wl, Bertrand.Marquis, andrew.cooper3, George Dunlap,
	Anthony PERARD

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

On Tue, 9 Mar 2021, Roger Pau Monné wrote:
> On Tue, Mar 09, 2021 at 10:05:18AM +0000, Julien Grall wrote:
> > Hi Stefano,
> > 
> > On 09/03/2021 00:59, Stefano Stabellini wrote:
> > > On Sun, 7 Mar 2021, Julien Grall wrote:
> > > > On 06/03/2021 23:04, no-reply@patchew.org wrote:
> 
> Where does one has to register to get the patchew build failures?

By asking Fam (CC'ed) very nicely :-)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09 10:32       ` Roger Pau Monné
@ 2021-03-09 11:11         ` Anthony PERARD
  2021-03-09 22:24         ` Stefano Stabellini
  1 sibling, 0 replies; 22+ messages in thread
From: Anthony PERARD @ 2021-03-09 11:11 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Julien Grall, Stefano Stabellini, xen-devel, famzheng, cardoe,
	wl, Bertrand.Marquis, andrew.cooper3, George Dunlap

On Tue, Mar 09, 2021 at 11:32:47AM +0100, Roger Pau Monné wrote:
> Does CentOS has something like Debian backports to get updated major
> versions of packages?

Something similar, it's done via SIG. We have SIG virt in which we
build Xen packages among others. And there's a SIG which building newer
version of GCC, we already use it to build the Linux kernel with a newer
version of GCC.

So we should be able to setup the Xen packages to build with a newer
version of GCC, at least in CentOS 7. (I still have to look again at
CentOS 8 or Stream.)

> Also, I think we do maintain Xen on CentOS using a separate
> repository, so newer versions of Xen will be added to CentOS. Adding
> George and Anthony since they know about that.
> 
> Thanks, Roger.

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09 10:05     ` Julien Grall
@ 2021-03-09 10:32       ` Roger Pau Monné
  2021-03-09 11:11         ` Anthony PERARD
  2021-03-09 22:24         ` Stefano Stabellini
  0 siblings, 2 replies; 22+ messages in thread
From: Roger Pau Monné @ 2021-03-09 10:32 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, xen-devel, famzheng, cardoe, wl,
	Bertrand.Marquis, andrew.cooper3, George Dunlap, Anthony PERARD

On Tue, Mar 09, 2021 at 10:05:18AM +0000, Julien Grall wrote:
> Hi Stefano,
> 
> On 09/03/2021 00:59, Stefano Stabellini wrote:
> > On Sun, 7 Mar 2021, Julien Grall wrote:
> > > On 06/03/2021 23:04, no-reply@patchew.org wrote:

Where does one has to register to get the patchew build failures?

> > > > Hi,
> > > > 
> > > > Patchew automatically ran gitlab-ci pipeline with this patch (series)
> > > > applied, but the job failed. Maybe there's a bug in the patches?
> > > > 
> > > > You can find the link to the pipeline near the end of the report below:
> > > > 
> > > > Type: series
> > > > Message-id: 20210306214148.27021-1-julien@xen.org
> > > > Subject: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to
> > > > 4.9 (5.1 on arm64)
> > > > 
> > > > === TEST SCRIPT BEGIN ===
> > > > #!/bin/bash
> > > > sleep 10
> > > > patchew gitlab-pipeline-check -p xen-project/patchew/xen
> > > > === TEST SCRIPT END ===
> > > > 
> > > > warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> > > > warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> > > >   From https://gitlab.com/xen-project/patchew/xen
> > > >    * [new tag]               patchew/20210306214148.27021-1-julien@xen.org ->
> > > > patchew/20210306214148.27021-1-julien@xen.org
> > > > Switched to a new branch 'test'
> > > > f332859b88 xen: Bump the minimum version of GCC supported to 4.9 (5.1 on
> > > > arm64)
> > > > 
> > > > === OUTPUT BEGIN ===
> > > > [2021-03-06 21:45:17] Looking up pipeline...
> > > > [2021-03-06 21:45:18] Found pipeline 266507477:
> > > > 
> > > > https://gitlab.com/xen-project/patchew/xen/-/pipelines/266507477
> > > > 
> > > > [2021-03-06 21:45:18] Waiting for pipeline to finish...
> > > > [2021-03-06 22:00:21] Still waiting...
> > > > [2021-03-06 22:15:26] Still waiting...
> > > > [2021-03-06 22:30:31] Still waiting...
> > > > [2021-03-06 22:45:35] Still waiting...
> > > > [2021-03-06 23:00:41] Still waiting...
> > > > [2021-03-06 23:04:42] Pipeline failed
> > > > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang-pvh' in stage 'test' is
> > > > skipped
> > > > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc-pvh' in stage 'test' is
> > > > skipped
> > > > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang' in stage 'test' is
> > > > skipped
> > > > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc' in stage 'test' is skipped
> > > > [2021-03-06 23:04:42] Job 'qemu-smoke-arm64-gcc' in stage 'test' is skipped
> > > > [2021-03-06 23:04:42] Job 'qemu-alpine-arm64-gcc' in stage 'test' is skipped
> > > > [2021-03-06 23:04:42] Job 'build-each-commit-gcc' in stage 'test' is skipped
> > > > [2021-03-06 23:04:42] Job 'alpine-3.12-clang' in stage 'build' is failed
> > > 
> > > The following jobs are failing:
> > > 
> > > alpine-3.12-clang [1]
> > > debian-unstable-32-gcc-debug
> > > debian-unstable-clang
> > > debian-unstable-gcc-randconfig
> > > ubuntu-bionic-clang
> > > 
> > > Cloning into 'qemu-xen-dir-remote.tmp'...
> > > error: inflate: data stream error (incorrect data check)
> > > fatal: serious inflate inconsistency
> > > fatal: index-pack failed
> > > 
> > > This looks unrelated to my patch. Possibly a network error?
> > 
> > Yeah.
> > 
> > 
> > > =========
> > > 
> > > centos-7-2-gcc [2]
> > > centos-7-2-gcc-debug
> > > centos-7-gcc
> > > centos-7-gcc-debug
> > > 
> > > Centos 7 is using GCC 4.8.5. This patch is bumping the minimum version to 4.9.
> > > So the failure is expected.
> > > 
> > > We may have to disable the jobs if my patch goes ahead. May I ask why we chose
> > > to build on Centos 7?
> > 
> > CentOS 7 is still the latest and it is maintained until 2024:
> > 
> > https://wiki.centos.org/About/Product
> > 
> > I don't have a lot of direct experience with x86 users any longer, but
> > this seems to be problematic?
> 
> The last full updates was on the Q3 2020, so it looks unlikely that Centos 7
> will upgrade to a new Xen.
> 
> So this is only going to be a problem for anyone that decides to build there
> own Xen.
> 
> I can't tell whether someone may want to build upstream Xen on Centos 7.
> What I can say is the Arm64 bug in GCC 4.9 will lead to corruption, the bug
> in GCC < 4.8 looks somewhat less problematic.
> 
> So, I am sure any user would appreciate if we tell (e.g. throw an error)
> them when there is a risk that GCC X.Y may miscompile Xen.
> 
> In the ideal world (e.g if we had unlimited time), we should workaround it
> in Xen. But this doesn't look easy/possible here.
> 
> Therefore, for now, I think dropping support for GCC < 4.9 (< 5.1 for Arm)
> is the best approach. The support can be revived when someone is willing to
> spend time on it.

Does CentOS has something like Debian backports to get updated major
versions of packages?

Also, I think we do maintain Xen on CentOS using a separate
repository, so newer versions of Xen will be added to CentOS. Adding
George and Anthony since they know about that.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-09  0:59   ` Stefano Stabellini
@ 2021-03-09 10:05     ` Julien Grall
  2021-03-09 10:32       ` Roger Pau Monné
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-09 10:05 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, famzheng, cardoe, wl, Bertrand.Marquis, andrew.cooper3

Hi Stefano,

On 09/03/2021 00:59, Stefano Stabellini wrote:
> On Sun, 7 Mar 2021, Julien Grall wrote:
>> On 06/03/2021 23:04, no-reply@patchew.org wrote:
>>> Hi,
>>>
>>> Patchew automatically ran gitlab-ci pipeline with this patch (series)
>>> applied, but the job failed. Maybe there's a bug in the patches?
>>>
>>> You can find the link to the pipeline near the end of the report below:
>>>
>>> Type: series
>>> Message-id: 20210306214148.27021-1-julien@xen.org
>>> Subject: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to
>>> 4.9 (5.1 on arm64)
>>>
>>> === TEST SCRIPT BEGIN ===
>>> #!/bin/bash
>>> sleep 10
>>> patchew gitlab-pipeline-check -p xen-project/patchew/xen
>>> === TEST SCRIPT END ===
>>>
>>> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
>>> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
>>>   From https://gitlab.com/xen-project/patchew/xen
>>>    * [new tag]               patchew/20210306214148.27021-1-julien@xen.org ->
>>> patchew/20210306214148.27021-1-julien@xen.org
>>> Switched to a new branch 'test'
>>> f332859b88 xen: Bump the minimum version of GCC supported to 4.9 (5.1 on
>>> arm64)
>>>
>>> === OUTPUT BEGIN ===
>>> [2021-03-06 21:45:17] Looking up pipeline...
>>> [2021-03-06 21:45:18] Found pipeline 266507477:
>>>
>>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/266507477
>>>
>>> [2021-03-06 21:45:18] Waiting for pipeline to finish...
>>> [2021-03-06 22:00:21] Still waiting...
>>> [2021-03-06 22:15:26] Still waiting...
>>> [2021-03-06 22:30:31] Still waiting...
>>> [2021-03-06 22:45:35] Still waiting...
>>> [2021-03-06 23:00:41] Still waiting...
>>> [2021-03-06 23:04:42] Pipeline failed
>>> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang-pvh' in stage 'test' is
>>> skipped
>>> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc-pvh' in stage 'test' is
>>> skipped
>>> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang' in stage 'test' is
>>> skipped
>>> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc' in stage 'test' is skipped
>>> [2021-03-06 23:04:42] Job 'qemu-smoke-arm64-gcc' in stage 'test' is skipped
>>> [2021-03-06 23:04:42] Job 'qemu-alpine-arm64-gcc' in stage 'test' is skipped
>>> [2021-03-06 23:04:42] Job 'build-each-commit-gcc' in stage 'test' is skipped
>>> [2021-03-06 23:04:42] Job 'alpine-3.12-clang' in stage 'build' is failed
>>
>> The following jobs are failing:
>>
>> alpine-3.12-clang [1]
>> debian-unstable-32-gcc-debug
>> debian-unstable-clang
>> debian-unstable-gcc-randconfig
>> ubuntu-bionic-clang
>>
>> Cloning into 'qemu-xen-dir-remote.tmp'...
>> error: inflate: data stream error (incorrect data check)
>> fatal: serious inflate inconsistency
>> fatal: index-pack failed
>>
>> This looks unrelated to my patch. Possibly a network error?
> 
> Yeah.
> 
> 
>> =========
>>
>> centos-7-2-gcc [2]
>> centos-7-2-gcc-debug
>> centos-7-gcc
>> centos-7-gcc-debug
>>
>> Centos 7 is using GCC 4.8.5. This patch is bumping the minimum version to 4.9.
>> So the failure is expected.
>>
>> We may have to disable the jobs if my patch goes ahead. May I ask why we chose
>> to build on Centos 7?
> 
> CentOS 7 is still the latest and it is maintained until 2024:
> 
> https://wiki.centos.org/About/Product
> 
> I don't have a lot of direct experience with x86 users any longer, but
> this seems to be problematic?

The last full updates was on the Q3 2020, so it looks unlikely that 
Centos 7 will upgrade to a new Xen.

So this is only going to be a problem for anyone that decides to build 
there own Xen.

I can't tell whether someone may want to build upstream Xen on Centos 7. 
What I can say is the Arm64 bug in GCC 4.9 will lead to corruption, the 
bug in GCC < 4.8 looks somewhat less problematic.

So, I am sure any user would appreciate if we tell (e.g. throw an error) 
them when there is a risk that GCC X.Y may miscompile Xen.

In the ideal world (e.g if we had unlimited time), we should workaround 
it in Xen. But this doesn't look easy/possible here.

Therefore, for now, I think dropping support for GCC < 4.9 (< 5.1 for 
Arm) is the best approach. The support can be revived when someone is 
willing to spend time on it.

> 
> 
>> ==========
>>
>> ubuntu-trusty-gcc [3]
>> ubuntu-trusty-gcc-debug
>>
>> Ubuntu trusty is using GCC 4.8.4. This patch is bumping the minimum version to
>> 4.9. So the failure is expected. We may have to disable the jobs if my patch
>> goes ahead. May I ask why we chose to build on Ubuntu trusty?
> 
> Ubuntu Trusty is in "Extended Security Maintenance", so technically
> still supported also:
> 
> https://ubuntu.com/about/release-cycle

See above.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
  2021-03-07 13:00 ` Julien Grall
@ 2021-03-09  0:59   ` Stefano Stabellini
  2021-03-09 10:05     ` Julien Grall
  0 siblings, 1 reply; 22+ messages in thread
From: Stefano Stabellini @ 2021-03-09  0:59 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, famzheng, sstabellini, cardoe, wl, Bertrand.Marquis,
	andrew.cooper3

On Sun, 7 Mar 2021, Julien Grall wrote:
> On 06/03/2021 23:04, no-reply@patchew.org wrote:
> > Hi,
> > 
> > Patchew automatically ran gitlab-ci pipeline with this patch (series)
> > applied, but the job failed. Maybe there's a bug in the patches?
> > 
> > You can find the link to the pipeline near the end of the report below:
> > 
> > Type: series
> > Message-id: 20210306214148.27021-1-julien@xen.org
> > Subject: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to
> > 4.9 (5.1 on arm64)
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > sleep 10
> > patchew gitlab-pipeline-check -p xen-project/patchew/xen
> > === TEST SCRIPT END ===
> > 
> > warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> > warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> >  From https://gitlab.com/xen-project/patchew/xen
> >   * [new tag]               patchew/20210306214148.27021-1-julien@xen.org ->
> > patchew/20210306214148.27021-1-julien@xen.org
> > Switched to a new branch 'test'
> > f332859b88 xen: Bump the minimum version of GCC supported to 4.9 (5.1 on
> > arm64)
> > 
> > === OUTPUT BEGIN ===
> > [2021-03-06 21:45:17] Looking up pipeline...
> > [2021-03-06 21:45:18] Found pipeline 266507477:
> > 
> > https://gitlab.com/xen-project/patchew/xen/-/pipelines/266507477
> > 
> > [2021-03-06 21:45:18] Waiting for pipeline to finish...
> > [2021-03-06 22:00:21] Still waiting...
> > [2021-03-06 22:15:26] Still waiting...
> > [2021-03-06 22:30:31] Still waiting...
> > [2021-03-06 22:45:35] Still waiting...
> > [2021-03-06 23:00:41] Still waiting...
> > [2021-03-06 23:04:42] Pipeline failed
> > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang-pvh' in stage 'test' is
> > skipped
> > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc-pvh' in stage 'test' is
> > skipped
> > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang' in stage 'test' is
> > skipped
> > [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc' in stage 'test' is skipped
> > [2021-03-06 23:04:42] Job 'qemu-smoke-arm64-gcc' in stage 'test' is skipped
> > [2021-03-06 23:04:42] Job 'qemu-alpine-arm64-gcc' in stage 'test' is skipped
> > [2021-03-06 23:04:42] Job 'build-each-commit-gcc' in stage 'test' is skipped
> > [2021-03-06 23:04:42] Job 'alpine-3.12-clang' in stage 'build' is failed
> 
> The following jobs are failing:
> 
> alpine-3.12-clang [1]
> debian-unstable-32-gcc-debug
> debian-unstable-clang
> debian-unstable-gcc-randconfig
> ubuntu-bionic-clang
> 
> Cloning into 'qemu-xen-dir-remote.tmp'...
> error: inflate: data stream error (incorrect data check)
> fatal: serious inflate inconsistency
> fatal: index-pack failed
> 
> This looks unrelated to my patch. Possibly a network error?

Yeah.


> =========
> 
> centos-7-2-gcc [2]
> centos-7-2-gcc-debug
> centos-7-gcc
> centos-7-gcc-debug
> 
> Centos 7 is using GCC 4.8.5. This patch is bumping the minimum version to 4.9.
> So the failure is expected.
> 
> We may have to disable the jobs if my patch goes ahead. May I ask why we chose
> to build on Centos 7?

CentOS 7 is still the latest and it is maintained until 2024:

https://wiki.centos.org/About/Product

I don't have a lot of direct experience with x86 users any longer, but
this seems to be problematic?


> ==========
> 
> ubuntu-trusty-gcc [3]
> ubuntu-trusty-gcc-debug
> 
> Ubuntu trusty is using GCC 4.8.4. This patch is bumping the minimum version to
> 4.9. So the failure is expected. We may have to disable the jobs if my patch
> goes ahead. May I ask why we chose to build on Ubuntu trusty?

Ubuntu Trusty is in "Extended Security Maintenance", so technically
still supported also:

https://ubuntu.com/about/release-cycle

 
> Cheers,
> 
> [1] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273595
> [2] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273539
> [3] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273575
> 
> 
> > === OUTPUT END ===
> > 
> > Test command exited with code: 1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
       [not found] <161507188317.11025.6744028462008264481@c667a6b167f6>
@ 2021-03-07 13:00 ` Julien Grall
  2021-03-09  0:59   ` Stefano Stabellini
  0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2021-03-07 13:00 UTC (permalink / raw)
  To: xen-devel, famzheng
  Cc: sstabellini, cardoe, wl, Bertrand.Marquis, andrew.cooper3

Hi,

On 06/03/2021 23:04, no-reply@patchew.org wrote:
> Hi,
> 
> Patchew automatically ran gitlab-ci pipeline with this patch (series) applied, but the job failed. Maybe there's a bug in the patches?
> 
> You can find the link to the pipeline near the end of the report below:
> 
> Type: series
> Message-id: 20210306214148.27021-1-julien@xen.org
> Subject: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> sleep 10
> patchew gitlab-pipeline-check -p xen-project/patchew/xen
> === TEST SCRIPT END ===
> 
> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
>  From https://gitlab.com/xen-project/patchew/xen
>   * [new tag]               patchew/20210306214148.27021-1-julien@xen.org -> patchew/20210306214148.27021-1-julien@xen.org
> Switched to a new branch 'test'
> f332859b88 xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)
> 
> === OUTPUT BEGIN ===
> [2021-03-06 21:45:17] Looking up pipeline...
> [2021-03-06 21:45:18] Found pipeline 266507477:
> 
> https://gitlab.com/xen-project/patchew/xen/-/pipelines/266507477
> 
> [2021-03-06 21:45:18] Waiting for pipeline to finish...
> [2021-03-06 22:00:21] Still waiting...
> [2021-03-06 22:15:26] Still waiting...
> [2021-03-06 22:30:31] Still waiting...
> [2021-03-06 22:45:35] Still waiting...
> [2021-03-06 23:00:41] Still waiting...
> [2021-03-06 23:04:42] Pipeline failed
> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang-pvh' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc-pvh' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-clang' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'qemu-smoke-x86-64-gcc' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'qemu-smoke-arm64-gcc' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'qemu-alpine-arm64-gcc' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'build-each-commit-gcc' in stage 'test' is skipped
> [2021-03-06 23:04:42] Job 'alpine-3.12-clang' in stage 'build' is failed

The following jobs are failing:

alpine-3.12-clang [1]
debian-unstable-32-gcc-debug
debian-unstable-clang
debian-unstable-gcc-randconfig
ubuntu-bionic-clang

Cloning into 'qemu-xen-dir-remote.tmp'...
error: inflate: data stream error (incorrect data check)
fatal: serious inflate inconsistency
fatal: index-pack failed

This looks unrelated to my patch. Possibly a network error?

=========

centos-7-2-gcc [2]
centos-7-2-gcc-debug
centos-7-gcc
centos-7-gcc-debug

Centos 7 is using GCC 4.8.5. This patch is bumping the minimum version 
to 4.9. So the failure is expected.

We may have to disable the jobs if my patch goes ahead. May I ask why we 
chose to build on Centos 7?


==========

ubuntu-trusty-gcc [3]
ubuntu-trusty-gcc-debug

Ubuntu trusty is using GCC 4.8.4. This patch is bumping the minimum 
version to 4.9. So the failure is expected. We may have to disable the 
jobs if my patch goes ahead. May I ask why we chose to build on Ubuntu 
trusty?


Cheers,

[1] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273595
[2] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273539
[3] https://gitlab.com/xen-project/patchew/xen/-/jobs/1078273575


> === OUTPUT END ===
> 
> Test command exited with code: 1
> 

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-03-11 11:43 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-06 21:41 [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64) Julien Grall
2021-03-07 10:57 ` Roger Pau Monné
2021-03-07 12:37   ` Julien Grall
2021-03-08  8:09 ` Jan Beulich
2021-03-08 10:51   ` Julien Grall
2021-03-08 11:20     ` Jan Beulich
2021-03-08 20:22       ` Julien Grall
2021-03-09 11:20         ` Jan Beulich
2021-03-10 18:05           ` Julien Grall
2021-03-11  8:17             ` Jan Beulich
2021-03-11  9:31               ` Julien Grall
2021-03-11 11:43                 ` Jan Beulich
2021-03-08  9:49 ` Ian Jackson
2021-03-10 20:48 ` Stefano Stabellini
     [not found] <161507188317.11025.6744028462008264481@c667a6b167f6>
2021-03-07 13:00 ` Julien Grall
2021-03-09  0:59   ` Stefano Stabellini
2021-03-09 10:05     ` Julien Grall
2021-03-09 10:32       ` Roger Pau Monné
2021-03-09 11:11         ` Anthony PERARD
2021-03-09 22:24         ` Stefano Stabellini
2021-03-10  7:52           ` Roger Pau Monné
2021-03-10 10:24             ` Zheng, Fam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.