[Skiboot] [PATCH 12/12] add little endian support

Oliver O'Halloran oohall at gmail.com
Tue Oct 1 16:04:06 AEST 2019


On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
> This adds support for building LE skiboot with LITTLE_ENDIAN=1.
> 
> This is not complete, notably PHB3, NPU* and *CAPI*, but it is
> sufficient to build and boot on mambo and OpenPOWER POWER9 systems.
> 
> LE/ELFv2 is a nicer calling convention, and results in smaller image and
> less stack usage. It also follows the rest of the Linux/OpenPOWER stack
> moving to LE.
> 
> The OPALv3 call interface still requires an ugly transition through BE
> for compatibility, but that is all handled on the OPAL side.
> 
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
>  Makefile.main                            | 38 ++++++++++--
>  asm/cvc_entry.S                          | 39 +++++-------
>  asm/head.S                               | 76 +++++++++++++++++-------
>  core/cpu.c                               | 24 +++++++-
>  core/init.c                              |  1 +
>  hdata/spira.c                            |  2 +-
>  include/asm-utils.h                      | 17 ++++++
>  include/cpu.h                            |  3 +
>  include/elf.h                            |  4 ++
>  libpore/p9_cpu_reg_restore_instruction.H | 23 +++----
>  libstb/cvc.c                             |  2 +-
>  11 files changed, 163 insertions(+), 66 deletions(-)
> 
> diff --git a/Makefile.main b/Makefile.main
> index 2d60bbbf5..1d834e81b 100644
> --- a/Makefile.main
> +++ b/Makefile.main
> @@ -65,21 +65,35 @@ CPPFLAGS += -I$(SRC)/libfdt -I$(SRC)/libflash -I$(SRC)/libxz -I$(SRC)/libc/inclu
>  CPPFLAGS += -I$(SRC)/libpore
>  CPPFLAGS += -D__SKIBOOT__ -nostdinc
>  CPPFLAGS += -isystem $(shell $(CC) -print-file-name=include)
> -CPPFLAGS += -DBITS_PER_LONG=64 -DHAVE_BIG_ENDIAN
> +CPPFLAGS += -DBITS_PER_LONG=64
> +
>  # We might want to remove our copy of stdint.h
>  # but that means uint64_t becomes an ulong instead of an ullong
>  # causing all our printf's to warn
>  CPPFLAGS += -ffreestanding
>  
> +ifeq ($(LITTLE_ENDIAN),1)
> +CPPFLAGS += -DHAVE_LITTLE_ENDIAN
> +else
> +CPPFLAGS += -DHAVE_BIG_ENDIAN
> +endif
> +
>  ifeq ($(DEBUG),1)
>  CPPFLAGS += -DDEBUG -DCCAN_LIST_DEBUG
>  endif
>  
> -CFLAGS := -fno-strict-aliasing -pie -fpie -fno-pic -mbig-endian -m64 -fno-asynchronous-unwind-tables
> +CFLAGS := -fno-strict-aliasing -pie -fpie -fno-pic -m64 -fno-asynchronous-unwind-tables
>  CFLAGS += -mcpu=power8
>  CFLAGS += -Wl,--oformat,elf64-powerpc -ggdb
>  CFLAGS += $(call try-cflag,$(CC),-ffixed-r13)
>  CFLAGS += $(call try-cflag,$(CC),-std=gnu11)
> +
> +ifeq ($(LITTLE_ENDIAN),1)
> +CFLAGS += -mlittle-endian
> +else
> +CFLAGS += -mbig-endian
> +endif
> +
>  ifeq ($(ELF_ABI_v2),1)
>  CFLAGS += $(call try-cflag,$(CC),-mabi=elfv2)
>  else
> @@ -135,8 +149,8 @@ LDFLAGS := -m64 -static -nostdlib -pie
>  LDFLAGS += -Wl,-pie
>  LDFLAGS += -Wl,-Ttext-segment,$(LD_TEXT) -Wl,-N -Wl,--build-id=none
>  LDFLAGS += -Wl,--no-multi-toc
> -LDFLAGS += -mcpu=power8 -mbig-endian -Wl,--oformat,elf64-powerpc
> -LDFLAGS_FINAL = -EB -m elf64ppc --no-multi-toc -N --build-id=none --whole-archive
> +LDFLAGS += -mcpu=power8 -Wl,--oformat,elf64-powerpc
> +LDFLAGS_FINAL = -m elf64lppc --no-multi-toc -N --build-id=none --whole-archive
>  LDFLAGS_FINAL += -static -nostdlib -pie -Ttext-segment=$(LD_TEXT) --oformat=elf64-powerpc
>  LDFLAGS_FINAL += --orphan-handling=warn
>  
> @@ -144,11 +158,25 @@ LDRFLAGS=-melf64ppc
>  # Debug stuff
>  #LDFLAGS += -Wl,-v -Wl,-Map,foomap 
>  
> +ifeq ($(LITTLE_ENDIAN),1)
> +LDFLAGS += -mlittle-endian
> +LDFLAGS_FINAL += -EL
> +else
> +LDFLAGS += -mbig-endian
> +LDFLAGS_FINAL += -EB
> +endif
> +
>  ifeq ($(DEAD_CODE_ELIMINATION),1)
>  LDFLAGS += -Wl,--gc-sections
>  endif
>  
> -AFLAGS := -D__ASSEMBLY__ -mbig-endian -m64
> +AFLAGS := -D__ASSEMBLY__ -m64
> +ifeq ($(LITTLE_ENDIAN),1)
> +AFLAGS += -mlittle-endian
> +else
> +AFLAGS += -mbig-endian
> +endif
> +
>  ifeq ($(ELF_ABI_v2),1)
>  AFLAGS += $(call try-cflag,$(CC),-mabi=elfv2)
>  else
> diff --git a/asm/cvc_entry.S b/asm/cvc_entry.S
> index 1296e88fe..1561f7cf9 100644
> --- a/asm/cvc_entry.S
> +++ b/asm/cvc_entry.S
> @@ -1,28 +1,9 @@
>  # SPDX-License-Identifier: Apache-2.0
> -# IBM_PROLOG_BEGIN_TAG
> -# This is an automatically generated prolog.
> -#
> -# $Source: src/usr/secureboot/base/rom_entry.S $
> -#
> -# OpenPOWER HostBoot Project
> -#
> -# COPYRIGHT International Business Machines Corp. 2013,2016
> -#
> -# Licensed under the Apache License, Version 2.0 (the "License");
> -# you may not use this file except in compliance with the License.
> -# You may obtain a copy of the License at
> -#
> -#     http://www.apache.org/licenses/LICENSE-2.0
> -#
> -# Unless required by applicable law or agreed to in writing, software
> -# distributed under the License is distributed on an "AS IS" BASIS,
> -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> -# implied. See the License for the specific language governing
> -# permissions and limitations under the License.
> -#
> -# IBM_PROLOG_END_TAG
>  
> -#.include "kernel/ppcconsts.S"
> +# Derived from automatically generated HostBoot rom_entry.S
> +
> +#include <asm-utils.h>
> +#include <processor.h>
>  
>  .section .text
>  
> @@ -44,7 +25,19 @@ call_rom_entry:
>      mr %r5, %r6
>      mr %r6, %r7
>      mr %r7, %r8
> +#if HAVE_BIG_ENDIAN
>      bctrl
> +#else
> +    bl $+4
> +1:  mflr %r9
> +    addi %r9,%r9,2f - 1b
> +    mtspr SPR_HSRR0, %r9
> +    LOAD_IMM64(%r9, (MSR_HV | MSR_SF))
> +    mtspr SPR_HSRR1, %r9
> +    hrfid
> +2:  .long 0x2104804e /* bctrl */
> +    FIXUP_ENDIAN
> +#endif
>      addi %r1, %r1, 128
>      ld %r2, 40(%r1)
>      ld %r0, 16(%r1)
> diff --git a/asm/head.S b/asm/head.S
> index 54199be85..71337cd22 100644
> --- a/asm/head.S
> +++ b/asm/head.S
> @@ -43,8 +43,9 @@ __head:
>  	. = 0x10
>  .global fdt_entry
>  fdt_entry:
> +	FIXUP_ENDIAN
>  	mr	%r27,%r3
> -	b	boot_entry
> +	b	boot_entry_fixed
>  
>  	/* This is a pointer to a descriptor used by debugging tools
>  	 * on the service processor to get to various trace buffers
> @@ -89,6 +90,7 @@ hir_trigger:
>  	. = 0x100
>  sreset_vector:
>  	/* BML entry, load up r3 with device tree location */
> +	FIXUP_ENDIAN
>  	li	%r3, 0
>  	oris	%r3, %r3, 0xa
>  	b	fdt_entry /* hack for lab boot */
> @@ -96,8 +98,9 @@ sreset_vector:
>  	/* Entry point set by the FSP */
>  	.= 0x180
>  hdat_entry:
> +	FIXUP_ENDIAN
>  	li	%r27,0
> -	b	boot_entry
> +	b	boot_entry_fixed
>  
>  	/* See naca.c */
>  	.= 0x1c0
> @@ -333,6 +336,8 @@ boot_offset:
>   */
>  .global boot_entry
>  boot_entry:
> +	FIXUP_ENDIAN
> +boot_entry_fixed:

Is there any point having boot_entry and boot_entry_fixed? IIRC
boot_entry is internal to skiboot so we would have to be in the correct
endian to get there in the first place.

>  	/* Check PVR and set some CR bits */
>  	mfspr	%r28,SPR_PVR
>  	li	%r26,3	/* Default to SMT4 */

> @@ -347,6 +352,7 @@ boot_entry:
>  	beq 	1f
>  	cmpwi	cr0,%r3,PVR_TYPE_P9P
>  	beq 	1f
> +	b	.
>  	attn		/* Unsupported CPU type... what do we do ? */
>  	b 	.	/* loop here, just in case attn is disabled */

Debug hack?


> @@ -369,7 +375,11 @@ boot_entry:
>  	add	%r2,%r2,%r29
>  
>  	/* Fixup our MSR (remove TA) */
> +#if HAVE_BIG_ENDIAN
>  	LOAD_IMM64(%r3, (MSR_HV | MSR_SF))
> +#else
> +	LOAD_IMM64(%r3, (MSR_HV | MSR_SF | MSR_LE))
> +#endif
>  	mtmsrd	%r3,0
>  
>  	/* Check our PIR, avoid threads */
> @@ -703,14 +713,18 @@ init_shared_sprs:
>  	mtspr	SPR_TSCR, %r3
>  
>  	/* HID0: Clear bit 13 (enable core recovery)
> -	 *       Clear bit 19 (HILE)
> +	 *       Set/clear bit 19 (HILE) depending on skiboot endian
>  	 */
>  	mfspr	%r3,SPR_HID0
>  	li	%r0,1
>  	sldi	%r4,%r0,(63-13)
> -	sldi	%r5,%r0,(63-19)
> -	or	%r0,%r4,%r5
> -	andc	%r3,%r3,%r0
> +	andc	%r3,%r3,%r4
> +	sldi	%r4,%r0,(63-19)
> +#if HAVE_BIG_ENDIAN
> +	andc	%r3,%r3,%r4
> +#else
> +	or	%r3,%r3,%r4
> +#endif
>  	sync
>  	mtspr	SPR_HID0,%r3
>  	mfspr	%r3,SPR_HID0
> @@ -737,17 +751,21 @@ init_shared_sprs:
>  	LOAD_IMM32(%r3,0x80287880)
>  	mtspr	SPR_TSCR, %r3
>  	/* HID0: Clear bit 5 (enable core recovery)
> -	 *       Clear bit 4 (HILE)
> +	 *       Set/clear bit 4 (HILE) depending on skiboot endian
>  	 *       Set bit 8 (radix)
>  	 */
>  	mfspr	%r3,SPR_HID0
>  	li	%r0,1
> -	sldi	%r4,%r0,(63-8)
> +	sldi	%r4,%r0,(63-4)
> +#if HAVE_BIG_ENDIAN
> +	andc	%r3,%r3,%r4
> +#else
>  	or	%r3,%r3,%r4
> +#endif
>  	sldi	%r4,%r0,(63-5)
> -	sldi	%r5,%r0,(63-4)
> -	or	%r0,%r4,%r5
> -	andc	%r3,%r3,%r0
> +	andc	%r3,%r3,%r4
> +	sldi	%r4,%r0,(63-8)
> +	or	%r3,%r3,%r4
>  	sync
>  	mtspr	SPR_HID0,%r3
>  	isync
> @@ -819,8 +837,9 @@ enter_nap:
>  	 * got to the same entry we use for pHyp and FDT HB.
>  	 */
>  opal_boot_trampoline:
> +	FIXUP_ENDIAN
>  	li	%r27,-1
> -	ba	boot_entry - __head
> +	ba	boot_entry_fixed - __head
>  
>  /*
>   *
> @@ -837,6 +856,8 @@ opal_boot_trampoline:
>  	.balign	0x10
>  .global opal_entry
>  opal_entry:
> +	OPAL_ENTRY_TO_SKIBOOT_ENDIAN
> +
>  	/* Get our per CPU pointer in r12 to check for quiesce */
>  	mfspr	%r12,SPR_PIR
>  	GET_STACK(%r12,%r12)
> @@ -982,20 +1003,33 @@ opal_entry:
>  	lwz	%r11,CPUTHREAD_IN_OPAL_CALL(%r12)
>  	subi	%r11,%r11,1
>  	stw	%r11,CPUTHREAD_IN_OPAL_CALL(%r12)
> +#if HAVE_BIG_ENDIAN
>  	/*
>  	 * blr with BH=01b means it's not a function return, OPAL was entered
>  	 * via (h)rfid not bl, so we don't have a corresponding link stack
>  	 * prediction to return to here.
>  	 */
>  	bclr	20,0,1
> +#else
> +	mflr	%r12
> +	mtspr	SPR_HSRR0,%r12
> +	mfmsr	%r11
> +	li	%r12,MSR_LE
> +	andc	%r11,%r11,%r12
> +	mtspr	SPR_HSRR1,%r11
> +	hrfid
> +#endif
>  
>  .global start_kernel
>  start_kernel:
> +	LOAD_IMM64(%r10,MSR_HV|MSR_SF)
> +__start_kernel:
>  	sync
>  	icbi	0,%r3
>  	sync
>  	isync
> -	mtctr	%r3
> +	mtspr	SPR_HSRR0,%r3
> +	mtspr	SPR_HSRR1,%r10
>  	mr	%r3,%r4
>  	LOAD_IMM64(%r8,SKIBOOT_BASE);
>  	LOAD_IMM32(%r10, opal_entry - __head)
> @@ -1004,21 +1038,19 @@ start_kernel:
>  	addi	%r7,%r5,1
>  	li	%r4,0
>  	li	%r5,0
> -	bctr
> +	hrfid
>  
>  	.global start_kernel32
>  start_kernel32:
> -	mfmsr	%r10
> -	clrldi	%r10,%r10,1
> -	mtmsrd	%r10,0
> -	sync
> -	isync
> -	b	start_kernel
> +	LOAD_IMM64(%r10,MSR_HV)
> +	b	__start_kernel
>  
>  .global start_kernel_secondary
>  start_kernel_secondary:
>  	sync
>  	isync
> -	mtctr	%r3
> +	LOAD_IMM64(%r10,MSR_HV|MSR_SF)
> +	mtspr	SPR_HSRR0,%r3
> +	mtspr	SPR_HSRR1,%r10
>  	mfspr	%r3,SPR_PIR
> -	bctr
> +	hrfid
> diff --git a/core/cpu.c b/core/cpu.c
> index b3433aef5..3d6a59033 100644
> --- a/core/cpu.c
> +++ b/core/cpu.c
> @@ -42,7 +42,7 @@ static unsigned long hid0_attn;
>  static bool sreset_enabled;
>  static bool ipi_enabled;
>  static bool pm_enabled;
> -static bool current_hile_mode;
> +static bool current_hile_mode = HAVE_LITTLE_ENDIAN;
>  static bool current_radix_mode = true;
>  static bool tm_suspend_enabled;
>  
> @@ -1415,6 +1415,24 @@ static int64_t cpu_change_all_hid0(struct hid0_change_req *req)
>  	return OPAL_SUCCESS;
>  }
>  
> +void cpu_set_hile_mode(bool hile)
> +{
> +	struct hid0_change_req req;
> +
> +	if (hile == current_hile_mode)
> +		return;
> +
> +	if (hile) {
> +		req.clr_bits = 0;
> +		req.set_bits = hid0_hile;
> +	} else {
> +		req.clr_bits = hid0_hile;
> +		req.set_bits = 0;
> +	}
> +	cpu_change_all_hid0(&req);
> +	current_hile_mode = hile;
> +}
> +
>  static void cpu_cleanup_one(void *param __unused)
>  {
>  	mtspr(SPR_AMR, 0);
> @@ -1453,8 +1471,8 @@ static int64_t cpu_cleanup_all(void)
>  
>  void cpu_fast_reboot_complete(void)
>  {
> -	/* Fast reboot will have cleared HID0:HILE */
> -	current_hile_mode = false;
> +	/* Fast reboot will have set HID0:HILE to skiboot endian */
> +	current_hile_mode = HAVE_LITTLE_ENDIAN;
>  
>  	/* and set HID0:RADIX */
>  	current_radix_mode = true;
> diff --git a/core/init.c b/core/init.c
> index 62d9c709f..7fa8bb9c2 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -620,6 +620,7 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>  	cpu_disable_ME_RI_all();
>  
>  	patch_traps(false);
> +	cpu_set_hile_mode(false); /* Clear HILE on all CPUs */
>  
>  	debug_descriptor.state_flags |= OPAL_BOOT_COMPLETE;
>  
> diff --git a/hdata/spira.c b/hdata/spira.c
> index 8dd05e923..f50ba1e47 100644
> --- a/hdata/spira.c
> +++ b/hdata/spira.c
> @@ -34,7 +34,7 @@ __section(".procin.data") struct proc_init_data proc_init_data = {
>  	.regs_ptr = HDIF_IDATA_PTR(offsetof(struct proc_init_data, regs), 0x10),
>  	.regs = {
>  		.nia = CPU_TO_BE64(0x180),
> -		.msr = CPU_TO_BE64(0x9000000000000000ULL), /* SF | HV */
> +		.msr = CPU_TO_BE64(MSR_SF | MSR_HV),
>  	},
>  };
>  
> diff --git a/include/asm-utils.h b/include/asm-utils.h
> index 2d26545e7..e3b970c28 100644
> --- a/include/asm-utils.h
> +++ b/include/asm-utils.h
> @@ -28,6 +28,23 @@
>  /* Load an address via the TOC */
>  #define LOAD_ADDR_FROM_TOC(r, e)	ld r,e at got(%r2)
>  
> +#if HAVE_BIG_ENDIAN
> +#define OPAL_ENTRY_TO_SKIBOOT_ENDIAN
> +#else
> +/* This must preserve LR, so can't use FIXUP_ENDIAN */
> +#define OPAL_ENTRY_TO_SKIBOOT_ENDIAN				   \
> +	.long 0xa600607d; /* mfmsr r11				*/ \
> +	.long 0x01006b69; /* xori r11,r11,1			*/ \
> +	.long 0xa64b7b7d; /* mthsrr1 r11			*/ \
> +	.long 0xa602687d; /* mflr r11				*/ \
> +	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
> +	.long 0xa602487d; /* mflr r10				*/ \
> +	.long 0x14004a39; /* addi r10,r10,20			*/ \
> +	.long 0xa64b5a7d; /* mthsrr0 r10			*/ \
> +	.long 0xa603687d; /* mtlr r11				*/ \
> +	.long 0x2402004c  /* hrfid				*/
> +#endif

Any reason we can't just use this for FIXUP_ENDIAN instead of having
two slightly different versions?

> +
>  #define FIXUP_ENDIAN						   \
>  	tdi   0,0,0x48;	  /* Reverse endian of b . + 8		*/ \
>  	b     191f;	  /* Skip trampoline if endian is good	*/ \
> diff --git a/include/cpu.h b/include/cpu.h
> index cda78644d..008f08a68 100644
> --- a/include/cpu.h
> +++ b/include/cpu.h
> @@ -282,6 +282,9 @@ extern void cpu_process_local_jobs(void);
>  /* Check if there's any job pending */
>  bool cpu_check_jobs(struct cpu_thread *cpu);
>  
> +/* Set/clear HILE on all CPUs */
> +void cpu_set_hile_mode(bool hile);
> +
>  /* OPAL sreset vector in place at 0x100 */
>  void cpu_set_sreset_enable(bool sreset_enabled);
>  
> diff --git a/include/elf.h b/include/elf.h
> index 93524bb99..8ce37fad4 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -9,7 +9,11 @@
>  /* Generic ELF header */
>  struct elf_hdr {
>  	uint32_t ei_ident;
> +#ifdef _BIG_ENDIAN
>  #define ELF_IDENT	0x7F454C46
> +#else
> +#define ELF_IDENT	0x464C457F
> +#endif
>  	uint8_t ei_class;
>  #define ELF_CLASS_32	1
>  #define ELF_CLASS_64	2

> diff --git a/libpore/p9_cpu_reg_restore_instruction.H b/libpore/p9_cpu_reg_restore_instruction.H
> index dd4358a82..cf00ff5e5 100644
> --- a/libpore/p9_cpu_reg_restore_instruction.H
> +++ b/libpore/p9_cpu_reg_restore_instruction.H
> @@ -61,23 +61,24 @@ enum
>      RLDICR_CONST        =   1,
>      MTSPR_CONST1        =   467,
>      MTMSRD_CONST1       =   178,
> -    MR_R0_TO_R10        =   0x7c0a0378, //mr r10, r0
> -    MR_R0_TO_R21        =   0x7c150378, //mr r21, r0
> -    MR_R0_TO_R9         =   0x7c090378, //mr r9, r0
> -    URMOR_CORRECTION    =   0x7d397ba6,
>      MFSPR_CONST         =   339,
> -    BLR_INST            =   0x4e800020,
> -    MTSPR_BASE_OPCODE   =   0x7c0003a6,
> -    ATTN_OPCODE         =   0x00000200,
>      OPCODE_18           =   18,
>      SELF_SAVE_FUNC_ADD  =   0x2300,
>      SELF_SAVE_OFFSET    =   0x180,
> -    SKIP_SPR_REST_INST  =   0x4800001c, //b . +0x01c
> -    MFLR_R30            =   0x7fc802a6,
> -    SKIP_SPR_SELF_SAVE  =   0x3bff0020, //addi r31 r31, 0x20
> -    MTLR_INST           =   0x7fc803a6  //mtlr r30
>  };
>  
> +#define MR_R0_TO_R10            0x7c0a0378UL //mr r10 r0
> +#define MR_R0_TO_R21            0x7c150378UL //mr r21 r0
> +#define MR_R0_TO_R9             0x7c090378UL //mr r9 r0
> +#define URMOR_CORRECTION        0x7d397ba6UL
> +#define BLR_INST                0x4e800020UL
> +#define MTSPR_BASE_OPCODE       0x7c0003a6UL
> +#define ATTN_OPCODE             0x00000200UL
> +#define SKIP_SPR_REST_INST      0x4800001cUL //b . +0x01c
> +#define MFLR_R30                0x7fc802a6UL
> +#define SKIP_SPR_SELF_SAVE      0x3bff0020UL //addi r31 r31 0x20
> +#define MTLR_INST               0x7fc803a6UL //mtlr r30
> +
>  #ifdef __cplusplus
>  } // namespace stopImageSection ends

I thought we were trying to keep this in sync with hostboot since
that's where we got it in the first place? That said, it probably
doesn't matter since I doubt we'll see any real changes to it at this
point in the P9 cycle.


> diff --git a/libstb/cvc.c b/libstb/cvc.c
> index dca4ac857..fd0b59bb8 100644
> --- a/libstb/cvc.c
> +++ b/libstb/cvc.c
> @@ -343,7 +343,7 @@ int call_cvc_verify(void *container, size_t len, const void *hw_key_hash,
>  		return OPAL_UNSUPPORTED;
>  
>  	if (log)
> -		*log = hw_params.log;
> +		*log = be64_to_cpu(hw_params.log);
>  
>  	if (rc != ROM_DONE)
>  		return OPAL_PARTIAL;



More information about the Skiboot mailing list