RE: [EXTERNAL] Re: [PATCH v8 6/7] coresight: tmc: Stop trace capture on FlIn

[Linu Cherian]

Hi James,

> -----Original Message-----
> From: James Clark <james.clark@xxxxxxx>
> Sent: Friday, May 31, 2024 3:47 PM
> To: Linu Cherian <lcherian@xxxxxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; coresight@xxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Sunil Kovvuri Goutham
> <sgoutham@xxxxxxxxxxx>; George Cherian <gcherian@xxxxxxxxxxx>;
> suzuki.poulose@xxxxxxx; mike.leach@xxxxxxxxxx
> Subject: [EXTERNAL] Re: [PATCH v8 6/7] coresight: tmc: Stop trace capture on
> FlIn
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> 
> 
> On 31/05/2024 05:27, Linu Cherian wrote:
> > Configure TMC ETR and ETF to flush and stop trace capture on FlIn
> > event based on sysfs attribute,
> > /sys/bus/coresight/devices/tmc_etXn/stop_on_flush.
> >
> > Signed-off-by: Linu Cherian <lcherian@xxxxxxxxxxx>
> > ---
> > Changelog from v7:
> > * Honour stop_on_flush sysfs attribute for perf sessions as well.
> >   This allows user to start trace sessions using perf method as
> >   well to use the Coresight kernel panic support.
> >
> >  .../hwtracing/coresight/coresight-tmc-core.c  | 31
> +++++++++++++++++++
> >  .../hwtracing/coresight/coresight-tmc-etf.c   | 18 ++++++++---
> >  .../hwtracing/coresight/coresight-tmc-etr.c   | 18 ++++++++---
> >  drivers/hwtracing/coresight/coresight-tmc.h   |  4 +++
> >  4 files changed, 61 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > index aaa9de9f4cd2..24edd9d750c3 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > @@ -452,9 +452,40 @@ static ssize_t buffer_size_store(struct device
> > *dev,
> >
> >  static DEVICE_ATTR_RW(buffer_size);
> >
> > +static ssize_t stop_on_flush_show(struct device *dev,
> > +				struct device_attribute *attr, char *buf) {
> > +	struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +
> > +	return sprintf(buf, "%#x\n", drvdata->stop_on_flush); }
> > +
> > +static ssize_t stop_on_flush_store(struct device *dev,
> > +				 struct device_attribute *attr,
> > +				 const char *buf, size_t size)
> > +{
> > +	int ret;
> > +	u8 val;
> > +	struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +
> > +	ret = kstrtou8(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +	if (val)
> > +		drvdata->stop_on_flush = true;
> > +	else
> > +		drvdata->stop_on_flush = false;
> > +
> > +	return size;
> > +}
> > +
> > +static DEVICE_ATTR_RW(stop_on_flush);
> > +
> > +
> >  static struct attribute *coresight_tmc_attrs[] = {
> >  	&dev_attr_trigger_cntr.attr,
> >  	&dev_attr_buffer_size.attr,
> > +	&dev_attr_stop_on_flush.attr,
> >  	NULL,
> >  };
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > index 655c0c0ba54b..1529b9cc210a 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > @@ -19,6 +19,7 @@ static int tmc_set_etf_buffer(struct
> > coresight_device *csdev,  static int __tmc_etb_enable_hw(struct
> > tmc_drvdata *drvdata)  {
> >  	int rc = 0;
> > +	u32 ffcr;
> >
> >  	CS_UNLOCK(drvdata->base);
> >
> > @@ -32,10 +33,12 @@ static int __tmc_etb_enable_hw(struct
> tmc_drvdata *drvdata)
> >  	}
> >
> >  	writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base +
> TMC_MODE);
> > -	writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> > -		       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
> > -		       TMC_FFCR_TRIGON_TRIGIN,
> > -		       drvdata->base + TMC_FFCR);
> > +
> > +	ffcr = TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> TMC_FFCR_FON_FLIN |
> > +		TMC_FFCR_FON_TRIG_EVT | TMC_FFCR_TRIGON_TRIGIN;
> > +	if (drvdata->stop_on_flush_en)
> > +		ffcr |= TMC_FFCR_STOP_ON_FLUSH;
> > +	writel_relaxed(ffcr, drvdata->base + TMC_FFCR);
> >
> >  	writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
> >  	tmc_enable_hw(drvdata);
> > @@ -225,7 +228,8 @@ static int tmc_enable_etf_sink_sysfs(struct
> coresight_device *csdev)
> >  		used = true;
> >  		drvdata->buf = buf;
> >  	}
> > -
> > +	if (drvdata->stop_on_flush)
> > +		drvdata->stop_on_flush_en = true;
> 
> Does this do anything different than only using stop_on_flush in
> __tmc_etr_enable_hw()?
> 
> Seems like stop_on_flush_en == stop_on_flush whenever
> stop_on_flush_en is read, so you might as well just read stop_on_flush and
> get rid of the _en copy?
> 

Ack.