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 88BABC4345F for ; Thu, 25 Apr 2024 10:26:49 +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=gMEXhfHMv25i/Fpf0Y0r6ngGlufaAIcogvwQBk099XI=; b=da7xENTWk27FEx oSKo6BvyqdyVSQ9/tsW7wE9CdTft8bVR66aewt1TcFmPQRKoGo33oddaBYkm/dm8TlIEwDdpj1WAk tpKwdPiYHzVYeATWyTLA2Y4tbHFIjs3D68ZnJPfsBxVSYUqpg1mDASo11XhGvk6toZHNgBnud/k9D yeg4T/PHPSp9SdQmgFBsd71xW9z7lHqGVsLlqe3fzR+UXTfYvdZca1IN1wd9c/lr0DoSJt8ZFPzHd yx6RRk8XbI592f3arLE55UrQJNMa46EU420JQtRpgd1OpTj1K+mskcmcOL0/F177ib1+TcQ4BYFa7 Pc2+ICneN1SoNizf8dpg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rzwJJ-00000007pZl-0GrW; Thu, 25 Apr 2024 10:26:37 +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 1rzwJB-00000007pYw-1pIU for linux-arm-kernel@lists.infradead.org; Thu, 25 Apr 2024 10:26:34 +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 D8DB71007; Thu, 25 Apr 2024 03:26:56 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.21.118]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8637E3F64C; Thu, 25 Apr 2024 03:26:27 -0700 (PDT) Date: Thu, 25 Apr 2024 11:26:24 +0100 From: Mark Rutland To: Tarun Sahu Cc: will@kernel.org, linux-arm-kernel@lists.infradead.org, nsekhar@google.com, nikhilnd@google.com, praan@google.com Subject: Re: [PATCH] drivers/perf/arm_smmuv3_pmu: Support for PMU Snapshot Message-ID: References: <20240423130647.1861668-1-tarunsahu@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240423130647.1861668-1-tarunsahu@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240425_032629_586049_5A6C2348 X-CRM114-Status: GOOD ( 36.25 ) 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 Tue, Apr 23, 2024 at 01:06:47PM +0000, Tarun Sahu wrote: > This patch adds support to capture snapshot of all event counters for > SMMUV3 PMU counters. The following functionalities are added: > 1. Manually capture the snapshot > 2. Capture the snapshot when an event counter overflows. > > Test: > To manually capture the snapshot: > $ echo 1 > /sys/devices/smmuv3_pmcg_/capture > 'OR' > Configure an event for capturing the snapshot when it overflows > $ perf stat -e /smmuv3_pmcg_ff88840/transaction,filter_enable=1, > overflow_capture=1,filter_span=1,filter_stream_id=0x42/ -a > netperf > > To read the snapshot: > $ cat /sys/devices/smmuv3_pmcg_/capture Why is that a magic file in sysfs rather than being exposed via perf? How does the first case even work? Where were the events configured? Looking at the code, that just iteratres over the counters. You should be able to achieve similar by creating a group of events, then reading the group leader to snapshot the group (though this won't currently use the HW capture feature). There's no rationale given here; what are you trying to use this for? As it stands (and condsidering the above) I do not think this is the right approach -- if nothing else I think the values should come via perf and not via sysfs. Mark. > > Signed-off-by: Tarun Sahu > --- > drivers/perf/arm_smmuv3_pmu.c | 77 +++++++++++++++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index 6303b82566f9..6e2baa752ccc 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -15,6 +15,7 @@ > * filter_enable - 0 = no filtering, 1 = filtering enabled > * filter_span - 0 = exact match, 1 = pattern match > * filter_stream_id - pattern to filter against > + * overflow_capture - capture all events when this event overflows > * > * To match a partial StreamID where the X most-significant bits must match > * but the Y least-significant bits might differ, STREAMID is programmed > @@ -56,6 +57,8 @@ > > #define SMMU_PMCG_EVCNTR0 0x0 > #define SMMU_PMCG_EVCNTR(n, stride) (SMMU_PMCG_EVCNTR0 + (n) * (stride)) > +#define SMMU_PMCG_SVR0 0x0 > +#define SMMU_PMCG_SVR(n, stride) (SMMU_PMCG_SVR0 + (n) * (stride)) > #define SMMU_PMCG_EVTYPER0 0x400 > #define SMMU_PMCG_EVTYPER(n) (SMMU_PMCG_EVTYPER0 + (n) * 4) > #define SMMU_PMCG_SID_SPAN_SHIFT 29 > @@ -67,8 +70,11 @@ > #define SMMU_PMCG_INTENCLR0 0xC60 > #define SMMU_PMCG_OVSCLR0 0xC80 > #define SMMU_PMCG_OVSSET0 0xCC0 > +#define SMMU_PMCG_CAPR 0xD88 > +#define SMMU_PMCG_CAPR_CAPTURE BIT(0) > #define SMMU_PMCG_CFGR 0xE00 > #define SMMU_PMCG_CFGR_SID_FILTER_TYPE BIT(23) > +#define SMMU_PMCG_CFGR_CAPTURE BIT(22) > #define SMMU_PMCG_CFGR_MSI BIT(21) > #define SMMU_PMCG_CFGR_RELOC_CTRS BIT(20) > #define SMMU_PMCG_CFGR_SIZE GENMASK(13, 8) > @@ -135,6 +141,7 @@ struct smmu_pmu { > u32 options; > u32 iidr; > bool global_filter; > + bool capture; > }; > > #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu)) > @@ -147,6 +154,7 @@ struct smmu_pmu { > } \ > > SMMU_PMU_EVENT_ATTR_EXTRACTOR(event, config, 0, 15); > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(overflow_capture, config, 31, 31); > SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_stream_id, config1, 0, 31); > SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_span, config1, 32, 32); > SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_enable, config1, 33, 33); > @@ -219,6 +227,18 @@ static inline u64 smmu_pmu_counter_get_value(struct smmu_pmu *smmu_pmu, u32 idx) > return value; > } > > +static inline u64 smmu_pmu_svr_get_value(struct smmu_pmu *smmu_pmu, u32 idx) > +{ > + u64 value; > + > + if (smmu_pmu->counter_mask & BIT(32)) > + value = readq(smmu_pmu->reloc_base + SMMU_PMCG_SVR(idx, 8)); > + else > + value = readl(smmu_pmu->reloc_base + SMMU_PMCG_SVR(idx, 4)); > + > + return value; > +} > + > static inline void smmu_pmu_counter_enable(struct smmu_pmu *smmu_pmu, u32 idx) > { > writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_CNTENSET0); > @@ -306,6 +326,7 @@ static void smmu_pmu_set_event_filter(struct perf_event *event, > u32 evtyper; > > evtyper = get_event(event) | span << SMMU_PMCG_SID_SPAN_SHIFT; > + evtyper |= get_overflow_capture(event); > smmu_pmu_set_evtyper(smmu_pmu, idx, evtyper); > smmu_pmu_set_smr(smmu_pmu, idx, sid); > } > @@ -549,6 +570,54 @@ static const struct attribute_group smmu_pmu_cpumask_group = { > .attrs = smmu_pmu_cpumask_attrs, > }; > > +static ssize_t smmu_pmu_capture_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev)); > + u32 capture, i; > + > + if (!smmu_pmu->capture) > + return -EPERM; > + capture = readl_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CAPR) & > + SMMU_PMCG_CAPR_CAPTURE; > + if (capture == 0) { > + for (i = 0; i < SMMU_PMCG_MAX_COUNTERS; i++) { > + capture = sprintf(buf, "%s%d %llu\n", buf, i, > + smmu_pmu_svr_get_value(smmu_pmu, i)); > + } > + } > + return capture; > +} > + > +static ssize_t smmu_pmu_capture_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev)); > + unsigned long val; > + > + if (!smmu_pmu->capture) > + return -EPERM; > + if (kstrtoul(buf, 0, &val) == 0 && val == 1) { > + writeq_relaxed(1, smmu_pmu->reg_base + SMMU_PMCG_CAPR); > + return count; > + } > + return -EINVAL; > +} > + > +static struct device_attribute smmu_pmu_capture_attr = > + __ATTR(capture, 0644, smmu_pmu_capture_show, smmu_pmu_capture_store); > + > +static struct attribute *smmu_pmu_capture_attrs[] = { > + &smmu_pmu_capture_attr.attr, > + NULL > +}; > + > +static const struct attribute_group smmu_pmu_capture_group = { > + .attrs = smmu_pmu_capture_attrs, > +}; > + > /* Events */ > > static ssize_t smmu_pmu_event_show(struct device *dev, > @@ -633,12 +702,14 @@ static const struct attribute_group smmu_pmu_identifier_group = { > > /* Formats */ > PMU_FORMAT_ATTR(event, "config:0-15"); > +PMU_FORMAT_ATTR(overflow_capture, "config:31"); > PMU_FORMAT_ATTR(filter_stream_id, "config1:0-31"); > PMU_FORMAT_ATTR(filter_span, "config1:32"); > PMU_FORMAT_ATTR(filter_enable, "config1:33"); > > static struct attribute *smmu_pmu_formats[] = { > &format_attr_event.attr, > + &format_attr_overflow_capture.attr, > &format_attr_filter_stream_id.attr, > &format_attr_filter_span.attr, > &format_attr_filter_enable.attr, > @@ -651,6 +722,7 @@ static const struct attribute_group smmu_pmu_format_group = { > }; > > static const struct attribute_group *smmu_pmu_attr_grps[] = { > + &smmu_pmu_capture_group, > &smmu_pmu_cpumask_group, > &smmu_pmu_events_group, > &smmu_pmu_format_group, > @@ -888,6 +960,11 @@ static int smmu_pmu_probe(struct platform_device *pdev) > smmu_pmu->reloc_base = smmu_pmu->reg_base; > } > > + if (cfgr & SMMU_PMCG_CFGR_CAPTURE) > + smmu_pmu->capture = true; > + else > + smmu_pmu->capture = false; > + > irq = platform_get_irq_optional(pdev, 0); > if (irq > 0) > smmu_pmu->irq = irq; > -- > 2.44.0.769.g3c40516874-goog > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel