linux-kernel - Re: [PATCH v3 3/4] perf annotate: add powerpc support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1467377311.12509.3.camel@gmail.com>
Date:	Fri, 01 Jul 2016 22:48:31 +1000
From:	Balbir Singh <bsingharora@...il.com>
To:	Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	linux-kernel@...r.kernel.org, acme@...nel.org,
	linuxppc-dev@...ts.ozlabs.org
Cc:	ananth@...ibm.com, David.Laight@...LAB.COM,
	naveen.n.rao@...ux.vnet.ibm.com, dja@...ens.net
Subject: Re: [PATCH v3 3/4] perf annotate: add powerpc support

On Fri, 2016-07-01 at 14:13 +0530, Ravi Bangoria wrote:
> Thanks Michael for your suggestion.
> 
> On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote:
> > 
> > On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote:
> > > 
> > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > index 36a5825..b87eac7 100644
> > > --- a/tools/perf/util/annotate.c
> > > +++ b/tools/perf/util/annotate.c
> > > @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b)
> > ...
> > > 
> > > +
> > > +static struct ins *ins__find_powerpc(const char *name)
> > > +{
> > > +	int i;
> > > +	struct ins *ins;
> > > +	struct ins_ops *ops;
> > > +	static struct instructions_powerpc head;
> > > +	static bool list_initialized;
> > > +
> > > +	/*
> > > +	 * - Interested only if instruction starts with 'b'.
> > > +	 * - Few start with 'b', but aren't branch instructions.
> > > +	 * - Let's also ignore instructions involving 'ctr' and
> > > +	 *   'tar' since target branch addresses for those can't
> > > +	 *   be determined statically.
> > > +	 */
> > > +	if (name[0] != 'b'             ||
> > > +	    !strncmp(name, "bcd", 3)   ||
> > > +	    !strncmp(name, "brinc", 5) ||
> > > +	    !strncmp(name, "bper", 4)  ||
> > > +	    strstr(name, "ctr")        ||
> > > +	    strstr(name, "tar"))
> > > +		return NULL;
> > It would be good if 'bctr' was at least recognised as a branch, even if we
> > can't determine the target. They are very common.
> We can not show arrow for this since we don't know the target location.
> can you please suggest how you intends perf to display bctr?
> 
> bctr can be classified into two variants -- 'bctr' and 'bctrl'.
> 
> 'bctr' will be considered as jump instruction but jump__parse() won't
> be able to find any target location and hence it will set target to
> UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This
> looks misleading.
> 
> bctrl will be considered as call instruction but call_parse() won't
> be able to find any target function and hence it won't show any
> navigation arrow for this instruction. Which is same as filter it
> beforehand.
>

The target location and function are in the counter. Can't we add
this to instruction ops? Is it a major change to add it?
 
Balbir Singh.