In the commit being fixed, coresight_enable_path() was changed to call coresight_enable_helpers() by passing a struct coresight_path* as the final void* 'path' argument, rather passing 'sink_data' as was being passed previously. This set the groundwork for the subsequent addition of the ctcu_enable function which interprets void* data as a struct coresight_path*, but it also broke the existing catu_enable function which interprets void* data as a struct perf_output_handle*.
The compiler could not flag the error since there are several layers of function calls treating the pointer as void*.
Fix both users simultaneously by adding an additional argument to the enable helper interface. So now both the struct coresight_path and the struct perf_output_handle pointers are passed. Also, eliminate all usages of the void* from these code paths and use explicit types instead to make all of this code more robust.
Note: The disable function is also changed from void* to struct coresight_path* but no implementations of this function need the struct perf_output_handle* so it is not added to the interface.
The existing bug can be reproduced with:
# perf record -e cs_etm//k -C 0-9 dd if=/dev/zero of=/dev/null
Showing an oops as follows:
Unable to handle kernel paging request at virtual address 000f6e84934ed19e
Call trace: tmc_etr_get_buffer+0x30/0x80 [coresight_tmc] (P) catu_enable_hw+0xbc/0x3d0 [coresight_catu] catu_enable+0x70/0xe0 [coresight_catu] coresight_enable_path+0xb0/0x258 [coresight]
Fixes: 080ee83cc361 ("Coresight: Change functions to accept the coresight_path") Signed-off-by: Carl Worth carl@os.amperecomputing.com ---
Jie, I looked into your suggestion of adding the struct perf_output_handle* to the struct coresight_path, but I couldn't justify adding event-related data to the path structure, which has nothing to do with it.
So, instead I took the approach in this patch to plumb both arguments through the enable path, (and changed away from void* as you agreed to).
Note: I've tested that this fixes the bug for catu_enable. This patch also obviously touches ctcu_enable, which I believe is correct, but I have not tested since I don't have ready access to the necessary hardware. I will appreciate other testing.
-Carl
drivers/hwtracing/coresight/coresight-catu.c | 12 ++++++---- drivers/hwtracing/coresight/coresight-core.c | 23 +++++++++++-------- .../hwtracing/coresight/coresight-ctcu-core.c | 11 ++++----- .../hwtracing/coresight/coresight-cti-core.c | 7 ++++-- .../hwtracing/coresight/coresight-cti-sysfs.c | 2 +- drivers/hwtracing/coresight/coresight-cti.h | 6 +++-- drivers/hwtracing/coresight/coresight-priv.h | 2 +- .../hwtracing/coresight/coresight-tmc-etr.c | 4 ++-- drivers/hwtracing/coresight/coresight-tmc.h | 3 ++- include/linux/coresight.h | 6 +++-- 10 files changed, 45 insertions(+), 31 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 5058432233da..724c25d0afa4 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -397,7 +397,7 @@ static int catu_wait_for_ready(struct catu_drvdata *drvdata) }
static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode, - void *data) + struct perf_output_handle *handle) { int rc; u32 control, mode; @@ -425,7 +425,7 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode, 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); + etr_buf = tmc_etr_get_buffer(etrdev, cs_mode, handle); if (IS_ERR(etr_buf)) return PTR_ERR(etr_buf); } @@ -455,7 +455,8 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode, }
static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, - void *data) + struct coresight_path *path, + struct perf_output_handle *handle) { int rc = 0; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); @@ -463,7 +464,7 @@ static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock); if (csdev->refcnt == 0) { CS_UNLOCK(catu_drvdata->base); - rc = catu_enable_hw(catu_drvdata, mode, data); + rc = catu_enable_hw(catu_drvdata, mode, handle); CS_LOCK(catu_drvdata->base); } if (!rc) @@ -488,7 +489,8 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) return rc; }
-static int catu_disable(struct coresight_device *csdev, void *__unused) +static int catu_disable(struct coresight_device *csdev, + struct coresight_path *__unused) { int rc = 0; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index fa758cc21827..cc449d5196a4 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -353,14 +353,17 @@ static bool coresight_is_helper(struct coresight_device *csdev) }
static int coresight_enable_helper(struct coresight_device *csdev, - enum cs_mode mode, void *data) + enum cs_mode mode, + struct coresight_path *path, + struct perf_output_handle *handle) { - return helper_ops(csdev)->enable(csdev, mode, data); + return helper_ops(csdev)->enable(csdev, mode, path, handle); }
-static void coresight_disable_helper(struct coresight_device *csdev, void *data) +static void coresight_disable_helper(struct coresight_device *csdev, + struct coresight_path *path) { - helper_ops(csdev)->disable(csdev, data); + helper_ops(csdev)->disable(csdev, path); }
static void coresight_disable_helpers(struct coresight_device *csdev, void *data) @@ -477,7 +480,9 @@ void coresight_disable_path(struct coresight_path *path) EXPORT_SYMBOL_GPL(coresight_disable_path);
static int coresight_enable_helpers(struct coresight_device *csdev, - enum cs_mode mode, void *data) + enum cs_mode mode, + struct coresight_path *path, + struct perf_output_handle *handle) { int i, ret = 0; struct coresight_device *helper; @@ -487,7 +492,7 @@ static int coresight_enable_helpers(struct coresight_device *csdev, if (!helper || !coresight_is_helper(helper)) continue;
- ret = coresight_enable_helper(helper, mode, data); + ret = coresight_enable_helper(helper, mode, path, handle); if (ret) return ret; } @@ -496,7 +501,7 @@ static int coresight_enable_helpers(struct coresight_device *csdev, }
int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, - void *sink_data) + struct perf_output_handle *handle) { int ret = 0; u32 type; @@ -510,7 +515,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, type = csdev->type;
/* Enable all helpers adjacent to the path first */ - ret = coresight_enable_helpers(csdev, mode, path); + ret = coresight_enable_helpers(csdev, mode, path, handle); if (ret) goto err_disable_path; /* @@ -526,7 +531,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
switch (type) { case CORESIGHT_DEV_TYPE_SINK: - ret = coresight_enable_sink(csdev, mode, sink_data); + ret = coresight_enable_sink(csdev, mode, handle); /* * Sink is the first component turned on. If we * failed to enable the sink, there are no components diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c index c6bafc96db96..9f6d71c59d4e 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c @@ -156,17 +156,16 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight return __ctcu_set_etr_traceid(csdev, traceid, port_num, enable); }
-static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, void *data) +static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, + struct coresight_path *path, + struct perf_output_handle *handle) { - struct coresight_path *path = (struct coresight_path *)data; - return ctcu_set_etr_traceid(csdev, path, true); }
-static int ctcu_disable(struct coresight_device *csdev, void *data) +static int ctcu_disable(struct coresight_device *csdev, + struct coresight_path *path) { - struct coresight_path *path = (struct coresight_path *)data; - return ctcu_set_etr_traceid(csdev, path, false); }
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 8fb30dd73fd2..f92e3be4607c 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -799,14 +799,17 @@ static void cti_pm_release(struct cti_drvdata *drvdata) }
/** cti ect operations **/ -int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data) +int cti_enable(struct coresight_device *csdev, enum cs_mode mode, + struct coresight_path *path, + struct perf_output_handle *handle) { struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
return cti_enable_hw(drvdata); }
-int cti_disable(struct coresight_device *csdev, void *data) +int cti_disable(struct coresight_device *csdev, + struct coresight_path *path) { struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 572b80ee96fb..2bb6929eeb19 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -112,7 +112,7 @@ static ssize_t enable_store(struct device *dev, ret = pm_runtime_resume_and_get(dev->parent); if (ret) return ret; - ret = cti_enable(drvdata->csdev, CS_MODE_SYSFS, NULL); + ret = cti_enable(drvdata->csdev, CS_MODE_SYSFS, NULL, NULL); if (ret) pm_runtime_put(dev->parent); } else { diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h index 8362a47c939c..73954369654c 100644 --- a/drivers/hwtracing/coresight/coresight-cti.h +++ b/drivers/hwtracing/coresight/coresight-cti.h @@ -216,8 +216,10 @@ int cti_add_connection_entry(struct device *dev, struct cti_drvdata *drvdata, const char *assoc_dev_name); struct cti_trig_con *cti_allocate_trig_con(struct device *dev, int in_sigs, int out_sigs); -int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data); -int cti_disable(struct coresight_device *csdev, void *data); +int cti_enable(struct coresight_device *csdev, enum cs_mode mode, + struct coresight_path *path, + struct perf_output_handle *handle); +int cti_disable(struct coresight_device *csdev, struct coresight_path *path); void cti_write_all_hw_regs(struct cti_drvdata *drvdata); void cti_write_intack(struct device *dev, u32 ackval); void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value); diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 33e22b1ba043..65c4984d98c0 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -136,7 +136,7 @@ static inline void CS_UNLOCK(void __iomem *addr)
void coresight_disable_path(struct coresight_path *path); int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, - void *sink_data); + struct perf_output_handle *handle); struct coresight_device *coresight_get_sink(struct coresight_path *path); struct coresight_device *coresight_get_sink_by_id(u32 id); struct coresight_device * diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index b07fcdb3fe1a..2ed7fa2366ce 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1325,9 +1325,9 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) }
struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, - enum cs_mode mode, void *data) + enum cs_mode mode, + struct perf_output_handle *handle) { - struct perf_output_handle *handle = data; struct etr_perf_buffer *etr_perf;
switch (mode) { diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 6541a27a018e..b6e2b00d393a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -440,7 +440,8 @@ struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata); void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu); void tmc_etr_remove_catu_ops(void); struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, - enum cs_mode mode, void *data); + enum cs_mode mode, + struct perf_output_handle *handle); extern const struct attribute_group coresight_etr_group;
#endif diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 4ac65c68bbf4..450cccd46f38 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -422,8 +422,10 @@ struct coresight_ops_source { */ struct coresight_ops_helper { int (*enable)(struct coresight_device *csdev, enum cs_mode mode, - void *data); - int (*disable)(struct coresight_device *csdev, void *data); + struct coresight_path *path, + struct perf_output_handle *data); + int (*disable)(struct coresight_device *csdev, + struct coresight_path *path); };
base-commit: 46a51f4f5edade43ba66b3c151f0e25ec8b69cb6