Re: [PATCH v2 3/5] coresight-tnoc: add nodes to configure flush

[Leo Yan <leo.yan@arm.com>]

On Thu, Mar 06, 2025 at 04:39:27PM +0800, Yuanfang Zhang wrote:

[...]

> >> +static ssize_t flush_req_store(struct device *dev,
> >> +                              struct device_attribute *attr,
> >> +                              const char *buf,
> >> +                              size_t size)
> >> +{

...

> >> +       reg = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> >> +       reg = reg | TRACE_NOC_CTRL_FLUSHREQ;
> >> +       writel_relaxed(reg, drvdata->base + TRACE_NOC_CTRL);
> > 
> > How can userspace determine when to trigger a flush?
> It can be triggered under any circumstances.
> > 
> > Generally, a driver kicks off a flush operation for a hardware before
> > reading data from buffer or when disable a link path.  I don't know the
> > hardware mechanism of TNOC, but seems to me, it does not make sense to
> > let the userspace to trigger a hardware flush, given the userspace has
> > no knowledge for device's state.
>
> TNOC supports the aforementioned flush operation, and it also adds this
> flush functionality, allowing users to set the flush themselves.

I am still not convinced for providing knobs to allow userspace to
directly control hardware.

A low level driver should have sufficient information to know when and
how it triggers a flush.  E.g., CoreSight ETF (coresight-tmc-etf.c) can
act as a link, in this case, it calls the tmc_flush_and_stop() function
to flush its buffer when it is stopped.  A flushing is triggered when a
session is terminated (either is a perf session or a Sysfs session).

Why not TNOC driver do the flushing same as other drivers?  It can flush
the data before a hardware link is to be disabled.  I don't think flush
operations are required at any time.

Seems to me, exposing APIs to userspace for flushing operations also
will introduce potential security risk.  A malicious software might
attack system with triggering tons of flushing in short time.

> > Furthermore, based on my understanding for patch 02 and 03, the working
> > flow is also concerned me.  IIUC, you want to use the driver to create
> > a linkage and then use userspace program to poll state and trigger
> > flushing.  Could you explain why use this way for managing the device?
> > 
> TNOC support flush just like other links. This interface simply provides
> customers with an additional option to trigger the flush.

This is not true for Arm CoreSight components.  My understanding is Arm
CoreSight drivers never provides an API to userspace to manually trigger
flush operations.

Thanks,
Leo