| 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
| ||
|
Message-ID: <00f05970-c793-242e-485a-f987201651ad@arm.com>
Date: Tue, 14 Nov 2023 12:11:50 +0000
From: James Clark <james.clark@....com>
To: Anshuman Khandual <anshuman.khandual@....com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Marc Zyngier <maz@...nel.org>,
Suzuki Poulose <suzuki.poulose@....com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-perf-users@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
will@...nel.org, catalin.marinas@....com, mark.rutland@....com
Subject: Re: [V14 4/8] drivers: perf: arm_pmuv3: Enable branch stack sampling
via FEAT_BRBE
On 14/11/2023 05:13, Anshuman Khandual wrote:
[...]
> +/*
> + * BRBE supports the following functional branch type filters while
> + * generating branch records. These branch filters can be enabled,
> + * either individually or as a group i.e ORing multiple filters
> + * with each other.
> + *
> + * BRBFCR_EL1_CONDDIR - Conditional direct branch
> + * BRBFCR_EL1_DIRCALL - Direct call
> + * BRBFCR_EL1_INDCALL - Indirect call
> + * BRBFCR_EL1_INDIRECT - Indirect branch
> + * BRBFCR_EL1_DIRECT - Direct branch
> + * BRBFCR_EL1_RTN - Subroutine return
> + */
> +static u64 branch_type_to_brbfcr(int branch_type)
> +{
> + u64 brbfcr = 0;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY) {
> + brbfcr |= BRBFCR_EL1_BRANCH_FILTERS;
> + return brbfcr;
> + }
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
> + brbfcr |= BRBFCR_EL1_INDCALL;
> + brbfcr |= BRBFCR_EL1_DIRCALL;
> + }
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
> + brbfcr |= BRBFCR_EL1_RTN;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_IND_CALL)
> + brbfcr |= BRBFCR_EL1_INDCALL;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_COND)
> + brbfcr |= BRBFCR_EL1_CONDDIR;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_IND_JUMP)
> + brbfcr |= BRBFCR_EL1_INDIRECT;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_CALL)
> + brbfcr |= BRBFCR_EL1_DIRCALL;
> +
> + return brbfcr;
> +}
> +
> +/*
> + * BRBE supports the following privilege mode filters while generating
> + * branch records.
> + *
> + * BRBCR_ELx_E0BRE - EL0 branch records
> + * BRBCR_ELx_ExBRE - EL1/EL2 branch records
> + *
> + * BRBE also supports the following additional functional branch type
> + * filters while generating branch records.
> + *
> + * BRBCR_ELx_EXCEPTION - Exception
> + * BRBCR_ELx_ERTN - Exception return
> + */
> +static u64 branch_type_to_brbcr(int branch_type)
> +{
> + u64 brbcr = BRBCR_ELx_DEFAULT_TS;
> +
> + /*
> + * BRBE should be paused on PMU interrupt while tracing kernel
> + * space to stop capturing further branch records. Otherwise
> + * interrupt handler branch records might get into the samples
> + * which is not desired.
> + *
> + * BRBE need not be paused on PMU interrupt while tracing only
> + * the user space, because it will automatically be inside the
> + * prohibited region. But even after PMU overflow occurs, the
> + * interrupt could still take much more cycles, before it can
> + * be taken and by that time BRBE will have been overwritten.
> + * Hence enable pause on PMU interrupt mechanism even for user
> + * only traces as well.
> + */
> + brbcr |= BRBCR_ELx_FZP;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_USER)
> + brbcr |= BRBCR_ELx_E0BRE;
> +
> + /*
> + * When running in the hyp mode, writing into BRBCR_EL1
> + * actually writes into BRBCR_EL2 instead. Field E2BRE
> + * is also at the same position as E1BRE.
> + */
> + if (branch_type & PERF_SAMPLE_BRANCH_KERNEL)
> + brbcr |= BRBCR_ELx_ExBRE;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_HV) {
> + if (is_kernel_in_hyp_mode())
> + brbcr |= BRBCR_ELx_ExBRE;
> + }
> +
> + if (!(branch_type & PERF_SAMPLE_BRANCH_NO_CYCLES))
> + brbcr |= BRBCR_ELx_CC;
> +
> + if (!(branch_type & PERF_SAMPLE_BRANCH_NO_FLAGS))
> + brbcr |= BRBCR_ELx_MPRED;
> +
> + /*
> + * The exception and exception return branches could be
> + * captured, irrespective of the perf event's privilege.
> + * If the perf event does not have enough privilege for
> + * a given exception level, then addresses which falls
> + * under that exception level will be reported as zero
> + * for the captured branch record, creating source only
> + * or target only records.
> + */
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY) {
> + brbcr |= BRBCR_ELx_EXCEPTION;
> + brbcr |= BRBCR_ELx_ERTN;
> + }
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY_CALL)
> + brbcr |= BRBCR_ELx_EXCEPTION;
> +
> + if (branch_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
> + brbcr |= BRBCR_ELx_ERTN;
> +
> + return brbcr & BRBCR_ELx_CONFIG_MASK;
> +}
> +
> +void armv8pmu_branch_enable(struct arm_pmu *arm_pmu)
> +{
> + struct pmu_hw_events *cpuc = this_cpu_ptr(arm_pmu->hw_events);
> + u64 brbfcr, brbcr;
> +
> + if (!(cpuc->brbe_sample_type && cpuc->brbe_users))
> + return;
> +
> + /*
> + * BRBE gets configured with a new mismatched branch sample
> + * type request, overriding any previous branch filters.
> + */
> + brbfcr = read_sysreg_s(SYS_BRBFCR_EL1);
> + brbfcr &= ~BRBFCR_EL1_DEFAULT_CONFIG;
This is called default_config, but is being used semantically the same
way as BRBCR_ELx_CONFIG_MASK below to clear out the fields. Doesn't that
mean that it's a mask rather than a default config? It's only ever used
in this way. default_config implies it's written or used as an
initialiser at some point.
> + brbfcr |= branch_type_to_brbfcr(cpuc->brbe_sample_type);
> + write_sysreg_s(brbfcr, SYS_BRBFCR_EL1);
> + isb();
> +
> + brbcr = read_sysreg_s(SYS_BRBCR_EL1);
> + brbcr &= ~BRBCR_ELx_CONFIG_MASK;
> + brbcr |= branch_type_to_brbcr(cpuc->brbe_sample_type);
BRBCR_ELx_CONFIG_MASK is already &'d at the end of
branch_type_to_brbcr(), so isn't it easier and equivalent to just do the
following instead of the read(), &= and then |= ?
write_sysreg_s(branch_type_to_brbcr(...), SYS_BRBCR_EL1);
Or at least make branch_type_to_brbfcr() consistent and &
BRBFCR_EL1_DEFAULT_CONFIG at the end of that function too.
> + write_sysreg_s(brbcr, SYS_BRBCR_EL1);
> + isb();
> +}
> +
Powered by blists - more mailing lists