[PATCH v2 1/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU
John Garry
john.garry at huawei.com
Mon Apr 12 18:21:24 BST 2021
On 12/04/2021 14:34, liuqi (BA) wrote:
>
> Hi John,
>
> Thanks for reviewing this.
> On 2021/4/9 18:22, John Garry wrote:
>> On 09/04/2021 10:05, Qi Liu wrote:
>>> PCIe PMU Root Complex Integrated End Point(RCiEP) device is supported
>>> to sample bandwidth, latency, buffer occupation etc.
>>>
>>> Each PMU RCiEP device monitors multiple Root Ports, and each RCiEP is
>>> registered as a PMU in /sys/bus/event_source/devices, so users can
>>> select target PMU, and use filter to do further sets.
side note: it would be good to mention what baseline the series is based
on in the cover letter
>>>
>>> Filtering options contains:
>>> event - select the event.
>>> subevent - select the subevent.
>>> port - select target Root Ports. Information of Root Ports
>>> are shown under sysfs.
>>> bdf - select requester_id of target EP device.
>>> trig_len - set trigger condition for starting event statistics.
>>> trigger_mode - set trigger mode. 0 means starting to statistic when
>>> bigger than trigger condition, and 1 means smaller.
>>> thr_len - set threshold for statistics.
>>> thr_mode - set threshold mode. 0 means count when bigger than
>>> threshold, and 1 means smaller.
>>>
>>> Signed-off-by: Qi Liu <liuqi115 at huawei.com>
>>> ---
>>> MAINTAINERS | 6 +
>>> drivers/perf/Kconfig | 2 +
>>> drivers/perf/Makefile | 1 +
>>> drivers/perf/pci/Kconfig | 16 +
>>> drivers/perf/pci/Makefile | 2 +
>>> drivers/perf/pci/hisilicon/Makefile | 5 +
>>> drivers/perf/pci/hisilicon/hisi_pcie_pmu.c | 1011
>>> ++++++++++++++++++++++++++++
>>> include/linux/cpuhotplug.h | 1 +
>>> 8 files changed, 1044 insertions(+)
>>> create mode 100644 drivers/perf/pci/Kconfig
>>> create mode 100644 drivers/perf/pci/Makefile
>>> create mode 100644 drivers/perf/pci/hisilicon/Makefile
>>> create mode 100644 drivers/perf/pci/hisilicon/hisi_pcie_pmu.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 3353de0..46c7861 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -8023,6 +8023,12 @@ W: http://www.hisilicon.com
>>> F: Documentation/admin-guide/perf/hisi-pmu.rst
>>> F: drivers/perf/hisilicon
>>> +HISILICON PCIE PMU DRIVER
>>> +M: Qi Liu <liuqi115 at huawei.com>
>>> +S: Maintained
>>> +F: Documentation/admin-guide/perf/hisi-pcie-pmu.rst
>>
>> nit: this does not exist yet...
>>
> thanks, I'll move this add-maintainer-part to the second patch.
that's why I advocate the documentation first :)
>>> +F: drivers/perf/pci/hisilicon/hisi_pcie_pmu.c
>>> +
>>> HISILICON QM AND ZIP Controller DRIVER
>>> M: Zhou Wang <wangzhou1 at hisilicon.com>
>>> L: linux-crypto at vger.kernel.org
>>> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
>>> index 3075cf1..99d4760 100644
>>> --- a/drivers/perf/Kconfig
>>> +++ b/drivers/perf/Kconfig
>>> @@ -139,4 +139,6 @@ config ARM_DMC620_PMU
>>> source "drivers/perf/hisilicon/Kconfig"
>>> +source "drivers/perf/pci/Kconfig"
>>> +
>>> endmenu
>>> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
>>> index 5260b11..1208c08 100644
>>> --- a/drivers/perf/Makefile
>>> +++ b/drivers/perf/Makefile
>>> @@ -14,3 +14,4 @@ obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
>>> obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
>>> obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
>>> obj-$(CONFIG_ARM_DMC620_PMU) += arm_dmc620_pmu.o
>>> +obj-y += pci/
>>> diff --git a/drivers/perf/pci/Kconfig b/drivers/perf/pci/Kconfig
>>> new file mode 100644
>>> index 0000000..a119486
>>> --- /dev/null
>>> +++ b/drivers/perf/pci/Kconfig
>>> @@ -0,0 +1,16 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +#
>>> +# PCIe Performance Monitor Drivers
>>> +#
>>> +menu "PCIe Performance Monitor"
>>> +
>>> +config HISI_PCIE_PMU
>>> + tristate "HiSilicon PCIE PERF PMU"
>>> + depends on ARM64 && PCI && HISI_PMU
>>
>> What from HISI_PMU is needed? I couldn't find anything here
> The event_sysfs_show() and format_sysfs_show() function of
> hisi_uncore_pmu.h can be reused in hisi_pcie_pmu.c, So I add path in
> Makefile and include "hisi_uncore_pmu.h".
Right, but it would be nice to be able to build this under COMPILE_TEST.
CONFIG_HISI_PMU cannot be built under COMPILE_TEST, so nice to not
depend on it.
So you could put hisi_event_sysfs_show() as a static inline in
hisi_uncore_pmu.h, so the dependency can be removed
Having said that, there is nothing really hisi specific in those
functions like hisi_event_sysfs_show().
Can't we just create generic functions here?
hisi_event_sysfs_show() == cci400_pmu_cycle_event_show() ==
xgene_pmu_event_show()
>
>>
>>> + help
>>> + Provide support for HiSilicon PCIe performance monitoring unit
>>> (PMU)
>>> + RCiEP devices.
>>> + Adds the PCIe PMU into perf events system for monitoring latency,
>>> + bandwidth etc.
>>> +
>>> +#define HISI_PCIE_CHECK_EVENTS(name, list) \
>>
>> "check" in a function name is too vague, as it does not imply any
>> side-effect from calling this function.
>>
>> And I think "build" or similar would be good to be included in the macro
>> name.
>>
>>> + static inline bool is_##name##_event(u32 idx) \
>>
>> why inline?
>>
>>> +
>>> { \
>>> + if ((idx >= list[0] && idx <= list[1]) || idx == list[2]) \
>>> + return true; \
>>> + return false; \
>>
>> isn't this just:
>> {
>> return (idx >= list[0] && idx <= list[1]) || idx == list[2])
>> }
>
> thanks, will address in next version.
>>
>>> + }
>>> +
>>> +/*
>>> + * The first element of event list is minimum index of TL-layer events
>>> + * and the second element is maximum index. The third element is index
>>> + * of a DL-layer event.
>>> + */
>>> +static const u32 bw_events_list[] = {0x04, 0x08, 0x84};
>>> +static const u32 latency_events_list[] = {0x10, 0x15, 0x85};
>>> +static const u32 bus_util_events_list[] = {0x20, 0x24, 0x09};
>>> +static const u32 buf_util_events_list[] = {0x28, 0x2a, 0x33};
>>> +
>>> +HISI_PCIE_CHECK_EVENTS(bw, bw_events_list);
>>> +HISI_PCIE_CHECK_EVENTS(latency, latency_events_list);
>>> +HISI_PCIE_CHECK_EVENTS(bus_util, bus_util_events_list);
>>> +HISI_PCIE_CHECK_EVENTS(buf_util, buf_util_events_list);
>>
>> Surely the macro can be fixed such that you need to have:
>> HISI_PCIE_CHECK_EVENTS(buf_util);
>>
> will address in next version.
>>
>>> +
>>> +static ssize_t hisi_pcie_cpumask_show(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(dev_get_drvdata(dev));
>>> +
>>> + return cpumap_print_to_pagebuf(true, buf, &pcie_pmu->cpumask);
>>> +}
>>> +
>>> +static ssize_t hisi_pcie_identifier_show(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(dev_get_drvdata(dev));
>>> +
>>> + return sysfs_emit(buf, "0x%x\n", pcie_pmu->identifier);
>>> +}
>>> +
>>> +static ssize_t hisi_pcie_bus_show(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(dev_get_drvdata(dev));
>>> +
>>> + return sysfs_emit(buf, "0x%02x\n", PCI_BUS_NUM(pcie_pmu->bdf_min));
>>> +}
>>> +
>>> +static void hisi_pcie_parse_data(struct hisi_pcie_pmu *pcie_pmu, u32
>>> reg_off,
>>
>> hisi_pcie_parse_data() name is way too vague. I mean, parse what data?
>>
>> is there is some standard format register we use for various data types,
>> then make that clear please
> Seems that we only have lower_32_bits() but do not have lower_16_bits().
> perhaps we could change the function name to hisi_pcie_parse_reg_value()?
ok, so even that name is better.
>
>>
>>> + u16 *arg0, u16 *arg1)
>>> +{
>>> + u32 val = readl(pcie_pmu->base + reg_off);
>>> +
>>> + *arg0 = val & 0xffff;
>>> + *arg1 = (val & 0xffff0000) >> 16;
>>> +}
>>> +
>>> +static u32 hisi_pcie_pmu_get_offset(u32 offset, u32 idx)
>>> +{
>>> + return offset + HISI_PCIE_REG_STEP * idx;
>>> +}
>>> +
>>> +static u32 hisi_pcie_pmu_readl(struct hisi_pcie_pmu *pcie_pmu, u32
>>> reg_offset,
>>> + u32 idx)
>>> +{
>>> + u32 offset = hisi_pcie_pmu_get_offset(reg_offset, idx);
>>> +
>>> + return readl(pcie_pmu->base + offset);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_writel(struct hisi_pcie_pmu *pcie_pmu, u32
>>> reg_offset,
>>> + u32 idx, u32 val)
>>> +{
>>> + u32 offset = hisi_pcie_pmu_get_offset(reg_offset, idx);
>>> +
>>> + writel(val, pcie_pmu->base + offset);
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_readq(struct hisi_pcie_pmu *pcie_pmu, u32
>>> reg_offset,
>>> + u32 idx)
>>> +{
>>> + u32 offset = hisi_pcie_pmu_get_offset(reg_offset, idx);
>>> +
>>> + return readq(pcie_pmu->base + offset);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_writeq(struct hisi_pcie_pmu *pcie_pmu, u32
>>> reg_offset,
>>> + u32 idx, u64 val)
>>> +{
>>> + u32 offset = hisi_pcie_pmu_get_offset(reg_offset, idx);
>>> +
>>> + writeq(val, pcie_pmu->base + offset);
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_get_event(struct perf_event *event)
>>
>> that function name is too vague
>>
>>> +{
>>> + return FIELD_PREP(HISI_PCIE_EVENT_M, hisi_pcie_get_event(event)) |
>>> + FIELD_PREP(HISI_PCIE_SUBEVENT_M,
>>> hisi_pcie_get_subevent(event));
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_get_target(struct perf_event *event)
>>
>> same here
>>
>>> +{
>>> + u64 port, val;
>>> +
>>> + port = hisi_pcie_get_port(event);
>>> + if (port) {
>>> + val = FIELD_PREP(HISI_PCIE_TARGET_M, port);
>>> + } else {
>>> + val = FIELD_PREP(HISI_PCIE_TARGET_M, hisi_pcie_get_bdf(event));
>>> + val |= HISI_PCIE_TARGET_EN;
>>> + }
>>> +
>>> + return val;
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_get_trigger(struct perf_event *event)
>>
>> and here
>>
>> what you really seem to be doing is getting the formatted fields for the
>> register
>>
>>
>> Having all these call-once functions seem to make life a bit more
>> difficult...
>>
> Do you mean config all the filters in one function?, will fix it, thanks.
Yes, I would suggest one function
>
>>> +{
>>> + u64 trig_len, val;
>>> +
>>> + trig_len = hisi_pcie_get_trig_len(event);
>>> + if (!trig_len)
>>> + return 0;
>>> +
>>> + val = FIELD_PREP(HISI_PCIE_TRIG_M, trig_len);
>>> + val |= FIELD_PREP(HISI_PCIE_TRIG_MODE_M,
>>> + hisi_pcie_get_trig_mode(event));
>>> + val |= HISI_PCIE_TRIG_EN;
>>> +
>>> + return val;
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_get_thr(struct perf_event *event)
>>> +{
>>> + u64 thr_len, val;
>>> +
>>> + thr_len = hisi_pcie_get_thr_len(event);
>>> + if (!thr_len)
>>> + return 0;
>>> +
>>> + val = FIELD_PREP(HISI_PCIE_THR_M, thr_len);
>>> + val |= FIELD_PREP(HISI_PCIE_THR_MODE_M,
>>> hisi_pcie_get_thr_mode(event));
>>> + val |= HISI_PCIE_THR_EN;
>>> +
>>> + return val;
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_config_filter(struct perf_event *event)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> + struct hw_perf_event *hwc = &event->hw;
>>> + u32 idx = hwc->idx;
>>> + u64 val;
>>> +
>>> + val = HISI_PCIE_DEFAULT_SET;
>>> + val |= hisi_pcie_pmu_get_event(event);
>>> + val |= hisi_pcie_pmu_get_target(event);
>>> + val |= hisi_pcie_pmu_get_trigger(event);
>>> + val |= hisi_pcie_pmu_get_thr(event);
>>> + hisi_pcie_pmu_writeq(pcie_pmu, HISI_PCIE_EVENT_CTRL, idx, val);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_clear_filter(struct perf_event *event)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> + struct hw_perf_event *hwc = &event->hw;
>>> +
>>> + hisi_pcie_pmu_writeq(pcie_pmu, HISI_PCIE_EVENT_CTRL, hwc->idx,
>>> + HISI_PCIE_DEFAULT_SET);
>>> +}
>>> +
>>> +static bool hisi_pcie_pmu_valid_port(struct hisi_pcie_pmu *pcie_pmu,
>>> u16 bdf)
>>> +{
>>> + return bdf <= pcie_pmu->bdf_max && bdf >= pcie_pmu->bdf_min;
>>
>> nit: it is standard to have:
>>
>> if (x >= min && x <= max)
>>
>> i.e. have min on the left
>>
>> 2nd nit: it is passed bdf, yet "port" is in the name
>>
> will fix the function, bdf means the BDF of Root Port, I'll use rp_bdf
> in bext version to describe it.
ok
>>
>>> +}
>>> +
>>> +static bool hisi_pcie_pmu_valid_requester_id(struct hisi_pcie_pmu
>>> *pcie_pmu,
>>> + u32 bdf)
>>> +{
>>> + struct pci_dev *root_port, *pdev;
>>> +
>>> + pdev =
>>> pci_get_domain_bus_and_slot(pci_domain_nr(pcie_pmu->pdev->bus),
>>> + PCI_BUS_NUM(bdf),
>>> + GET_PCI_DEVFN(bdf));
>>> + if (!pdev)
>>> + return false;
>>> +
>>> + root_port = pcie_find_root_port(pdev);
>>> + if (!root_port)
>>> + return false;
>>> +
>>> + pci_dev_put(pdev);
>>> + return hisi_pcie_pmu_valid_port(pcie_pmu, pci_dev_id(root_port));
>>> +}
>>> +
>>> +static bool hisi_pcie_pmu_valid_filter(struct perf_event *event,
>>> + struct hisi_pcie_pmu *pcie_pmu)
>>> +{
>>> + u32 subev_idx = hisi_pcie_get_subevent(event);
>>> + u32 event_idx = hisi_pcie_get_event(event);
>>> + u32 requester_id = hisi_pcie_get_bdf(event);
>>> +
>>> + if (subev_idx > HISI_PCIE_SUBEVENT_MAX ||
>>> + event_idx > HISI_PCIE_EVENT_MAX) {
>>> + pci_err(pcie_pmu->pdev,
>>> + "Max event index and max subevent index is: %d, %d.\n",
>>> + HISI_PCIE_EVENT_MAX, HISI_PCIE_SUBEVENT_MAX);
>>
>> if this is just going to be fed back to userspace, I don't see why we
>> need a kernel log
>>
>> and the only caller also triggers an error message, which I doubt is needed
>>
> Print out the HISI_PCIE_EVENT_MAX and HISI_PCIE_SUBEVENT_MAX here may be
> more convenient for users to get the right value.
> If you this is redundant I'll remove it. :)
Don't we already tell this to userspace?
>>> + return false;
>>> + }
>>> +
>>> + if (hisi_pcie_get_thr_len(event) > HISI_PCIE_THR_MAX_VAL)
>>> + return false;
>>> +
>>> + 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)) {
>>> + pci_err(pcie_pmu->pdev, "Invalid requester id.\n");
>>
>> see previous comments
>>
>>> + return false;
>>> + }
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool hisi_pcie_pmu_validate_event_group(struct perf_event *event)
>>> +{
>>> + struct perf_event *sibling, *leader = event->group_leader;
>>> + int counters = 1;
>>> +
>>> + if (!is_software_event(leader)) {
>>> + if (leader->pmu != event->pmu)
>>> + return false;
>>> +
>>> + if (leader != event)
>>> + counters++;
>>> + }
>>> +
>>> + for_each_sibling_event(sibling, event->group_leader) {
>>> + if (is_software_event(sibling))
>>> + continue;
>>> +
>>> + if (sibling->pmu != event->pmu)
>>> + return false;
>>> +
>>> + counters++;
>>> + }
>>> +
>>> + return counters <= HISI_PCIE_MAX_COUNTERS;
>>> +}
>>> +
>>> +static int hisi_pcie_pmu_event_init(struct perf_event *event)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> +
>>> + event->cpu = cpumask_first(&pcie_pmu->cpumask);
>>> +
>>> + if (event->attr.type != event->pmu->type)
>>> + return -ENOENT;
>>> +
>>> + /* Sampling is not supported. */
>>> + if (is_sampling_event(event) || event->attach_state &
>>> PERF_ATTACH_TASK)
>>> + return -EOPNOTSUPP;
>>> +
>>> + /* Per-task mode is not supported. */
>>> + if (event->cpu < 0)
>>
>> cpumask_first() gives an unsigned int - this can never happen
please fix this!
>>
>>> + return -EINVAL;
>>> +
>>> + if (!hisi_pcie_pmu_valid_filter(event, pcie_pmu)) {
>>> + pci_err(pcie_pmu->pdev, "Invalid filter!\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (!hisi_pcie_pmu_validate_event_group(event))
>>> + return -EINVAL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static u64 hisi_pcie_pmu_process_data(struct perf_event *event, u64 val,
>>> +
>>> +static void hisi_pcie_pmu_disable_int(struct hisi_pcie_pmu *pcie_pmu,
>>> + struct hw_perf_event *hwc)
>>> +{
>>> + u32 idx = hwc->idx;
>>> +
>>> + hisi_pcie_pmu_writel(pcie_pmu, HISI_PCIE_INT_MASK, idx, 1);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_reset_counter(struct hisi_pcie_pmu *pcie_pmu,
>>> + struct hw_perf_event *hwc)
>>> +{
>>> + u32 idx = hwc->idx;
>>> +
>>> + hisi_pcie_pmu_writeq(pcie_pmu, HISI_PCIE_EVENT_CTRL, idx,
>>> + HISI_PCIE_RESET_CNT);
>>> + hisi_pcie_pmu_writeq(pcie_pmu, HISI_PCIE_EVENT_CTRL, idx,
>>> + HISI_PCIE_DEFAULT_SET);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_start(struct perf_event *event, int flags)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> + struct hw_perf_event *hwc = &event->hw;
>>> + struct hw_perf_event_extra *hwc_ext;
>>> + u64 prev_cnt, prev_cnt_ext;
>>> +
>>> + hwc_ext = &hwc->extra_reg;
>>> + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
>>> + return;
>>> +
>>> + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
>>> + hwc->state = 0;
>>> +
>>> + hisi_pcie_pmu_config_filter(event);
>>> + hisi_pcie_pmu_enable_counter(pcie_pmu, hwc);
>>> + hisi_pcie_pmu_enable_int(pcie_pmu, hwc);
>>> + hisi_pcie_pmu_set_period(event);
>>> +
>>> + if (flags & PERF_EF_RELOAD) {
>>> + prev_cnt = local64_read(&hwc->prev_count);
>>> + prev_cnt_ext = hwc_ext->config;
>>> + hisi_pcie_pmu_write_counter(event, prev_cnt, prev_cnt_ext);
>>> + }
>>> +
>>> + perf_event_update_userpage(event);
>>> +}
>>> +
>>> +static void hisi_pcie_pmu_stop(struct perf_event *event, int flags)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> + struct hw_perf_event *hwc = &event->hw;
>>> +
>>> + hisi_pcie_pmu_read(event);
>>
>> why do this - it just reads a register? and so why not check the return
>> value?
>>
> hisi_pcie_pmu_read() is a void function, perhaps I should use
> hisi_pcie_pmu_update_counter().
I am not sure what your intention is with calling hisi_pcie_pmu_read(),
so cannot advise
>>> + hisi_pcie_pmu_disable_int(pcie_pmu, hwc);
>>> + hisi_pcie_pmu_disable_counter(pcie_pmu, hwc);
>>> + hisi_pcie_pmu_clear_filter(event);
>>> + WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
>>> + hwc->state |= PERF_HES_STOPPED;
>>> +
>>> + if (hwc->state & PERF_HES_UPTODATE)
>>> + return;
>>> +
>>> + hwc->state |= PERF_HES_UPTODATE;
>>> +}
>>> +
>>> +static int hisi_pcie_pmu_add(struct perf_event *event, int flags)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = to_pcie_pmu(event->pmu);
>>> + struct hw_perf_event *hwc = &event->hw;
>>> + int idx;
>>> +
>>> + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
>>> + idx = hisi_pcie_pmu_get_event_idx(pcie_pmu);
>>> + if (idx < 0)
>>> + return idx;
>>> +
>>> + hwc->idx = idx;
>>> + pcie_pmu->hw_events[idx] = event;
>>> +
>>> + /* Reset PMC to avoid interference caused by previous statistic. */
>>> +
>>> +static void hisi_pcie_pmu_irq_unregister(struct pci_dev *pdev,
>>> + struct hisi_pcie_pmu *pcie_pmu)
>>> +{
>>> + free_irq(pcie_pmu->irq, pcie_pmu);
>>> + pci_free_irq_vectors(pdev);
>>> +}
>>> +
>>> +static int hisi_pcie_pmu_online_cpu(unsigned int cpu, struct
>>> hlist_node *node)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = hlist_entry_safe(node,
>>> + struct hisi_pcie_pmu, node);
>>> +
>>> + if (cpumask_empty(&pcie_pmu->cpumask)) {
>>> + cpumask_set_cpu(cpu, &pcie_pmu->cpumask);
>>> + WARN_ON(irq_set_affinity_hint(pcie_pmu->irq, cpumask_of(cpu)));
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int hisi_pcie_pmu_offline_cpu(unsigned int cpu, struct
>>> hlist_node *node)
>>> +{
>>> + struct hisi_pcie_pmu *pcie_pmu = hlist_entry_safe(node,
>>> + struct hisi_pcie_pmu, node);
>>> + unsigned int target;
>>> +
>>> + if (!cpumask_test_and_clear_cpu(cpu, &pcie_pmu->cpumask))
>>
>> I do wonder why we even need maintain pcie_pmu->cpumask
>>
>> Can't we just use cpu_online_mask as appropiate instead?
?
>>
>>> + return 0;
>>> +
>>> + /* Choose a new CPU from all online cpus. */
>>> + target = cpumask_any_but(cpu_online_mask, cpu);
>>> + if (target >= nr_cpu_ids)
>>> + return 0;
>>> +
>>> + perf_pmu_migrate_context(&pcie_pmu->pmu, cpu, target);
>>> + WARN_ON(irq_set_affinity_hint(pcie_pmu->irq, cpumask_of(target)));
>>> +
>>> + return 0;
>>> +}
>>> +
More information about the linux-arm-kernel
mailing list