Hi Leo, On Tue, 1 Apr 2025 at 19:07, Leo Yan <leo.yan@xxxxxxx> wrote: > > The buffer update callbacks disable the sink before syncing data but > misses to re-enable it afterward. This is fine in the general flow, > because the sink will be re-enabled the next time the PMU event is > activated. > > However, during AUX pause and resume, if the sink is disabled in the > buffer update callback, there is no chance to re-enable it when AUX > resumes. > > To address this, the callbacks now check the event state > 'event->hw.state'. If the event is an active state (0), the sink is > re-enabled. > > For the TMC ETR driver, buffer updates are not fully protected by > the driver's spinlock. In this case, the sink is not re-enabled if its > reference counter is 0, in order to avoid race conditions where the sink > may have been completely disabled. > > Signed-off-by: Leo Yan <leo.yan@xxxxxxx> > --- > drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++++++ > drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > index d858740001c2..7584cc03d8e6 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > @@ -482,6 +482,7 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > unsigned long offset, to_read = 0, flags; > struct cs_buffers *buf = sink_config; > struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct perf_event *event = handle->event; > > if (!buf) > return 0; > @@ -586,6 +587,14 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > * is expected by the perf ring buffer. > */ > CS_LOCK(drvdata->base); > + > + /* > + * If the event is active, it is triggered during an AUX pause. > + * Re-enable the sink so that it is ready when AUX resume is invoked. > + */ > + if (!event->hw.state) > + __tmc_etb_enable_hw(drvdata); > + Think that the refcnt should be checked here too. Does the ETB case need to be handled? - somewhat confusingly the coresight-tmc-etf.c file handles both ETF and ETB. Regards Mike > out: > raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 76a8cb29b68a..8923fbc6e1a0 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1636,6 +1636,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > struct etr_perf_buffer *etr_perf = config; > struct etr_buf *etr_buf = etr_perf->etr_buf; > + struct perf_event *event = handle->event; > > raw_spin_lock_irqsave(&drvdata->spinlock, flags); > > @@ -1705,6 +1706,15 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > */ > smp_wmb(); > > + /* > + * If the event is active, it is triggered during an AUX pause. > + * Re-enable the sink so that it is ready when AUX resume is invoked. > + */ > + raw_spin_lock_irqsave(&drvdata->spinlock, flags); > + if (csdev->refcnt && !event->hw.state) > + __tmc_etr_enable_hw(drvdata); > + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > + > out: > /* > * Don't set the TRUNCATED flag in snapshot mode because 1) the > -- > 2.34.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
