| 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
| ||
|
Message-ID: <CAJ9a7VgS3Bjnocxkg1mY4Vi6v7KpQFk1nrDa2z2O-Z_+4OekfA@mail.gmail.com>
Date: Mon, 17 Apr 2023 13:00:38 +0100
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@....com>
Cc: coresight@...ts.linaro.org, quic_jinlmao@...cinc.com,
suzuki.poulose@....com,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Leo Yan <leo.yan@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 12/13] coresight: Enable and disable helper devices
adjacent to the path
Hi James
On Tue, 4 Apr 2023 at 16:52, James Clark <james.clark@....com> wrote:
>
> Currently CATU is the only helper device, and its enable and disable
> calls are hard coded. To allow more helper devices to be added in a
> generic way, remove these hard coded calls and just enable and disable
> all helper devices.
>
> This has to apply to helpers adjacent to the path, because they will
> never be in the path. CATU was already discovered in this way, so
> there is no change there.
>
> One change that is needed is for CATU to call back into ETR to allocate
> the buffer. Because the enable call was previously hard coded, it was
> done at a point where the buffer was already allocated, but this is no
> longer the case.
>
> Signed-off-by: James Clark <james.clark@....com>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 21 ++-
> drivers/hwtracing/coresight/coresight-core.c | 161 ++++++++++++++++--
> .../hwtracing/coresight/coresight-etm-perf.c | 4 +-
> drivers/hwtracing/coresight/coresight-priv.h | 3 +
> .../hwtracing/coresight/coresight-tmc-etr.c | 43 +----
> include/linux/coresight.h | 11 +-
> 6 files changed, 188 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index bc90a03f478f..3949ded0d4fa 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -395,13 +395,18 @@ static inline int catu_wait_for_ready(struct catu_drvdata *drvdata)
> return coresight_timeout(csa, CATU_STATUS, CATU_STATUS_READY, 1);
> }
>
> -static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
> +static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode,
> + void *data)
> {
> int rc;
> u32 control, mode;
> - struct etr_buf *etr_buf = data;
> + struct etr_buf *etr_buf = NULL;
> struct device *dev = &drvdata->csdev->dev;
> struct coresight_device *csdev = drvdata->csdev;
> + struct coresight_device *etrdev;
> + union coresight_dev_subtype etr_subtype = {
> + .sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM
> + };
>
> if (catu_wait_for_ready(drvdata))
> dev_warn(dev, "Timeout while waiting for READY\n");
> @@ -416,6 +421,13 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
> if (rc)
> return rc;
>
> + etrdev = coresight_find_input_type(
> + csdev->pdata, CORESIGHT_DEV_TYPE_SINK, etr_subtype);
> + if (etrdev) {
> + etr_buf = tmc_etr_get_buffer(etrdev, cs_mode, data);
> + if (IS_ERR(etr_buf))
> + return PTR_ERR(etr_buf);
> + }
> control |= BIT(CATU_CONTROL_ENABLE);
>
> if (etr_buf && etr_buf->mode == ETR_MODE_CATU) {
> @@ -441,13 +453,14 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
> return 0;
> }
>
> -static int catu_enable(struct coresight_device *csdev, void *data)
> +static int catu_enable(struct coresight_device *csdev, enum cs_mode mode,
> + void *data)
> {
> int rc;
> struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);
>
> CS_UNLOCK(catu_drvdata->base);
> - rc = catu_enable_hw(catu_drvdata, data);
> + rc = catu_enable_hw(catu_drvdata, mode, data);
> CS_LOCK(catu_drvdata->base);
> return rc;
> }
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 939b7fb751b5..16689fe4ba98 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -403,8 +403,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
> csdev->enable = false;
> }
>
> -static int coresight_enable_source(struct coresight_device *csdev,
> - enum cs_mode mode)
> +int coresight_enable_source(struct coresight_device *csdev, enum cs_mode mode,
> + void *data)
> {
> int ret;
>
> @@ -413,7 +413,7 @@ static int coresight_enable_source(struct coresight_device *csdev,
> ret = coresight_control_assoc_ectdev(csdev, true);
> if (ret)
> return ret;
> - ret = source_ops(csdev)->enable(csdev, NULL, mode);
> + ret = source_ops(csdev)->enable(csdev, data, mode);
> if (ret) {
> coresight_control_assoc_ectdev(csdev, false);
> return ret;
> @@ -426,25 +426,75 @@ static int coresight_enable_source(struct coresight_device *csdev,
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(coresight_enable_source);
> +
> +static bool coresight_is_helper(struct coresight_device *csdev)
> +{
> + return csdev->type == CORESIGHT_DEV_TYPE_HELPER;
> +}
> +
> +static int coresight_enable_helper(struct coresight_device *csdev,
> + enum cs_mode mode, void *data)
> +{
> + int ret;
> +
> + if (!helper_ops(csdev)->enable)
> + return 0;
> + ret = helper_ops(csdev)->enable(csdev, mode, data);
> + if (ret)
> + return ret;
> +
> + csdev->enable = true;
> + return 0;
> +}
> +
> +static void coresight_disable_helper(struct coresight_device *csdev)
> +{
> + int ret;
> +
> + if (!helper_ops(csdev)->disable)
> + return;
> +
> + ret = helper_ops(csdev)->disable(csdev, NULL);
> + if (ret)
> + return;
> + csdev->enable = false;
> +}
> +
> +static void coresight_disable_helpers(struct coresight_device *csdev)
> +{
> + int i;
> + struct coresight_device *helper;
> +
> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
> + helper = csdev->pdata->out_conns[i]->dest_dev;
> + if (helper && coresight_is_helper(helper))
> + coresight_disable_helper(helper);
> + }
> +}
>
> /**
> * coresight_disable_source - Drop the reference count by 1 and disable
> * the device if there are no users left.
> *
> * @csdev: The coresight device to disable
> + * @data: Opaque data to pass on to the disable function of the source device.
> + * For example in perf mode this is a pointer to the struct perf_event.
> *
> * Returns true if the device has been disabled.
> */
> -static bool coresight_disable_source(struct coresight_device *csdev)
> +bool coresight_disable_source(struct coresight_device *csdev, void *data)
> {
> if (atomic_dec_return(&csdev->refcnt) == 0) {
> if (source_ops(csdev)->disable)
> - source_ops(csdev)->disable(csdev, NULL);
> + source_ops(csdev)->disable(csdev, data);
> coresight_control_assoc_ectdev(csdev, false);
> + coresight_disable_helpers(csdev);
> csdev->enable = false;
> }
> return !csdev->enable;
> }
> +EXPORT_SYMBOL_GPL(coresight_disable_source);
>
> /*
> * coresight_disable_path_from : Disable components in the given path beyond
> @@ -495,6 +545,9 @@ static void coresight_disable_path_from(struct list_head *path,
> default:
> break;
> }
> +
> + /* Disable all helpers adjacent along the path last */
> + coresight_disable_helpers(csdev);
> }
> }
>
> @@ -504,9 +557,28 @@ void coresight_disable_path(struct list_head *path)
> }
> EXPORT_SYMBOL_GPL(coresight_disable_path);
>
> -int coresight_enable_path(struct list_head *path, enum cs_mode mode, void *sink_data)
> +static int coresight_enable_helpers(struct coresight_device *csdev,
> + enum cs_mode mode, void *data)
> {
> + int i, ret = 0;
> + struct coresight_device *helper;
> +
> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
> + helper = csdev->pdata->out_conns[i]->dest_dev;
> + if (!helper || helper->type != CORESIGHT_DEV_TYPE_HELPER)
minor issue: Use coresight_is_helper() here too?
> + continue;
> +
> + ret = coresight_enable_helper(helper, mode, data);
> + if (ret)
> + return ret;
> + }
>
> + return 0;
> +}
> +
> +int coresight_enable_path(struct list_head *path, enum cs_mode mode,
> + void *sink_data)
> +{
> int ret = 0;
> u32 type;
> struct coresight_node *nd;
> @@ -516,6 +588,10 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode, void *sink_
> csdev = nd->csdev;
> type = csdev->type;
>
> + /* Enable all helpers adjacent to the path first */
> + ret = coresight_enable_helpers(csdev, mode, sink_data);
> + if (ret)
> + goto err;
> /*
> * ETF devices are tricky... They can be a link or a sink,
> * depending on how they are configured. If an ETF has been
> @@ -710,7 +786,7 @@ static int coresight_grab_device(struct coresight_device *csdev)
> struct coresight_device *child;
>
> child = csdev->pdata->out_conns[i]->dest_dev;
> - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
> + if (child && coresight_is_helper(child))
> if (!coresight_get_ref(child))
> goto err;
> }
> @@ -721,7 +797,7 @@ static int coresight_grab_device(struct coresight_device *csdev)
> struct coresight_device *child;
>
> child = csdev->pdata->out_conns[i]->dest_dev;
> - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
> + if (child && coresight_is_helper(child))
> coresight_put_ref(child);
> }
> return -ENODEV;
> @@ -740,7 +816,7 @@ static void coresight_drop_device(struct coresight_device *csdev)
> struct coresight_device *child;
>
> child = csdev->pdata->out_conns[i]->dest_dev;
> - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
> + if (child && coresight_is_helper(child))
> coresight_put_ref(child);
> }
> }
> @@ -1102,7 +1178,7 @@ int coresight_enable(struct coresight_device *csdev)
> if (ret)
> goto err_path;
>
> - ret = coresight_enable_source(csdev, CS_MODE_SYSFS);
> + ret = coresight_enable_source(csdev, CS_MODE_SYSFS, NULL);
> if (ret)
> goto err_source;
>
> @@ -1159,7 +1235,7 @@ void coresight_disable(struct coresight_device *csdev)
> if (ret)
> goto out;
>
> - if (!csdev->enable || !coresight_disable_source(csdev))
> + if (!csdev->enable || !coresight_disable_source(csdev, NULL))
> goto out;
>
> switch (csdev->subtype.source_subtype) {
> @@ -1644,6 +1720,69 @@ static inline int coresight_search_device_idx(struct coresight_dev_list *dict,
> return -ENOENT;
> }
>
> +static bool coresight_compare_type(enum coresight_dev_type type_a,
> + union coresight_dev_subtype subtype_a,
> + enum coresight_dev_type type_b,
> + union coresight_dev_subtype subtype_b)
> +{
> + if (type_a != type_b)
> + return false;
> +
> + switch (type_a) {
> + case CORESIGHT_DEV_TYPE_SINK:
> + return subtype_a.sink_subtype == subtype_b.sink_subtype;
> + case CORESIGHT_DEV_TYPE_LINK:
> + return subtype_a.link_subtype == subtype_b.link_subtype;
> + case CORESIGHT_DEV_TYPE_LINKSINK:
> + return subtype_a.link_subtype == subtype_b.link_subtype &&
> + subtype_a.sink_subtype == subtype_b.sink_subtype;
> + case CORESIGHT_DEV_TYPE_SOURCE:
> + return subtype_a.source_subtype == subtype_b.source_subtype;
> + case CORESIGHT_DEV_TYPE_HELPER:
> + return subtype_a.helper_subtype == subtype_b.helper_subtype;
> + default:
> + return false;
> + }
> +}
> +
> +struct coresight_device *
> +coresight_find_input_type(struct coresight_platform_data *pdata,
> + enum coresight_dev_type type,
> + union coresight_dev_subtype subtype)
> +{
> + int i;
> + struct coresight_connection *conn;
> +
> + for (i = 0; i < pdata->nr_inconns; ++i) {
> + conn = pdata->in_conns[i];
> + if (conn &&
> + coresight_compare_type(type, subtype, conn->src_dev->type,
> + conn->src_dev->subtype))
> + return conn->src_dev;
> + }
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_find_input_type);
> +
> +struct coresight_device *
> +coresight_find_output_type(struct coresight_platform_data *pdata,
> + enum coresight_dev_type type,
> + union coresight_dev_subtype subtype)
> +{
> + int i;
> + struct coresight_connection *conn;
> +
> + for (i = 0; i < pdata->nr_outconns; ++i) {
> + conn = pdata->out_conns[i];
> + if (conn->dest_dev &&
> + coresight_compare_type(type, subtype, conn->dest_dev->type,
> + conn->dest_dev->subtype))
> + return conn->dest_dev;
> + }
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_find_output_type);
> +
> bool coresight_loses_context_with_cpu(struct device *dev)
> {
> return fwnode_property_present(dev_fwnode(dev),
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index a48c97da8165..166adcf592cd 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -492,7 +492,7 @@ static void etm_event_start(struct perf_event *event, int flags)
> goto fail_end_stop;
>
> /* Finally enable the tracer */
> - if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF))
> + if (coresight_enable_source(csdev, CS_MODE_PERF, event))
> goto fail_disable_path;
>
> /*
> @@ -586,7 +586,7 @@ static void etm_event_stop(struct perf_event *event, int mode)
> return;
>
> /* stop tracer */
> - source_ops(csdev)->disable(csdev, event);
> + coresight_disable_source(csdev, event);
>
> /* tell the core */
> event->hw.state = PERF_HES_STOPPED;
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 65ae6d161c57..5575014f73e0 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -216,5 +216,8 @@ void coresight_set_assoc_ectdev_mutex(struct coresight_device *csdev,
>
> void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
> struct coresight_device *coresight_get_percpu_sink(int cpu);
> +int coresight_enable_source(struct coresight_device *csdev, enum cs_mode mode,
> + void *data);
> +bool coresight_disable_source(struct coresight_device *csdev, void *data);
>
> #endif
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 00a0c2aa8481..37afe8b52760 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -775,40 +775,19 @@ static const struct etr_buf_operations etr_sg_buf_ops = {
> struct coresight_device *
> tmc_etr_get_catu_device(struct tmc_drvdata *drvdata)
> {
> - int i;
> - struct coresight_device *tmp, *etr = drvdata->csdev;
> + struct coresight_device *etr = drvdata->csdev;
> + union coresight_dev_subtype catu_subtype = {
> + .helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU
> + };
>
> if (!IS_ENABLED(CONFIG_CORESIGHT_CATU))
> return NULL;
>
> - for (i = 0; i < etr->pdata->nr_outconns; i++) {
> - tmp = etr->pdata->out_conns[i]->dest_dev;
> - if (tmp && coresight_is_catu_device(tmp))
> - return tmp;
> - }
> -
> - return NULL;
> + return coresight_find_output_type(etr->pdata, CORESIGHT_DEV_TYPE_HELPER,
> + catu_subtype);
> }
> EXPORT_SYMBOL_GPL(tmc_etr_get_catu_device);
>
> -static inline int tmc_etr_enable_catu(struct tmc_drvdata *drvdata,
> - struct etr_buf *etr_buf)
> -{
> - struct coresight_device *catu = tmc_etr_get_catu_device(drvdata);
> -
> - if (catu && helper_ops(catu)->enable)
> - return helper_ops(catu)->enable(catu, etr_buf);
> - return 0;
> -}
> -
> -static inline void tmc_etr_disable_catu(struct tmc_drvdata *drvdata)
> -{
> - struct coresight_device *catu = tmc_etr_get_catu_device(drvdata);
> -
> - if (catu && helper_ops(catu)->disable)
> - helper_ops(catu)->disable(catu, drvdata->etr_buf);
> -}
> -
> static const struct etr_buf_operations *etr_buf_ops[] = {
> [ETR_MODE_FLAT] = &etr_flat_buf_ops,
> [ETR_MODE_ETR_SG] = &etr_sg_buf_ops,
> @@ -1058,13 +1037,6 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
> if (WARN_ON(drvdata->etr_buf))
> return -EBUSY;
>
> - /*
> - * If this ETR is connected to a CATU, enable it before we turn
> - * this on.
> - */
> - rc = tmc_etr_enable_catu(drvdata, etr_buf);
> - if (rc)
> - return rc;
> rc = coresight_claim_device(drvdata->csdev);
> if (!rc) {
> drvdata->etr_buf = etr_buf;
> @@ -1072,7 +1044,6 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
> if (rc) {
> drvdata->etr_buf = NULL;
> coresight_disclaim_device(drvdata->csdev);
> - tmc_etr_disable_catu(drvdata);
> }
> }
>
> @@ -1162,8 +1133,6 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> {
> __tmc_etr_disable_hw(drvdata);
> - /* Disable CATU device if this ETR is connected to one */
> - tmc_etr_disable_catu(drvdata);
> coresight_disclaim_device(drvdata->csdev);
> /* Reset the ETR buf used by hardware */
> drvdata->etr_buf = NULL;
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index b97edd24f3ec..61dfbab5fa98 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -375,7 +375,8 @@ struct coresight_ops_source {
> * @disable : Disable the device
> */
> struct coresight_ops_helper {
> - int (*enable)(struct coresight_device *csdev, void *data);
> + int (*enable)(struct coresight_device *csdev, enum cs_mode mode,
> + void *data);
> int (*disable)(struct coresight_device *csdev, void *data);
> };
>
> @@ -646,5 +647,13 @@ coresight_add_out_conn(struct device *dev,
> struct coresight_platform_data *pdata,
> const struct coresight_connection *new_conn);
> int coresight_add_in_conn(struct coresight_connection *conn);
> +struct coresight_device *
> +coresight_find_input_type(struct coresight_platform_data *pdata,
> + enum coresight_dev_type type,
> + union coresight_dev_subtype subtype);
> +struct coresight_device *
> +coresight_find_output_type(struct coresight_platform_data *pdata,
> + enum coresight_dev_type type,
> + union coresight_dev_subtype subtype);
>
> #endif /* _LINUX_COREISGHT_H */
> --
> 2.34.1
>
Reviewed-by: Mike Leach <mike.leach@...aro.org>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists