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 40AA4C2BA4C for ; Wed, 26 Jan 2022 17:33:51 +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:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HClNcqMfJKyFNdI11WxGV8ROPMYKM0V3UKHTKv+ezng=; b=yQBFCSYyZQQNGN RGzsP5OJEp7qQpl/oSz73uYXXKjSc63F6ILOS2yhAEdjE3iXBHZ/Gz3LnUGXWH2Ve9rJ/PxrLbGJX NATj46dJax+Re2kb/1ROhyrLyBys0q66aWtY/DUWncXN9kOV57XRas65tzRlttOwws74OdwyUX+Qu vrLrZ3Dw++X7iKC+T2BS14GEBFdN/sy4qLtvszNSzxKEImxo2hB96emL67DqV5KvAeH4t4RHHWD6y cx7KLq4D1ZS+fWjyUj4ODNqA7TnxEVLbImu6olctLEA+a5tco0l9b4/iEMUOVpq2nRllxfAgGkTqf HJMOhui0VQOpmDf8Yufg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCm9U-00D1IL-GX; Wed, 26 Jan 2022 17:32:12 +0000 Received: from mail-ot1-f54.google.com ([209.85.210.54]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCle7-00Cp0v-7i for linux-arm-kernel@lists.infradead.org; Wed, 26 Jan 2022 16:59:48 +0000 Received: by mail-ot1-f54.google.com with SMTP id b17-20020a9d4791000000b005a17fc2dfc1so2477425otf.1 for ; Wed, 26 Jan 2022 08:59:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YQ5BAI7S/KVmQ/LV/p1EHUUf4M859iY+jX0AOgHG8QE=; b=0oqhC/0ojhDmY020Oc+WQNKWsYz6gLjjvL3IYQhRsHXvChqHeMLu6Ji1aG2GNJVR4S uYbHdOTCevC/9t9g/CK8TDlCfGugFId5JGnluzETp+fnqwHkGmYWMfydEkN39F4JHwrj Oau1C4UpCpI9X73z2HH2Ei9BYqy92q0h/nLqnzKFRMLFeyFrJW1ZQOnmGEBVqMAunq7p tDk/8E/UhDLReH5IkPJ5DNNAfThXSBFOtf9F0kcLPYYhl+KYxVJHqlXwH/iIehJ4hFiV 4Qp5tbc8AVFt7MtOgkOA7Nh7DGEt7RBWIEFJqr5mSKsfs4jXWlvycCojztGZK1GQlkuA pVwg== X-Gm-Message-State: AOAM5327GfmCYQBEEkP8+puefnPIFMuYfnO2uokIL98Rqkb+vffMSqQK 4OamBF5tFMuFNt7zOHD44g== X-Google-Smtp-Source: ABdhPJwMUHlaE6s6swqJbVgEUfJw9eAQzgx+aBQaduDIHooIsdoIcZWSjGo489GbmMtxKCkfy9zBSA== X-Received: by 2002:a05:6830:1508:: with SMTP id k8mr4401551otp.382.1643216385703; Wed, 26 Jan 2022 08:59:45 -0800 (PST) Received: from robh.at.kernel.org (66-90-148-213.dyn.grandenetworks.net. [66.90.148.213]) by smtp.gmail.com with ESMTPSA id 59sm5170318oti.74.2022.01.26.08.59.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jan 2022 08:59:45 -0800 (PST) Received: (nullmailer pid 968366 invoked by uid 1000); Wed, 26 Jan 2022 16:59:44 -0000 Date: Wed, 26 Jan 2022 10:59:44 -0600 From: Rob Herring To: Anshuman Khandual Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Mark Rutland , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , linux-perf-users@vger.kernel.org Subject: Re: [RFC V1 03/11] arm64/perf: Update struct arm_pmu for BRBE Message-ID: References: <1642998653-21377-1-git-send-email-anshuman.khandual@arm.com> <1642998653-21377-4-git-send-email-anshuman.khandual@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1642998653-21377-4-git-send-email-anshuman.khandual@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220126_085947_359843_D5E3F222 X-CRM114-Status: GOOD ( 27.06 ) 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 Mon, Jan 24, 2022 at 10:00:45AM +0530, Anshuman Khandual wrote: > This updates struct arm_pmu to include all required helpers that will drive >From submitting-patches.rst: Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour. > BRBE functionality for a given PMU implementation. These are the following. Don't describe what the change is, one can read the diff for that. Answer why it is needed. 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. > > - 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 The function names seem pretty self-explanatory, but the text is needed, shouldn't it be in the struct declaration. I'm not really a fan of patches adding dead code. That's not any less to review. Restructuring with 'no functional change' OTOH is helpful in reviewing. > A BRBE driver implementation needs to provide these functionalities. > > Cc: Will Deacon > Cc: Mark Rutland > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Catalin Marinas > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-perf-users@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual > --- > arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++ > include/linux/perf/arm_pmu.h | 7 +++++++ > 2 files changed, 43 insertions(+) > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c > index cab678ed6618..f6a47036b0b4 100644 > --- a/arch/arm64/kernel/perf_event.c > +++ b/arch/arm64/kernel/perf_event.c > @@ -1015,6 +1015,35 @@ static int armv8pmu_filter_match(struct perf_event *event) > return evtype != ARMV8_PMUV3_PERFCTR_CHAIN; > } > > +static void armv8pmu_brbe_filter(struct pmu_hw_events *hw_event, struct perf_event *event) > +{ > +} > + > +static void armv8pmu_brbe_enable(struct pmu_hw_events *hw_event) > +{ > +} > + > +static void armv8pmu_brbe_disable(struct pmu_hw_events *hw_event) > +{ > +} > + > +static void armv8pmu_brbe_read(struct pmu_hw_events *hw_event, struct perf_event *event) > +{ > +} > + > +static void armv8pmu_brbe_probe(struct pmu_hw_events *hw_event) > +{ > +} > + > +static void armv8pmu_brbe_reset(struct pmu_hw_events *hw_event) > +{ > +} > + > +static bool armv8pmu_brbe_supported(struct perf_event *event) > +{ > + return false; > +} > + > static void armv8pmu_reset(void *info) > { > struct arm_pmu *cpu_pmu = (struct arm_pmu *)info; > @@ -1247,6 +1276,13 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name, > > cpu_pmu->pmu.event_idx = armv8pmu_user_event_idx; > > + cpu_pmu->brbe_filter = armv8pmu_brbe_filter; > + cpu_pmu->brbe_enable = armv8pmu_brbe_enable; > + cpu_pmu->brbe_disable = armv8pmu_brbe_disable; > + cpu_pmu->brbe_read = armv8pmu_brbe_read; > + cpu_pmu->brbe_probe = armv8pmu_brbe_probe; > + cpu_pmu->brbe_reset = armv8pmu_brbe_reset; > + cpu_pmu->brbe_supported = armv8pmu_brbe_supported; > cpu_pmu->name = name; > cpu_pmu->map_event = map_event; > cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] = events ? > diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h > index 2512e2f9cd4e..c0dd0d6c5883 100644 > --- a/include/linux/perf/arm_pmu.h > +++ b/include/linux/perf/arm_pmu.h > @@ -98,6 +98,13 @@ struct arm_pmu { > void (*reset)(void *); > int (*map_event)(struct perf_event *event); > int (*filter_match)(struct perf_event *event); > + void (*brbe_filter)(struct pmu_hw_events *hw_events, struct perf_event *event); > + void (*brbe_probe)(struct pmu_hw_events *hw_events); > + void (*brbe_enable)(struct pmu_hw_events *hw_events); > + void (*brbe_disable)(struct pmu_hw_events *hw_events); > + void (*brbe_read)(struct pmu_hw_events *hw_events, struct perf_event *event); > + void (*brbe_reset)(struct pmu_hw_events *hw_events); > + bool (*brbe_supported)(struct perf_event *event); > int num_events; > bool secure_access; /* 32-bit ARM only */ > #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40 > -- > 2.25.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel