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 B3B74C54798 for ; Fri, 23 Feb 2024 11:26:57 +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=JJ/TQ6clvLtkzzs+3il+pRKfmuSixcIxBeMNjsGRIow=; b=cMnl3EdvBH+8zY 5XJU8D9jimyeyilmkK+3Z43UOY6ib5ZBqpUtJ/uoWv/PEmVvXShiGDlSodMzkwtfskmTiiMwKMsXU KyTO7VUficU1NonDhfuN2K3zIrblx4Q9lk5KjmY1hiIRZBsREQr9/MMvGa/ToYrwvyIUEFLIxQSmz KNC0F+qPo/6Qg8BUsBzm7hQxH/6IG+MskQpxwJF8PJW9bGZEaR97vBaf/Nx5QpKrKUi4+Rf1nDPvI tz1kBvKAZRWNNDZvnZGBBoS87B9qSaYbVddv4cvove2yC9Hp+wULLKy3rrIdeHCGYSjljm6d4Vpwx U5+dveiTd9z9mmICWXgQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdThV-000000096yE-0j2s; Fri, 23 Feb 2024 11:26:45 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdThQ-000000096rL-3Awm for linux-arm-kernel@lists.infradead.org; Fri, 23 Feb 2024 11:26:42 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0B91063459; Fri, 23 Feb 2024 11:26:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC829C43390; Fri, 23 Feb 2024 11:26:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1708687599; bh=pPKLLS6SVTpg1tKiUiKHfl/jdui9YbrMYB1Xo5+H3qw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=p1VBBDuws5kOxlmM1rNRc6tfcBgeJ2kZK52q+QUTB5RLSJfE2lKpDbEhCMyDz540s y7GndBI06OoCvSFiXEBS9MvP8dvCgbvSSdd1cm2VIWRnmGIcB8CE4tC0mRBuOtBWot 0Uvo0VyLpfGF2I5xkN0J5vpjPmx/5zkNqXkX6HMDVZHMkhD9S+rs3dZib/JrtyS1K2 svl5QMxTb+rCBVM2+QrSXXLE2xF6Jad4uIU5mzlKZoBgzsNuWjcbnDalrEf2BdxYf9 3fmPGEoT9ZmVbm1iGWRJwCpHDN7tJ7zK76OGAzGjR1xQFMKaqD7nRNLh6+yQZkyEM5 CnPZ0fRkDk6/w== Date: Fri, 23 Feb 2024 11:26:33 +0000 From: Will Deacon To: Ji Sheng Teoh Cc: Jonathan Corbet , Mark Rutland , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Dan Williams , Ilkka Koskinen , Jonathan Cameron , Dave Jiang , Ley Foon Tan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v6 1/4] perf: starfive: Add StarLink PMU support Message-ID: <20240223112633.GA10403@willie-the-truck> References: <20240129095141.3262366-1-jisheng.teoh@starfivetech.com> <20240129095141.3262366-2-jisheng.teoh@starfivetech.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240129095141.3262366-2-jisheng.teoh@starfivetech.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240223_032641_058530_02B4D253 X-CRM114-Status: GOOD ( 28.98 ) 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 Hi, On Mon, Jan 29, 2024 at 05:51:38PM +0800, Ji Sheng Teoh wrote: > This patch adds support for StarFive's StarLink PMU (Performance > Monitor Unit). StarLink PMU integrates one or more CPU cores with > a shared L3 memory system. The PMU supports overflow interrupt, > up to 16 programmable 64bit event counters, and an independent > 64bit cycle counter. StarLink PMU is accessed via MMIO. Since Palmer acked this (thanks!), I queued it locally but then ran into a few small issues with my build testing. Comments below. > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig > index 273d67ecf6d2..41278742ef88 100644 > --- a/drivers/perf/Kconfig > +++ b/drivers/perf/Kconfig > @@ -86,6 +86,15 @@ config RISCV_PMU_SBI > full perf feature support i.e. counter overflow, privilege mode > filtering, counter configuration. > > +config STARFIVE_STARLINK_PMU > + depends on ARCH_STARFIVE Please can you add "|| COMPILE_TEST" to this dependency so that you get build coverage from other architectures? > + bool "StarFive StarLink PMU" > + help > + Provide support for StarLink Performance Monitor Unit. > + StarLink Performance Monitor Unit integrates one or more cores with > + an L3 memory system. The L3 cache events are added into perf event > + subsystem, allowing monitoring of various L3 cache perf events. > + > config ARM_PMU_ACPI > depends on ARM_PMU && ACPI > def_bool y [...] > diff --git a/drivers/perf/starfive_starlink_pmu.c b/drivers/perf/starfive_starlink_pmu.c > new file mode 100644 > index 000000000000..2447ca09a471 > --- /dev/null > +++ b/drivers/perf/starfive_starlink_pmu.c > @@ -0,0 +1,643 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * StarFive's StarLink PMU driver > + * > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > + * > + * Author: Ji Sheng Teoh > + * > + */ [...] > +static void starlink_pmu_counter_start(struct perf_event *event, > + struct starlink_pmu *starlink_pmu) > +{ > + struct hw_perf_event *hwc = &event->hw; > + int idx = event->hw.idx; > + u64 val; > + > + /* > + * Enable counter overflow interrupt[63:0], > + * which is mapped as follow: > + * > + * event counter 0 - Bit [0] > + * event counter 1 - Bit [1] > + * ... > + * cycle counter - Bit [63] > + */ > + val = readq(starlink_pmu->pmu_base + STARLINK_PMU_INTERRUPT_ENABLE); > + > + if (hwc->config == STARLINK_CYCLES) { > + /* > + * Cycle count has its dedicated register, and it starts > + * counting as soon as STARLINK_PMU_GLOBAL_ENABLE is set. > + */ > + val |= STARLINK_PMU_CYCLE_OVERFLOW_MASK; > + } else { > + writeq(event->hw.config, starlink_pmu->pmu_base + > + STARLINK_PMU_EVENT_SELECT + idx * sizeof(u64)); > + > + val |= (1 << idx); > + } I think this needs to be a u64 on the right hand side, or just use the BIT_ULL() macro. > + > + writeq(val, starlink_pmu->pmu_base + STARLINK_PMU_INTERRUPT_ENABLE); > + > + writeq(STARLINK_PMU_GLOBAL_ENABLE, starlink_pmu->pmu_base + > + STARLINK_PMU_CONTROL); > +} [...] > +static irqreturn_t starlink_pmu_handle_irq(int irq_num, void *data) > +{ > + struct starlink_pmu *starlink_pmu = data; > + struct starlink_hw_events *hw_events = > + this_cpu_ptr(starlink_pmu->hw_events); > + bool handled = false; > + int idx; > + u64 overflow_status; > + > + for (idx = 0; idx < STARLINK_PMU_MAX_COUNTERS; idx++) { > + struct perf_event *event = hw_events->events[idx]; > + > + if (!event) > + continue; > + > + overflow_status = readq(starlink_pmu->pmu_base + > + STARLINK_PMU_COUNTER_OVERFLOW_STATUS); > + if (!(overflow_status & BIT(idx))) > + continue; > + > + writeq(1 << idx, starlink_pmu->pmu_base + > + STARLINK_PMU_COUNTER_OVERFLOW_STATUS); Same shifting problem here. > +static int starlink_pmu_probe(struct platform_device *pdev) > +{ > + struct starlink_pmu *starlink_pmu; > + struct starlink_hw_events *hw_events; > + struct resource *res; > + int cpuid, i, ret; > + > + starlink_pmu = devm_kzalloc(&pdev->dev, sizeof(*starlink_pmu), GFP_KERNEL); > + if (!starlink_pmu) > + return -ENOMEM; > + > + starlink_pmu->pmu_base = > + devm_platform_get_and_ioremap_resource(pdev, 0, &res); > + if (IS_ERR(starlink_pmu->pmu_base)) > + return PTR_ERR(starlink_pmu->pmu_base); > + > + starlink_pmu->hw_events = alloc_percpu_gfp(struct starlink_hw_events, > + GFP_KERNEL); > + if (!starlink_pmu->hw_events) { > + dev_err(&pdev->dev, "Failed to allocate per-cpu PMU data\n"); > + kfree(starlink_pmu); You shouldn't call kfree() on a device-managed object (i.e. allocated with devm_kzalloc()). Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel