From: Junhao He <[email protected]>
The PMU can monitor traffic of certain target Root Port or downstream
target Endpoint. User can specify the target filter by the "port" or
"bdf" option respectively. The PMU can only monitor the Root Port or
Endpoint on the same PCIe core so the value of "port" or "bdf" should
be valid and will be checked by the driver.
Currently at least and only one of "port" and "bdf" option must be set.
If "port" filter is not set or is set explicitly to zero (default),
driver will regard the user specifies a "bdf" option since "port" option
is a bitmask of the target Root Ports and zero is not a valid
value.
If user not explicitly set "port" or "bdf" filter, the driver uses "bdf"
default value (zero) to set target filter, but driver will skip the
check of bdf=0, although it's a valid value (meaning 0000:000:00.0).
Then the user just gets zero.
Therefore, we need to check if both "port" and "bdf" are invalid, then
return failure and report warning.
Testing:
before the patch:
0 hisi_pcie0_core1/rx_mrd_flux/
0 hisi_pcie0_core1/rx_mrd_flux,port=0/
24,124 hisi_pcie0_core1/rx_mrd_flux,port=1/
0 hisi_pcie0_core1/rx_mrd_flux,bdf=0/
0 hisi_pcie0_core1/rx_mrd_flux,port=0x800/
<not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=1/
24,132 hisi_pcie0_core1/rx_mrd_flux,bdf=0x1700/
<not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x0/
<not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1/
24,138 hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1700/
24,126 hisi_pcie0_core1/rx_mrd_flux,port=0x1,bdf=0x0/
after the patch:
<not supported> hisi_pcie0_core1/rx_mrd_flux/
<not supported> hisi_pcie0_core1/rx_mrd_flux,port=0/
24,153 hisi_pcie0_core1/rx_mrd_flux,port=1/
0 hisi_pcie0_core1/rx_mrd_flux,port=0x800/
<not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=0/
<not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=1/
24,117 hisi_pcie0_core1/rx_mrd_flux,bdf=0x1700/
<not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x0/
<not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1/
24,120 hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1700/
24,123 hisi_pcie0_core1/rx_mrd_flux,port=0x1,bdf=0x0/
Signed-off-by: Junhao He <[email protected]>
Signed-off-by: Yicong Yang <[email protected]>
---
drivers/perf/hisilicon/hisi_pcie_pmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
index 6f39cb82661e..b2dde7559639 100644
--- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
+++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
@@ -306,10 +306,10 @@ static bool hisi_pcie_pmu_valid_filter(struct perf_event *event,
if (hisi_pcie_get_trig_len(event) > HISI_PCIE_TRIG_MAX_VAL)
return false;
- if (requester_id) {
- if (!hisi_pcie_pmu_valid_requester_id(pcie_pmu, requester_id))
- return false;
- }
+ /* Need to explicitly set filter of "port" or "bdf" */
+ if (!hisi_pcie_get_port(event) &&
+ !hisi_pcie_pmu_valid_requester_id(pcie_pmu, requester_id))
+ return false;
return true;
}
--
2.24.0
On Fri, 23 Feb 2024 18:33:56 +0800
Yicong Yang <[email protected]> wrote:
> From: Junhao He <[email protected]>
>
> The PMU can monitor traffic of certain target Root Port or downstream
> target Endpoint. User can specify the target filter by the "port" or
> "bdf" option respectively. The PMU can only monitor the Root Port or
> Endpoint on the same PCIe core so the value of "port" or "bdf" should
> be valid and will be checked by the driver.
>
> Currently at least and only one of "port" and "bdf" option must be set.
> If "port" filter is not set or is set explicitly to zero (default),
> driver will regard the user specifies a "bdf" option since "port" option
> is a bitmask of the target Root Ports and zero is not a valid
> value.
>
> If user not explicitly set "port" or "bdf" filter, the driver uses "bdf"
> default value (zero) to set target filter, but driver will skip the
> check of bdf=0, although it's a valid value (meaning 0000:000:00.0).
> Then the user just gets zero.
>
> Therefore, we need to check if both "port" and "bdf" are invalid, then
> return failure and report warning.
>
> Testing:
> before the patch:
> 0 hisi_pcie0_core1/rx_mrd_flux/
> 0 hisi_pcie0_core1/rx_mrd_flux,port=0/
> 24,124 hisi_pcie0_core1/rx_mrd_flux,port=1/
> 0 hisi_pcie0_core1/rx_mrd_flux,bdf=0/
> 0 hisi_pcie0_core1/rx_mrd_flux,port=0x800/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=1/
> 24,132 hisi_pcie0_core1/rx_mrd_flux,bdf=0x1700/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x0/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1/
> 24,138 hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1700/
> 24,126 hisi_pcie0_core1/rx_mrd_flux,port=0x1,bdf=0x0/
>
> after the patch:
> <not supported> hisi_pcie0_core1/rx_mrd_flux/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,port=0/
> 24,153 hisi_pcie0_core1/rx_mrd_flux,port=1/
> 0 hisi_pcie0_core1/rx_mrd_flux,port=0x800/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=0/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,bdf=1/
> 24,117 hisi_pcie0_core1/rx_mrd_flux,bdf=0x1700/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x0/
> <not supported> hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1/
> 24,120 hisi_pcie0_core1/rx_mrd_flux,port=0x0,bdf=0x1700/
> 24,123 hisi_pcie0_core1/rx_mrd_flux,port=0x1,bdf=0x0/
>
> Signed-off-by: Junhao He <[email protected]>
> Signed-off-by: Yicong Yang <[email protected]>
Thanks for explanation on v1. I'm fine with this now.
Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> drivers/perf/hisilicon/hisi_pcie_pmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> index 6f39cb82661e..b2dde7559639 100644
> --- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> @@ -306,10 +306,10 @@ static bool hisi_pcie_pmu_valid_filter(struct perf_event *event,
> if (hisi_pcie_get_trig_len(event) > HISI_PCIE_TRIG_MAX_VAL)
> return false;
>
> - if (requester_id) {
> - if (!hisi_pcie_pmu_valid_requester_id(pcie_pmu, requester_id))
> - return false;
> - }
> + /* Need to explicitly set filter of "port" or "bdf" */
> + if (!hisi_pcie_get_port(event) &&
> + !hisi_pcie_pmu_valid_requester_id(pcie_pmu, requester_id))
> + return false;
>
> return true;
> }