On Fri, Aug 30, 2019 at 10:11:55AM +0100, Andrew Murray wrote:

On Fri, Aug 30, 2019 at 08:52:20AM +0100, Will Deacon wrote:

quoted

On Fri, Aug 30, 2019 at 01:08:03AM +0100, Andrew Murray wrote:

quoted

On Thu, Aug 29, 2019 at 05:54:58PM +0100, Will Deacon wrote:

quoted

On Thu, Aug 29, 2019 at 04:48:34PM +0100, Will Deacon wrote:

quoted

diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
index 95091f72228b..7fa042f5444e 100644
--- a/arch/arm64/include/asm/atomic_ll_sc.h
+++ b/arch/arm64/include/asm/atomic_ll_sc.h

@@ -23,6 +23,10 @@ asm_ops "\n"								\
 #define __LL_SC_FALLBACK(asm_ops) asm_ops
 #endif

I downloaded your original patches and tried them, and also got the
build error. After playing with this I think something isn't quite right...

This is your current test:

 echo 'int main(void) {asm volatile("and w0, w0, %w0" :: "K" (4294967295)); return 0; }' |  aarch64-linux-gnu-gcc -S -x c  - ; echo $?

But on my machine this returns 0, i.e. no error. If I drop the -S:

 echo 'int main(void) {asm volatile("and w0, w0, %w0" :: "K" (4294967295)); return 0; }' |  aarch64-linux-gnu-gcc -x c  - ; echo $?

Then this returns 1.

So I guess the -S flag or something similar is needed.

This seems correct to me, and is the reason we pass -S in the Makefile. Why
are you dropping it?

In the first case, the (broken) compiler is emitted an assembly file
containing "and w0, w0, 4294967295", and so we will not define
CONFIG_CC_HAS_K_CONSTRAINT.

In the second case, you're passing the bad assembly file to GAS, which
rejects it.

quoted

quoted

quoted

quoted

+#ifndef CONFIG_CC_HAS_K_CONSTRAINT
+#define K
+#endif

Also, isn't this the wrong way around?

No. If the compiler doesn't support the K constraint, then we get:

	[old] "" "r" (old)

because we've defined K as being nothing. Otherwise, we get:

	[old] "K" "r" (old)

because K isn't defined as anything.

It looks like when using $(call try-run,echo - it's the last argument that is
used when the condition is false. Thus at present we seem to be setting 
CONFIG_CC_HAS_K_CONSTRAINT when 'K' is broken.

No. We set CONFIG_CC_HAS_K_CONSTRAINT when the compiler fails to generate
an assembly file with the invalid immediate.

quoted

Without the __stringify I get a compilation failure when building
kernel/panic.o because it tries to cmpxchg a 32-bit variable with -1
(PANIC_CPU_INVALID). Looking at panic.s, I see that constraint parameter
isn't being expanded. For example if I do:

  #ifndef CONFIG_CC_HAS_K_CONSTRAINT
  #define INVALID_CONSTRAINT
  #else
  #define INVALID_CONSTRAINT	K
  #endif

and then pass INVALID_CONSTRAINT to the generator macros, we'll end up
with INVALID_CONSTRAINT in the .s file and gas will barf.

This still isn't an issue for me. Your patches cause the build to fail because
it's using the K flag - if I invert the CONFIG_CC_HAS_K_CONSTRAINT test then
it builds correctly (because it expands the K to nothing).

That doesn't make any sense :/ Is this after you've dropped the -S
parameter?

If you think there's a bug, please can you send a patch? However, inverting
the check breaks the build for me. Which toolchain are you using?

If there is an issue with the expansion of constraint, shouldn't we also
__stringify 'asm_op'?

It would be harmless, but there's no need because asm_op doesn't ever
require further expansion.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel