linux-kernel - Re: [PATCH v4 4/8] hisi_ptt: Add support for dynamically updating the filter list

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220221115108.000033bf@Huawei.com>
Date:   Mon, 21 Feb 2022 11:51:08 +0000
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Yicong Yang <yangyicong@...ilicon.com>
CC:     <gregkh@...uxfoundation.org>, <helgaas@...nel.org>,
        <alexander.shishkin@...ux.intel.com>, <lorenzo.pieralisi@....com>,
        <will@...nel.org>, <mark.rutland@....com>,
        <mathieu.poirier@...aro.org>, <suzuki.poulose@....com>,
        <mike.leach@...aro.org>, <leo.yan@...aro.org>,
        <daniel.thompson@...aro.org>, <joro@...tes.org>,
        <john.garry@...wei.com>, <shameerali.kolothum.thodi@...wei.com>,
        <robin.murphy@....com>, <peterz@...radead.org>, <mingo@...hat.com>,
        <acme@...nel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <coresight@...ts.linaro.org>, <linux-pci@...r.kernel.org>,
        <linux-perf-users@...r.kernel.org>,
        <iommu@...ts.linux-foundation.org>, <prime.zeng@...wei.com>,
        <liuqi115@...wei.com>, <zhangshaokun@...ilicon.com>,
        <linuxarm@...wei.com>, <song.bao.hua@...ilicon.com>
Subject: Re: [PATCH v4 4/8] hisi_ptt: Add support for dynamically updating
 the filter list

On Mon, 21 Feb 2022 16:43:03 +0800
Yicong Yang <yangyicong@...ilicon.com> wrote:

> 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@...ilicon.com>

One comment following on from ordering of mixed devm and manual cleanup
in earlier patches.

Otherwise looks fine to me.

> ---
>  drivers/hwtracing/ptt/hisi_ptt.c | 138 ++++++++++++++++++++++++++++---
>  drivers/hwtracing/ptt/hisi_ptt.h |  34 ++++++++
>  2 files changed, 160 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
> index c2b6f8aa9f1e..50193a331faa 100644
> --- a/drivers/hwtracing/ptt/hisi_ptt.c
> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> @@ -269,25 +269,118 @@ static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt)
>  	return 0;
>  }
>  


...

> @@ -313,6 +406,9 @@ static void hisi_ptt_init_ctrls(struct hisi_ptt *hisi_ptt)
>  	struct pci_bus *bus;
>  	u32 reg;
>  
> +	INIT_DELAYED_WORK(&hisi_ptt->work, hisi_ptt_update_filters);
> +	spin_lock_init(&hisi_ptt->filter_update_lock);
> +	INIT_KFIFO(hisi_ptt->filter_update_kfifo);
>  	INIT_LIST_HEAD(&hisi_ptt->port_filters);
>  	INIT_LIST_HEAD(&hisi_ptt->req_filters);
>  
> @@ -329,6 +425,13 @@ static void hisi_ptt_init_ctrls(struct hisi_ptt *hisi_ptt)
>  	hisi_ptt->upper = FIELD_GET(HISI_PTT_DEVICE_RANGE_UPPER, reg);
>  	hisi_ptt->lower = FIELD_GET(HISI_PTT_DEVICE_RANGE_LOWER, reg);
>  
> +	/*
> +	 * No need to fail if the bus is NULL here as the device
> +	 * maybe hotplugged after the PTT driver probe, in which
> +	 * case we can detect the event and update the list as
> +	 * we register a bus notifier for dynamically updating
> +	 * the filter list.
> +	 */
>  	bus = pci_find_bus(pci_domain_nr(pdev->bus), PCI_BUS_NUM(hisi_ptt->upper));
>  	if (bus)
>  		pci_walk_bus(bus, hisi_ptt_init_filters, hisi_ptt);
> @@ -832,6 +935,12 @@ static int hisi_ptt_probe(struct pci_dev *pdev,
>  		return ret;
>  	}
>  
> +	/* Register the bus notifier for dynamically updating the filter list */
> +	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)
> +		pci_warn(pdev, "failed to register filter update notifier, ret = %d", ret);
> +
>  	return 0;
>  }
>  
> @@ -839,6 +948,11 @@ void hisi_ptt_remove(struct pci_dev *pdev)
>  {
>  	struct hisi_ptt *hisi_ptt = pci_get_drvdata(pdev);
>  
> +	bus_unregister_notifier(&pci_bus_type, &hisi_ptt->hisi_ptt_nb);
> +

wrt to earlier comment on ordering you'll also need to move these to
a devm_action() call to keep the ordering clean wrt to probe vs remove().

> +	/* Cancel any work that has been queued */
> +	cancel_delayed_work_sync(&hisi_ptt->work);
> +
>  	if (hisi_ptt->trace_ctrl.status == HISI_PTT_TRACE_STATUS_ON)
>  		hisi_ptt_trace_end(hisi_ptt);
>  

...