From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 51A1CC25B75 for ; Thu, 6 Jun 2024 03:58:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RUejy8Yp7V+EUTQ547lNVh7sB+zS5ZxfGLv+N1YcNTE=; b=wsCXde7aHVd5yp c/Xw/lEG98DUa3EXUzrGMxF1LulOizRxWIHlleK6QskvLPb2By/GzzP2wqtGw5igoVPsGF3UA44fF 1O47Upt2SoWQCO4Pk7UD0/6NycnViDn2t0Z/xv1f4n8FY69OTWTUmGelR94lzvu7aS8VZoExWKNCP Ir4btVMrNuMctHfI66Ee0QBx5a9uMZhrKLmf+69/1SXHmq16xI/Oj+ITCtZcNUkZasKbP8Csgge+J hpCaimbHQWKx3fUHOfh27ypmbsvXoUwVR+kQmwaZBkxV3CYrEVQYm44BpkgS8LkoEC6huihgG6wzp 1hh+fEGeqXM090Y3QTpw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sF4Ge-00000008IAO-2vri; Thu, 06 Jun 2024 03:58:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sF4Ga-00000008I9b-49Zf for linux-arm-kernel@lists.infradead.org; Thu, 06 Jun 2024 03:58:22 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5915E339; Wed, 5 Jun 2024 20:58:40 -0700 (PDT) Received: from [10.162.40.16] (a077893.blr.arm.com [10.162.40.16]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6F2E23F64C; Wed, 5 Jun 2024 20:58:11 -0700 (PDT) Message-ID: Date: Thu, 6 Jun 2024 09:27:49 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V17 0/9] arm64/perf: Enable branch stack sampling Content-Language: en-US To: James Clark , mark.rutland@arm.com Cc: Mark Brown , Rob Herring , Marc Zyngier , Suzuki Poulose , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, catalin.marinas@arm.com References: <20240405024639.1179064-1-anshuman.khandual@arm.com> <80d33844-bdd2-4fee-81dd-9cd37c63d42c@arm.com> From: Anshuman Khandual In-Reply-To: <80d33844-bdd2-4fee-81dd-9cd37c63d42c@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240605_205821_170868_7971CBE5 X-CRM114-Status: GOOD ( 27.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 5/30/24 15:17, James Clark wrote: > > > On 05/04/2024 03:46, Anshuman Khandual wrote: >> This series enables perf branch stack sampling support on arm64 platform >> via a new arch feature called Branch Record Buffer Extension (BRBE). All >> the relevant register definitions could be accessed here. >> >> https://developer.arm.com/documentation/ddi0601/2021-12/AArch64-Registers >> >> This series applies on 6.9-rc2. >> >> Also this series is being hosted below for quick access, review and test. >> >> https://git.gitlab.arm.com/linux-arm/linux-anshuman.git (brbe_v17) >> >> There are still some open questions regarding handling multiple perf events >> with different privilege branch filters getting on the same PMU, supporting >> guest branch stack tracing from the host etc. Finally also looking for some >> suggestions regarding supporting BRBE inside the guest. The series has been >> re-organized completely as suggested earlier. >> >> - Anshuman >> > [...] >> >> ------------------ Possible 'branch_sample_type' Mismatch ----------------- >> >> Branch stack sampling attributes 'event->attr.branch_sample_type' generally >> remain the same for all the events during a perf record session. >> >> $perf record -e -e -j [workload] >> >> event_1->attr.branch_sample_type == event_2->attr.branch_sample_type >> >> This 'branch_sample_type' is used to configure the BRBE hardware, when both >> events i.e and get scheduled on a given PMU. But during >> PMU HW event's privilege filter inheritance, 'branch_sample_type' does not >> remain the same for all events. Let's consider the following example >> >> $perf record -e cycles:u -e instructions:k -j any,save_type ls >> >> cycles->attr.branch_sample_type != instructions->attr.branch_sample_type >> >> Because cycles event inherits PERF_SAMPLE_BRANCH_USER and instruction event >> inherits PERF_SAMPLE_BRANCH_KERNEL. The proposed solution here configures >> BRBE hardware with 'branch_sample_type' from last event to be added in the >> PMU and hence captured branch records only get passed on to matching events >> during a PMU interrupt. >> > > Hi Anshuman, > > Surely because of this example we should merge? At least we have to try > to make the most common basic command lines work. Unless we expect all > tools to know whether the branch buffer is shared between PMUs on each > architecture or not. The driver knows though, so can merge the settings > because it all has to go into one BRBE. > > Merging the settings in tools would be a much harder problem. Alright, makes sense. > > Any user that doesn't have permission for anything in the merged result > can continue to get nothing. > > And we can always add filtering in the kernel later on if we want to to > make it appear to behave even more normally. Understood. > >> static int >> armpmu_add(struct perf_event *event, int flags) >> { >> ........ >> if (has_branch_stack(event)) { >> /* >> * Reset branch records buffer if a new task event gets >> * scheduled on a PMU which might have existing records. >> * Otherwise older branch records present in the buffer >> * might leak into the new task event. >> */ >> if (event->ctx->task && hw_events->brbe_context != event->ctx) { >> hw_events->brbe_context = event->ctx; >> if (armpmu->branch_reset) >> armpmu->branch_reset(); >> } >> hw_events->brbe_users++; >> Here -------> hw_events->brbe_sample_type = event->attr.branch_sample_type; >> } >> ........ >> } >> >> Instead of overriding existing 'branch_sample_type', both could be merged. >> > > I can't see any use case where anyone would want the override behavior. > Especially if you imagine multiple users not even aware of each other. > Either the current "no records for mismatches" or the merged one make sense. Hence I had enlisted all the three available options. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel