Received: by 2002:a05:7412:da14:b0:e2:908c:2ebd with SMTP id fe20csp2345120rdb; Tue, 10 Oct 2023 00:35:32 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH9J1bSotTC20ykSkKMd54XVtdaus/UyQQgF2Zngo0IDOjLLGPjyYeGZRW+um0MrrolBRHm X-Received: by 2002:a05:6870:888b:b0:1c8:bfd1:ecb8 with SMTP id m11-20020a056870888b00b001c8bfd1ecb8mr20715871oam.30.1696923332573; Tue, 10 Oct 2023 00:35:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696923332; cv=none; d=google.com; s=arc-20160816; b=TpcIHRmcMqmP7eH5spythelGUt9ONhAiyLWgtoxXa7Nqcz1GP2TkkHoWACOigRGP5X NfVREfmXm3eq2/BK+G0veDdMrpU4gPrQ/C9qBi/igb4JPMtOX9fm5dNMupSNbwHc4ExZ mM/eSEntSWn2QXBOv8G6ruSqyHQ+SgsTToPo/9fc0gGQ1xNjrclNBowA8rpayCMfirAp OMoZglYQS4fGZRMfu+aAjoxe3Olbg425iTVqrbOiBvpMv8gPRbyb/ztrH+GruA8u/DFc TY0jQPA1VC8u+so3YjXRoklquXXdh2pQPMOrVvFSgZ3oABgD4HCxxh+6NkTKDBuVYY6C 9IHA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:references:to:subject :cc; bh=3sZKtc5eXJPBgdKNYl/5Imk5vyfn3dQg3UKH7ag2ujw=; fh=/2qddH48XrjKoVMYkUXu+eNcF9RsF3zVE3BkXFDs6fI=; b=nHe6f9cWUxvwRtS6tpi2O6n1ecB6vvUN5UMIknA2oPQD/CM+r5av9KNBzmQliCHO48 9uj5Nlqa8/SOXmECO15akiN2DePzgUUiroBd8pr1po9bz4tlU2tmkQV6sDta7EKW+yXa 0sK87xWWxOhvvL6TSiuRjsxXCoiYJ+Wj2RCrVXAITP15u/+Aozg/nYadWJXsprxk1Ywy UXrSXD6dOryCiKRB4FBW9+h1K4LChJwB7JdAfj4UvvaXiNve0VDpzMXV3q08IGu9n/km KY0nefFP9TYqBsrvg/CBqVT9O4BTZNTZZwzNiEnvnRPvdNZZlB6izY4cz1k/xx5JIraj BF5g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Return-Path: Received: from lipwig.vger.email (lipwig.vger.email. [23.128.96.33]) by mx.google.com with ESMTPS id l133-20020a633e8b000000b0054fdfd26dffsi12033411pga.337.2023.10.10.00.35.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 00:35:32 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id A581E802466C; Tue, 10 Oct 2023 00:35:29 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1442460AbjJJHfT (ORCPT + 99 others); Tue, 10 Oct 2023 03:35:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1442324AbjJJHfS (ORCPT ); Tue, 10 Oct 2023 03:35:18 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A867C92; Tue, 10 Oct 2023 00:35:15 -0700 (PDT) Received: from canpemm500009.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4S4SHK5Vn0ztSwY; Tue, 10 Oct 2023 15:30:37 +0800 (CST) Received: from [10.67.121.177] (10.67.121.177) by canpemm500009.china.huawei.com (7.192.105.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Tue, 10 Oct 2023 15:35:13 +0800 CC: , , , , , , , , , , , , Subject: Re: [PATCH v6 3/4] drivers/perf: add DesignWare PCIe PMU driver To: Shuai Xue , Jonathan Cameron References: <20230606074938.97724-1-xueshuai@linux.alibaba.com> <20230606074938.97724-4-xueshuai@linux.alibaba.com> <31e2b012-3a29-d063-842d-e3f7736816e7@huawei.com> <20230727103929.00000544@Huawei.com> <12958abe-4bdb-8532-bf67-8e772ed2a9dd@linux.alibaba.com> <10176239-78c1-e276-e74f-a6e79b9c0751@linux.alibaba.com> <1ebf5862-441f-729a-32c1-8e59ac452168@linux.alibaba.com> <262abac1-39ea-7612-e198-6bc08c76dd39@linux.alibaba.com> From: Yicong Yang Message-ID: <0e4f5e36-57bb-8c3e-4757-b1b8ff76c09d@huawei.com> Date: Tue, 10 Oct 2023 15:35:13 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.1 MIME-Version: 1.0 In-Reply-To: <262abac1-39ea-7612-e198-6bc08c76dd39@linux.alibaba.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.177] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To canpemm500009.china.huawei.com (7.192.105.203) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-0.5 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 10 Oct 2023 00:35:29 -0700 (PDT) On 2023/10/9 21:08, Shuai Xue wrote: > > > On 2023/8/4 11:09, Shuai Xue wrote: >> >> >> On 2023/8/4 10:28, Yicong Yang wrote: >>> On 2023/8/4 9:39, Shuai Xue wrote: >>>> >>>> >>>> On 2023/8/1 19:46, Yicong Yang wrote: >>>>> On 2023/7/28 20:41, Shuai Xue wrote: >>>>>> >>>>>> >>>>>> On 2023/7/27 17:39, Jonathan Cameron wrote: >>>>>>> On Tue, 6 Jun 2023 23:14:07 +0800 >>>>>>> Yicong Yang wrote: >>>>>>> >>>>>>>> On 2023/6/6 15:49, Shuai Xue wrote: >>>>>>>>> This commit adds the PCIe Performance Monitoring Unit (PMU) driver support >>>>>>>>> for T-Head Yitian SoC chip. Yitian is based on the Synopsys PCI Express >>>>>>>>> Core controller IP which provides statistics feature. The PMU is not a PCIe >>>>>>>>> Root Complex integrated End Point(RCiEP) device but only register counters >>>>>>>>> provided by each PCIe Root Port. >>>>>>>>> >>>>>>>>> To facilitate collection of statistics the controller provides the >>>>>>>>> following two features for each Root Port: >>>>>>>>> >>>>>>>>> - Time Based Analysis (RX/TX data throughput and time spent in each >>>>>>>>> low-power LTSSM state) >>>>>>>>> - Event counters (Error and Non-Error for lanes) >>>>>>>>> >>>>>>>>> Note, only one counter for each type and does not overflow interrupt. >>>>>>>>> >>>>>>>>> This driver adds PMU devices for each PCIe Root Port. And the PMU device is >>>>>>>>> named based the BDF of Root Port. For example, >>>>>>>>> >>>>>>>>> 30:03.0 PCI bridge: Device 1ded:8000 (rev 01) >>>>>>>>> >>>>>>>>> the PMU device name for this Root Port is dwc_rootport_3018. >>>>>>>>> >>>>>>>>> Example usage of counting PCIe RX TLP data payload (Units of 16 bytes):: >>>>>>>>> >>>>>>>>> $# perf stat -a -e dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/ >>>>>>>>> >>>>>>>>> average RX bandwidth can be calculated like this: >>>>>>>>> >>>>>>>>> PCIe TX Bandwidth = PCIE_TX_DATA * 16B / Measure_Time_Window >>>>>>>>> >>>>>>>>> Signed-off-by: Shuai Xue >>>>>>>>> Reported-by: kernel test robot >>>>>>>>> Link: https://lore.kernel.org/oe-kbuild-all/202305170639.XU3djFZX-lkp@intel.com/ >>>>>>>>> Reviewed-by: Baolin Wang >>>>>>> >>>>>>> I'll review on top to avoid any duplication with Yicong. >>>>>> >>>>>> Thank you! It also served as a reminder that I missed Yicong's email. It appears >>>>>> that Thunderbird mistakenly moved his email to the junk folder, resulting in me >>>>>> overlooking it. >>>>>> >>>>>>> >>>>>>> Note I've cropped the stuff neither of us commented on so it's >>>>>>> easier to spot the feedback. >>>>>> >>>>>> Thank you for noting that. My feedback is replied inline. >>>>>> >>>>>>> >>>>>>> Jonathan >>>>>>> >>>>>>>>> --- >>>>>>>>> drivers/perf/Kconfig | 7 + >>>>>>>>> drivers/perf/Makefile | 1 + >>>>>>>>> drivers/perf/dwc_pcie_pmu.c | 706 ++++++++++++++++++++++++++++++++++++ >>>>>>>>> 3 files changed, 714 insertions(+) >>>>>>>>> create mode 100644 drivers/perf/dwc_pcie_pmu.c >>>>>>>>> >>>>>>>>> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig >>>>>>>>> index 711f82400086..6ff3921d7a62 100644 >>>>>>>>> --- a/drivers/perf/Kconfig >>>>>>>>> +++ b/drivers/perf/Kconfig >>>>>>>>> @@ -209,6 +209,13 @@ config MARVELL_CN10K_DDR_PMU >>>>>>>>> Enable perf support for Marvell DDR Performance monitoring >>>>>>>>> event on CN10K platform. >>>>>>>>> >>>>>>>>> +config DWC_PCIE_PMU >>>>>>>>> + tristate "Enable Synopsys DesignWare PCIe PMU Support" >>>>>>>>> + depends on (ARM64 && PCI) >>>>>>>>> + help >>>>>>>>> + Enable perf support for Synopsys DesignWare PCIe PMU Performance >>>>>>>>> + monitoring event on Yitian 710 platform. >>>>>>> >>>>>>> The documentation kind of implies this isn't platform specific. >>>>>>> If some parts are (such as which events exist) then you may want to push >>>>>>> that to userspace / perftool with appropriate matching against specific SoC. >>>>>>> >>>>>>> If it is generic, then change this text to "event on platform including the Yitian 710." >>>>>> >>>>>> It is generic without any platform specific, so I will change it as you expected. >>>>>> >>>>>>> >>>>>>>>> + >>>>>>>>> source "drivers/perf/arm_cspmu/Kconfig" >>>>>>>>> >>>>>>>>> source "drivers/perf/amlogic/Kconfig" >>>>>>> >>>>>>>>> new file mode 100644 >>>>>>>>> index 000000000000..8bfcf6e0662d >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/drivers/perf/dwc_pcie_pmu.c >>>>>>>>> @@ -0,0 +1,706 @@ >>>>>>> >>>>>>> ... >>>>>>> >>>>>>>>> + >>>>>>>>> +struct dwc_pcie_pmu { >>>>>>>>> + struct pci_dev *pdev; /* Root Port device */ >>>>>>>> >>>>>>>> If the root port removed after the probe of this PCIe PMU driver, we'll access the NULL >>>>>>>> pointer. I didn't see you hold the root port to avoid the removal. >>>>>> >>>>>> Do you mean that I should have a reference count of rootport by pci_dev_get() when allocating >>>>>> pcie_pmu? >>>>>> >>>>>> pcie_pmu->pdev = pci_dev_get(); >>>>> >>>>> It could be one option, but will block the removal of device from userspace. Another option >>>>> is to register a PCI bus notifier then on removal/added the driver can get notified and handle >>>>> it, for example, remove the related PMU on the removal of the root ports. >>>> >>>> I see, but can root port be removed from userspace? I check the hotplug slot interface, no root >>>> port is available to power off. >>>> >>> >>> For hotplug maybe not, but user can remove certian device through sysfs: >>> >>> echo 1 > /sys/bus/pci/devices//remove >>> >> >> Thank you, I will add a notifier for removal/added action. >> >> Best Regards, >> Shuai > > Hi, Yicong, > > I am confused when adding a notifier by bus_register_notifier(). If I have added a action to > pdev->dev to unregister pmu: > > ret = perf_pmu_register(&pcie_pmu->pmu, name, -1); > if (ret) { > pci_err(pcie_pmu->pdev, > "Error %d registering PMU @%x\n", ret, bdf); > goto out; > } > ret = devm_add_action_or_reset( > &pdev->dev, dwc_pcie_pmu_unregister_pmu, &pcie_pmu->pmu); > > the pmu will be unregister when the port removes, so accessing the NULL pointer will never happen, > right? Do we still need the bus_register_notifier()? > Not necessary, a notifier is used to notice the device removal and avoid dereferencing the NULL pointer. If you find another way like above to avoid the issue it will be ok. Since your pmu is 1:1 related to the root port, add a devm action to unregister the PMU on root port removal looks better. Thanks.