On 6/2/23 07:15, Namhyung Kim wrote:

Hello,

On Tue, May 30, 2023 at 9:21 PM Anshuman Khandual
[off-list ref] wrote:

quoted

This enables branch stack sampling events in ARMV8 PMU, via an architecture
feature FEAT_BRBE aka branch record buffer extension. This defines required
branch helper functions pmuv8pmu_branch_XXXXX() and the implementation here
is wrapped with a new config option CONFIG_ARM64_BRBE.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Tested-by: James Clark <redacted>
Signed-off-by: Anshuman Khandual <redacted>
---

[SNIP]

quoted

+void armv8pmu_branch_read(struct pmu_hw_events *cpuc, struct perf_event *event)
+{
+       struct brbe_hw_attr *brbe_attr = (struct brbe_hw_attr *)cpuc->percpu_pmu->private;
+       u64 brbfcr, brbcr;
+       int idx, loop1_idx1, loop1_idx2, loop2_idx1, loop2_idx2, count;
+
+       brbcr = read_sysreg_s(SYS_BRBCR_EL1);
+       brbfcr = read_sysreg_s(SYS_BRBFCR_EL1);
+
+       /* Ensure pause on PMU interrupt is enabled */
+       WARN_ON_ONCE(!(brbcr & BRBCR_EL1_FZP));
+
+       /* Pause the buffer */
+       write_sysreg_s(brbfcr | BRBFCR_EL1_PAUSED, SYS_BRBFCR_EL1);
+       isb();
+
+       /* Determine the indices for each loop */
+       loop1_idx1 = BRBE_BANK0_IDX_MIN;
+       if (brbe_attr->brbe_nr <= BRBE_BANK_MAX_ENTRIES) {
+               loop1_idx2 = brbe_attr->brbe_nr - 1;
+               loop2_idx1 = BRBE_BANK1_IDX_MIN;
+               loop2_idx2 = BRBE_BANK0_IDX_MAX;

Is this to disable the bank1?  Maybe need a comment.

Sure, will add a comment.

quoted

+       } else {
+               loop1_idx2 = BRBE_BANK0_IDX_MAX;
+               loop2_idx1 = BRBE_BANK1_IDX_MIN;
+               loop2_idx2 = brbe_attr->brbe_nr - 1;
+       }

The loop2_idx1 is the same for both cases.  Maybe better
to move it out of the if statement.

Sure, will do the following change as suggested but wondering whether should
the change be implemented from this patch onwards or in the later patch that
adds capture_brbe_regset().
 

--- a/drivers/perf/arm_brbe.c
+++ b/drivers/perf/arm_brbe.c

@@ -56,13 +56,14 @@ static int capture_brbe_regset(struct brbe_hw_attr *brbe_attr, struct brbe_regse
        int idx, count;
 
        loop1_idx1 = BRBE_BANK0_IDX_MIN;
+       loop2_idx1 = BRBE_BANK1_IDX_MIN;
        if (brbe_attr->brbe_nr <= BRBE_BANK_MAX_ENTRIES) {
                loop1_idx2 = brbe_attr->brbe_nr - 1;
-               loop2_idx1 = BRBE_BANK1_IDX_MIN;
+
+               /* Disable capturing the bank 1 */
                loop2_idx2 = BRBE_BANK0_IDX_MAX;
        } else {
                loop1_idx2 = BRBE_BANK0_IDX_MAX;
-               loop2_idx1 = BRBE_BANK1_IDX_MIN;
                loop2_idx2 = brbe_attr->brbe_nr - 1;
        }

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel