Subject: Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

From: Balbir Singh <[email-masked]>
Date: Tue, 29 Mar 2016 16:45:59 +1100

>> At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx?
> Sorry I'm not sure what you mean.
>
> None of this relates to the current CPUs thread info.
Oh! I meant that klp_init_thread_info points to the end of (struct thread_info {} + 1) in the stack of the thread/task, but with the irq_contexts they are a separate array and not on stack
>>> 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 <asm/kvm_ppc.h>
>>>  #include <asm/hugetlb.h>
>>>  #include <asm/epapr_hcalls.h>
>>> +#include <asm/livepatch.h>
>>>  
>>>  #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?
> It should end up THREAD_SIZE aligned as before, due to the memblock_alloc_base().
Yep.. I missed it.. so where is the space for ti? The stack is going to go grow downwards from ti+THREAD_SIZE
>>>  #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?
> Well we probably don't want to be doing live patching when we're on the
> emergency stacks. But we have no control over whether that happens so we have
> to support it.
>

OK.. I was wondering if the code will even work.. I wonder if the ftrace data structures will work in real mode, including the hash/etc.

Balbir