| Message ID | 20201207154341.1004276-1-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Work around broken GCC handling of "S" constraint | expand |
On Mon, 7 Dec 2020 at 16:43, Marc Zyngier <maz@kernel.org> wrote: > > 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 <maz@kernel.org> > --- > arch/arm64/Makefile | 9 +++++++++ > arch/arm64/include/asm/kvm_asm.h | 8 +++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 5789c2d18d43..c4ee8e64ad1a 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -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) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index 7ccf770c53d9..fa8e886998a3 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -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? > /* > * 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; \ > }) > > -- > 2.29.2 >
(resend with David's email address fixed) On Mon, 7 Dec 2020 at 18:17, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 7 Dec 2020 at 16:43, Marc Zyngier <maz@kernel.org> wrote: > > > > 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 <maz@kernel.org> > > > --- > > arch/arm64/Makefile | 9 +++++++++ > > arch/arm64/include/asm/kvm_asm.h | 8 +++++++- > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > index 5789c2d18d43..c4ee8e64ad1a 100644 > > --- a/arch/arm64/Makefile > > +++ b/arch/arm64/Makefile > > @@ -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) > > > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > > index 7ccf770c53d9..fa8e886998a3 100644 > > --- a/arch/arm64/include/asm/kvm_asm.h > > +++ b/arch/arm64/include/asm/kvm_asm.h > > @@ -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? > > > /* > > * 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; \ > > }) > > > > -- > > 2.29.2 > >
On 2020-12-07 17:19, Ard Biesheuvel wrote: > (resend with David's email address fixed) Irk. Thanks for that. >> > +#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.
On Mon, 7 Dec 2020 at 18:41, Marc Zyngier <maz@kernel.org> wrote: > > On 2020-12-07 17:19, Ard Biesheuvel wrote: > > (resend with David's email address fixed) > > Irk. Thanks for that. > > >> > +#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). > 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.
On 2020-12-07 17:47, Ard Biesheuvel wrote: > On Mon, 7 Dec 2020 at 18:41, Marc Zyngier <maz@kernel.org> wrote: >> >> On 2020-12-07 17:19, Ard Biesheuvel wrote: >>> (resend with David's email address fixed) >> >> Irk. Thanks for that. >> >>>>> +#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). >> > > 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.
Hi Robin, On Mon, 07 Dec 2020 18:42:23 +0000, Robin Murphy <robin.murphy@arm.com> wrote: > > On 2020-12-07 17:47, Ard Biesheuvel wrote: > > On Mon, 7 Dec 2020 at 18:41, Marc Zyngier <maz@kernel.org> wrote: > >> > >> On 2020-12-07 17:19, Ard Biesheuvel wrote: > >>> (resend with David's email address fixed) > >> > >> Irk. Thanks for that. > >> > >>>>> +#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). > >> > > > > 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.
On 2020-12-07 19:04, Marc Zyngier wrote: > Hi Robin, > > On Mon, 07 Dec 2020 18:42:23 +0000, > Robin Murphy <robin.murphy@arm.com> wrote: >> >> On 2020-12-07 17:47, Ard Biesheuvel wrote: >>> On Mon, 7 Dec 2020 at 18:41, Marc Zyngier <maz@kernel.org> wrote: >>>> >>>> On 2020-12-07 17:19, Ard Biesheuvel wrote: >>>>> (resend with David's email address fixed) >>>> >>>> Irk. Thanks for that. >>>> >>>>>>> +#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). >>>> >>> >>> 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. 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.
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 5789c2d18d43..c4ee8e64ad1a 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -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) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 7ccf770c53d9..fa8e886998a3 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -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; \ })
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 <maz@kernel.org> --- arch/arm64/Makefile | 9 +++++++++ arch/arm64/include/asm/kvm_asm.h | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-)