Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755547AbcC2AAF (ORCPT ); Mon, 28 Mar 2016 20:00:05 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:32774 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751049AbcC2AAB (ORCPT ); Mon, 28 Mar 2016 20:00:01 -0400 Subject: Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info To: Michael Ellerman , linuxppc-dev@ozlabs.org References: <1458817445-5855-1-git-send-email-mpe@ellerman.id.au> <1458817445-5855-5-git-send-email-mpe@ellerman.id.au> Cc: duwe@lst.de, linux-kernel@vger.kernel.org, rostedt@goodmis.org, kamalesh@linux.vnet.ibm.com, pmladek@suse.com, jeyu@redhat.com, jkosina@suse.cz, live-patching@vger.kernel.org, mbenes@suse.cz From: Balbir Singh Message-ID: <56F9C531.3090703@gmail.com> Date: Tue, 29 Mar 2016 10:58:41 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <1458817445-5855-5-git-send-email-mpe@ellerman.id.au> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5798 Lines: 160 On 24/03/16 22:04, Michael Ellerman wrote: > In order to support live patching we need to maintain an alternate > stack of TOC & LR values. We use the base of the stack for this, and > store the "live patch stack pointer" in struct thread_info. > > Unlike the other fields of thread_info, we can not statically initialise > that value, so it must be done at run time. > > This patch just adds the code to support that, it is not enabled until > the next patch which actually adds live patch support. > > Signed-off-by: Michael Ellerman > --- > arch/powerpc/include/asm/livepatch.h | 8 ++++++++ > arch/powerpc/include/asm/thread_info.h | 4 +++- > arch/powerpc/kernel/irq.c | 3 +++ > arch/powerpc/kernel/process.c | 6 +++++- > arch/powerpc/kernel/setup_64.c | 17 ++++++++++------- > 5 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h > index ad36e8e34fa1..a402f7f94896 100644 > --- a/arch/powerpc/include/asm/livepatch.h > +++ b/arch/powerpc/include/asm/livepatch.h > @@ -49,6 +49,14 @@ static inline unsigned long klp_get_ftrace_location(unsigned long faddr) > */ > return ftrace_location_range(faddr, faddr + 16); > } > + > +static inline void klp_init_thread_info(struct thread_info *ti) > +{ > + /* + 1 to account for STACK_END_MAGIC */ > + ti->livepatch_sp = (unsigned long *)(ti + 1) + 1; > +} > +#else > +static void klp_init_thread_info(struct thread_info *ti) { } > #endif /* CONFIG_LIVEPATCH */ > > #endif /* _ASM_POWERPC_LIVEPATCH_H */ > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h > index 7efee4a3240b..8febc3f66d53 100644 > --- a/arch/powerpc/include/asm/thread_info.h > +++ b/arch/powerpc/include/asm/thread_info.h > @@ -43,7 +43,9 @@ struct thread_info { > int preempt_count; /* 0 => preemptable, > <0 => BUG */ > unsigned long local_flags; /* private flags for thread */ > - > +#ifdef CONFIG_LIVEPATCH > + unsigned long *livepatch_sp; > +#endif > /* low level flags - has atomic operations done on it */ > unsigned long flags ____cacheline_aligned_in_smp; > }; > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index 290559df1e8b..3cb46a3b1de7 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -66,6 +66,7 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_PPC64 > #include > @@ -607,10 +608,12 @@ void irq_ctx_init(void) > memset((void *)softirq_ctx[i], 0, THREAD_SIZE); > tp = softirq_ctx[i]; > tp->cpu = i; > + klp_init_thread_info(tp); At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx? > > memset((void *)hardirq_ctx[i], 0, THREAD_SIZE); > tp = hardirq_ctx[i]; > tp->cpu = i; > + klp_init_thread_info(tp); Same question here > } > } > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index 612df305886b..c27215e6ef48 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -55,6 +55,8 @@ > #include > #endif > #include > +#include > + > #include > #include > > @@ -1400,13 +1402,15 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, > extern void ret_from_kernel_thread(void); > void (*f)(void); > unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE; > + struct thread_info *ti = task_thread_info(p); > + > + klp_init_thread_info(ti); > > /* Copy registers */ > sp -= sizeof(struct pt_regs); > childregs = (struct pt_regs *) sp; > if (unlikely(p->flags & PF_KTHREAD)) { > /* kernel thread */ > - struct thread_info *ti = (void *)task_stack_page(p); > memset(childregs, 0, sizeof(struct pt_regs)); > childregs->gpr[1] = sp + sizeof(struct pt_regs); > /* function */ > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index 3807fb05b6de..1b6cabb8e715 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -69,6 +69,7 @@ > #include > #include > #include > +#include > > #ifdef DEBUG > #define DBG(fmt...) udbg_printf(fmt) > @@ -667,16 +668,16 @@ static void __init emergency_stack_init(void) > limit = min(safe_stack_limit(), ppc64_rma_size); > > for_each_possible_cpu(i) { > - unsigned long sp; > - sp = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit); > - sp += THREAD_SIZE; > - paca[i].emergency_sp = __va(sp); > + struct thread_info *ti; > + ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit)); > + klp_init_thread_info(ti); > + paca[i].emergency_sp = (void *)ti + THREAD_SIZE; > Does emergency_sp still end up 128 byte aligned after this? > #ifdef CONFIG_PPC_BOOK3S_64 > /* emergency stack for machine check exception handling. */ > - sp = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit); > - sp += THREAD_SIZE; > - paca[i].mc_emergency_sp = __va(sp); > + ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit)); > + klp_init_thread_info(ti); Do we care about live-patching in this context? Are we mixing per-thread and per-cpu contexts? > + paca[i].mc_emergency_sp = (void *)ti + THREAD_SIZE; > #endif > } > } > @@ -700,6 +701,8 @@ void __init setup_arch(char **cmdline_p) > if (ppc_md.panic) > setup_panic(); > > + klp_init_thread_info(&init_thread_info); > + > init_mm.start_code = (unsigned long)_stext; > init_mm.end_code = (unsigned long) _etext; > init_mm.end_data = (unsigned long) _edata;