[PATCH] MIPS: Support binutils configured with --enable-mips-fix-loongson3-llsc=yes

[PATCH] MIPS: Support binutils configured with --enable-mips-fix-loongson3-llsc=yes

[Aurelien Jarno]

From version 2.35, binutils can be configured with
--enable-mips-fix-loongson3-llsc=yes, which means it defaults to
-mfix-loongson3-llsc. This breaks labels which might then point at the
wrong instruction.

The workaround to explicitly pass -mno-fix-loongson3-llsc has been
added in Linux version 5.1, but is only enabled when building a Loongson
64 kernel. As vendors might use a common toolchain for building Loongson
and non-Loongson kernels, just move that workaround to
arch/mips/Makefile. At the same time update the comments to reflect the
current status.

Cc: stable@vger.kernel.org # 5.1+
Cc: YunQiang Su <syq@debian.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 arch/mips/Makefile            | 19 +++++++++++++++++++
 arch/mips/loongson64/Platform | 22 ----------------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index cd4343edeb11..5ffdd67093bc 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -136,6 +136,25 @@ cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
 #
 cflags-y += -fno-stack-check
 
+# binutils from v2.35 when built with --enable-mips-fix-loongson3-llsc=yes,
+# supports an -mfix-loongson3-llsc flag which emits a sync prior to each ll
+# instruction to work around a CPU bug (see __SYNC_loongson3_war in asm/sync.h
+# for a description).
+#
+# We disable this in order to prevent the assembler meddling with the
+# instruction that labels refer to, ie. if we label an ll instruction:
+#
+# 1: ll v0, 0(a0)
+#
+# ...then with the assembler fix applied the label may actually point at a sync
+# instruction inserted by the assembler, and if we were using the label in an
+# exception table the table would no longer contain the address of the ll
+# instruction.
+#
+# Avoid this by explicitly disabling that assembler behaviour.
+#
+cflags-y += $(call as-option,-Wa$(comma)-mno-fix-loongson3-llsc,)
+
 #
 # CPU-dependent compiler/assembler options for optimization.
 #
diff --git a/arch/mips/loongson64/Platform b/arch/mips/loongson64/Platform
index ec42c5085905..e2354e128d9a 100644
--- a/arch/mips/loongson64/Platform
+++ b/arch/mips/loongson64/Platform
@@ -5,28 +5,6 @@
 
 cflags-$(CONFIG_CPU_LOONGSON64)	+= -Wa,--trap
 
-#
-# Some versions of binutils, not currently mainline as of 2019/02/04, support
-# an -mfix-loongson3-llsc flag which emits a sync prior to each ll instruction
-# to work around a CPU bug (see __SYNC_loongson3_war in asm/sync.h for a
-# description).
-#
-# We disable this in order to prevent the assembler meddling with the
-# instruction that labels refer to, ie. if we label an ll instruction:
-#
-# 1: ll v0, 0(a0)
-#
-# ...then with the assembler fix applied the label may actually point at a sync
-# instruction inserted by the assembler, and if we were using the label in an
-# exception table the table would no longer contain the address of the ll
-# instruction.
-#
-# Avoid this by explicitly disabling that assembler behaviour. If upstream
-# binutils does not merge support for the flag then we can revisit & remove
-# this later - for now it ensures vendor toolchains don't cause problems.
-#
-cflags-$(CONFIG_CPU_LOONGSON64)	+= $(call as-option,-Wa$(comma)-mno-fix-loongson3-llsc,)
-
 #
 # binutils from v2.25 on and gcc starting from v4.9.0 treat -march=loongson3a
 # as MIPS64 R2; older versions as just R1.  This leaves the possibility open
-- 
2.29.2

Re: [PATCH] MIPS: Support binutils configured with --enable-mips-fix-loongson3-llsc=yes

[Maciej W. Rozycki]

On Sat, 9 Jan 2021, Aurelien Jarno wrote:

> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index cd4343edeb11..5ffdd67093bc 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -136,6 +136,25 @@ cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>  #
>  cflags-y += -fno-stack-check
>  
> +# binutils from v2.35 when built with --enable-mips-fix-loongson3-llsc=yes,
> +# supports an -mfix-loongson3-llsc flag which emits a sync prior to each ll
> +# instruction to work around a CPU bug (see __SYNC_loongson3_war in asm/sync.h
> +# for a description).
> +#
> +# We disable this in order to prevent the assembler meddling with the
> +# instruction that labels refer to, ie. if we label an ll instruction:
> +#
> +# 1: ll v0, 0(a0)
> +#
> +# ...then with the assembler fix applied the label may actually point at a sync
> +# instruction inserted by the assembler, and if we were using the label in an
> +# exception table the table would no longer contain the address of the ll
> +# instruction.

 Interesting.  Given that a MIPS assembler is generally free to shuffle 
instructions as it sees fit in its default reorder mode as long as that 
does not change the semantics of the code executed, shouldn't we instead 
place all label/instruction pairs used for exception handling in noreorder 
blocks so as to make sure the label refers to the instruction an exception 
handler expects it to?

 E.g. for the case quoted above:

	.set	push
	.set	noreorder
1:	ll	v0, 0(a0)
	.set	pop

  Maciej

Re: [PATCH] MIPS: Support binutils configured with --enable-mips-fix-loongson3-llsc=yes

[Thomas Bogendoerfer]

On Sat, Jan 09, 2021 at 08:30:47PM +0100, Aurelien Jarno wrote:
> >From version 2.35, binutils can be configured with
> --enable-mips-fix-loongson3-llsc=yes, which means it defaults to
> -mfix-loongson3-llsc. This breaks labels which might then point at the
> wrong instruction.
> 
> The workaround to explicitly pass -mno-fix-loongson3-llsc has been
> added in Linux version 5.1, but is only enabled when building a Loongson
> 64 kernel. As vendors might use a common toolchain for building Loongson
> and non-Loongson kernels, just move that workaround to
> arch/mips/Makefile. At the same time update the comments to reflect the
> current status.
> 
> Cc: stable@vger.kernel.org # 5.1+
> Cc: YunQiang Su <syq@debian.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  arch/mips/Makefile            | 19 +++++++++++++++++++
>  arch/mips/loongson64/Platform | 22 ----------------------
>  2 files changed, 19 insertions(+), 22 deletions(-)

I've applied it to mips-next, but I consider such changes as rather rude.
I would have expected, that the workaround is only enabled via command
line option and not by default.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]