| Message ID | 20230523093228.48149-3-yangyicong@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Improve PTT filter interface | expand |
On Tue, 23 May 2023 17:32:26 +0800 Yicong Yang <yangyicong@huawei.com> wrote: > From: Yicong Yang <yangyicong@hisilicon.com> > > The PCIe devices supported by the PTT trace can be removed/rescanned by > hotplug or through sysfs. Add support for dynamically updating the > available filter list by registering a PCI bus notifier block. Then user > can always get latest information about available tracing filters and > driver can block the invalid filters of which related devices no longer > exist in the system. > > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> Hi Yangyicong, A few comments inline Jonathan > --- > Documentation/trace/hisi-ptt.rst | 6 +- > drivers/hwtracing/ptt/hisi_ptt.c | 165 +++++++++++++++++++++++++++++-- > drivers/hwtracing/ptt/hisi_ptt.h | 39 ++++++++ > 3 files changed, 201 insertions(+), 9 deletions(-) > > diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst > index 4f87d8e21065..3641aca4287a 100644 > --- a/Documentation/trace/hisi-ptt.rst > +++ b/Documentation/trace/hisi-ptt.rst > @@ -153,9 +153,9 @@ Endpoint function can be specified in one trace. Specifying both Root Port > and function at the same time is not supported. Driver maintains a list of > available filters and will check the invalid inputs. > > -Currently the available filters are detected in driver's probe. If the supported > -devices are removed/added after probe, you may need to reload the driver to update > -the filters. > +The available filters will be dynamically updates, which means you will always updated, > +get correct filter information when hotplug events happen, or when you manually > +remove/rescan the devices. > > 2. Type > ------- > diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c > index 548cfef51ace..9b4acbc434b0 100644 > --- a/drivers/hwtracing/ptt/hisi_ptt.c > +++ b/drivers/hwtracing/ptt/hisi_ptt.c > @@ -357,24 +357,41 @@ static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt) ... > static struct hisi_ptt_filter_desc * > -hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) > +hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, u16 devid, bool is_port) > { > struct hisi_ptt_filter_desc *filter; > + u8 devfn = devid & 0xff; > + char *filter_name; > + > + filter_name = kasprintf(GFP_KERNEL, "%04x:%02x:%02x.%d", pci_domain_nr(hisi_ptt->pdev->bus), > + PCI_BUS_NUM(devid), PCI_SLOT(devfn), PCI_FUNC(devfn)); > + if (!filter_name) { > + pci_err(hisi_ptt->pdev, "failed to allocate name for filter %s\n", > + filter_name); > + kfree(filter); Don't think you've allocated filter yet. > + return NULL; > + } > > filter = kzalloc(sizeof(*filter), GFP_KERNEL); > if (!filter) { > pci_err(hisi_ptt->pdev, "failed to add filter for %s\n", > - pci_name(pdev)); > + filter_name); > return NULL; > } > > - filter->devid = PCI_DEVID(pdev->bus->number, pdev->devfn); > - filter->is_port = pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT; > + filter->name = filter_name; > + filter->is_port = is_port; > + filter->devid = devid; > + > if (filter->is_port) { > list_add_tail(&filter->list, &hisi_ptt->port_filters); > > @@ -387,6 +404,102 @@ hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) > return filter; > } ... > @@ -633,14 +752,19 @@ static int hisi_ptt_trace_valid_filter(struct hisi_ptt *hisi_ptt, u64 config) > * For Requester ID filters, walk the available filter list to see > * whether we have one matched. > */ > + mutex_lock(&hisi_ptt->filter_lock); > if (!hisi_ptt->trace_ctrl.is_port) { > list_for_each_entry(filter, &hisi_ptt->req_filters, list) { > - if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) > + if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) { > + mutex_unlock(&hisi_ptt->filter_lock); Perhaps a single exit location makes more sense given we need to unlock in all paths? goto unlock; etc > return 0; > + } > } > } else if (bitmap_subset(&val, &port_mask, BITS_PER_LONG)) { > + mutex_unlock(&hisi_ptt->filter_lock); > return 0; > } > + mutex_unlock(&hisi_ptt->filter_lock); > > return -EINVAL; > } > @@ -916,6 +1040,31 @@ static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt) > &hisi_ptt->hisi_ptt_pmu); > } > diff --git a/drivers/hwtracing/ptt/hisi_ptt.h b/drivers/hwtracing/ptt/hisi_ptt.h > index 5beb1648c93a..d080b306af77 100644 > --- a/drivers/hwtracing/ptt/hisi_ptt.h > +++ b/drivers/hwtracing/ptt/hisi_ptt.h ... > @@ -161,6 +182,7 @@ struct hisi_ptt_pmu_buf { > * struct hisi_ptt - Per PTT device data > * @trace_ctrl: the control information of PTT trace > * @hotplug_node: node for register cpu hotplug event > + * @hisi_ptt_nb: dynamic filter update notifier Wrong location. Order should match the fields in the structure. So this belongs just above hotplug_node > * @hisi_ptt_pmu: the pum device of trace > * @iobase: base IO address of the device > * @pdev: pci_dev of this PTT device > @@ -170,10 +192,15 @@ struct hisi_ptt_pmu_buf { > * @lower_bdf: the lower BDF range of the PCI devices managed by this PTT device > * @port_filters: the filter list of root ports > * @req_filters: the filter list of requester ID > + * @filter_lock: lock to protect the filters > * @port_mask: port mask of the managed root ports > + * @work: delayed work for filter updating > + * @filter_update_lock: spinlock to protect the filter update fifo > + * @filter_update_fifo: fifo of the filters waiting to update the filter list > */ > struct hisi_ptt { > struct hisi_ptt_trace_ctrl trace_ctrl; > + struct notifier_block hisi_ptt_nb; > struct hlist_node hotplug_node; > struct pmu hisi_ptt_pmu; > void __iomem *iobase; > @@ -192,7 +219,19 @@ struct hisi_ptt { > */ > struct list_head port_filters; > struct list_head req_filters; > + struct mutex filter_lock; > u16 port_mask; > + > + /* > + * We use a delayed work here to avoid indefinitely waiting for > + * the hisi_ptt->mutex which protecting the filter list. The > + * work will be delayed only if the mutex can not be held, > + * otherwise no delay will be applied. > + */ > + struct delayed_work work; > + spinlock_t filter_update_lock; > + DECLARE_KFIFO(filter_update_kfifo, struct hisi_ptt_filter_update_info, > + HISI_PTT_FILTER_UPDATE_FIFO_SIZE); > }; > > #define to_hisi_ptt(pmu) container_of(pmu, struct hisi_ptt, hisi_ptt_pmu)
On 2023/6/6 17:56, Jonathan Cameron wrote: > On Tue, 23 May 2023 17:32:26 +0800 > Yicong Yang <yangyicong@huawei.com> wrote: > >> From: Yicong Yang <yangyicong@hisilicon.com> >> >> The PCIe devices supported by the PTT trace can be removed/rescanned by >> hotplug or through sysfs. Add support for dynamically updating the >> available filter list by registering a PCI bus notifier block. Then user >> can always get latest information about available tracing filters and >> driver can block the invalid filters of which related devices no longer >> exist in the system. >> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > Hi Yangyicong, > > A few comments inline Thanks for the comments! Will fix them up in next version. > > Jonathan > >> --- >> Documentation/trace/hisi-ptt.rst | 6 +- >> drivers/hwtracing/ptt/hisi_ptt.c | 165 +++++++++++++++++++++++++++++-- >> drivers/hwtracing/ptt/hisi_ptt.h | 39 ++++++++ >> 3 files changed, 201 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst >> index 4f87d8e21065..3641aca4287a 100644 >> --- a/Documentation/trace/hisi-ptt.rst >> +++ b/Documentation/trace/hisi-ptt.rst >> @@ -153,9 +153,9 @@ Endpoint function can be specified in one trace. Specifying both Root Port >> and function at the same time is not supported. Driver maintains a list of >> available filters and will check the invalid inputs. >> >> -Currently the available filters are detected in driver's probe. If the supported >> -devices are removed/added after probe, you may need to reload the driver to update >> -the filters. >> +The available filters will be dynamically updates, which means you will always > > updated, > >> +get correct filter information when hotplug events happen, or when you manually >> +remove/rescan the devices. >> >> 2. Type >> ------- >> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c >> index 548cfef51ace..9b4acbc434b0 100644 >> --- a/drivers/hwtracing/ptt/hisi_ptt.c >> +++ b/drivers/hwtracing/ptt/hisi_ptt.c >> @@ -357,24 +357,41 @@ static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt) > > ... > >> static struct hisi_ptt_filter_desc * >> -hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) >> +hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, u16 devid, bool is_port) >> { >> struct hisi_ptt_filter_desc *filter; >> + u8 devfn = devid & 0xff; >> + char *filter_name; >> + >> + filter_name = kasprintf(GFP_KERNEL, "%04x:%02x:%02x.%d", pci_domain_nr(hisi_ptt->pdev->bus), >> + PCI_BUS_NUM(devid), PCI_SLOT(devfn), PCI_FUNC(devfn)); >> + if (!filter_name) { >> + pci_err(hisi_ptt->pdev, "failed to allocate name for filter %s\n", >> + filter_name); >> + kfree(filter); > > Don't think you've allocated filter yet. > >> + return NULL; >> + } >> >> filter = kzalloc(sizeof(*filter), GFP_KERNEL); >> if (!filter) { >> pci_err(hisi_ptt->pdev, "failed to add filter for %s\n", >> - pci_name(pdev)); >> + filter_name); >> return NULL; >> } >> >> - filter->devid = PCI_DEVID(pdev->bus->number, pdev->devfn); >> - filter->is_port = pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT; >> + filter->name = filter_name; >> + filter->is_port = is_port; >> + filter->devid = devid; >> + >> if (filter->is_port) { >> list_add_tail(&filter->list, &hisi_ptt->port_filters); >> >> @@ -387,6 +404,102 @@ hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) >> return filter; >> } > > > > > ... > >> @@ -633,14 +752,19 @@ static int hisi_ptt_trace_valid_filter(struct hisi_ptt *hisi_ptt, u64 config) >> * For Requester ID filters, walk the available filter list to see >> * whether we have one matched. >> */ >> + mutex_lock(&hisi_ptt->filter_lock); >> if (!hisi_ptt->trace_ctrl.is_port) { >> list_for_each_entry(filter, &hisi_ptt->req_filters, list) { >> - if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) >> + if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) { >> + mutex_unlock(&hisi_ptt->filter_lock); > > Perhaps a single exit location makes more sense given we need to unlock in > all paths? goto unlock; etc > >> return 0; >> + } >> } >> } else if (bitmap_subset(&val, &port_mask, BITS_PER_LONG)) { >> + mutex_unlock(&hisi_ptt->filter_lock); >> return 0; >> } >> + mutex_unlock(&hisi_ptt->filter_lock); >> >> return -EINVAL; >> } >> @@ -916,6 +1040,31 @@ static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt) >> &hisi_ptt->hisi_ptt_pmu); >> } > > > >> diff --git a/drivers/hwtracing/ptt/hisi_ptt.h b/drivers/hwtracing/ptt/hisi_ptt.h >> index 5beb1648c93a..d080b306af77 100644 >> --- a/drivers/hwtracing/ptt/hisi_ptt.h >> +++ b/drivers/hwtracing/ptt/hisi_ptt.h > > ... > >> @@ -161,6 +182,7 @@ struct hisi_ptt_pmu_buf { >> * struct hisi_ptt - Per PTT device data >> * @trace_ctrl: the control information of PTT trace >> * @hotplug_node: node for register cpu hotplug event >> + * @hisi_ptt_nb: dynamic filter update notifier > > Wrong location. Order should match the fields in the structure. > So this belongs just above hotplug_node > > >> * @hisi_ptt_pmu: the pum device of trace >> * @iobase: base IO address of the device >> * @pdev: pci_dev of this PTT device >> @@ -170,10 +192,15 @@ struct hisi_ptt_pmu_buf { >> * @lower_bdf: the lower BDF range of the PCI devices managed by this PTT device >> * @port_filters: the filter list of root ports >> * @req_filters: the filter list of requester ID >> + * @filter_lock: lock to protect the filters >> * @port_mask: port mask of the managed root ports >> + * @work: delayed work for filter updating >> + * @filter_update_lock: spinlock to protect the filter update fifo >> + * @filter_update_fifo: fifo of the filters waiting to update the filter list >> */ >> struct hisi_ptt { >> struct hisi_ptt_trace_ctrl trace_ctrl; >> + struct notifier_block hisi_ptt_nb; >> struct hlist_node hotplug_node; >> struct pmu hisi_ptt_pmu; >> void __iomem *iobase; >> @@ -192,7 +219,19 @@ struct hisi_ptt { >> */ >> struct list_head port_filters; >> struct list_head req_filters; >> + struct mutex filter_lock; >> u16 port_mask; >> + >> + /* >> + * We use a delayed work here to avoid indefinitely waiting for >> + * the hisi_ptt->mutex which protecting the filter list. The >> + * work will be delayed only if the mutex can not be held, >> + * otherwise no delay will be applied. >> + */ >> + struct delayed_work work; >> + spinlock_t filter_update_lock; >> + DECLARE_KFIFO(filter_update_kfifo, struct hisi_ptt_filter_update_info, >> + HISI_PTT_FILTER_UPDATE_FIFO_SIZE); >> }; >> >> #define to_hisi_ptt(pmu) container_of(pmu, struct hisi_ptt, hisi_ptt_pmu) > > . >
diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst index 4f87d8e21065..3641aca4287a 100644 --- a/Documentation/trace/hisi-ptt.rst +++ b/Documentation/trace/hisi-ptt.rst @@ -153,9 +153,9 @@ Endpoint function can be specified in one trace. Specifying both Root Port and function at the same time is not supported. Driver maintains a list of available filters and will check the invalid inputs. -Currently the available filters are detected in driver's probe. If the supported -devices are removed/added after probe, you may need to reload the driver to update -the filters. +The available filters will be dynamically updates, which means you will always +get correct filter information when hotplug events happen, or when you manually +remove/rescan the devices. 2. Type ------- diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c index 548cfef51ace..9b4acbc434b0 100644 --- a/drivers/hwtracing/ptt/hisi_ptt.c +++ b/drivers/hwtracing/ptt/hisi_ptt.c @@ -357,24 +357,41 @@ static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt) static void hisi_ptt_del_free_filter(struct hisi_ptt *hisi_ptt, struct hisi_ptt_filter_desc *filter) { + if (filter->is_port) + hisi_ptt->port_mask &= ~hisi_ptt_get_filter_val(filter->devid, true); + list_del(&filter->list); + kfree(filter->name); kfree(filter); } static struct hisi_ptt_filter_desc * -hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) +hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, u16 devid, bool is_port) { struct hisi_ptt_filter_desc *filter; + u8 devfn = devid & 0xff; + char *filter_name; + + filter_name = kasprintf(GFP_KERNEL, "%04x:%02x:%02x.%d", pci_domain_nr(hisi_ptt->pdev->bus), + PCI_BUS_NUM(devid), PCI_SLOT(devfn), PCI_FUNC(devfn)); + if (!filter_name) { + pci_err(hisi_ptt->pdev, "failed to allocate name for filter %s\n", + filter_name); + kfree(filter); + return NULL; + } filter = kzalloc(sizeof(*filter), GFP_KERNEL); if (!filter) { pci_err(hisi_ptt->pdev, "failed to add filter for %s\n", - pci_name(pdev)); + filter_name); return NULL; } - filter->devid = PCI_DEVID(pdev->bus->number, pdev->devfn); - filter->is_port = pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT; + filter->name = filter_name; + filter->is_port = is_port; + filter->devid = devid; + if (filter->is_port) { list_add_tail(&filter->list, &hisi_ptt->port_filters); @@ -387,6 +404,102 @@ hisi_ptt_alloc_add_filter(struct hisi_ptt *hisi_ptt, struct pci_dev *pdev) return filter; } +static void hisi_ptt_update_filters(struct work_struct *work) +{ + struct delayed_work *delayed_work = to_delayed_work(work); + struct hisi_ptt_filter_update_info info; + struct hisi_ptt_filter_desc *filter; + struct hisi_ptt *hisi_ptt; + + hisi_ptt = container_of(delayed_work, struct hisi_ptt, work); + + if (!mutex_trylock(&hisi_ptt->filter_lock)) { + schedule_delayed_work(&hisi_ptt->work, HISI_PTT_WORK_DELAY_MS); + return; + } + + while (kfifo_get(&hisi_ptt->filter_update_kfifo, &info)) { + if (info.is_add) { + /* + * Notify the users if failed to add this filter, others + * still work and available. See the comments in + * hisi_ptt_init_filters(). + */ + filter = hisi_ptt_alloc_add_filter(hisi_ptt, info.devid, info.is_port); + if (!filter) + continue; + } else { + struct hisi_ptt_filter_desc *tmp; + struct list_head *target_list; + + target_list = info.is_port ? &hisi_ptt->port_filters : + &hisi_ptt->req_filters; + + list_for_each_entry_safe(filter, tmp, target_list, list) + if (filter->devid == info.devid) { + hisi_ptt_del_free_filter(hisi_ptt, filter); + break; + } + } + } + + mutex_unlock(&hisi_ptt->filter_lock); +} + +/* + * A PCI bus notifier is used here for dynamically updating the filter + * list. + */ +static int hisi_ptt_notifier_call(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct hisi_ptt *hisi_ptt = container_of(nb, struct hisi_ptt, hisi_ptt_nb); + struct hisi_ptt_filter_update_info info; + struct pci_dev *pdev, *root_port; + struct device *dev = data; + u32 port_devid; + + pdev = to_pci_dev(dev); + root_port = pcie_find_root_port(pdev); + if (!root_port) + return 0; + + port_devid = PCI_DEVID(root_port->bus->number, root_port->devfn); + if (port_devid < hisi_ptt->lower_bdf || + port_devid > hisi_ptt->upper_bdf) + return 0; + + info.is_port = pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT; + info.devid = PCI_DEVID(pdev->bus->number, pdev->devfn); + + switch (action) { + case BUS_NOTIFY_ADD_DEVICE: + info.is_add = true; + break; + case BUS_NOTIFY_DEL_DEVICE: + info.is_add = false; + break; + default: + return 0; + } + + /* + * The FIFO size is 16 which is sufficient for almost all the cases, + * since each PCIe core will have most 8 Root Ports (typically only + * 1~4 Root Ports). On failure log the failed filter and let user + * handle it. + */ + if (kfifo_in_spinlocked(&hisi_ptt->filter_update_kfifo, &info, 1, + &hisi_ptt->filter_update_lock)) + schedule_delayed_work(&hisi_ptt->work, 0); + else + pci_warn(hisi_ptt->pdev, + "filter update fifo overflow for target %s\n", + pci_name(pdev)); + + return 0; +} + static int hisi_ptt_init_filters(struct pci_dev *pdev, void *data) { struct pci_dev *root_port = pcie_find_root_port(pdev); @@ -407,7 +520,8 @@ static int hisi_ptt_init_filters(struct pci_dev *pdev, void *data) * should be partial initialized and users would know which filter fails * through the log. Other functions of PTT device are still available. */ - filter = hisi_ptt_alloc_add_filter(hisi_ptt, pdev); + filter = hisi_ptt_alloc_add_filter(hisi_ptt, PCI_DEVID(pdev->bus->number, pdev->devfn), + pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT); if (!filter) return -ENOMEM; @@ -466,8 +580,13 @@ static int hisi_ptt_init_ctrls(struct hisi_ptt *hisi_ptt) int ret; u32 reg; + INIT_DELAYED_WORK(&hisi_ptt->work, hisi_ptt_update_filters); + INIT_KFIFO(hisi_ptt->filter_update_kfifo); + spin_lock_init(&hisi_ptt->filter_update_lock); + INIT_LIST_HEAD(&hisi_ptt->port_filters); INIT_LIST_HEAD(&hisi_ptt->req_filters); + mutex_init(&hisi_ptt->filter_lock); ret = hisi_ptt_config_trace_buf(hisi_ptt); if (ret) @@ -633,14 +752,19 @@ static int hisi_ptt_trace_valid_filter(struct hisi_ptt *hisi_ptt, u64 config) * For Requester ID filters, walk the available filter list to see * whether we have one matched. */ + mutex_lock(&hisi_ptt->filter_lock); if (!hisi_ptt->trace_ctrl.is_port) { list_for_each_entry(filter, &hisi_ptt->req_filters, list) { - if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) + if (val == hisi_ptt_get_filter_val(filter->devid, filter->is_port)) { + mutex_unlock(&hisi_ptt->filter_lock); return 0; + } } } else if (bitmap_subset(&val, &port_mask, BITS_PER_LONG)) { + mutex_unlock(&hisi_ptt->filter_lock); return 0; } + mutex_unlock(&hisi_ptt->filter_lock); return -EINVAL; } @@ -916,6 +1040,31 @@ static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt) &hisi_ptt->hisi_ptt_pmu); } +static void hisi_ptt_unregister_filter_update_notifier(void *data) +{ + struct hisi_ptt *hisi_ptt = data; + + bus_unregister_notifier(&pci_bus_type, &hisi_ptt->hisi_ptt_nb); + + /* Cancel any work that has been queued */ + cancel_delayed_work_sync(&hisi_ptt->work); +} + +/* Register the bus notifier for dynamically updating the filter list */ +static int hisi_ptt_register_filter_update_notifier(struct hisi_ptt *hisi_ptt) +{ + int ret; + + hisi_ptt->hisi_ptt_nb.notifier_call = hisi_ptt_notifier_call; + ret = bus_register_notifier(&pci_bus_type, &hisi_ptt->hisi_ptt_nb); + if (ret) + return ret; + + return devm_add_action_or_reset(&hisi_ptt->pdev->dev, + hisi_ptt_unregister_filter_update_notifier, + hisi_ptt); +} + /* * The DMA of PTT trace can only use direct mappings due to some * hardware restriction. Check whether there is no IOMMU or the @@ -987,6 +1136,10 @@ static int hisi_ptt_probe(struct pci_dev *pdev, return ret; } + ret = hisi_ptt_register_filter_update_notifier(hisi_ptt); + if (ret) + pci_warn(pdev, "failed to register filter update notifier, ret = %d", ret); + ret = hisi_ptt_register_pmu(hisi_ptt); if (ret) { pci_err(pdev, "failed to register PMU device, ret = %d", ret); diff --git a/drivers/hwtracing/ptt/hisi_ptt.h b/drivers/hwtracing/ptt/hisi_ptt.h index 5beb1648c93a..d080b306af77 100644 --- a/drivers/hwtracing/ptt/hisi_ptt.h +++ b/drivers/hwtracing/ptt/hisi_ptt.h @@ -11,12 +11,15 @@ #include <linux/bits.h> #include <linux/cpumask.h> +#include <linux/kfifo.h> #include <linux/list.h> #include <linux/mutex.h> +#include <linux/notifier.h> #include <linux/pci.h> #include <linux/perf_event.h> #include <linux/spinlock.h> #include <linux/types.h> +#include <linux/workqueue.h> #define DRV_NAME "hisi_ptt" @@ -71,6 +74,11 @@ #define HISI_PTT_WAIT_TRACE_TIMEOUT_US 100UL #define HISI_PTT_WAIT_POLL_INTERVAL_US 10UL +/* FIFO size for dynamically updating the PTT trace filter list. */ +#define HISI_PTT_FILTER_UPDATE_FIFO_SIZE 16 +/* Delay time for filter updating work */ +#define HISI_PTT_WORK_DELAY_MS 100UL + #define HISI_PCIE_CORE_PORT_ID(devfn) ((PCI_SLOT(devfn) & 0x7) << 1) /* Definition of the PMU configs */ @@ -135,11 +143,24 @@ struct hisi_ptt_trace_ctrl { * struct hisi_ptt_filter_desc - Descriptor of the PTT trace filter * @list: entry of this descriptor in the filter list * @is_port: the PCI device of the filter is a Root Port or not + * @name: name of this filter, same as the name of the related PCI device * @devid: the PCI device's devid of the filter */ struct hisi_ptt_filter_desc { struct list_head list; bool is_port; + char *name; + u16 devid; +}; + +/** + * struct hisi_ptt_filter_update_info - Information for PTT filter updating + * @pdev: the PCI device to update in the filter list + * @is_add: adding to the filter or not + */ +struct hisi_ptt_filter_update_info { + bool is_port; + bool is_add; u16 devid; }; @@ -161,6 +182,7 @@ struct hisi_ptt_pmu_buf { * struct hisi_ptt - Per PTT device data * @trace_ctrl: the control information of PTT trace * @hotplug_node: node for register cpu hotplug event + * @hisi_ptt_nb: dynamic filter update notifier * @hisi_ptt_pmu: the pum device of trace * @iobase: base IO address of the device * @pdev: pci_dev of this PTT device @@ -170,10 +192,15 @@ struct hisi_ptt_pmu_buf { * @lower_bdf: the lower BDF range of the PCI devices managed by this PTT device * @port_filters: the filter list of root ports * @req_filters: the filter list of requester ID + * @filter_lock: lock to protect the filters * @port_mask: port mask of the managed root ports + * @work: delayed work for filter updating + * @filter_update_lock: spinlock to protect the filter update fifo + * @filter_update_fifo: fifo of the filters waiting to update the filter list */ struct hisi_ptt { struct hisi_ptt_trace_ctrl trace_ctrl; + struct notifier_block hisi_ptt_nb; struct hlist_node hotplug_node; struct pmu hisi_ptt_pmu; void __iomem *iobase; @@ -192,7 +219,19 @@ struct hisi_ptt { */ struct list_head port_filters; struct list_head req_filters; + struct mutex filter_lock; u16 port_mask; + + /* + * We use a delayed work here to avoid indefinitely waiting for + * the hisi_ptt->mutex which protecting the filter list. The + * work will be delayed only if the mutex can not be held, + * otherwise no delay will be applied. + */ + struct delayed_work work; + spinlock_t filter_update_lock; + DECLARE_KFIFO(filter_update_kfifo, struct hisi_ptt_filter_update_info, + HISI_PTT_FILTER_UPDATE_FIFO_SIZE); }; #define to_hisi_ptt(pmu) container_of(pmu, struct hisi_ptt, hisi_ptt_pmu)