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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2183C4332F for ; Fri, 18 Nov 2022 06:39:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241129AbiKRGjU (ORCPT ); Fri, 18 Nov 2022 01:39:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230287AbiKRGjS (ORCPT ); Fri, 18 Nov 2022 01:39:18 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 65B505C769; Thu, 17 Nov 2022 22:39:16 -0800 (PST) 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 4BF3923A; Thu, 17 Nov 2022 22:39:22 -0800 (PST) Received: from [10.162.41.7] (unknown [10.162.41.7]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 174033F73B; Thu, 17 Nov 2022 22:39:10 -0800 (PST) Message-ID: <4efc0ae1-564e-dd05-842a-46fb1aeb4ad8@arm.com> Date: Fri, 18 Nov 2022 12:09:07 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH V5 2/7] arm64/perf: Update struct arm_pmu for BRBE Content-Language: en-US To: Suzuki K Poulose , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, peterz@infradead.org, acme@kernel.org, mark.rutland@arm.com, will@kernel.org, catalin.marinas@arm.com Cc: Mark Brown , James Clark , Rob Herring , Marc Zyngier , Ingo Molnar References: <20221107062514.2851047-1-anshuman.khandual@arm.com> <20221107062514.2851047-3-anshuman.khandual@arm.com> <8f6d3424-2650-8e8b-61f7-1431aec4633b@arm.com> From: Anshuman Khandual In-Reply-To: <8f6d3424-2650-8e8b-61f7-1431aec4633b@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/9/22 17:00, Suzuki K Poulose wrote: > On 07/11/2022 06:25, Anshuman Khandual wrote: >> Although BRBE is an armv8 speciifc HW feature, abstracting out its various >> function callbacks at the struct arm_pmu level is preferred, as it cleaner >> , easier to follow and maintain. >> >> Besides some helpers i.e brbe_supported(), brbe_probe() and brbe_reset() >> might not fit seamlessly, when tried to be embedded via existing arm_pmu >> helpers in the armv8 implementation. >> >> Updates the struct arm_pmu to include all required helpers that will drive >> BRBE functionality for a given PMU implementation. These are the following. >> >> - brbe_filter    : Convert perf event filters into BRBE HW filters >> - brbe_probe    : Probe BRBE HW and capture its attributes >> - brbe_enable    : Enable BRBE HW with a given config >> - brbe_disable    : Disable BRBE HW >> - brbe_read    : Read BRBE buffer for captured branch records >> - brbe_reset    : Reset BRBE buffer >> - brbe_supported: Whether BRBE is supported or not >> >> A BRBE driver implementation needs to provide these functionalities. > > Could these not be hidden from the generic arm_pmu and kept in the > arm64 pmu backend  ? It looks like they are quite easy to simply > move these to the corresponding hooks in arm64 pmu. We have had this discussion multiple times in the past [1], but I still believe, keeping BRBE implementation hooks at the PMU level rather than embedding them with other PMU events handling, is a much better logical abstraction. [1] https://lore.kernel.org/all/c3804290-bdb1-d1eb-3526-9b0ce4c8e8b1@arm.com/ -------------------------------------------------------------------------- > > One thing to answer in the commit msg is why we need the hooks here. > Have we concluded that adding BRBE hooks to struct arm_pmu for what is > an armv8 specific feature is the right approach? I don't recall > reaching that conclusion. Although it might be possible to have this implementation embedded in the existing armv8 PMU implementation, I still believe that the BRBE functionalities abstracted out at the arm_pmu level with a separate config option is cleaner, easier to follow and to maintain as well. Besides some helpers i.e brbe_supported(), brbe_probe() and brbe_reset() might not fit seamlessly, when tried to be embedded via existing arm_pmu helpers in the armv8 implementation. Nonetheless if arm_pmu based additional BRBE helpers is absolutely a no go for folks here in general, will explore arm64 based implementation. ---------------------------------------------------------------------------- I am still waiting for maintainer's take on this issue. I will be happy to rework this series to move all these implementation inside arm64 callbacks instead, if that is required or preferred by the maintainers. But according to me, this current abstraction layout is much better. - Anshuman