linux-kernel - RE: [V3 02/10] powerpc, perf: Enable conditional branch filter for POWER8

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 26 Nov 2013 17:06:19 +1100 (EST)
From:	michael@...abs.org (mpe@...erman.id.au)
To:	linuxppc-dev@...abs.org, linux-kernel@...r.kernel.org
Cc:	mikey@...ling.org, sukadev@...ux.vnet.ibm.com,
	michaele@....ibm.com, eranian@...gle.com
Subject: RE: [V3 02/10] powerpc, perf: Enable conditional branch filter for POWER8

On Wed, 2013-16-10 at 06:56:49 UTC, Anshuman Khandual wrote:
> Enables conditional branch filter support for POWER8
> utilizing MMCRA register based filter and also invalidates
> a BHRB branch filter combination involving conditional
> branches.
> 
> Signed-off-by: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/power8-pmu.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index 2ee4a70..6e28587 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -580,11 +580,21 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
>  	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
>  		return -1;
>  
> +	/* Invalid branch filter combination - HW does not support */
> +	if ((branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) &&
> +			(branch_sample_type & PERF_SAMPLE_BRANCH_COND))
> +		return -1;

What this doesn't make obvious is that the hardware doesn't support any
combinations. It just happens that these are the only two possibilities we
allow, and so this is the only combination we have to disallow.

>
>  	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
>  		pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
>  		return pmu_bhrb_filter;
>  	}
>  
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
> +		pmu_bhrb_filter |= POWER8_MMCRA_IFM3;
> +		return pmu_bhrb_filter;
> +	}
> +
>  	/* Every thing else is unsupported */
>  	return -1;
>  }

I think it would be clearer if we actually checked for the possibilities we
allow and let everything else fall through, eg:

	/* Ignore user/kernel/hv bits */
	branch_sample_type &= ~PERF_SAMPLE_BRANCH_PLM_ALL;

	if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY)
		return 0;

	if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY_CALL)
		return POWER8_MMCRA_IFM1;
 
	if (branch_sample_type == PERF_SAMPLE_BRANCH_COND)
		return POWER8_MMCRA_IFM3;
	
	return -1;

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/