lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250227162349.GB2157064@e132581.arm.com>
Date: Thu, 27 Feb 2025 16:23:49 +0000
From: Leo Yan <leo.yan@....com>
To: Yuanfang Zhang <quic_yuanfang@...cinc.com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>,
	James Clark <james.clark@...aro.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, kernel@...cinc.com,
	linux-kernel@...r.kernel.org, coresight@...ts.linaro.org,
	linux-arm-kernel@...ts.infradead.org, kernel@....qualcomm.com,
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/5] coresight-tnoc: add nodes to configure flush

On Wed, Feb 26, 2025 at 07:05:52PM +0800, Yuanfang Zhang wrote:
> 
> Two nodes for configure flush are added here:
> 1. flush_req: write 1 to initiates a flush sequence.
> 
> 2. flush_state: read this node to get flush status. 0: sequence in
> progress; 1: sequence has been completed.
> 
> Signed-off-by: Yuanfang Zhang <quic_yuanfang@...cinc.com>
> ---
>  drivers/hwtracing/coresight/coresight-tnoc.c | 73 ++++++++++++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-tnoc.h |  4 ++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
> index fad8e61f05ef25989aba1be342c547f835e8953a..20231f28ddcb6a60d9b3c1ca3e0ca4d731dac39c 100644
> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
> @@ -16,6 +16,78 @@
>  #include "coresight-tnoc.h"
>  #include "coresight-trace-id.h"
> 
> +static ssize_t flush_req_store(struct device *dev,
> +                              struct device_attribute *attr,
> +                              const char *buf,
> +                              size_t size)
> +{
> +       struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       struct coresight_device *csdev = drvdata->csdev;
> +       unsigned long val;
> +       u32 reg;
> +
> +       if (kstrtoul(buf, 10, &val))
> +               return -EINVAL;
> +
> +       if (val != 1)
> +               return -EINVAL;
> +
> +       spin_lock(&drvdata->spinlock);
> +       if (csdev->refcnt == 0) {
> +               spin_unlock(&drvdata->spinlock);
> +               return -EPERM;
> +       }
> +
> +       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?

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.

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?

Thanks,
Leo

> +
> +       spin_unlock(&drvdata->spinlock);
> +
> +       return size;
> +}
> +static DEVICE_ATTR_WO(flush_req);
> +
> +/*
> + * flush-sequence status:
> + * value 0: sequence in progress;
> + * value 1: sequence has been completed.
> + */
> +static ssize_t flush_status_show(struct device *dev,
> +                                struct device_attribute *attr,
> +                                char *buf)
> +{
> +       struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       struct coresight_device *csdev = drvdata->csdev;
> +       u32 val;
> +
> +       spin_lock(&drvdata->spinlock);
> +       if (csdev->refcnt == 0) {
> +               spin_unlock(&drvdata->spinlock);
> +               return -EPERM;
> +       }
> +
> +       val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> +       spin_unlock(&drvdata->spinlock);
> +       return sysfs_emit(buf, "%lu\n", BMVAL(val, 2, 2));
> +}
> +static DEVICE_ATTR_RO(flush_status);
> +
> +static struct attribute *trace_noc_attrs[] = {
> +       &dev_attr_flush_req.attr,
> +       &dev_attr_flush_status.attr,
> +       NULL,
> +};
> +
> +static struct attribute_group trace_noc_attr_grp = {
> +       .attrs = trace_noc_attrs,
> +};
> +
> +static const struct attribute_group *trace_noc_attr_grps[] = {
> +       &trace_noc_attr_grp,
> +       NULL,
> +};
> +
>  static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
>  {
>         u32 val;
> @@ -142,6 +214,7 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>                 return ret;
> 
>         desc.ops = &trace_noc_cs_ops;
> +       desc.groups = trace_noc_attr_grps;
>         desc.type = CORESIGHT_DEV_TYPE_LINK;
>         desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
>         desc.pdata = adev->dev.platform_data;
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.h b/drivers/hwtracing/coresight/coresight-tnoc.h
> index b6bd1ef659897d8e0994c5e8514e8cbdd16eebd8..d0fe8f52709ff4147d66dbf90987595012cfaa4e 100644
> --- a/drivers/hwtracing/coresight/coresight-tnoc.h
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.h
> @@ -10,6 +10,10 @@
> 
>  /* Enable generation of output ATB traffic.*/
>  #define TRACE_NOC_CTRL_PORTEN  BIT(0)
> +/* Writing 1 to initiate a flush sequence.*/
> +#define TRACE_NOC_CTRL_FLUSHREQ        BIT(1)
> +/* 0: sequence in progress; 1: sequence has been completed.*/
> +#define TRACE_NOC_CTRL_FLUSHSTATUS     BIT(2)
>  /* Writing 1 to issue a FREQ or FREQ_TS packet*/
>  #define TRACE_NOC_CTRL_FREQTSREQ       BIT(5)
>  /* Sets the type of issued ATB FLAG packets. 0: 'FLAG' packets; 1: 'FLAG_TS' packets.*/
> 
> --
> 2.34.1
> 
> _______________________________________________
> CoreSight mailing list -- coresight@...ts.linaro.org
> To unsubscribe send an email to coresight-leave@...ts.linaro.org

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