Skip to contentSkip to conversation

arm64: Work around broken GCC handling of "S" constraint

PATCH In review Marc Zyngier 3 participants 8 messages Dec 7, 2020

Subsystems (2)
arm64 KVM arm64
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

PATCH 1/1 arm64: Work around broken GCC handling of "S" constraint

GCC 4.9 seems to have a problem with the "S" asm constraint when the symbol lives in the same compilation unit, and pretends the constraint is impossible:

$ cat x.c
void *foo(void)
{
	static int x;
	int *addr;
	asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
	return addr;
}
$ ~/Work/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc -S -x c -O2 x.c
x.c: In function ‘foo’:
x.c:5:2: error: impossible constraint in ‘asm’
  asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
  ^

Boo. Following revisions of the compiler work just fine, though.

We can fallback to the "i" constraint in that case, which *seems* to do the right thing. Hopefully we will be able to remove this at some point, but in the meantime this gets us going.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/Makefile              | 9 +++++++++
 arch/arm64/include/asm/kvm_asm.h | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)
arch/arm64/Makefile +9 -0
index 5789c2d18d43..c4ee8e64ad1a 100644
@@ -44,12 +44,21 @@ cc_has_k_constraint := $(call try-run,echo				\
 		return 0;						\
 	}' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
 
+cc_has_broken_s_constraint := $(call try-run,echo			\
+	'void *foo(void) {						\
+		static int x;						\
+		int *addr;						\
+		asm("adrp %0, %1" : "=r" (addr) : "S" (&x));		\
+		return addr;						\
+	}' | $(CC) -S -x c -c -O2 -o "$$TMP" -,,-DCONFIG_CC_HAS_BROKEN_S_CONSTRAINT=1)
+
 ifeq ($(CONFIG_BROKEN_GAS_INST),y)
 $(warning Detected assembler with broken .inst; disassembly will be unreliable)
 endif
 
 KBUILD_CFLAGS	+= -mgeneral-regs-only	\
 		   $(compat_vdso) $(cc_has_k_constraint)
+KBUILD_CFLAGS	+= $(cc_has_broken_s_constraint)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, psabi)
 KBUILD_AFLAGS	+= $(compat_vdso)
