Skip to contentSkip to conversation

arm64: Fix fallout of asm/opcodes.h removal

PATCH New Marc Zyngier 3 participants 8 messages Dec 6, 2016

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v2 cover arm64: Fix fallout of asm/opcodes.h removal

As part of the asm/opcodes.h removal, the SET_PSTATE_{PAN,UAO} macros have been switch to using the .inst directive instead of .long (as this has the advantage of providing a correct disassembly). This had the side effect of bringing back an ugly gas bug that shows up when .inst is used within alternative sequences.

This series works around the problem by:
- adding detection code for the broken binutils,
- work around the issue by falling back to .long when using a buggy assembler.

This has been tested with both binutils 2.25 (broken) and 2.27 (fixed), with both LE and BE builds.

Marc Zyngier (2):
arm64: Add detection code for broken .inst support in binutils
arm64: Work around broken .inst when defective gas is detected

 arch/arm64/Makefile             | 10 ++++++++--
 arch/arm64/include/asm/sysreg.h | 29 +++++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 6 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH v2 1/2 arm64: Add detection code for broken .inst support in binutils

Binutils version up to (and including) 2.25 have a pathological behaviour when it comes to mixing .inst directive and arithmetic involving labels. The assembler complains about non-constant expressions and compilation stops pretty quickly.

In order to detect this and work around it, let's add a bit of detection code that will set the CONFIG_BROKEN_GAS_INST option should a broken gas be detected.

---
 arch/arm64/Makefile | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
arch/arm64/Makefile +8 -2
index 3635b86..b9a4a93 100644
@@ -37,10 +37,16 @@ $(warning LSE atomics not supported by binutils)
   endif
 endif
 
-KBUILD_CFLAGS	+= -mgeneral-regs-only $(lseinstr)
+brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
+
+ifneq ($(brokengasinst),)
+$(warning Detected assembler with broken .inst; disassembly will be unreliable)
+endif
+
+KBUILD_CFLAGS	+= -mgeneral-regs-only $(lseinstr) $(brokengasinst)
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
 KBUILD_CFLAGS	+= $(call cc-option, -mpc-relative-literal-loads)
-KBUILD_AFLAGS	+= $(lseinstr)
+KBUILD_AFLAGS	+= $(lseinstr) $(brokengasinst)
 
 ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
 KBUILD_CPPFLAGS	+= -mbig-endian
Signature · 1 line
2.1.4
#2 lore raw

PATCH v2 1/1 arm/arm64: xen: Move shared architecture headers to include/xen/arm

In reply to Marc Zyngier

ARM and arm64 Xen ports share a number of headers, leading to packaging issues when these headers needs to be exported, as it breaks the reasonable requirement that an architecture port has self-contained headers.

Fix the issue by moving the 5 header files to include/xen/arm, and keep local placeholders to include the relevant files.

---
* From v1:
- Move offending include files to include/xen/arm

 arch/arm/include/asm/xen/hypercall.h               |  88 +--------------
 arch/arm/include/asm/xen/hypervisor.h              |  40 +------
 arch/arm/include/asm/xen/interface.h               |  86 +-------------
 arch/arm/include/asm/xen/page-coherent.h           |  99 +----------------
 arch/arm/include/asm/xen/page.h                    | 123 +--------------------
 arch/arm64/include/asm/xen/hypercall.h             |   2 +-
 arch/arm64/include/asm/xen/hypervisor.h            |   2 +-
 arch/arm64/include/asm/xen/interface.h             |   2 +-
 arch/arm64/include/asm/xen/page-coherent.h         |   2 +-
 arch/arm64/include/asm/xen/page.h                  |   2 +-
 .../asm/xen => include/xen/arm}/hypercall.h        |   0
 .../asm/xen => include/xen/arm}/hypervisor.h       |   0
 .../asm/xen => include/xen/arm}/interface.h        |   0
 .../asm/xen => include/xen/arm}/page-coherent.h    |   0
 .../arm/include/asm/xen => include/xen/arm}/page.h |   0
 15 files changed, 10 insertions(+), 436 deletions(-)
 copy {arch/arm/include/asm/xen => include/xen/arm}/hypercall.h (100%)
 copy {arch/arm/include/asm/xen => include/xen/arm}/hypervisor.h (100%)
 copy {arch/arm/include/asm/xen => include/xen/arm}/interface.h (100%)
 copy {arch/arm/include/asm/xen => include/xen/arm}/page-coherent.h (100%)
 copy {arch/arm/include/asm/xen => include/xen/arm}/page.h (100%)
arch/arm/include/asm/xen/hypercall.h +1 -87
index 9d874db..3522cba 100644
@@ -1,87 +1 @@
-/******************************************************************************
- * hypercall.h
- *
- * Linux-specific hypervisor handling.
- *
- * Stefano Stabellini <[email protected]>, Citrix, 2012
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation; or, when distributed
- * separately from the Linux kernel or incorporated into other
- * software packages, subject to the following license:
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this source file (the "Software"), to deal in the Software without
- * restriction, including without limitation the rights to use, copy, modify,
- * merge, publish, distribute, sublicense, and/or sell copies of the Software,
- * and to permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#ifndef _ASM_ARM_XEN_HYPERCALL_H
-#define _ASM_ARM_XEN_HYPERCALL_H
-
-#include <linux/bug.h>
-
-#include <xen/interface/xen.h>
-#include <xen/interface/sched.h>
-#include <xen/interface/platform.h>
-
-long privcmd_call(unsigned call, unsigned long a1,
-		unsigned long a2, unsigned long a3,
-		unsigned long a4, unsigned long a5);
-int HYPERVISOR_xen_version(int cmd, void *arg);
-int HYPERVISOR_console_io(int cmd, int count, char *str);
-int HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count);
-int HYPERVISOR_sched_op(int cmd, void *arg);
-int HYPERVISOR_event_channel_op(int cmd, void *arg);
-unsigned long HYPERVISOR_hvm_op(int op, void *arg);
-int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
-int HYPERVISOR_physdev_op(int cmd, void *arg);
-int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
-int HYPERVISOR_tmem_op(void *arg);
-int HYPERVISOR_vm_assist(unsigned int cmd, unsigned int type);
-int HYPERVISOR_platform_op_raw(void *arg);
-static inline int HYPERVISOR_platform_op(struct xen_platform_op *op)
-{
-	op->interface_version = XENPF_INTERFACE_VERSION;
-	return HYPERVISOR_platform_op_raw(op);
-}
-int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
-
-static inline int
-HYPERVISOR_suspend(unsigned long start_info_mfn)
-{
-	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
-
-	/* start_info_mfn is unused on ARM */
-	return HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
-}
-
-static inline void
-MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
-			unsigned int new_val, unsigned long flags)
-{
-	BUG();
-}
-
-static inline void
-MULTI_mmu_update(struct multicall_entry *mcl, struct mmu_update *req,
-		 int count, int *success_count, domid_t domid)
-{
-	BUG();
-}
-
-#endif /* _ASM_ARM_XEN_HYPERCALL_H */
+#include <xen/arm/hypercall.h>
arch/arm/include/asm/xen/hypervisor.h +1 -39
index 9525151..d6e7709 100644
@@ -1,39 +1 @@
-#ifndef _ASM_ARM_XEN_HYPERVISOR_H
-#define _ASM_ARM_XEN_HYPERVISOR_H
-
-#include <linux/init.h>
-
-extern struct shared_info *HYPERVISOR_shared_info;
-extern struct start_info *xen_start_info;
-
-/* Lazy mode for batching updates / context switch */
-enum paravirt_lazy_mode {
-	PARAVIRT_LAZY_NONE,
-	PARAVIRT_LAZY_MMU,
-	PARAVIRT_LAZY_CPU,
-};
-
-static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
-{
-	return PARAVIRT_LAZY_NONE;
-}
-
-extern struct dma_map_ops *xen_dma_ops;
-
-#ifdef CONFIG_XEN
-void __init xen_early_init(void);
-#else
-static inline void xen_early_init(void) { return; }
-#endif
-
-#ifdef CONFIG_HOTPLUG_CPU
-static inline void xen_arch_register_cpu(int num)
-{
-}
-
-static inline void xen_arch_unregister_cpu(int num)
-{
-}
-#endif
-
-#endif /* _ASM_ARM_XEN_HYPERVISOR_H */
+#include <xen/arm/hypervisor.h>
arch/arm/include/asm/xen/interface.h +1 -85

86 lines not on this page · raw message

arch/arm/include/asm/xen/page-coherent.h +1 -98

99 lines not on this page · raw message

arch/arm/include/asm/xen/page.h +1 -122

123 lines not on this page · raw message

arch/arm64/include/asm/xen/hypercall.h +1 -1
index 74b0c42..3522cba 100644
@@ -1 +1 @@
-#include <../../arm/include/asm/xen/hypercall.h>
+#include <xen/arm/hypercall.h>
arch/arm64/include/asm/xen/hypervisor.h +1 -1
index f263da8..d6e7709 100644
@@ -1 +1 @@
-#include <../../arm/include/asm/xen/hypervisor.h>
+#include <xen/arm/hypervisor.h>
arch/arm64/include/asm/xen/interface.h +1 -1
index 44457ae..88c0d75 100644
@@ -1 +1 @@
-#include <../../arm/include/asm/xen/interface.h>
+#include <xen/arm/interface.h>
arch/arm64/include/asm/xen/page-coherent.h +1 -1
index 2052102..b3ef061 100644
@@ -1 +1 @@
-#include <../../arm/include/asm/xen/page-coherent.h>
+#include <xen/arm/page-coherent.h>
arch/arm64/include/asm/xen/page.h +1 -1
index bed87ec..31bbc80 100644
@@ -1 +1 @@
-#include <../../arm/include/asm/xen/page.h>
+#include <xen/arm/page.h>
include/xen/arm/hypercall.h renamed from arch/arm/include/asm/xen/hypercall.h +0 -0
similarity index 100%
copy from arch/arm/include/asm/xen/hypercall.h
copy to include/xen/arm/hypercall.h
include/xen/arm/hypervisor.h renamed from arch/arm/include/asm/xen/hypervisor.h +0 -0
similarity index 100%
copy from arch/arm/include/asm/xen/hypervisor.h
copy to include/xen/arm/hypervisor.h
include/xen/arm/interface.h renamed from arch/arm/include/asm/xen/interface.h +0 -0
similarity index 100%
copy from arch/arm/include/asm/xen/interface.h
copy to include/xen/arm/interface.h
include/xen/arm/page-coherent.h renamed from arch/arm/include/asm/xen/page-coherent.h +0 -0
similarity index 100%
copy from arch/arm/include/asm/xen/page-coherent.h
copy to include/xen/arm/page-coherent.h
include/xen/arm/page.h renamed from arch/arm/include/asm/xen/page.h +0 -0
similarity index 100%
copy from arch/arm/include/asm/xen/page.h
copy to include/xen/arm/page.h
Signature · 1 line
2.1.4
#3 lore raw

PATCH v2 1/1 arm/arm64: xen: Move shared architecture headers to include/xen/arm

Quoted 5 lines · Marc Zyngier

ARM and arm64 Xen ports share a number of headers, leading to packaging issues when these headers needs to be exported, as it breaks the reasonable requirement that an architecture port has self-contained headers.

Quoted 7 lines

Fix the issue by moving the 5 header files to include/xen/arm, and keep local placeholders to include the relevant files.

---
* From v1:
- Move offending include files to include/xen/arm

Duh. Apologies for the spurious resend, I messed up.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#4 lore raw

PATCH v2 2/2 arm64: Work around broken .inst when defective gas is detected

In reply to Marc Zyngier

.inst being largely broken with older binutils, it'd be better not to emit it altogether when detecting such configuration (as it leads to all kind of horrors when using alternatives).

Generalize the __emit_inst macro and use it extensively in asm/sysreg.h, and make it generate a .long when a broken gas is detected. The disassembly will be crap, but at least we can write semi-sane code.

---
 arch/arm64/include/asm/sysreg.h | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)
