arm64: minor fixups and enhancements

The recent addition of EFI support has changed the way the arm64 kernel image is entered based on whether or not CONFIG_EFI is defined. To support this conditional entry point add a new global symbol start to head.S that marks the entry, and change the the linker script ENTRY() command to define start as the kernel entry.

Quoted 5 lines · Geoff Levand

The recent addition of EFI support has changed the way the arm64 kernel image is entered based on whether or not CONFIG_EFI is defined. To support this conditional entry point add a new global symbol start to head.S that marks the entry, and change the the linker script ENTRY() command to define start as the kernel entry.

I don't yet understand what the problem is. The _text symbol is currently defined in vmlinux.lds.S to be the beginning of the .head.text section. Whether we have CONFIG_EFI or not, the entry point is the same.

First of all, the 'add x13, x18, #0x16' was carefully chosen to be both a "MZ" prefix and an executable instruction without any harmful side effects.

So currently, the EFI stub jumps to that add
instruction, and not to the 'b stext' that comes after. There is an issue with that, which I have already proposed a patch for (arm64/efi:
efistub: jump to 'stext' directly, not through the header), but this is related to the guarantees the UEFI spec gives about where the header gets loaded (if at all).

However, going back to your patch, setting ENTRY() only affects the vmlinux ELF image, and this information gets stripped when creating the binary. Do you need the entry point to be set so you can load vmlinux using the debugger, perhaps? In that case, did you have any problems branching to the add instruction? If so, I would like to know about it.

Change the preprocessor macro INVALID_HWID definition from ULONG_MAX to (~0) to allow INVALID_HWID to be used within assembly source files.

Commit 3e98fdacc59bbbdbb659be1a144ccc48ed4860fa (arm64: kernel: make the pen of the secondary a 64-bit unsigned value) added the preprocessor macro INVALID_HWID to asm/cputype.h with it defined to be ULONG_MAX. Use of INVALID_HWID in an assembly source file that includes cputype.h will generate an assembler undefined symbol ULONG_MAX build error. The kernel does define a ULONG_MAX in kernel.h, but that file not setup to be included in assembly files.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/include/asm/cputype.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/cputype.h +0 -0

index 379d0b8..6b68380 100644

Quoted 9 lines · Geoff Levand

Change the preprocessor macro INVALID_HWID definition from ULONG_MAX to (~0) to allow INVALID_HWID to be used within assembly source files.

arch/arm64/include/asm/cputype.h +0 -0

@@ -16,7 +16,7 @@
 #ifndef __ASM_CPUTYPE_H
 #define __ASM_CPUTYPE_H
 

arch/arm64/include/asm/cputype.h line 19

Quoted 2 lines

arch/arm64/include/asm/cputype.h +1 -1

-#define INVALID_HWID		ULONG_MAX
+#define INVALID_HWID		UL(~0)

Does it actually expand to ULONG_MAX? ~0 is an int.

Quoted 10 lines · Catalin Marinas

Quoted 9 lines · Geoff Levand

Change the preprocessor macro INVALID_HWID definition from ULONG_MAX to (~0) to allow INVALID_HWID to be used within assembly source files.

arch/arm64/include/asm/cputype.h +0 -0

@@ -16,7 +16,7 @@
 #ifndef __ASM_CPUTYPE_H
 #define __ASM_CPUTYPE_H
 

Quoted 4 lines

arch/arm64/include/asm/cputype.h line 19

Quoted 2 lines

arch/arm64/include/asm/cputype.h +1 -1

-#define INVALID_HWID		ULONG_MAX
+#define INVALID_HWID		UL(~0)

Does it actually expand to ULONG_MAX? ~0 is an int.

It seems to be OK, in C:

volatile unsigned long secondary_holding_pen_release = INVALID_HWID;

Disassembly of section .data:

0000000000000000 <secondary_holding_pen_release>:

   0:   ffffffff        .word   0xffffffff
   4:   ffffffff        .word   0xffffffff

Change the preprocessor macro INVALID_HWID definition from ULONG_MAX to (~0) to allow INVALID_HWID to be used within assembly source files.

Commit 3e98fdacc59bbbdbb659be1a144ccc48ed4860fa (arm64: kernel: make the pen of the secondary a 64-bit unsigned value) added the preprocessor macro INVALID_HWID to asm/cputype.h with it defined to be ULONG_MAX. Use of INVALID_HWID in an assembly source file that includes cputype.h will generate an assembler undefined symbol ULONG_MAX build error. The kernel does define a ULONG_MAX in kernel.h, but that file not setup to be included in assembly files.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/include/asm/cputype.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/cputype.h +1 -1

index 379d0b8..6b68380 100644
@@ -16,7 +16,7 @@
 #ifndef __ASM_CPUTYPE_H
 #define __ASM_CPUTYPE_H
 
-#define INVALID_HWID		ULONG_MAX
+#define INVALID_HWID		UL(~0)
 
 #define MPIDR_UP_BITMASK	(0x1 << 30)
 #define MPIDR_MT_BITMASK	(0x1 << 24)

Signature · 8 lines

1.9.1



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

Quoted 9 lines · Geoff Levand

Change the preprocessor macro INVALID_HWID definition from ULONG_MAX to (~0) to allow INVALID_HWID to be used within assembly source files.

Commit 3e98fdacc59bbbdbb659be1a144ccc48ed4860fa (arm64: kernel: make the pen of the secondary a 64-bit unsigned value) added the preprocessor macro INVALID_HWID to asm/cputype.h with it defined to be ULONG_MAX. Use of INVALID_HWID in an assembly source file that includes cputype.h will generate an assembler undefined symbol ULONG_MAX build error. The kernel does define a ULONG_MAX in kernel.h, but that file not setup to be included in assembly files.

We don't seem to be using INVALID_HWID in any assembly at the moment, and I'm not sure I follow what we'd need it for.

Why do we need this in assembly?

Function prototypes are never definitions, so remove any 'extern' keyword from the funcion prototypes in cpu_ops.h. Fixes warnings emited by checkpatch.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/include/asm/cpu_ops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/include/asm/cpu_ops.h +2 -2

index d7b4b38..150ce63 100644
@@ -61,7 +61,7 @@ struct cpu_operations {
 };
 
 extern const struct cpu_operations *cpu_ops[NR_CPUS];
-extern int __init cpu_read_ops(struct device_node *dn, int cpu);
-extern void __init cpu_read_bootcpu_ops(void);
+int __init cpu_read_ops(struct device_node *dn, int cpu);
+void __init cpu_read_bootcpu_ops(void);
 
 #endif /* ifndef __ASM_CPU_OPS_H */

Signature · 8 lines

1.9.1



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

Remove an unused local variable from head.S. It seems this was never used even from the initial commit 9703d9d7f77ce129621f7d80a844822e2daa7008 (arm64: Kernel booting and initialisation), and is a left over from a previous implementation of __calc_phys_offset.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/kernel/head.S | 4 ----
 1 file changed, 4 deletions(-)

arch/arm64/kernel/head.S +0 -0

index 4019b85..607d4bb 100644

asm-generic/vmlinux.lds.h should be included after the arch specific headers so that the arch headers can override the generic macro defs in asm-generic/vmlinux.lds.h.

Fixes preprosessor redefined warnings when adding arch specific macros.

Signed-off-by Geoff Levand <[email protected]> for Huawei, Linaro

---
 arch/arm64/kernel/vmlinux.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/kernel/vmlinux.lds.S +1 -1

index fbb1af7..8dc1d46 100644
@@ -4,10 +4,10 @@
  * Written by Martin Mares <[email protected]>
  */
 
-#include <asm-generic/vmlinux.lds.h>
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/page.h>
+#include <asm-generic/vmlinux.lds.h>
 
 #include "image.h"
 

Signature · 8 lines

1.9.1



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

Quoted 6 lines · Geoff Levand

asm-generic/vmlinux.lds.h should be included after the arch specific headers so that the arch headers can override the generic macro defs in asm-generic/vmlinux.lds.h.

Fixes preprosessor redefined warnings when adding arch specific macros.

A sample of those warnings would be nice. What do you see being redefined?

Quoted 10 lines · Mark Rutland

Quoted 6 lines · Geoff Levand

asm-generic/vmlinux.lds.h should be included after the arch specific headers so that the arch headers can override the generic macro defs in asm-generic/vmlinux.lds.h.

Fixes preprosessor redefined warnings when adding arch specific macros.

A sample of those warnings would be nice. What do you see being redefined?

In testing I wanted to set LOAD_OFFSET. If gave warnings like these:

arch/arm64/include/asm/page.h: warning: "LOAD_OFFSET" redefined [enabled by default]

I'll send out another patch with updated message.

Quoted 15 lines · Geoff Levand

Quoted 10 lines · Mark Rutland

Quoted 6 lines · Geoff Levand

asm-generic/vmlinux.lds.h should be included after the arch specific headers so that the arch headers can override the generic macro defs in asm-generic/vmlinux.lds.h.

Fixes preprosessor redefined warnings when adding arch specific macros.

A sample of those warnings would be nice. What do you see being redefined?

In testing I wanted to set LOAD_OFFSET. If gave warnings like these:

Quoted 3 lines

arch/arm64/include/asm/page.h: warning: "LOAD_OFFSET" redefined [enabled by default]

I'll send out another patch with updated message.

We currently don't define LOAD_OFFSET, so it would be good to see what/why it is defined to. You could keep this patch as part of the series introducing LOAD_OFFSET.

Add the new arm64 routine local_disable() to allow the masking of several DAIF flags in one operation. Currently, we only have routines to mask individual flags, and to mask several flags multiple calls to daifset are required.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/include/asm/irqflags.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

arch/arm64/include/asm/irqflags.h +2 -0

index 11cc941..28521d4 100644
@@ -113,5 +113,18 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
 #define local_dbg_enable()	asm("msr	daifclr, #8" : : : "memory")
 #define local_dbg_disable()	asm("msr	daifset, #8" : : : "memory")
 
+enum daif_flag {
+	DAIF_FIQ   = (1UL << 6),

Quoted 17 lines · Geoff Levand

Add the new arm64 routine local_disable() to allow the masking of several DAIF flags in one operation. Currently, we only have routines to mask individual flags, and to mask several flags multiple calls to daifset are required.

Signed-off-by Geoff Levand <[email protected]>

---
 arch/arm64/include/asm/irqflags.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

arch/arm64/include/asm/irqflags.h +0 -0

index 11cc941..28521d4 100644
@@ -113,5 +113,18 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
 #define local_dbg_enable()	asm("msr	daifclr, #8" : : : "memory")
 #define local_dbg_disable()	asm("msr	daifset, #8" : : : "memory")
 

arch/arm64/include/asm/irqflags.h static inline int arch_irqs_disabled_flags(unsigned long flags) · line 127

Quoted 12 lines

arch/arm64/include/asm/irqflags.h +12 -0

+enum daif_flag {
+	DAIF_FIQ   = (1UL << 6),
+	DAIF_IRQ   = (1UL << 7),
+	DAIF_ASYNC = (1UL << 8),
+	DAIF_DBG   = (1UL << 9),
+	DAIF_ALL   = (0xffUL << 6),
+};
+
+static inline void local_disable(unsigned long daif_flags)
+{
+	arch_local_irq_restore(daif_flags | arch_local_save_flags());
+}

Who's using this function? I don't see any patch in this series calling it.

Add an assembler macro definition 'atomic' for performing generic atomic operations. Also add macro definitions for atomic_add, atomic_sub, atomic_inc, and atomic_dec.

Quoted 3 lines · Geoff Levand

Add an assembler macro definition 'atomic' for performing generic atomic operations. Also add macro definitions for atomic_add, atomic_sub, atomic_inc, and atomic_dec.

Same question as before. Why are this needed?

Quoted 6 lines · Catalin Marinas

Quoted 3 lines · Geoff Levand

Add an assembler macro definition 'atomic' for performing generic atomic operations. Also add macro definitions for atomic_add, atomic_sub, atomic_inc, and atomic_dec.

Same question as before. Why are this needed?

I use these in the smp-spin-table re-work I am doing to keep a count of CPUs in the secondary_holding_pen, but those patches are not yet ready for review.

I thought I'd post this now thinking theses macros might be of use to someone else. I could just put them into the smp-spin-table series, please let me know.

The existing arm64 linker script uses some pre-defined section macros from vmlinux.lds.h, all of which include a section load address attribute of 'AT(ADDR(section) - LOAD_OFFSET)'. LOAD_OFFSET is not defined for arm64, and defaults to zero, so this mix of section attributions has no ill effect.

For consistency and to clear the way for a possible definition of LOAD_OFFSET add any missing AT() macros to vmlinux.lds.S.

Quoted 7 lines · Geoff Levand

The existing arm64 linker script uses some pre-defined section macros from vmlinux.lds.h, all of which include a section load address attribute of 'AT(ADDR(section) - LOAD_OFFSET)'. LOAD_OFFSET is not defined for arm64, and defaults to zero, so this mix of section attributions has no ill effect.

For consistency and to clear the way for a possible definition of LOAD_OFFSET add any missing AT() macros to vmlinux.lds.S.

So, what's the possible definition for LOAD_OFFSET on arm64?

Catalin,

Quoted 9 lines · Catalin Marinas

Quoted 7 lines · Geoff Levand

The existing arm64 linker script uses some pre-defined section macros from vmlinux.lds.h, all of which include a section load address attribute of 'AT(ADDR(section) - LOAD_OFFSET)'. LOAD_OFFSET is not defined for arm64, and defaults to zero, so this mix of section attributions has no ill effect.

For consistency and to clear the way for a possible definition of LOAD_OFFSET add any missing AT() macros to vmlinux.lds.S.

Quoted 1 line

So, what's the possible definition for LOAD_OFFSET on arm64?

Well, this is just intended as a cleanup for vmlinux.lds.S, but I think a LOAD_OFFSET definition may be needed for kdump.

Takahiro, could you comment?

-Geoff

Add a new arm64 device tree binding cpu-return-addr. This binding is recomended for all ARM v8 CPUs that have an "enable-method" property value of "spin-table". The value is a 64 bit physical address that secondary CPU execution will transfer to upon CPU shutdown.

Signed-off-by Geoff Levand <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Documentation/devicetree/bindings/arm/cpus.txt +0 -0

index 1fe72a0..42d5d5f 100644
@@ -201,6 +201,15 @@ nodes to be present and contain the properties described below.
 			  property identifying a 64-bit zero-initialised

Quoted 18 lines · Geoff Levand

Add a new arm64 device tree binding cpu-return-addr. This binding is recomended for all ARM v8 CPUs that have an "enable-method" property value of "spin-table". The value is a 64 bit physical address that secondary CPU execution will transfer to upon CPU shutdown.

Signed-off-by Geoff Levand <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Documentation/devicetree/bindings/arm/cpus.txt +0 -0

index 1fe72a0..42d5d5f 100644
@@ -201,6 +201,15 @@ nodes to be present and contain the properties described below.
 			  property identifying a 64-bit zero-initialised
 			  memory location.
 

Documentation/devicetree/bindings/arm/cpus.txt nodes to be present and contain the properties described below. · line 211

Quoted 8 lines

Documentation/devicetree/bindings/arm/cpus.txt +8 -0

+	- cpu-return-addr
+		Usage: recomended for all ARM v8 CPUs that have an
+		       "enable-method" property value of "spin-table".
+		Value type: <prop-encoded-array>
+		Definition:
+			# On ARM v8 64-bit systems must be a two cell property.
+			The value is a 64 bit physical address that secondary
+			CPU execution will transfer to upon CPU shutdown.

As I've been away for most of the past four weeks, I haven't read all the threads around this topic. But I don't think we ended up with a clearly agreed recommendation for cpu-return-addr. If we do, we also need to be clear on what state the CPU should be in when returned to such address (ELx, MMU, caches).

In general, if we need returning to firmware I would strongly recommend PSCI but I know there is the Applied board which does not have EL3, so something like this may work. But we need to get them into discussion as well since I assume cpu-return-addr would be a firmware provided address.