Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 02 Aug 2023 20:16:17 +0100
Message-ID: <86il9x5k72.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Rajnesh Kanwal <rkanwal@rivosinc.com>
Cc:     anshuman.khandual@arm.com, acme@kernel.org, broonie@kernel.org,
        catalin.marinas@arm.com, james.clark@arm.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, mark.rutland@arm.com,
        mingo@redhat.com, peterz@infradead.org, robh@kernel.org,
        suzuki.poulose@arm.com, will@kernel.org, atishp@rivosinc.com
Subject: Re: [PATCH V13 - RESEND 09/10] arm64/perf: Implement branch records save on task sched out
In-Reply-To: <20230802115931.12212-1-rkanwal@rivosinc.com>
References: <20230711082455.215983-10-anshuman.khandual@arm.com>
        <20230802115931.12212-1-rkanwal@rivosinc.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 02 Aug 2023 12:59:31 +0100,
Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
> 
> >diff --git a/drivers/perf/arm_brbe.c b/drivers/perf/arm_brbe.c
> >index 203cd4f350d5..2177632befa6 100644
> >--- a/drivers/perf/arm_brbe.c
> >+++ b/drivers/perf/arm_brbe.c
> >@@ -165,6 +165,36 @@ static int stitch_stored_live_entries(struct brbe_regset *stored,
> > 	return min(nr_live + nr_stored, nr_max);
> > }
> > 
> >+static int brbe_branch_save(struct brbe_regset *live, int nr_hw_entries)
> >+{
> >+	u64 brbfcr = read_sysreg_s(SYS_BRBFCR_EL1);
> >+	int nr_live;
> >+
> >+	write_sysreg_s(brbfcr | BRBFCR_EL1_PAUSED, SYS_BRBFCR_EL1);
> >+	isb();
> >+
> >+	nr_live = capture_brbe_regset(live, nr_hw_entries);
> >+
> >+	write_sysreg_s(brbfcr & ~BRBFCR_EL1_PAUSED, SYS_BRBFCR_EL1);
> >+	isb();
> >+
> >+	return nr_live;
> >+}
> >+
> >+void armv8pmu_branch_save(struct arm_pmu *arm_pmu, void *ctx)
> >+{
> >+	struct arm64_perf_task_context *task_ctx = ctx;
> >+	struct brbe_regset live[BRBE_MAX_ENTRIES];
> >+	int nr_live, nr_store, nr_hw_entries;
> >+
> >+	nr_hw_entries = brbe_get_numrec(arm_pmu->reg_brbidr);
> >+	nr_live = brbe_branch_save(live, nr_hw_entries);
> >+	nr_store = task_ctx->nr_brbe_records;
> >+	nr_store = stitch_stored_live_entries(task_ctx->store, live, nr_store,
> >+					      nr_live, nr_hw_entries);
> >+	task_ctx->nr_brbe_records = nr_store;
> >+}
> 
> Asking out-of-curiosity. Have you thought about virtualization use
> case. Current LBR implementation create an event for the guest
> and save/restore happens using the sched_task callback. (Correct me
> if I am wrong). Given you are only saving and processing those saved
> entries, how do you plan to expose the entries to the guest?

Two possibilities:

- either we perform a full save/restore of the registers so that host
  and guest have isolated states

- or we trap all BRBE accesses and piggy-back on the perf framework
  for that, much like we already do for the PMU.

	M.

-- 
Without deviation from the norm, progress is not possible.