[Skiboot] [RFC PATCH 1/2] move the __this_cpu register to r16, reserve r13-r15

Nicholas Piggin npiggin at gmail.com
Mon Dec 2 18:43:15 AEDT 2019


There have been several bugs between Linux and OPAL caused by both
using r13 for their primary per-CPU data address. This patch moves
OPAL to use r16 for this, and prevents the compiler from touching
r13-r15. This helps things to be a little more robust, and also makes
crashes in OPAL easier to debug.

Later, if we allow interrupts (other than non-maskable) to be taken when
running in skiboot, Linux's interrupt return handler does not restore
r13 if the interrupt was taken in PR=0 state, which would corrupt the
skiboot r13 register.

Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
---
 Makefile.main |  7 +++++--
 asm/head.S    | 28 ++++++++++++++--------------
 asm/misc.S    |  8 ++++----
 include/cpu.h |  2 +-
 4 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/Makefile.main b/Makefile.main
index e26a9ab17..527cd8ae9 100644
--- a/Makefile.main
+++ b/Makefile.main
@@ -87,7 +87,10 @@ endif
 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 += -ffixed-r13
+CFLAGS += -ffixed-r14
+CFLAGS += -ffixed-r15
+CFLAGS += -ffixed-r16
 CFLAGS += $(call try-cflag,$(CC),-std=gnu11)
 
 ifeq ($(LITTLE_ENDIAN),1)
@@ -118,7 +121,7 @@ endif
 
 # Check if the new parametrized stack protector option is supported
 # by gcc, otherwise disable stack protector
-STACK_PROT_CFLAGS := -mstack-protector-guard=tls -mstack-protector-guard-reg=r13
+STACK_PROT_CFLAGS := -mstack-protector-guard=tls -mstack-protector-guard-reg=r16
 STACK_PROT_CFLAGS += -mstack-protector-guard-offset=0
 HAS_STACK_PROT := $(call test_cflag,$(CC),$(STACK_PROT_CFLAGS))
 
diff --git a/asm/head.S b/asm/head.S
index 0b4b1a5f0..825f4e74a 100644
--- a/asm/head.S
+++ b/asm/head.S
@@ -25,7 +25,7 @@
 	addi	stack_reg,stack_reg,EMERGENCY_CPU_STACKS_OFFSET at l;
 
 #define GET_CPU()							\
-	clrrdi	%r13,%r1,STACK_SHIFT
+	clrrdi	%r16,%r1,STACK_SHIFT
 
 #define SAVE_GPR(reg,sp)	std %r##reg,STACK_GPR##reg(sp)
 #define REST_GPR(reg,sp)	ld %r##reg,STACK_GPR##reg(sp)
@@ -464,18 +464,18 @@ boot_entry:
 	addi	%r3,%r3,8
 	bdnz	1b
 
-	/* Get our per-cpu pointer into r13 */
+	/* Get our per-cpu pointer into r16 */
 	GET_CPU()
 
 #ifdef STACK_CHECK_ENABLED
 	/* Initialize stack bottom mark to 0, it will be updated in C code */
 	li	%r0,0
-	std	%r0,CPUTHREAD_STACK_BOT_MARK(%r13)
+	std	%r0,CPUTHREAD_STACK_BOT_MARK(%r16)
 #endif
 	/* Initialize the stack guard */
 	LOAD_IMM64(%r3,STACK_CHECK_GUARD_BASE);
 	xor	%r3,%r3,%r31
-	std	%r3,0(%r13)
+	std	%r3,0(%r16)
 
 	/* Jump to C */
 	mr	%r3,%r27
@@ -592,12 +592,12 @@ reset_wakeup:
 	/* Get PIR */
 	mfspr	%r31,SPR_PIR
 
-	/* Get that CPU stack base and use it to restore r13 */
+	/* Get that CPU stack base and use it to restore r16 */
 	GET_STACK(%r1,%r31)
 	GET_CPU()
 
 	/* Restore original stack pointer */
-	ld	%r1,CPUTHREAD_SAVE_R1(%r13)
+	ld	%r1,CPUTHREAD_SAVE_R1(%r16)
 
 	/* Restore more stuff */
 	lwz	%r4,STACK_CR(%r1)
@@ -655,7 +655,7 @@ reset_fast_reboot_wakeup:
 	/* Get PIR */
 	mfspr	%r31,SPR_PIR
 
-	/* Get that CPU stack base and use it to restore r13 */
+	/* Get that CPU stack base and use it to restore r16 */
 	GET_STACK(%r1,%r31)
 	GET_CPU()
 
@@ -923,17 +923,17 @@ opal_entry:
 	std	%r9,STACK_GPR9(%r1)
 	std	%r10,STACK_GPR10(%r1)
 
-	/* Save Token (r0), LR and r13 */
+	/* Save Token (r0), LR and r16 */
 	mflr	%r12
 	std	%r0,STACK_GPR0(%r1)
-	std	%r13,STACK_GPR13(%r1)
+	std	%r16,STACK_GPR16(%r1)
 	std	%r12,STACK_LR(%r1)
 
 	/* Get the CPU thread */
 	GET_CPU()
 
 	/* Store token in CPU thread */
-	std	%r0,CPUTHREAD_CUR_TOKEN(%r13)
+	std	%r0,CPUTHREAD_CUR_TOKEN(%r16)
 
 	/* Mark the stack frame */
 	li	%r12,STACK_ENTRY_OPAL_API
@@ -975,14 +975,14 @@ opal_entry:
 	bl	opal_exit_check /* r3 is preserved */
 
 	/*
-	 * Restore r1 and r13 before decrementing in_opal_call.
-	 * Move per-cpu pointer to volatile r12, restore lr, r1, r13.
+	 * Restore r1 and r16 before decrementing in_opal_call.
+	 * Move per-cpu pointer to volatile r12, restore lr, r1, r16.
 	 */
 .Lreturn:
 	ld	%r12,STACK_LR(%r1)
 	mtlr	%r12
-	mr	%r12,%r13
-	ld	%r13,STACK_GPR13(%r1)
+	mr	%r12,%r16
+	ld	%r16,STACK_GPR16(%r1)
 	ld	%r1,STACK_GPR1(%r1)
 .Lreject:
 	sync 	/* release barrier vs quiescing */
diff --git a/asm/misc.S b/asm/misc.S
index 647f60b26..9904b806f 100644
--- a/asm/misc.S
+++ b/asm/misc.S
@@ -213,7 +213,7 @@ enter_p8_pm_state:
 	bl	pm_save_regs
 
 	/* Save stack pointer in struct cpu_thread */
-	std	%r1,CPUTHREAD_SAVE_R1(%r13)
+	std	%r1,CPUTHREAD_SAVE_R1(%r16)
 
 	/* Winkle or nap ? */
 	cmpli	%cr0,0,%r3,0
@@ -221,7 +221,7 @@ enter_p8_pm_state:
 
 	/* nap sequence */
 	ptesync
-0:	ld	%r0,CPUTHREAD_SAVE_R1(%r13)
+0:	ld	%r0,CPUTHREAD_SAVE_R1(%r16)
 	cmpd	cr0,%r0,%r0
 	bne	0b
 	PPC_INST_NAP
@@ -229,7 +229,7 @@ enter_p8_pm_state:
 
 	/* rvwinkle sequence */
 1:	ptesync
-0:	ld	%r0,CPUTHREAD_SAVE_R1(%r13)
+0:	ld	%r0,CPUTHREAD_SAVE_R1(%r16)
 	cmpd	cr0,%r0,%r0
 	bne	0b
 	PPC_INST_RVWINKLE
@@ -250,7 +250,7 @@ enter_p9_pm_state:
 	bl	pm_save_regs
 
 	/* Save stack pointer in struct cpu_thread */
-	std	%r1,CPUTHREAD_SAVE_R1(%r13)
+	std	%r1,CPUTHREAD_SAVE_R1(%r16)
 
 	mtspr	SPR_PSSCR,%r3
 	PPC_INST_STOP
diff --git a/include/cpu.h b/include/cpu.h
index 686310d71..9b7f41dfb 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -212,7 +212,7 @@ extern u8 get_available_nr_cores_in_chip(u32 chip_id);
 		core = next_available_core_in_chip(core, chip_id))
 
 /* Return the caller CPU (only after init_cpu_threads) */
-register struct cpu_thread *__this_cpu asm("r13");
+register struct cpu_thread *__this_cpu asm("r16");
 static inline __nomcount struct cpu_thread *this_cpu(void)
 {
 	return __this_cpu;
-- 
2.23.0



More information about the Skiboot mailing list