Subject: Re: [PATCH v2] powerpc/64: handle linker stubs in low .text code

From: Balbir Singh <[email-masked]>
Date: Tue, 30 May 2017 09:02:30 +1000

On Mon, 2017-05-29 at 17:39 +1000, Nicholas Piggin wrote:
> Very large kernels may require linker stubs for branches from HEAD
> text code. The linker may place these stubs before the HEAD text
> sections, which breaks the assumption that HEAD text is located at 0
> (or the .text section being located at 0x7000/0x8000 on Book3S
> kernels).
> 
> Provide an option to create a small section just before the .text
> section with an empty 256 - 4 bytes, and adjust the start of the .text
> section to match. The linker will tend to put stubs in that section
> and not break our relative-to-absolute offset assumptions.
>

I wonder if the size should be configurable in case more than expected
number of stubs are created. I've seen at-least 1 with as I was playing
with linker script alignment of sections.
 
> This causes a small waste of space on common kernels, but allows large
> kernels to build and boot. For now, it is an EXPERT config option,
> defaulting to =n, but a reference is provided for it in the build-time
> check for such breakage. This is good enough for allyesconfig and
> custom users / hackers.
> 
> Signed-off-by: Nicholas Piggin <[email-masked]>
> ---
>  arch/powerpc/Kconfig               | 11 +++++++++++
>  arch/powerpc/include/asm/head-64.h | 18 ++++++++++++++++++
>  arch/powerpc/kernel/vmlinux.lds.S  |  5 +++++
>  arch/powerpc/tools/head_check.sh   |  7 +++++++
>  4 files changed, 41 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index f7c8f9972f61..6eb70c96ec5e 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -454,6 +454,17 @@ config PPC_TRANSACTIONAL_MEM
>         ---help---
>           Support user-mode Transactional Memory on POWERPC.
>  
> +config LD_HEAD_STUB_CATCH
> +	bool "Reserve 256 bytes to cope with linker stubs in HEAD text" if EXPERT
> +	depends on PPC64
> +	default n
> +	help
> +	  Very large kernels can cause linker branch stubs to be generated by
> +	  code in head_64.S, which moves the head text sections out of their
> +	  specified location. This option can work around the problem.

I think we should also say that this relies on binutils/linker doing the
right thing, I don't think this is documented, just an observation that
always works.

> +
> +	  If unsure, say "N".
> +
>  config DISABLE_MPROFILE_KERNEL
>  	bool "Disable use of mprofile-kernel for kernel tracing"
>  	depends on PPC64 && CPU_LITTLE_ENDIAN
> diff --git a/arch/powerpc/include/asm/head-64.h b/arch/powerpc/include/asm/head-64.h
> index 99c9f01d02df..7ab95798f170 100644
> --- a/arch/powerpc/include/asm/head-64.h
> +++ b/arch/powerpc/include/asm/head-64.h
> @@ -63,11 +63,29 @@
>  	. = 0x0;						\
>  start_##sname:
>  
> +/*
> + * .linker_stub_catch section is used to catch linker stubs from being
> + * inserted in our .text section, above the start_text label (which breaks
> + * the ABS_ADDR calculation). See kernel/vmlinux.lds.S and tools/head_check.sh
> + * for more details. We would prefer to just keep a cacheline (0x80), but
> + * 0x100 seems to be how the linker aligns branch stub groups.
> + */
> +#ifdef CONFIG_LD_HEAD_STUB_CATCH
> +#define OPEN_TEXT_SECTION(start)				\
> +	.section ".linker_stub_catch","ax",@progbits;		\
> +linker_stub_catch:						\
> +	. = 0x4;						\

Start at 4? 

> +	text_start = (start) + 0x100;				\
> +	.section ".text","ax",@progbits;			\
> +	.balign 0x100;						\
> +start_text:
> +#else
>  #define OPEN_TEXT_SECTION(start)				\
>  	text_start = (start);					\
>  	.section ".text","ax",@progbits;			\
>  	. = 0x0;						\
>  start_text:
> +#endif
>

I could apply these on linux-next, it does not have the head_check.sh bits.
I wanted to see what the final vmlinux layout looks like after this. I presume
start_text would be at 0x8100
  
>  #define ZERO_FIXED_SECTION(sname, start, end)			\
>  	sname##_start = (start);				\
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 5aa434e84605..e69155f0db36 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -81,6 +81,11 @@ SECTIONS
>  	 * section placement to work.
>  	 */
>  	.text BLOCK(0) : AT(ADDR(.text) - LOAD_OFFSET) {
> +#ifdef CONFIG_LD_HEAD_STUB_CATCH
> +		*(.linker_stub_catch);
> +		. = . ;
> +#endif
> +
>  #else
>  	.text : AT(ADDR(.text) - LOAD_OFFSET) {
>  		ALIGN_FUNCTION();
> diff --git a/arch/powerpc/tools/head_check.sh b/arch/powerpc/tools/head_check.sh
> index bfde937564f9..ad9e57209aa4 100755
> --- a/arch/powerpc/tools/head_check.sh
> +++ b/arch/powerpc/tools/head_check.sh
> @@ -21,6 +21,11 @@
>  # fixed section region (0 - 0x8000ish). Check what code is calling those stubs,
>  # and perhaps change so a direct branch can reach.
>  #
> +# A ".linker_stub_catch" section is used to catch some stubs generated by
> +# early .text code, which tend to get placed at the start of the section.
> +# If there are too many such stubs, they can overflow this section. Expanding
> +# it may help (or reducing the number of stub branches).
> +#
>  # Linker stubs use the TOC pointer, so even if fixed section code could
>  # tolerate them being inserted into head code, they can't be allowed in low
>  # level entry code (boot, interrupt vectors, etc) until r2 is set up. This
> @@ -50,6 +55,7 @@ start_head_addr=$(cat .tmp_symbols.txt | grep " t start_first_256B$" | cut -d' '
>  
>  if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
>  	echo "ERROR: head code starts at $start_head_addr, should be $expected_start_head_addr"
> +	echo "ERROR: try to enable LD_HEAD_STUB_CATCH config option"
>  	echo "ERROR: see comments in arch/powerpc/tools/head_check.sh"
>  
>  	exit 1
> @@ -63,6 +69,7 @@ start_text_addr=$(cat .tmp_symbols.txt | grep " t start_text$" | cut -d' ' -f1)
>  
>  if [ "$start_text_addr" != "$expected_start_text_addr" ]; then
>  	echo "ERROR: start_text address is $start_text_addr, should be $expected_start_text_addr"
> +	echo "ERROR: try to enable LD_HEAD_STUB_CATCH config option"
>  	echo "ERROR: see comments in arch/powerpc/tools/head_check.sh"
>  
>  	exit 1

Overall, I think this is very useful. I am not sure how to test this and
what features it would be incompatible with -- kexec/crash dump?

Balbir Singh.