arch/arm64/include/asm/sysreg.h +25 -4
index 9e16a18..98ae03f 100644
@@ -35,12 +35,33 @@
 #define sys_reg(op0, op1, crn, crm, op2) \
 	((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
+#ifndef CONFIG_BROKEN_GAS_INST
+
 #ifdef __ASSEMBLY__
 #define __emit_inst(x)			.inst (x)
 #else
 #define __emit_inst(x)			".inst " __stringify((x)) "\n\t"
 #endif
 
+#else  /* CONFIG_BROKEN_GAS_INST */
+
+#ifndef CONFIG_CPU_BIG_ENDIAN
+#define __INSTR_BSWAP(x)		(x)
+#else  /* CONFIG_CPU_BIG_ENDIAN */
+#define __INSTR_BSWAP(x)		((((x) << 24) & 0xff000000)	| \
+					 (((x) <<  8) & 0x00ff0000)	| \
+					 (((x) >>  8) & 0x0000ff00)	| \
+					 (((x) >> 24) & 0x000000ff))
+#endif	/* CONFIG_CPU_BIG_ENDIAN */
+
+#ifdef __ASSEMBLY__
+#define __emit_inst(x)			.long __INSTR_BSWAP(x)
+#else  /* __ASSEMBLY__ */
+#define __emit_inst(x)			".long " __stringify(__INSTR_BSWAP(x)) "\n\t"
+#endif	/* __ASSEMBLY__ */
+
+#endif	/* CONFIG_BROKEN_GAS_INST */
+
 #define SYS_MIDR_EL1			sys_reg(3, 0, 0, 0, 0)
 #define SYS_MPIDR_EL1			sys_reg(3, 0, 0, 0, 5)
 #define SYS_REVIDR_EL1			sys_reg(3, 0, 0, 0, 6)
@@ -232,11 +253,11 @@
 	.equ	.L__reg_num_xzr, 31
 
 	.macro	mrs_s, rt, sreg
-	.inst	0xd5200000|(\sreg)|(.L__reg_num_\rt)
+	 __emit_inst(0xd5200000|(\sreg)|(.L__reg_num_\rt))
 	.endm
 
 	.macro	msr_s, sreg, rt
-	.inst	0xd5000000|(\sreg)|(.L__reg_num_\rt)
+	__emit_inst(0xd5000000|(\sreg)|(.L__reg_num_\rt))
 	.endm
 
 #else
@@ -250,11 +271,11 @@ asm(
 "	.equ	.L__reg_num_xzr, 31\n"
 "\n"
 "	.macro	mrs_s, rt, sreg\n"
-"	.inst	0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+	__emit_inst(0xd5200000|(\\sreg)|(.L__reg_num_\\rt))
 "	.endm\n"
 "\n"
 "	.macro	msr_s, sreg, rt\n"
-"	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+	__emit_inst(0xd5000000|(\\sreg)|(.L__reg_num_\\rt))
 "	.endm\n"
 );
 
Signature · 1 line
2.1.4
#5 lore raw

PATCH v2 2/2 arm64: Work around broken .inst when defective gas is detected

Quoted 3 lines · Marc Zyngier

.inst being largely broken with older binutils, it'd be better not to emit it altogether when detecting such configuration (as it leads to all kind of horrors when using alternatives).

GAS seems to provide few guarantees about the constantness of anything other than an expression composed purely of literal constants. So, this may rather be lack of a useful feature rather than a bug per se.

Quoted 4 lines

Generalize the __emit_inst macro and use it extensively in asm/sysreg.h, and make it generate a .long when a broken gas is detected. The disassembly will be crap, but at least we can write semi-sane code.

Don't know how much this helps, but if we want decent disassembly then one option is to use macros instead of symbols, so that the argument to .inst involves only literals after macro expansion.

Something like:

.ifndef .L__with_reg_defined

	.set .L__with_reg_defined, 1

	.macro __def_reg name, num
		.macro __with_reg__\name instr:vararg
			\instr \num
		.endm
	.endm

	.irp n, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
		__def_reg x\n, \n
	.endr
	__def_reg xzr, 31

	.macro __with_reg regname, instr:vararg
		__with_reg__\regname \instr
	.endm

.endif

.ifndef .L__msr_s_defined

	.set .L__msr_s_defined, 1

	.macro __msr_s sreg, rn
		.inst	0xd5000000 | (\sreg) | (\rn)
	.endm

	.macro msr_s sreg, rn
		__with_reg \rn, __msr_s \sreg,
	.endm

.endif

For the inline asm case, gcc will think that the asm assembles to much larger code that it really does in this, but I suspect this won't matter in realistic situations.

For the .S case, we would define the macros once only, in the header (as at present) -- the guards won't be necessary for that.

Cheers
---Dave

Quoted 80 lines
---
 arch/arm64/include/asm/sysreg.h | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)
