| Message ID | 20180601180157.759-1-yann.morin.1998@free.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [PATCHv2] linux: may fail to boot for binutils 2.29+ even without armv7m | expand |
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Commit f13477b (linux: config.in: add comment for Arm Cortex-M) added a > comment so that the user that the linux kernel may miscompile with > binutils 2.29+, when the target is an armv7m CPU. > However, the real trigger is a compilation in thumb2 mode, which happens > to be the only option for armv7m CPUs. > We can't know whether the kernel will be built in arm or thumb2 mode, > though, because we do not have that information: it is only available in > the Linux' .config file, which we don;t have access to at the time we > run our menuconfig. > So, relax the conditions under which the comment is made, so that it > appears as soon as binutils are >= 2.29 (i.e. not 2.28, which is the > oldest we support) for ARM CPUs. > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Christophe Priouzeau <christophe.priouzeau@st.com> > Cc: Laurent GONZALEZ <br22@gezedo.com> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > --- > Changes v1 -> v2: > - only applicable to ARM CPUs. (Peter) > --- > linux/Config.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/linux/Config.in b/linux/Config.in > index bffb52fd18..c29b1f6c88 100644 > --- a/linux/Config.in > +++ b/linux/Config.in > @@ -9,7 +9,7 @@ config BR2_LINUX_KERNEL > if BR2_LINUX_KERNEL > comment "Linux kernel may fail to boot with binutils >= 2.29" > - depends on BR2_ARM_CPU_ARMV7M > + depends on BR2_arm || BR2_armeb This only applies to a kernel built with CONFIG_THUMB2=y, so I've slightly reworded the comment to be: comment "Linux kernel in thumb mode may be broken with binutils >= 2.29" And applied, thanks.
On 01-06-18 20:01, Yann E. MORIN wrote: > Commit f13477b (linux: config.in: add comment for Arm Cortex-M) added a > comment so that the user that the linux kernel may miscompile with > binutils 2.29+, when the target is an armv7m CPU. > > However, the real trigger is a compilation in thumb2 mode, which happens > to be the only option for armv7m CPUs. > > We can't know whether the kernel will be built in arm or thumb2 mode, > though, because we do not have that information: it is only available in > the Linux' .config file, which we don;t have access to at the time we > run our menuconfig. > > So, relax the conditions under which the comment is made, so that it > appears as soon as binutils are >= 2.29 (i.e. not 2.28, which is the > oldest we support) for ARM CPUs. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Christophe Priouzeau <christophe.priouzeau@st.com> > Cc: Laurent GONZALEZ <br22@gezedo.com> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > > --- > Changes v1 -> v2: > - only applicable to ARM CPUs. (Peter) > --- > linux/Config.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/linux/Config.in b/linux/Config.in > index bffb52fd18..c29b1f6c88 100644 > --- a/linux/Config.in > +++ b/linux/Config.in > @@ -9,7 +9,7 @@ config BR2_LINUX_KERNEL > if BR2_LINUX_KERNEL > > comment "Linux kernel may fail to boot with binutils >= 2.29" > - depends on BR2_ARM_CPU_ARMV7M > + depends on BR2_arm || BR2_armeb I have a bit of a problem with this patch. IMO it's rather unlikely that people build the kernel in THUMB mode on non-M CPUs. But now we're always showing this warning. I think it will confuse a number of people, and I don't think it's very likely that it will actually help anyone. Regards, Arnout > depends on !BR2_BINUTILS_VERSION_2_28_X > > # Packages that need to have a kernel with support for loadable modules, >
Hello, On Tue, 5 Jun 2018 00:10:30 +0200, Arnout Vandecappelle wrote: > > comment "Linux kernel may fail to boot with binutils >= 2.29" > > - depends on BR2_ARM_CPU_ARMV7M > > + depends on BR2_arm || BR2_armeb > > I have a bit of a problem with this patch. IMO it's rather unlikely that people > build the kernel in THUMB mode on non-M CPUs. Are you sure ? There is definitely support for building a Thumb2 kernel on ARMv7-A, and I think it's a popular way to reduce a bit the size of the kernel, no? > But now we're always showing this > warning. I think it will confuse a number of people, and I don't think it's very > likely that it will actually help anyone. Actually the warning should not have a: depends on BR2_arm || BR2_armeb but instead a: depends on BR2_ARM_INSTRUCTIONS_THUMB || BR2_ARM_INSTRUCTIONS_THUMB2 because that's actually what we do in binutils to decide whether we want to fall back on binutils 2.28 by default or not. Best regards, Thomas
On 05/06/2018 07:49, Thomas Petazzoni wrote: > Hello, > > On Tue, 5 Jun 2018 00:10:30 +0200, Arnout Vandecappelle wrote: > >>> comment "Linux kernel may fail to boot with binutils >= 2.29" >>> - depends on BR2_ARM_CPU_ARMV7M >>> + depends on BR2_arm || BR2_armeb >> I have a bit of a problem with this patch. IMO it's rather unlikely that people >> build the kernel in THUMB mode on non-M CPUs. > Are you sure ? There is definitely support for building a Thumb2 kernel > on ARMv7-A, and I think it's a popular way to reduce a bit the size of > the kernel, no? I gently remember the list that we can alternatively fix the kernel with a patch. This solution will allow any version of binutils to work with any version of the kernel, and can be easily reverted. What about using: http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565390.html -- Laurent
Arnout, All, On 2018-06-05 00:10 +0200, Arnout Vandecappelle spake thusly: > > > On 01-06-18 20:01, Yann E. MORIN wrote: > > Commit f13477b (linux: config.in: add comment for Arm Cortex-M) added a > > comment so that the user that the linux kernel may miscompile with > > binutils 2.29+, when the target is an armv7m CPU. > > > > However, the real trigger is a compilation in thumb2 mode, which happens > > to be the only option for armv7m CPUs. > > > > We can't know whether the kernel will be built in arm or thumb2 mode, > > though, because we do not have that information: it is only available in > > the Linux' .config file, which we don;t have access to at the time we > > run our menuconfig. > > > > So, relax the conditions under which the comment is made, so that it > > appears as soon as binutils are >= 2.29 (i.e. not 2.28, which is the > > oldest we support) for ARM CPUs. > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Cc: Christophe Priouzeau <christophe.priouzeau@st.com> > > Cc: Laurent GONZALEZ <br22@gezedo.com> > > Cc: Peter Korsgaard <peter@korsgaard.com> > > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > > Cc: Arnout Vandecappelle <arnout@mind.be> > > > > --- > > Changes v1 -> v2: > > - only applicable to ARM CPUs. (Peter) > > --- > > linux/Config.in | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/linux/Config.in b/linux/Config.in > > index bffb52fd18..c29b1f6c88 100644 > > --- a/linux/Config.in > > +++ b/linux/Config.in > > @@ -9,7 +9,7 @@ config BR2_LINUX_KERNEL > > if BR2_LINUX_KERNEL > > > > comment "Linux kernel may fail to boot with binutils >= 2.29" > > - depends on BR2_ARM_CPU_ARMV7M > > + depends on BR2_arm || BR2_armeb > > I have a bit of a problem with this patch. IMO it's rather unlikely that people > build the kernel in THUMB mode on non-M CPUs. But now we're always showing this > warning. I think it will confuse a number of people, and I don't think it's very > likely that it will actually help anyone. You are wrong in saying that people are unlikely to build Thumb kernels, even on Cortex-A CPUs I does happen. I do know quite a few cases, for sure. I would not say that it is most people; I don't know. But it does happen quite a bit, yes. However, I am indeed not so sure that we do need the comment, but at least it is there now. And for sure, the previous conditions, although correct, were not entirely accurate, as they missed some quite common cases as well. Regards, Yann E. MORIN. > Regards, > Arnout > > > depends on !BR2_BINUTILS_VERSION_2_28_X > > > > # Packages that need to have a kernel with support for loadable modules, > > > > -- > Arnout Vandecappelle arnout at mind be > Senior Embedded Software Architect +32-16-286500 > Essensium/Mind http://www.mind.be > G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven > LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle > GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Thomas, All, On 2018-06-05 07:49 +0200, Thomas Petazzoni spake thusly: > Hello, > > On Tue, 5 Jun 2018 00:10:30 +0200, Arnout Vandecappelle wrote: > > > > comment "Linux kernel may fail to boot with binutils >= 2.29" > > > - depends on BR2_ARM_CPU_ARMV7M > > > + depends on BR2_arm || BR2_armeb > > > > I have a bit of a problem with this patch. IMO it's rather unlikely that people > > build the kernel in THUMB mode on non-M CPUs. > > Are you sure ? There is definitely support for building a Thumb2 kernel > on ARMv7-A, and I think it's a popular way to reduce a bit the size of > the kernel, no? Agreed. > > But now we're always showing this > > warning. I think it will confuse a number of people, and I don't think it's very > > likely that it will actually help anyone. > > Actually the warning should not have a: > depends on BR2_arm || BR2_armeb > > but instead a: > depends on BR2_ARM_INSTRUCTIONS_THUMB || BR2_ARM_INSTRUCTIONS_THUMB2 > > because that's actually what we do in binutils to decide whether we > want to fall back on binutils 2.28 by default or not. But as you can see in the commit log that introduced this change, what you suggest is not correct, because it is also very valid that you have a arm-mode userland running on a thumb-mode kernel (as Peter initially pointed out on IRC). So, the kernel warning should not be about he userland mode. And 'BR2_ARM_INSTRUCTIONS_THUMB || BR2_ARM_INSTRUCTIONS_THUMB2' represent the userland mode, while we have, from our kconfig, no way to know the kernel mode, because that is in the kernel .config file, which we only get late. As such, the only way is to depend on arm || armeb. Regards, Yann E. MORIN.
Laurent, All, On 2018-06-05 09:23 +0200, Laurent GONZALEZ spake thusly: > On 05/06/2018 07:49, Thomas Petazzoni wrote: > > On Tue, 5 Jun 2018 00:10:30 +0200, Arnout Vandecappelle wrote: > >>> comment "Linux kernel may fail to boot with binutils >= 2.29" > >>> - depends on BR2_ARM_CPU_ARMV7M > >>> + depends on BR2_arm || BR2_armeb > >> I have a bit of a problem with this patch. IMO it's rather unlikely that people > >> build the kernel in THUMB mode on non-M CPUs. > > Are you sure ? There is definitely support for building a Thumb2 kernel > > on ARMv7-A, and I think it's a popular way to reduce a bit the size of > > the kernel, no? > I gently remember the list that we can alternatively fix the kernel with a patch. Not really, because we don;t know where the kernel is coming from, and if it is already patched or not, or if it has a similar construct elsewhere in the code. A lot of users, especially enterprise-class citizen, have their own internal git tree(s) with their own kernel patched for their own bioard(s), to which they may also apply various unknown sets of patches, so they may already have that patch applied in those internal trees. Granted, we could have a conditional patch, like the perl deprecated stuff, but I don;t much like that idea, because it is not bullet-proof (e.g. their tree already has a patch that touches close to where our patch would be applied, so they conflict, so our patch does not apply, so we believe their kernl is OK while it is not, and the runtime fails, and we're back to square-one). Granted, this is very unlikely, but the more unlikely things tend to be in theory, the more they tend to happen in the real world! ;-) > This solution will allow any version of binutils to work with any version of the kernel, and can be easily reverted. > > What about using: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565390.html That patch has still not been applied in v4.17, so I'd be wary of using a patch that is not a backport anyway... And BTW, v4.17 is still unfixed as far as I can see... Regards, Yann E. MORIN.
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: Hi, >> comment "Linux kernel may fail to boot with binutils >= 2.29" >> - depends on BR2_ARM_CPU_ARMV7M >> + depends on BR2_arm || BR2_armeb > I have a bit of a problem with this patch. IMO it's rather unlikely that people > build the kernel in THUMB mode on non-M CPUs. But now we're always showing this > warning. I think it will confuse a number of people, and I don't think it's very > likely that it will actually help anyone. Why is it unlikely? Thumb(2) generates smaller code, so I could certainly think of use cases where it could be useful (and have used it myself in the past). But Ok, maybe the warning should have mentioned thumb.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: Hi, > But as you can see in the commit log that introduced this change, what > you suggest is not correct, because it is also very valid that you have > a arm-mode userland running on a thumb-mode kernel (as Peter initially > pointed out on IRC). > So, the kernel warning should not be about he userland mode. > And 'BR2_ARM_INSTRUCTIONS_THUMB || BR2_ARM_INSTRUCTIONS_THUMB2' > represent the userland mode, while we have, from our kconfig, no way to > know the kernel mode, because that is in the kernel .config file, which > we only get late. > As such, the only way is to depend on arm || armeb. Or alternatively on BR2_ARM_CPU_HAS_THUMB || BR2_ARM_CPU_HAS_THUMB2 But ok, either of those are true for most ARM cores.
Peter, All, On 2018-06-08 16:22 +0200, Peter Korsgaard spake thusly: > >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: [--SNIP--] > > As such, the only way is to depend on arm || armeb. > Or alternatively on > BR2_ARM_CPU_HAS_THUMB || BR2_ARM_CPU_HAS_THUMB2 Right, this would have been more correct, bt, as you say: > But ok, either of those are true for most ARM cores. What ARM core does not have Thumb nowadays? ;-) Regards, Yann E. MORIN.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Commit f13477b (linux: config.in: add comment for Arm Cortex-M) added a > comment so that the user that the linux kernel may miscompile with > binutils 2.29+, when the target is an armv7m CPU. > However, the real trigger is a compilation in thumb2 mode, which happens > to be the only option for armv7m CPUs. > We can't know whether the kernel will be built in arm or thumb2 mode, > though, because we do not have that information: it is only available in > the Linux' .config file, which we don;t have access to at the time we > run our menuconfig. > So, relax the conditions under which the comment is made, so that it > appears as soon as binutils are >= 2.29 (i.e. not 2.28, which is the > oldest we support) for ARM CPUs. > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Christophe Priouzeau <christophe.priouzeau@st.com> > Cc: Laurent GONZALEZ <br22@gezedo.com> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > --- > Changes v1 -> v2: > - only applicable to ARM CPUs. (Peter) Committed to 2018.02.x, thanks.
diff --git a/linux/Config.in b/linux/Config.in index bffb52fd18..c29b1f6c88 100644 --- a/linux/Config.in +++ b/linux/Config.in @@ -9,7 +9,7 @@ config BR2_LINUX_KERNEL if BR2_LINUX_KERNEL comment "Linux kernel may fail to boot with binutils >= 2.29" - depends on BR2_ARM_CPU_ARMV7M + depends on BR2_arm || BR2_armeb depends on !BR2_BINUTILS_VERSION_2_28_X # Packages that need to have a kernel with support for loadable modules,
Commit f13477b (linux: config.in: add comment for Arm Cortex-M) added a comment so that the user that the linux kernel may miscompile with binutils 2.29+, when the target is an armv7m CPU. However, the real trigger is a compilation in thumb2 mode, which happens to be the only option for armv7m CPUs. We can't know whether the kernel will be built in arm or thumb2 mode, though, because we do not have that information: it is only available in the Linux' .config file, which we don;t have access to at the time we run our menuconfig. So, relax the conditions under which the comment is made, so that it appears as soon as binutils are >= 2.29 (i.e. not 2.28, which is the oldest we support) for ARM CPUs. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Christophe Priouzeau <christophe.priouzeau@st.com> Cc: Laurent GONZALEZ <br22@gezedo.com> Cc: Peter Korsgaard <peter@korsgaard.com> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Arnout Vandecappelle <arnout@mind.be> --- Changes v1 -> v2: - only applicable to ARM CPUs. (Peter) --- linux/Config.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)