arch/arm64/include/asm/kvm_asm.h +7 -1
index 7ccf770c53d9..fa8e886998a3 100644
@@ -199,6 +199,12 @@ extern void __vgic_v3_init_lrs(void);
 
 extern u32 __kvm_get_mdcr_el2(void);
 
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT	"i"
+#else
+#define SYM_CONSTRAINT	"S"
+#endif
+
 /*
  * Obtain the PC-relative address of a kernel symbol
  * s: symbol

@@ -215,7 +221,7 @@ extern u32 __kvm_get_mdcr_el2(void);
 		typeof(s) *addr;					\
 		asm("adrp	%0, %1\n"				\
 		    "add	%0, %0, :lo12:%1\n"			\
-		    : "=r" (addr) : "S" (&s));				\
+		    : "=r" (addr) : SYM_CONSTRAINT (&s));		\
 		addr;							\
 	})
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#1 lore raw
Quoted 64 lines · Marc Zyngier

GCC 4.9 seems to have a problem with the "S" asm constraint when the symbol lives in the same compilation unit, and pretends the constraint is impossible:

$ cat x.c
void *foo(void)
{
        static int x;
        int *addr;
        asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
        return addr;
}
$ ~/Work/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc -S -x c -O2 x.c
x.c: In function ‘foo’:
x.c:5:2: error: impossible constraint in ‘asm’
  asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
  ^

Boo. Following revisions of the compiler work just fine, though.

We can fallback to the "i" constraint in that case, which *seems* to do the right thing. Hopefully we will be able to remove this at some point, but in the meantime this gets us going.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/Makefile              | 9 +++++++++
 arch/arm64/include/asm/kvm_asm.h | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)
arch/arm64/Makefile +9 -0
index 5789c2d18d43..c4ee8e64ad1a 100644
@@ -44,12 +44,21 @@ cc_has_k_constraint := $(call try-run,echo                          \
                return 0;                                               \
        }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
 
+cc_has_broken_s_constraint := $(call try-run,echo                      \
+       'void *foo(void) {                                              \
+               static int x;                                           \
+               int *addr;                                              \
+               asm("adrp %0, %1" : "=r" (addr) : "S" (&x));            \
+               return addr;                                            \
+       }' | $(CC) -S -x c -c -O2 -o "$$TMP" -,,-DCONFIG_CC_HAS_BROKEN_S_CONSTRAINT=1)
+
 ifeq ($(CONFIG_BROKEN_GAS_INST),y)
 $(warning Detected assembler with broken .inst; disassembly will be unreliable)
 endif
 
 KBUILD_CFLAGS  += -mgeneral-regs-only  \
                   $(compat_vdso) $(cc_has_k_constraint)
+KBUILD_CFLAGS  += $(cc_has_broken_s_constraint)
 KBUILD_CFLAGS  += $(call cc-disable-warning, psabi)
 KBUILD_AFLAGS  += $(compat_vdso)
arch/arm64/include/asm/kvm_asm.h +0 -0
index 7ccf770c53d9..fa8e886998a3 100644

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 9 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
@@ -199,6 +199,12 @@ extern void __vgic_v3_init_lrs(void);
 
 extern u32 __kvm_get_mdcr_el2(void);
 
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

Quoted 13 lines
arch/arm64/include/asm/kvm_asm.h +1 -2
 /*
  * Obtain the PC-relative address of a kernel symbol
  * s: symbol

@@ -215,7 +221,7 @@ extern u32 __kvm_get_mdcr_el2(void);
                typeof(s) *addr;                                        \
                asm("adrp       %0, %1\n"                               \
                    "add        %0, %0, :lo12:%1\n"                     \
-                   : "=r" (addr) : "S" (&s));                          \
+                   : "=r" (addr) : SYM_CONSTRAINT (&s));               \
                addr;                                                   \
        })
 
--
Quoted 2 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#2 lore raw

(resend with David's email address fixed)

Quoted 90 lines · Ard Biesheuvel
Quoted 64 lines · Marc Zyngier

GCC 4.9 seems to have a problem with the "S" asm constraint when the symbol lives in the same compilation unit, and pretends the constraint is impossible:

$ cat x.c
void *foo(void)
{
        static int x;
        int *addr;
        asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
        return addr;
}
$ ~/Work/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc -S -x c -O2 x.c
x.c: In function ‘foo’:
x.c:5:2: error: impossible constraint in ‘asm’
  asm("adrp %0, %1" : "=r" (addr) : "S" (&x));
  ^

Boo. Following revisions of the compiler work just fine, though.

We can fallback to the "i" constraint in that case, which *seems* to do the right thing. Hopefully we will be able to remove this at some point, but in the meantime this gets us going.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/Makefile              | 9 +++++++++
 arch/arm64/include/asm/kvm_asm.h | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)
arch/arm64/Makefile +9 -0
index 5789c2d18d43..c4ee8e64ad1a 100644
@@ -44,12 +44,21 @@ cc_has_k_constraint := $(call try-run,echo                          \
                return 0;                                               \
        }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
 
+cc_has_broken_s_constraint := $(call try-run,echo                      \
+       'void *foo(void) {                                              \
+               static int x;                                           \
+               int *addr;                                              \
+               asm("adrp %0, %1" : "=r" (addr) : "S" (&x));            \
+               return addr;                                            \
+       }' | $(CC) -S -x c -c -O2 -o "$$TMP" -,,-DCONFIG_CC_HAS_BROKEN_S_CONSTRAINT=1)
+
 ifeq ($(CONFIG_BROKEN_GAS_INST),y)
 $(warning Detected assembler with broken .inst; disassembly will be unreliable)
 endif
 
 KBUILD_CFLAGS  += -mgeneral-regs-only  \
                   $(compat_vdso) $(cc_has_k_constraint)
+KBUILD_CFLAGS  += $(cc_has_broken_s_constraint)
 KBUILD_CFLAGS  += $(call cc-disable-warning, psabi)
 KBUILD_AFLAGS  += $(compat_vdso)
arch/arm64/include/asm/kvm_asm.h +0 -0
index 7ccf770c53d9..fa8e886998a3 100644

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 9 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
@@ -199,6 +199,12 @@ extern void __vgic_v3_init_lrs(void);
 
 extern u32 __kvm_get_mdcr_el2(void);
 
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

Quoted 12 lines
arch/arm64/include/asm/kvm_asm.h +1 -1
 /*
  * Obtain the PC-relative address of a kernel symbol
  * s: symbol

@@ -215,7 +221,7 @@ extern u32 __kvm_get_mdcr_el2(void);
                typeof(s) *addr;                                        \
                asm("adrp       %0, %1\n"                               \
                    "add        %0, %0, :lo12:%1\n"                     \
-                   : "=r" (addr) : "S" (&s));                          \
+                   : "=r" (addr) : SYM_CONSTRAINT (&s));               \
                addr;                                                   \
        })
Quoted 3 lines
Quoted 3 lines

--
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#3 lore raw
Quoted 1 line · Ard Biesheuvel

(resend with David's email address fixed)

Irk. Thanks for that.

Quoted 8 lines
Quoted 8 lines

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 6 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

I guess we could. But I haven't investigated which exact range of compiler is broken (GCC 6.3 seems fixed, but that's the oldest I have apart from the offending 4.9).

        M.
Signature · 5 lines
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#4 lore raw
Quoted 15 lines · Marc Zyngier
Quoted 1 line · Ard Biesheuvel

(resend with David's email address fixed)

Irk. Thanks for that.

Quoted 8 lines
Quoted 8 lines

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 6 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

Quoted 4 lines

I guess we could. But I haven't investigated which exact range of compiler is broken (GCC 6.3 seems fixed, but that's the oldest I have apart from the offending 4.9).

I tried 5.4 on godbolt, and it seems happy. And the failure will be obvious, so we can afford to get it slightly wrong and refine it later.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#5 lore raw
Quoted 16 lines · Ard Biesheuvel
Quoted 15 lines · Marc Zyngier
Quoted 1 line · Ard Biesheuvel

(resend with David's email address fixed)

Irk. Thanks for that.

Quoted 8 lines
Quoted 8 lines

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 6 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

Quoted 8 lines
Quoted 4 lines

I guess we could. But I haven't investigated which exact range of compiler is broken (GCC 6.3 seems fixed, but that's the oldest I have apart from the offending 4.9).

I tried 5.4 on godbolt, and it seems happy. And the failure will be obvious, so we can afford to get it slightly wrong and refine it later.

FWIW the Linaro 14.11, 15.02 and 15.05 releases of GCC 4.9.3 seem to build rc7 without complaint. The only thing older that I have to hand is Ubuntu's GCC 4.8.4, which Kbuild chokes on entirely now.

Robin.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#6 lore raw

Hi Robin,

Quoted 23 lines · Robin Murphy
Quoted 21 lines · Ard Biesheuvel
Quoted 19 lines · Marc Zyngier
Quoted 1 line · Ard Biesheuvel

(resend with David's email address fixed)

Irk. Thanks for that.

Quoted 8 lines
Quoted 8 lines

arch/arm64/include/asm/kvm_asm.h extern void __vgic_v3_init_lrs(void); · line 207

Quoted 6 lines
arch/arm64/include/asm/kvm_asm.h +6 -0
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

I guess we could. But I haven't investigated which exact range of compiler is broken (GCC 6.3 seems fixed, but that's the oldest I have apart from the offending 4.9).

Quoted 7 lines
Quoted 3 lines

I tried 5.4 on godbolt, and it seems happy. And the failure will be obvious, so we can afford to get it slightly wrong and refine it later.

FWIW the Linaro 14.11, 15.02 and 15.05 releases of GCC 4.9.3 seem to build rc7 without complaint. The only thing older that I have to hand is Ubuntu's GCC 4.8.4, which Kbuild chokes on entirely now.

Can you try kvmarm/next? David's PSCI relay is breaking badly here.

Thanks,

	M.
Signature · 5 lines
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#7 lore raw
Quoted 31 lines · Marc Zyngier

Hi Robin,

Quoted 27 lines · Robin Murphy
Quoted 24 lines · Ard Biesheuvel
Quoted 15 lines · Marc Zyngier
Quoted 1 line · Ard Biesheuvel

(resend with David's email address fixed)

Irk. Thanks for that.

Quoted 8 lines
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+#ifdef CONFIG_CC_HAS_BROKEN_S_CONSTRAINT
+#define SYM_CONSTRAINT "i"
+#else
+#define SYM_CONSTRAINT "S"
+#endif
+

Could we just check GCC_VERSION here?

Quoted 4 lines

I guess we could. But I haven't investigated which exact range of compiler is broken (GCC 6.3 seems fixed, but that's the oldest I have apart from the offending 4.9).

I tried 5.4 on godbolt, and it seems happy. And the failure will be obvious, so we can afford to get it slightly wrong and refine it later.

Quoted 5 lines
Quoted 3 lines

FWIW the Linaro 14.11, 15.02 and 15.05 releases of GCC 4.9.3 seem to build rc7 without complaint. The only thing older that I have to hand is Ubuntu's GCC 4.8.4, which Kbuild chokes on entirely now.

Can you try kvmarm/next? David's PSCI relay is breaking badly here.

Ah, gotcha... Yes, they're all falling over on that :(

The 15.08 release of 5.1.1 is happy though, so Ard's probably right about generalising it to 4.x.

Cheers,
Robin.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm