Re: [PATCH v4 2/3] powerpc/modules: Don't try to restore r2 after a sibling call

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Josh Poimboeuf wrote:
On Wed, Nov 15, 2017 at 02:58:33PM +0530, Naveen N. Rao wrote:
> +int instr_is_link_branch(unsigned int instr)
> +{
> +	return (instr_is_branch_iform(instr) || instr_is_branch_bform(instr)) &&
> +	       (instr & BRANCH_SET_LINK);
> +}
> +

Nitpicking here, but since we're not considering the other branch forms,
perhaps this can be renamed to instr_is_link_relative_branch() (or maybe
instr_is_relative_branch_link()), just so we're clear :)

My understanding is that the absolute/relative bit isn't a "form", but
rather a bit that can be set for either the b-form (conditional) or the
i-form (unconditional).  And the above function isn't checking the
absolute bit, so it isn't necessarily a relative branch.  Or did I miss
something?

Ah, good point. I was coming from the fact that we are only considering the i-form and b-form branches and not the lr/ctr/tar based branches, which are always absolute branches, but can also set the link register.

Thinking about this more, aren't we only interested in relative branches
here (for relocations), so can we actually filter out the absolute branches? Something like this?

int instr_is_relative_branch_link(unsigned int instr)
{
	return ((instr_is_branch_iform(instr) || instr_is_branch_bform(instr)) &&
	       !(instr & BRANCH_ABSOLUTE) && (instr & BRANCH_SET_LINK));
}


- Naveen


--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux