On 08-Jul-2020, at 5:12 PM, Michael Ellerman <mpe@ellerman.id.au> = wrote:

Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:

PowerISA v3.1 has few updates for the = Branch History Rolling Buffer(BHRB).
          &nb= sp;       ^
          &nb= sp;       a
First = is the addition of BHRB disable bit and second new filtering
          &nb= sp;            = ;            &= nbsp;           &nb= sp;     ^
          &nb= sp;            = ;            &= nbsp;           &nb= sp;     is
modes for BHRB.

BHRB disable is controlled via Monitor Mode Control Register = A (MMCRA)
bit 26, namely "BHRB Recording Disable = (BHRBRD)". This field controls

Most people call that bit = 37.

whether= BHRB entries are written when BHRB recording is enabled by other
bits. Patch implements support for this BHRB disable bit.
      ^
      This

Secondly PowerISA v3.1 introduce filtering support for

.. that should be in a separate patch please.

PERF_SAMPLE_BRANCH_IND_CALL/COND. The patch adds BHRB filter = support
          &nb= sp;            = ;            ^=
          &nb= sp;            = ;            T= his
for = "ind_call" and "cond" in power10_bhrb_filter_map().

'commit bb19af816025 ("powerpc/perf: Prevent kernel address = leak to userspace via BHRB buffer")'

That doesn't need single quotes, = and should be wrapped at 72 columns
like the rest of the text.

added a check in bhrb_read() to = filter the kernel address from BHRB buffer. Patch here modified
it to avoid that check for PowerISA v3.1 based processors, = since PowerISA v3.1 allows
only MSR[PR]=3D1 address to be = written to BHRB buffer.

And that should be a separate = patch again please.

Sure, I will split these to separate = patches

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/perf/core-book3s.c =       | 27 +++++++++++++++++++++------
arch/powerpc/perf/isa207-common.c     | = 13 +++++++++++++
arch/powerpc/perf/power10-pmu.c =       | 13 +++++++++++--
arch/powerpc/platforms/powernv/idle.c | 14 ++++++++++++++
4 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c = b/arch/powerpc/perf/core-book3s.c
index fad5159..9709606 = 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -466,9 = +466,13 @@ static void power_pmu_bhrb_read(struct perf_event *event, = struct cpu_hw_events *
* addresses at this point. = Check the privileges before
* exporting it to userspace = (avoid exposure of regions
* where we could have = speculative execution)
+ * Incase of ISA 310, BHRB = will capture only user-space
          &nb= sp;            = ;   ^
          &nb= sp;            = ;   In case of ISA v3.1,

+ mmcra |=3D MMCRA_BHRB_DISABLE;
+
+ /* Write SPRN_MMCRA if mmcra has = either disabled

Comment format is wrong.

+ * instruction sampling or = BHRB

Full stop please.

Sure

+ */
+ if (val = !=3D mmcra) {
+ mtspr(SPRN_MMCRA, mmcra);
= = = mb();
isync();
}
diff --git a/arch/powerpc/perf/isa207-common.c = b/arch/powerpc/perf/isa207-common.c
index 7d4839e..463d925 = 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ = -404,6 +404,12 @@ int isa207_compute_mmcr(u64 event[], int n_ev,

mmcra =3D mmcr1 =3D mmcr2 =3D = mmcr3 =3D 0;

+ /* Disable bhrb unless explicitly = requested
+ * by setting MMCRA [:26] = bit.
+ */

Comment format again.

+ if = (cpu_has_feature(CPU_FTR_ARCH_31))
+ mmcra |=3D = MMCRA_BHRB_DISABLE;

Here we do a feature check before setting MMCRA_BHRB_DISABLE, = but you
didn't = above?

+
= /* Second pass: assign PMCs, set all MMCR1 fields */
= for (i =3D 0; i < n_ev; ++i) {
pmc = =3D (event[i] >> EVENT_PMC_SHIFT) & = EVENT_PMC_MASK;
@@ -475,10 +481,17 @@ int = isa207_compute_mmcr(u64 event[], int n_ev,
}

if (event[i] & = EVENT_WANTS_BHRB) {
+ /* set MMCRA[:26] to 0 for = Power10 to enable BHRB */

"set MMCRA[:26] to 0" =3D=3D = "clear MMCRA[:26]=E2=80=9D

Athira

if = (pevents[i]->attr.exclude_user)
mmcr2 |=3D = MMCR2_FCP(pmc);

diff --git = a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
index d64d69d..07fb919 100644
--- = a/arch/powerpc/perf/power10-pmu.c
+++ = b/arch/powerpc/perf/power10-pmu.c
@@ -82,6 +82,8 @@

/* MMCRA IFM bits - POWER10 */
#define POWER10_MMCRA_IFM1 0x0000000040000000UL
+#define POWER10_MMCRA_IFM2 0x0000000080000000UL
+#define POWER10_MMCRA_IFM3 0x00000000C0000000UL
#define POWER10_MMCRA_BHRB_MASK 0x00000000C0000000UL

/* Table of alternatives, sorted by column 0 = */
@@ -233,8 +235,15 @@ static u64 = power10_bhrb_filter_map(u64 branch_sample_type)
if = (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
= = return -1;

- if = (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
- = = return -1;
+ if (branch_sample_type & = PERF_SAMPLE_BRANCH_IND_CALL) {
+ = pmu_bhrb_filter |=3D POWER10_MMCRA_IFM2;
+ return = pmu_bhrb_filter;
+ }
+
+ = if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
+ = = pmu_bhrb_filter |=3D POWER10_MMCRA_IFM3;
+ return = pmu_bhrb_filter;
+ }

= if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
= = return -1;
diff --git = a/arch/powerpc/platforms/powernv/idle.c = b/arch/powerpc/platforms/powernv/idle.c
index = 2dd4673..7db99c7 100644
--- = a/arch/powerpc/platforms/powernv/idle.c
+++ = b/arch/powerpc/platforms/powernv/idle.c
@@ -611,6 +611,7 = @@ static unsigned long power9_idle_stop(unsigned long psscr, bool = mmu_on)
unsigned long srr1;
unsigned = long pls;
unsigned long mmcr0 =3D 0;
+ = unsigned long mmcra_bhrb =3D 0;
struct = p9_sprs sprs =3D {}; /* avoid false used-uninitialised */
= bool sprs_saved =3D false;

@@ -657,6 = +658,15 @@ static unsigned long power9_idle_stop(unsigned long psscr, = bool mmu_on)
*/
= mmcr0 = = =3D mfspr(SPRN_MMCR0);
}
+
+ = if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ /* = POWER10 uses MMCRA[:26] as BHRB disable bit
Comment format.

+ * to disable BHRB logic = when not used. Hence Save and
+ * restore MMCRA after a = state-loss idle.
+ */
+ = mmcra_bhrb =3D mfspr(SPRN_MMCRA);
+ }

It's the whole mmcra it should be called mmcra?

Yes, we are = saving the whole mmcra.

+
if = ((psscr & PSSCR_RL_MASK) >=3D pnv_first_spr_loss_level) {
= = sprs.lpcr =3D mfspr(SPRN_LPCR);
= sprs.hfscr =3D mfspr(SPRN_HFSCR);
@@ -721,6 +731,10 @@ = static unsigned long power9_idle_stop(unsigned long psscr, bool = mmu_on)
mtspr(SPRN_MMCR0, mmcr0);
}

+ /* Reload MMCRA to restore BHRB = disable bit for POWER10 */
+ if = (cpu_has_feature(CPU_FTR_ARCH_31))
+ = mtspr(SPRN_MMCRA, mmcra_bhrb);
+
/*
= = * DD2.2 and = earlier need to set then clear bit 60 in MMCRA
* to ensure the PMU starts = running.

cheers