arch/arm64/include/asm/sysreg.h +25 -4
index 9e16a18..98ae03f 100644
@@ -35,12 +35,33 @@
 #define sys_reg(op0, op1, crn, crm, op2) \
 	((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
+#ifndef CONFIG_BROKEN_GAS_INST
+
 #ifdef __ASSEMBLY__
 #define __emit_inst(x)			.inst (x)
 #else
 #define __emit_inst(x)			".inst " __stringify((x)) "\n\t"
 #endif
 
+#else  /* CONFIG_BROKEN_GAS_INST */
+
+#ifndef CONFIG_CPU_BIG_ENDIAN
+#define __INSTR_BSWAP(x)		(x)
+#else  /* CONFIG_CPU_BIG_ENDIAN */
+#define __INSTR_BSWAP(x)		((((x) << 24) & 0xff000000)	| \
+					 (((x) <<  8) & 0x00ff0000)	| \
+					 (((x) >>  8) & 0x0000ff00)	| \
+					 (((x) >> 24) & 0x000000ff))
+#endif	/* CONFIG_CPU_BIG_ENDIAN */
+
+#ifdef __ASSEMBLY__
+#define __emit_inst(x)			.long __INSTR_BSWAP(x)
+#else  /* __ASSEMBLY__ */
+#define __emit_inst(x)			".long " __stringify(__INSTR_BSWAP(x)) "\n\t"
+#endif	/* __ASSEMBLY__ */
+
+#endif	/* CONFIG_BROKEN_GAS_INST */
+
 #define SYS_MIDR_EL1			sys_reg(3, 0, 0, 0, 0)
 #define SYS_MPIDR_EL1			sys_reg(3, 0, 0, 0, 5)
 #define SYS_REVIDR_EL1			sys_reg(3, 0, 0, 0, 6)
@@ -232,11 +253,11 @@
 	.equ	.L__reg_num_xzr, 31
 
 	.macro	mrs_s, rt, sreg
-	.inst	0xd5200000|(\sreg)|(.L__reg_num_\rt)
+	 __emit_inst(0xd5200000|(\sreg)|(.L__reg_num_\rt))
 	.endm
 
 	.macro	msr_s, sreg, rt
-	.inst	0xd5000000|(\sreg)|(.L__reg_num_\rt)
+	__emit_inst(0xd5000000|(\sreg)|(.L__reg_num_\rt))
 	.endm
 
 #else
@@ -250,11 +271,11 @@ asm(
 "	.equ	.L__reg_num_xzr, 31\n"
 "\n"
 "	.macro	mrs_s, rt, sreg\n"
-"	.inst	0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+	__emit_inst(0xd5200000|(\\sreg)|(.L__reg_num_\\rt))
 "	.endm\n"
 "\n"
 "	.macro	msr_s, sreg, rt\n"
-"	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+	__emit_inst(0xd5000000|(\\sreg)|(.L__reg_num_\\rt))
 "	.endm\n"
 );
 
Signature · 7 lines
2.1.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#6 lore raw

PATCH v2 2/2 arm64: Work around broken .inst when defective gas is detected

Quoted 9 lines · Dave Martin
Quoted 3 lines · Marc Zyngier

.inst being largely broken with older binutils, it'd be better not to emit it altogether when detecting such configuration (as it leads to all kind of horrors when using alternatives).

GAS seems to provide few guarantees about the constantness of anything other than an expression composed purely of literal constants. So, this may rather be lack of a useful feature rather than a bug per se.

Quoted 8 lines
Quoted 4 lines

Generalize the __emit_inst macro and use it extensively in asm/sysreg.h, and make it generate a .long when a broken gas is detected. The disassembly will be crap, but at least we can write semi-sane code.

Don't know how much this helps, but if we want decent disassembly then one option is to use macros instead of symbols, so that the argument to .inst involves only literals after macro expansion.

[...]

It seems that this doesn't solve your problem though ... oh well.

Cheers
---Dave

#7 lore raw

v2 cover arm64: Fix fallout of asm/opcodes.h removal

In reply to Marc Zyngier
Quoted 11 lines · Marc Zyngier

As part of the asm/opcodes.h removal, the SET_PSTATE_{PAN,UAO} macros have been switch to using the .inst directive instead of .long (as this has the advantage of providing a correct disassembly). This had the side effect of bringing back an ugly gas bug that shows up when .inst is used within alternative sequences.

This series works around the problem by:
- adding detection code for the broken binutils,
- work around the issue by falling back to .long when using a buggy assembler.

Quoted 6 lines

This has been tested with both binutils 2.25 (broken) and 2.27 (fixed), with both LE and BE builds.

Marc Zyngier (2):
arm64: Add detection code for broken .inst support in binutils
arm64: Work around broken .inst when defective gas is detected

For the two patches (not the bonus xen one!):

Thanks for sticking with this...

Will