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: <CAJ9a7VhNW6cQt_O6NX+99LXfo-t3mFTejzdNjWcyssbMrXR4OQ@mail.gmail.com>
Date: Thu, 4 Dec 2025 09:04:03 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Leo Yan <leo.yan@....com>
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

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.

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.

Regards

Mike

> I think you should extend attrs for the new adding registers:
>
>   static struct attribute *coresight_cti_regs_attrs[] = {
>       ...
>       coresight_cti_reg(triginstatus, CTITRIGINSTATUS),
>       /* Qcom CTI only for triginstatus1/2/3 */
>       coresight_cti_reg(triginstatus1, CTITRIGINSTATUS + 0x4),
>       coresight_cti_reg(triginstatus2, CTITRIGINSTATUS + 0x8),
>       coresight_cti_reg(triginstatus3, CTITRIGINSTATUS + 0xc),
>       ...
>   }
>
> Then, you can add a is_visible() in coresight_cti_regs_group:
>
>   static umode_t coresight_cti_regs_is_visible(struct kobject *kobj,
>                   struct attribute *attr, int n)
>   {
>           struct device *dev = container_of(kobj, struct device, kobj);
>           struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>
>           /* Mute QCOM CTI registers for standard CTI module */
>           if (!drvdata->is_qcom_cti) {
>               if (attr == &triginstatus1.attr ||
>                   attr == &triginstatus2.attr ||
>                   attr == &triginstatus3.attr)
>                   return 0;
>           }
>
>           return attr->mode;
>   }
>
>   static const struct attribute_group coresight_cti_regs_group = {
>           .attrs = coresight_cti_regs_attrs,
>           .name = "regs",
>           .is_visible = coresight_cti_regs_is_visible,
>   };
>
> Thanks,
> Leo



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK