Re: [PATCH 05/14] powerpc: Add new code patching routines

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

On Tue, 2008-06-24 at 08:48 -0500, Kumar Gala wrote:
> On Jun 23, 2008, at 8:32 PM, Michael Ellerman wrote:
> 
> > This commit adds some new routines for patching code, they will be  
> > used
> > in a following commit.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> > arch/powerpc/lib/code-patching.c    |  107 ++++++++++++++++++++++++++ 
> > +++++++++
> > include/asm-powerpc/code-patching.h |    8 +++
> > 2 files changed, 115 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/ 
> > code-patching.c
> > index 430f4c1..27957c4 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -41,3 +41,110 @@ unsigned int create_branch(const unsigned int  
> > *addr,
> >
> > 	return instruction;
> > }
> > +
> > +unsigned int create_cond_branch(const unsigned int *addr,
> > +				unsigned long target, int flags)
> > +{
> 
> it would be nice to have some idea what flags is suppose to be.

Yeah this routine is a bit of kludge, it's really only written for
translate_branch(). As it is, flags just takes any of the bits in a
conditional branch that aren't the opcode or target.

To fully synthesise a conditional branch by hand you'd probably want
another routine which constructs the BO & BI fields for you.

> > +unsigned int translate_branch(const unsigned int *dest, const  
> > unsigned int *src)
> > +{
> 
> I'm not sure I get what this function is trying to do.

It takes a relative branch at src and returns a new branch of the same
type that could be placed at dest, and would jump to the same target as
the original instruction. I guess I should add some doco :)

> > +	unsigned long target;
> > +
> > +	target = branch_target(src);
> > +
> > +	if (instr_is_branch_iform(*src))
> > +		return create_branch(dest, target, *src);
> > +	else if (instr_is_branch_bform(*src))
> > +		return create_cond_branch(dest, target, *src);
> > +
> > +	return 0;
> > +}

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]