| Message ID | 20240223103359.18669-6-yangyicong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drivers/perf: hisi_pcie: Several updates for HiSilicon PCIe PMU driver | expand |
On Fri, 23 Feb 2024 18:33:56 +0800 Yicong Yang <yangyicong@huawei.com> wrote: > From: Junhao He <hejunhao3@huawei.com> > > 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 <hejunhao3@huawei.com> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> Thanks for explanation on v1. I'm fine with this now. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > 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; > }
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; }