linux-kernel - Re: [PATCH v6 2/2] coresight: cti: Add Qualcomm extended CTI support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251204100203.GJ724103@e132581.arm.com>
Date: Thu, 4 Dec 2025 10:02:03 +0000
From: Leo Yan <leo.yan@....com>
To: Mike Leach <mike.leach@...aro.org>
Cc: Yingchao Deng <yingchao.deng@....qualcomm.com>,
	James Clark <james.clark@...aro.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Tingwei Zhang <tingwei.zhang@....qualcomm.com>,
	quic_yingdeng@...cinc.com, coresight@...ts.linaro.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org,
	Jinlong Mao <jinlong.mao@....qualcomm.com>,
	Mao Jinlong <quic_jinlmao@...cinc.com>
Subject: Re: [PATCH v6 2/2] coresight: cti: Add Qualcomm extended CTI support

On Thu, Dec 04, 2025 at 09:04:03AM +0000, Mike Leach wrote:
> Hi,
> 
> On Thu, 4 Dec 2025 at 08:38, Leo Yan <leo.yan@....com> wrote:
> >
> > On Wed, Dec 03, 2025 at 06:29:44PM +0000, Coresight ML wrote:
> >
> > [...]
> >
> > > > +/* Read registers with power check only (no enable check). */
> > > > +static ssize_t coresight_cti_reg_show(struct device *dev,
> > > > +                                 struct device_attribute *attr, char *buf)
> > > > +{
> > > > +   struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > > > +   struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr);
> > > > +   u32 idx, val = 0;
> > > > +
> > > > +   pm_runtime_get_sync(dev->parent);
> > > > +   raw_spin_lock(&drvdata->spinlock);
> > > > +   idx = drvdata->config.ext_reg_sel;
> > > > +   if (drvdata->config.hw_powered) {
> > > > +           switch (cti_attr->off) {
> > > > +           case INDEX_CTITRIGINSTATUS:
> > > > +           case INDEX_CTITRIGOUTSTATUS:
> > > > +           case INDEX_ITTRIGINACK:
> > > > +           case INDEX_ITTRIGOUT:
> > > > +           case INDEX_ITTRIGOUTACK:
> > > > +           case INDEX_ITTRIGIN:
> >
> > I read again and now I understand why you need "config.ext_reg_sel"
> > as an index for these expending registers.
> >
> 
> Having this index for these extended registers matches what we do for
> the INEN and OUTEN registers. This gives the user a consistent
> approach. We do not want the unnecessary attributes as  it will
> increase the memory footprint for all cti instances, not just the qcom
> ones.

I agree with using index for CTI triggers, but it is not necessary to
add a new index for other registers (status, mode setting, ACK, etc).

It would be directive to present the status and mode setting
registers, given these registers are only from 0-3.  This will be easy
accessed from userspace, and avoid complexity in the driver.

> The first patch in this series works to reduce the memory footprint by
> only allocating resource based on the actual configuration. For
> example for an ARM designed CTI with 8 trigger registers, we no longer
> declare static 128 x 32 bit arrays for each of INEN and OUTEN which
> were required by the original design.
> 
> Given that there can be 10s or 100s of CTIs in a large multicore
> system, reducing the footprint to match the actual configuration, and
> offering a level of compression by using an index + single file to
> access a set of registers improves the efficiency of the driver.

It is good for reducing footprint, but I would give priority for a
neat implementation and easy use interfaces.

And the sysfs attr code and global structures (e.g. register conversion
struct) can shared by all instances, so I don't worry much the scale
issue if we extend them.

Thanks,
Leo