| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4147832a-1f24-5993-dfb6-59f420a17481@linux.alibaba.com>
Date: Mon, 16 Oct 2023 11:00:13 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: chengyou@...ux.alibaba.com, kaishen@...ux.alibaba.com,
yangyicong@...wei.com, will@...nel.org,
Jonathan.Cameron@...wei.com, baolin.wang@...ux.alibaba.com,
robin.murphy@....com, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
rdunlap@...radead.org, mark.rutland@....com,
zhuo.song@...ux.alibaba.com, renyu.zj@...ux.alibaba.com
Subject: Re: [PATCH v7 3/4] drivers/perf: add DesignWare PCIe PMU driver
On 2023/10/14 00:30, Bjorn Helgaas wrote:
> On Fri, Oct 13, 2023 at 11:46:44AM +0800, Shuai Xue wrote:
>>
>>
>> On 2023/10/13 00:25, Bjorn Helgaas wrote:
>>> On Thu, Oct 12, 2023 at 11:28:55AM +0800, 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.
>
> IIUC, the PMU is directly integrated into the Root Port: it's
> discovered and operated via the Root Port config space. If so, I
> wouldn't bother mentioning RCiEP because there's no need to list all
> the things it's *not*.
I see, will not mention RCiEP next version.
>
>>>> 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.
>>>
>>> Not sure what "does not overflow interrupt" means. Does it mean
>>> there's no interrupt generated when the counter overflows?
>>
>> Yes, exactly. The rootport does NOT generate interrupt when the
>> couter overflows. I think the assumption hidden in this design is
>> 64-bit counter will not overflow within observable time.
>>
>> PCIe 5.0 slots can now reach anywhere between ~4GB/sec for a x1 slot
>> up to ~64GB/sec for a x16 slot. The unit of counter is 16 byte.
>>
>> 2^64/(64/16*10^9)/60/60/24/365=146 years
>>
>> so, the counter will not overflow within 146 years.
>
> Certainly a reasonable assumption :)
>
> But I'm confused about how many counters there are. Clearly there are
> two features ((1) time-based analysis and (2) event counters).
>
> "One counter for each type" suggests there's one counter for
> time-based analysis and a second counter for event counting,
You are right, two counters, TIME_BASED_ANAL_DATA_REG for time-based
analysis and EVENT_COUNTER_DATA_REG for event counting. And both of them
do not support generate interrupt when the counter overflows.
> but from
> dwc_pcie_pmu_event_add(), it looks like each Root Port might have a
> single counter, and you can decide whether that counter is used for
> time-based analysis or event counting, but you can't do both at the
> same time?
dwc_pcie_pmu_event_add() now limit the PMU usage to stat event for either
time-based analysis or event counting.
I will extend to support stat them at the same time.
@@ -447,10 +447,10 @@ static int dwc_pcie_pmu_event_add(struct perf_event *event, int flags)
u32 ctrl;
/* Only one counter and it is in use */
- if (pcie_pmu->event)
+ if (pcie_pmu->event[type])
return -ENOSPC;
- pcie_pmu->event = event;
+ pcie_pmu->event[type] = event;
hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
if (type == DWC_PCIE_LANE_EVENT) {
@@ -486,10 +486,11 @@ static int dwc_pcie_pmu_event_add(struct perf_event *event, int flags)
static void dwc_pcie_pmu_event_del(struct perf_event *event, int flags)
{
struct dwc_pcie_pmu *pcie_pmu = to_dwc_pcie_pmu(event->pmu);
+ enum dwc_pcie_event_type type = DWC_PCIE_EVENT_TYPE(event);
dwc_pcie_pmu_event_stop(event, flags | PERF_EF_UPDATE);
perf_event_update_userpage(event);
- pcie_pmu->event = NULL;
+ pcie_pmu->event[type] = NULL;
}
> And the event counting is for a single lane, not for the
> link as a whole?
Yes.
>
> If so, I might word this as:
>
> Each Root Port contains one counter that can be used for either:
>
> - Time-Based Analysis (RX/TX data throughput and time spent in
> each low-power LTSSM state) or
>
> - Event counting (error and non-error events for a specified lane)
>
> There is no interrupt for counter overflow.
Based on above, I change the word to:
To facilitate collection of statistics the controller provides the
following two features for each Root Port:
- one 64-bit counter for Time Based Analysis (RX/TX data throughput and
time spent in each low-power LTSSM state) and
- one 32-bit counter for Event counting (error and non-error events for
a specified lane)
Note: There is no interrupt for counter overflow.
>
>>>> + Enable perf support for Synopsys DesignWare PCIe PMU Performance
>>>> + monitoring event on platform including the Yitian 710.
>>>
>>> Should this mention Alibaba or T-Head? I don't know how
>>> Alibaba/T-Head/Yitian are all related.
>>
>> The server chips, named Yitian 710, are custom-built by Alibaba Group's chip
>> development business, T-Head.
>>
>> Enable perf support for Synopsys DesignWare PCIe PMU Performance
>> monitoring event on platform including the Alibaba Yitian 710.
>>
>> Is this okay?
>
> Perfect :)
Thank you for valuable comments.
Best Regards,
Shuai
Powered by blists - more mailing lists