linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14
@ 2024-05-21 12:39 Michael Ellerman
  2024-05-21 12:39 ` [PATCH 2/2] powerpc/uaccess: Use YZ asm constraint for ld Michael Ellerman
  2024-05-21 17:09 ` [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Nick Desaulniers
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Ellerman @ 2024-05-21 12:39 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: nathan, ndesaulniers, linkw

Building ppc64le_defconfig with GCC 14 fails with assembler errors:

    CC      fs/readdir.o
  /tmp/ccdQn0mD.s: Assembler messages:
  /tmp/ccdQn0mD.s:212: Error: operand out of domain (18 is not a multiple of 4)
  /tmp/ccdQn0mD.s:226: Error: operand out of domain (18 is not a multiple of 4)
  ... [6 lines]
  /tmp/ccdQn0mD.s:1699: Error: operand out of domain (18 is not a multiple of 4)

A snippet of the asm shows:

  # ../fs/readdir.c:210:         unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
         ld 9,0(29)       # MEM[(u64 *)name_38(D) + _88 * 1], MEM[(u64 *)name_38(D) + _88 * 1]
  # 210 "../fs/readdir.c" 1
         1:      std 9,18(8)     # put_user       # *__pus_addr_52, MEM[(u64 *)name_38(D) + _88 * 1]

The 'std' instruction requires a 4-byte aligned displacement because
it is a DS-form instruction, and as the assembler says, 18 is not a
multiple of 4.

The fix is to change the constraint on the memory operand to put_user(),
from "m" which is a general memory reference to "YZ".

The "Z" constraint is documented in the GCC manual PowerPC machine
constraints, and specifies a "memory operand accessed with indexed or
indirect addressing". "Y" is not documented in the manual but specifies
a "memory operand for a DS-form instruction". Using both allows the
compiler to generate a DS-form "std" or X-form "stdx" as appropriate.

The change has to be conditional on CONFIG_PPC_KERNEL_PREFIXED because
the "Y" constraint does not guarantee 4-byte alignment when prefixed
instructions are enabled.

Unfortunately clang doesn't support the "Y" constraint so that has to be
behind an ifdef.

Although the build error is only seen with GCC 14, that appears to just
be luck. The constraint has been incorrect since it was first added.

Fixes: c20beffeec3c ("powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()")
Cc: stable@vger.kernel.org # v5.10+
Suggested-by: Kewen Lin <linkw@gcc.gnu.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/uaccess.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index de10437fd206..4cba724c8899 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -92,9 +92,25 @@ __pu_failed:							\
 		: label)
 #endif
 
+#ifdef CONFIG_CC_IS_CLANG
+#define DS_FORM_CONSTRAINT "Z<>"
+#else
+#define DS_FORM_CONSTRAINT "YZ<>"
+#endif
+
 #ifdef __powerpc64__
+#ifdef CONFIG_PPC_KERNEL_PREFIXED
 #define __put_user_asm2_goto(x, ptr, label)			\
 	__put_user_asm_goto(x, ptr, label, "std")
+#else
+#define __put_user_asm2_goto(x, addr, label)			\
+	asm goto ("1: std%U1%X1 %0,%1	# put_user\n"		\
+		EX_TABLE(1b, %l2)				\
+		:						\
+		: "r" (x), DS_FORM_CONSTRAINT (*addr)		\
+		:						\
+		: label)
+#endif // CONFIG_PPC_KERNEL_PREFIXED
 #else /* __powerpc64__ */
 #define __put_user_asm2_goto(x, addr, label)			\
 	asm goto(					\
-- 
2.45.1


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

* [PATCH 2/2] powerpc/uaccess: Use YZ asm constraint for ld
  2024-05-21 12:39 [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Michael Ellerman
@ 2024-05-21 12:39 ` Michael Ellerman
  2024-05-21 17:09 ` [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Nick Desaulniers
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2024-05-21 12:39 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: nathan, ndesaulniers, linkw

The 'ld' instruction requires a 4-byte aligned displacement because it
is a DS-form instruction. But the "m" asm constraint doesn't enforce
that.

Add a special case of __get_user_asm2_goto() so that the "YZ" constraint
can be used for "ld".

The "Z" constraint is documented in the GCC manual PowerPC machine
constraints, and specifies a "memory operand accessed with indexed or
indirect addressing". "Y" is not documented in the manual but specifies
a "memory operand for a DS-form instruction". Using both allows the
compiler to generate a DS-form "ld" or X-form "ldx" as appropriate.

The change has to be conditional on CONFIG_PPC_KERNEL_PREFIXED because
the "Y" constraint does not guarantee 4-byte alignment when prefixed
instructions are enabled.

No build errors have been reported due to this, but the possibility is
there depending on compiler code generation decisions.

Fixes: c20beffeec3c ("powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/uaccess.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 4cba724c8899..fd594bf6c6a9 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -181,8 +181,19 @@ do {								\
 #endif
 
 #ifdef __powerpc64__
+#ifdef CONFIG_PPC_KERNEL_PREFIXED
 #define __get_user_asm2_goto(x, addr, label)			\
 	__get_user_asm_goto(x, addr, label, "ld")
+#else
+#define __get_user_asm2_goto(x, addr, label)			\
+	asm_goto_output(					\
+		"1:	ld%U1%X1 %0, %1	# get_user\n"		\
+		EX_TABLE(1b, %l2)				\
+		: "=r" (x)					\
+		: DS_FORM_CONSTRAINT (*addr)			\
+		:						\
+		: label)
+#endif // CONFIG_PPC_KERNEL_PREFIXED
 #else /* __powerpc64__ */
 #define __get_user_asm2_goto(x, addr, label)			\
 	asm_goto_output(					\
-- 
2.45.1


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

* Re: [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14
  2024-05-21 12:39 [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Michael Ellerman
  2024-05-21 12:39 ` [PATCH 2/2] powerpc/uaccess: Use YZ asm constraint for ld Michael Ellerman
@ 2024-05-21 17:09 ` Nick Desaulniers
  2024-05-24  7:09   ` Michael Ellerman
  1 sibling, 1 reply; 4+ messages in thread
From: Nick Desaulniers @ 2024-05-21 17:09 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: nathan, linuxppc-dev, linkw

On Tue, May 21, 2024 at 5:39 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Building ppc64le_defconfig with GCC 14 fails with assembler errors:
>
>     CC      fs/readdir.o
>   /tmp/ccdQn0mD.s: Assembler messages:
>   /tmp/ccdQn0mD.s:212: Error: operand out of domain (18 is not a multiple of 4)
>   /tmp/ccdQn0mD.s:226: Error: operand out of domain (18 is not a multiple of 4)
>   ... [6 lines]
>   /tmp/ccdQn0mD.s:1699: Error: operand out of domain (18 is not a multiple of 4)
>
> A snippet of the asm shows:
>
>   # ../fs/readdir.c:210:         unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
>          ld 9,0(29)       # MEM[(u64 *)name_38(D) + _88 * 1], MEM[(u64 *)name_38(D) + _88 * 1]
>   # 210 "../fs/readdir.c" 1
>          1:      std 9,18(8)     # put_user       # *__pus_addr_52, MEM[(u64 *)name_38(D) + _88 * 1]
>
> The 'std' instruction requires a 4-byte aligned displacement because
> it is a DS-form instruction, and as the assembler says, 18 is not a
> multiple of 4.
>
> The fix is to change the constraint on the memory operand to put_user(),
> from "m" which is a general memory reference to "YZ".
>
> The "Z" constraint is documented in the GCC manual PowerPC machine
> constraints, and specifies a "memory operand accessed with indexed or
> indirect addressing". "Y" is not documented in the manual but specifies
> a "memory operand for a DS-form instruction". Using both allows the
> compiler to generate a DS-form "std" or X-form "stdx" as appropriate.
>
> The change has to be conditional on CONFIG_PPC_KERNEL_PREFIXED because
> the "Y" constraint does not guarantee 4-byte alignment when prefixed
> instructions are enabled.
>
> Unfortunately clang doesn't support the "Y" constraint so that has to be
> behind an ifdef.

Filed: https://github.com/llvm/llvm-project/issues/92939

-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14
  2024-05-21 17:09 ` [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Nick Desaulniers
@ 2024-05-24  7:09   ` Michael Ellerman
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2024-05-24  7:09 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: nathan, linuxppc-dev, linkw

Nick Desaulniers <ndesaulniers@google.com> writes:
> On Tue, May 21, 2024 at 5:39 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Building ppc64le_defconfig with GCC 14 fails with assembler errors:
>>
>>     CC      fs/readdir.o
>>   /tmp/ccdQn0mD.s: Assembler messages:
>>   /tmp/ccdQn0mD.s:212: Error: operand out of domain (18 is not a multiple of 4)
>>   /tmp/ccdQn0mD.s:226: Error: operand out of domain (18 is not a multiple of 4)
>>   ... [6 lines]
>>   /tmp/ccdQn0mD.s:1699: Error: operand out of domain (18 is not a multiple of 4)
>>
>> A snippet of the asm shows:
>>
>>   # ../fs/readdir.c:210:         unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
>>          ld 9,0(29)       # MEM[(u64 *)name_38(D) + _88 * 1], MEM[(u64 *)name_38(D) + _88 * 1]
>>   # 210 "../fs/readdir.c" 1
>>          1:      std 9,18(8)     # put_user       # *__pus_addr_52, MEM[(u64 *)name_38(D) + _88 * 1]
>>
>> The 'std' instruction requires a 4-byte aligned displacement because
>> it is a DS-form instruction, and as the assembler says, 18 is not a
>> multiple of 4.
>>
>> The fix is to change the constraint on the memory operand to put_user(),
>> from "m" which is a general memory reference to "YZ".
>>
>> The "Z" constraint is documented in the GCC manual PowerPC machine
>> constraints, and specifies a "memory operand accessed with indexed or
>> indirect addressing". "Y" is not documented in the manual but specifies
>> a "memory operand for a DS-form instruction". Using both allows the
>> compiler to generate a DS-form "std" or X-form "stdx" as appropriate.
>>
>> The change has to be conditional on CONFIG_PPC_KERNEL_PREFIXED because
>> the "Y" constraint does not guarantee 4-byte alignment when prefixed
>> instructions are enabled.
>>
>> Unfortunately clang doesn't support the "Y" constraint so that has to be
>> behind an ifdef.
>
> Filed: https://github.com/llvm/llvm-project/issues/92939

Thanks. I will file one to have the GCC constraint documented.

cheers

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

end of thread, other threads:[~2024-05-24  7:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-21 12:39 [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Michael Ellerman
2024-05-21 12:39 ` [PATCH 2/2] powerpc/uaccess: Use YZ asm constraint for ld Michael Ellerman
2024-05-21 17:09 ` [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14 Nick Desaulniers
2024-05-24  7:09   ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).