linux-kernel - Re: [PATCH] coresight: etm4x: fix trctraceid sysfs always invisible

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: <CAJ9a7VgX+mf-J6jipQgJY7=F2vJzLhF9XcBxHAV7pCwiqPdbnw@mail.gmail.com>
Date:   Fri, 12 May 2023 14:37:54 +0100
From:   Mike Leach <mike.leach@...aro.org>
To:     Junhao He <hejunhao3@...wei.com>
Cc:     mathieu.poirier@...aro.org, suzuki.poulose@....com,
        leo.yan@...aro.org, jonathan.cameron@...wei.com,
        coresight@...ts.linaro.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
        linuxarm@...wei.com, yangyicong@...wei.com, shenyang39@...wei.com,
        prime.zeng@...ilicon.com
Subject: Re: [PATCH] coresight: etm4x: fix trctraceid sysfs always invisible

Hi

Thanks for spotting this and supplying a patch.

Unfortunately, this solution moving the location of the file alters
the published ABI
(/Documents/ABI/testing/sysfs-bus-coresight-devices-etm4x), which is
generally not permitted

The file is associated with a register - TRCTRACIDR - so to fix this
we need a solution to ensure the offset is correctly set in the
attribute.

The first few versions of the Trace ID set that you reference did do
this correctly, but some code change after a review in a later version
introduced the error which we did not then notice.

Based on that earlier work have sent a fix patch that restores the
visibility of this register in the mgmt/ directory.

Thanks and regards

Mike

On Fri, 12 May 2023 at 03:41, Junhao He <hejunhao3@...wei.com> wrote:
>
> The trctraceid sysfs interface is current in etm4x mgmt group.
> Each attr in the mgmt group will call the function is_visible()
> to check whether the register is implemented. However the trctraceid
> does not bound to any register. So the trctraceid sysfs will
> always be invisible.
>
> Move it to etmv4 group to fix that.
>
> Fixes: df4871204e5d ("coresight: etm4x: Update ETM4 driver to use Trace ID API")
> Signed-off-by: Junhao He <hejunhao3@...wei.com>
> ---
>  .../coresight/coresight-etm4x-sysfs.c         | 42 +++++++++----------
>  1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 5e62aa40ecd0..0ea71de0f56b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2335,6 +2335,26 @@ static ssize_t ts_source_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(ts_source);
>
> +/*
> + * Trace ID allocated dynamically on enable - but also allocate on read
> + * in case sysfs or perf read before enable to ensure consistent metadata
> + * information for trace decode
> + */
> +static ssize_t trctraceid_show(struct device *dev,
> +                              struct device_attribute *attr,
> +                              char *buf)
> +{
> +       int trace_id;
> +       struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +       trace_id = etm4_read_alloc_trace_id(drvdata);
> +       if (trace_id < 0)
> +               return trace_id;
> +
> +       return sysfs_emit(buf, "0x%x\n", trace_id);
> +}
> +static DEVICE_ATTR_RO(trctraceid);
> +
>  static struct attribute *coresight_etmv4_attrs[] = {
>         &dev_attr_nr_pe_cmp.attr,
>         &dev_attr_nr_addr_cmp.attr,
> @@ -2390,29 +2410,10 @@ static struct attribute *coresight_etmv4_attrs[] = {
>         &dev_attr_vmid_masks.attr,
>         &dev_attr_cpu.attr,
>         &dev_attr_ts_source.attr,
> +       &dev_attr_trctraceid.attr,
>         NULL,
>  };
>
> -/*
> - * Trace ID allocated dynamically on enable - but also allocate on read
> - * in case sysfs or perf read before enable to ensure consistent metadata
> - * information for trace decode
> - */
> -static ssize_t trctraceid_show(struct device *dev,
> -                              struct device_attribute *attr,
> -                              char *buf)
> -{
> -       int trace_id;
> -       struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> -
> -       trace_id = etm4_read_alloc_trace_id(drvdata);
> -       if (trace_id < 0)
> -               return trace_id;
> -
> -       return sysfs_emit(buf, "0x%x\n", trace_id);
> -}
> -static DEVICE_ATTR_RO(trctraceid);
> -
>  struct etmv4_reg {
>         struct coresight_device *csdev;
>         u32 offset;
> @@ -2549,7 +2550,6 @@ static struct attribute *coresight_etmv4_mgmt_attrs[] = {
>         coresight_etm4x_reg(trcpidr3, TRCPIDR3),
>         coresight_etm4x_reg(trcoslsr, TRCOSLSR),
>         coresight_etm4x_reg(trcconfig, TRCCONFIGR),
> -       &dev_attr_trctraceid.attr,
>         coresight_etm4x_reg(trcdevarch, TRCDEVARCH),
>         NULL,
>  };
> --
> 2.33.0
>
> _______________________________________________
> CoreSight mailing list -- coresight@...ts.linaro.org
> To unsubscribe send an email to coresight-leave@...ts.linaro.org



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