From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id F30F71A00BC for ; Tue, 8 Mar 2016 21:46:02 +1100 (AEDT) Received: from newverein.lst.de (verein.lst.de [213.95.11.211]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8B210140BA7 for ; Tue, 8 Mar 2016 21:46:01 +1100 (AEDT) Date: Tue, 8 Mar 2016 11:45:52 +0100 From: Torsten Duwe To: Balbir Singh Cc: linuxppc-dev@ozlabs.org, pmladek@suse.com, jeyu@redhat.com, jkosina@suse.cz, jikos@kernel.org, linux-kernel@vger.kernel.org, rostedt@goodmis.org, kamalesh@linux.vnet.ibm.com, live-patching@vger.kernel.org, mbenes@suse.cz Subject: Re: [v5][PATCH] livepatch/ppc: Enable livepatching on powerpc Message-ID: <20160308104552.GA16502@lst.de> References: <1457422437-3357-1-git-send-email-bsingharora@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1457422437-3357-1-git-send-email-bsingharora@gmail.com> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, Mar 08, 2016 at 06:33:57PM +1100, Balbir Singh wrote: > Changelog v5: > 1. Removed the mini-stack frame created for klp_return_helper. > As a result of the mini-stack frame, function with > 8 > arguments could not be patched Did you get my previous mails? Those functions only require special care, the _can_ be patched. In general, writing replacement functions always requires attention! Have you *tested* this patch? Replacing a function in the kernel? Replacing a function in a module? For local calls? For global calls? I strongly doubt so because it does not work this way. To be fair, my last mail still was not 100% correct, but the conclusion that the mini frame is not needed at all is invalid. Please leave it as it was, I'm working on a test / demonstrator for how to handle these. > + * Why do we need this? > + * After patching we need to return to a trampoline return function > + * that guarantees that we restore the TOC and return to the correct > + * caller back > + */ > + std r2, 24(r1) /* save TOC now, unconditionally. */ > + subf r0, r2, r0 /* Calculate offset from current TOC */ > + stw r0, 12(r1) /* Of the final LR and save it in CR+4 */ > + bl 5f > +5: mflr r12 > + addi r12, r12, (klp_return_helper + 4 - .)@l > + std r12, LRSAVE(r1) [...] > + * maybe inserting a klp_return_helper frame or not. > +*/ > +klp_return_helper: > + ld r2, 24(r1) /* restore TOC (saved by ftrace_caller) */ > + lwa r0, 12(r1) /* Load from CR+4, offset of LR w.r.t TOC */ > + add r0, r0, r2 /* Add the offset to current TOC */ > + std r0, LRSAVE(r1) /* save the real return address */ > + mtlr r0 > + blr > +#endif NAKed-by: Torsten Duwe Torsten