| Message ID | 20250310090407.2069489-2-quic_jiegan@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | coresight: ctcu: Enable byte-cntr function for TMC ETR | expand |
On 10/03/2025 10:04, Jie Gan wrote: > +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf *etr_buf = drvdata->sysfs_buf; > + struct etr_sg_table *etr_table = etr_buf->private; > + struct tmc_sg_table *table = etr_table->sg_table; > + long w_offset; > + u64 rwp; > + > + rwp = tmc_read_rwp(drvdata); > + w_offset = tmc_sg_get_data_page_offset(table, rwp); > + > + return w_offset; > +} > + > +/* > + * Retrieve the offset to the write pointer of the ETR buffer based on whether > + * the memory mode is SG, flat or reserved. > + */ > +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) You need kerneldoc for exports. Best regards, Krzysztof
On 3/10/2025 5:07 PM, Krzysztof Kozlowski wrote: > On 10/03/2025 10:04, Jie Gan wrote: >> +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) >> +{ >> + struct etr_buf *etr_buf = drvdata->sysfs_buf; >> + struct etr_sg_table *etr_table = etr_buf->private; >> + struct tmc_sg_table *table = etr_table->sg_table; >> + long w_offset; >> + u64 rwp; >> + >> + rwp = tmc_read_rwp(drvdata); >> + w_offset = tmc_sg_get_data_page_offset(table, rwp); >> + >> + return w_offset; >> +} >> + >> +/* >> + * Retrieve the offset to the write pointer of the ETR buffer based on whether >> + * the memory mode is SG, flat or reserved. >> + */ >> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) > > You need kerneldoc for exports. Hi Krzysztof, Sorry for the insufficient description for an export function. Will fix it in next version. Thanks, Jie > > > Best regards, > Krzysztof
Hi, On Mon, 10 Mar 2025 at 09:04, Jie Gan <quic_jiegan@quicinc.com> wrote: > > The new functions calculate and return the offset to the write pointer of > the ETR buffer based on whether the memory mode is SG, flat or reserved. > The functions have the RWP offset can directly read data from ETR buffer, > enabling the transfer of data to any required location. > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > --- > .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tmc.h | 1 + > 2 files changed, 41 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index eda7cdad0e2b..ec636ab1fd75 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) > } > EXPORT_SYMBOL_GPL(tmc_free_sg_table); > > +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr; > + u64 rwp; > + It is not valid to read RWP if the TMC is running. It must be in the stopped or disabled state - see the specifications for TMC /ETR It is likely that CSUNLOCK / CSLOCK are needed here too, along with the spinlock that protects drvdata See the code in coresight_tmc_etr.c :- e.g. in tmc_update_etr_buffer() ... <take spinlock> ... CS_UNLOCK(drvdata->base); tmc_flush_and_stop(drvdata); // this ensures tmc is stopped and flushed to memory - essential to ensure full formatted frame is in memory. tmc_sync_etr_buf(drvdata); // this function reads rwp. CS_LOCK(drvdata->base); <release spinlokc> This type of program flow is common to both sysfs and perf handling of TMC buffers. > + rwp = tmc_read_rwp(drvdata); > + return rwp - paddr; > +} > + > +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf *etr_buf = drvdata->sysfs_buf; > + struct etr_sg_table *etr_table = etr_buf->private; > + struct tmc_sg_table *table = etr_table->sg_table; > + long w_offset; > + u64 rwp; > + Same comments as above > + rwp = tmc_read_rwp(drvdata); > + w_offset = tmc_sg_get_data_page_offset(table, rwp); > + > + return w_offset; > +} > + > +/* > + * Retrieve the offset to the write pointer of the ETR buffer based on whether > + * the memory mode is SG, flat or reserved. > + */ > +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf *etr_buf = drvdata->sysfs_buf; > + As this is an exported function, please ensure that the inputs are valid - check the pointers Code to ensure TMC is flushed and stopped could be inserted here. Regards Mike > + if (etr_buf->mode == ETR_MODE_ETR_SG) > + return tmc_sg_get_rwp_offset(drvdata); > + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV) > + return tmc_flat_resrv_get_rwp_offset(drvdata); > + else > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset); > + > /* > * Alloc pages for the table. Since this will be used by the device, > * allocate the pages closer to the device (i.e, dev_to_node(dev) > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index b48bc9a01cc0..baedb4dcfc3f 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void); > struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, > enum cs_mode mode, void *data); > extern const struct attribute_group coresight_etr_group; > +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); > > #endif > -- > 2.34.1 >
On 3/12/2025 12:49 AM, Mike Leach wrote: > Hi, > > On Mon, 10 Mar 2025 at 09:04, Jie Gan <quic_jiegan@quicinc.com> wrote: >> >> The new functions calculate and return the offset to the write pointer of >> the ETR buffer based on whether the memory mode is SG, flat or reserved. >> The functions have the RWP offset can directly read data from ETR buffer, >> enabling the transfer of data to any required location. >> >> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> >> --- >> .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-tmc.h | 1 + >> 2 files changed, 41 insertions(+) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c >> index eda7cdad0e2b..ec636ab1fd75 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c >> @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) >> } >> EXPORT_SYMBOL_GPL(tmc_free_sg_table); >> >> +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata) >> +{ >> + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr; >> + u64 rwp; >> + > > It is not valid to read RWP if the TMC is running. It must be in the > stopped or disabled state - see the specifications for TMC /ETR > > It is likely that CSUNLOCK / CSLOCK are needed here too, along with > the spinlock that protects drvdata > > See the code in coresight_tmc_etr.c :- > > e.g. in > > tmc_update_etr_buffer() > > ... > <take spinlock> > ... > CS_UNLOCK(drvdata->base); > tmc_flush_and_stop(drvdata); // this ensures tmc is stopped and > flushed to memory - essential to ensure full formatted frame is in > memory. > tmc_sync_etr_buf(drvdata); // this function reads rwp. > CS_LOCK(drvdata->base); > <release spinlokc> > > This type of program flow is common to both sysfs and perf handling of > TMC buffers. Hi Mike, I am fully understood your point here. The function is designed this way to read the w_offset (which may not be entirely accurate because the etr buffer is not synced) when the byte-cntr devnode is opened, aiming to reduce the length of redundant trace data. In this case, we cannot ensure the TMC is stopped or disabled. The byte-cntr only requires an offset to know where it can start before the expected trace data gets into ETR buffer. The w_offset is also read when the byte-cntr function stops, which occurs after the TMC is disabled. Maybe this is not a good idea and I should read r_offset upon open? The primary goal for byte-cntr is trying to transfer useful trace data from the ETR buffer to the userspace, if we start from r_offset, a large number of redundant trace data which the user does not expect will be transferred simultaneously. > >> + rwp = tmc_read_rwp(drvdata); >> + return rwp - paddr; >> +} >> + >> +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) >> +{ >> + struct etr_buf *etr_buf = drvdata->sysfs_buf; >> + struct etr_sg_table *etr_table = etr_buf->private; >> + struct tmc_sg_table *table = etr_table->sg_table; >> + long w_offset; >> + u64 rwp; >> + > > Same comments as above > >> + rwp = tmc_read_rwp(drvdata); >> + w_offset = tmc_sg_get_data_page_offset(table, rwp); >> + >> + return w_offset; >> +} >> + >> +/* >> + * Retrieve the offset to the write pointer of the ETR buffer based on whether >> + * the memory mode is SG, flat or reserved. >> + */ >> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) >> +{ >> + struct etr_buf *etr_buf = drvdata->sysfs_buf; >> + > > As this is an exported function, please ensure that the inputs are > valid - check the pointers Sure, will do. Thanks, Jie > > Code to ensure TMC is flushed and stopped could be inserted here. > > Regards > > Mike > >> + if (etr_buf->mode == ETR_MODE_ETR_SG) >> + return tmc_sg_get_rwp_offset(drvdata); >> + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV) >> + return tmc_flat_resrv_get_rwp_offset(drvdata); >> + else >> + return -EINVAL; >> +} >> +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset); >> + >> /* >> * Alloc pages for the table. Since this will be used by the device, >> * allocate the pages closer to the device (i.e, dev_to_node(dev) >> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h >> index b48bc9a01cc0..baedb4dcfc3f 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc.h >> +++ b/drivers/hwtracing/coresight/coresight-tmc.h >> @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void); >> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, >> enum cs_mode mode, void *data); >> extern const struct attribute_group coresight_etr_group; >> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); >> >> #endif >> -- >> 2.34.1 >> > >
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index eda7cdad0e2b..ec636ab1fd75 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) } EXPORT_SYMBOL_GPL(tmc_free_sg_table); +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata) +{ + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr; + u64 rwp; + + rwp = tmc_read_rwp(drvdata); + return rwp - paddr; +} + +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) +{ + struct etr_buf *etr_buf = drvdata->sysfs_buf; + struct etr_sg_table *etr_table = etr_buf->private; + struct tmc_sg_table *table = etr_table->sg_table; + long w_offset; + u64 rwp; + + rwp = tmc_read_rwp(drvdata); + w_offset = tmc_sg_get_data_page_offset(table, rwp); + + return w_offset; +} + +/* + * Retrieve the offset to the write pointer of the ETR buffer based on whether + * the memory mode is SG, flat or reserved. + */ +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) +{ + struct etr_buf *etr_buf = drvdata->sysfs_buf; + + if (etr_buf->mode == ETR_MODE_ETR_SG) + return tmc_sg_get_rwp_offset(drvdata); + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV) + return tmc_flat_resrv_get_rwp_offset(drvdata); + else + return -EINVAL; +} +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset); + /* * Alloc pages for the table. Since this will be used by the device, * allocate the pages closer to the device (i.e, dev_to_node(dev) diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index b48bc9a01cc0..baedb4dcfc3f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void); struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, enum cs_mode mode, void *data); extern const struct attribute_group coresight_etr_group; +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); #endif
The new functions calculate and return the offset to the write pointer of the ETR buffer based on whether the memory mode is SG, flat or reserved. The functions have the RWP offset can directly read data from ETR buffer, enabling the transfer of data to any required location. Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> --- .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 1 + 2 files changed, 41 insertions(+)