Re: [PATCH 1/2] coresight: etm4x: Expose default timestamp source in sysfs

Mathieu Poirier
2022-05-02T16:50:33Z

On Fri, Apr 29, 2022 at 01:30:59PM +0100, German Gomez wrote:
> Add a new sysfs interface in /sys/bus/coresight/devices/etm<N>/ts_source
> indicating the configured timestamp source when the ETM device driver
> was probed.
>
> The perf tool will use this information to detect if the trace data
> timestamp matches the kernel time, enabling correlation of CoreSight
> trace with perf events.
>
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: German Gomez <german.gomez@arm.com>
> ---
> arch/arm64/include/asm/sysreg.h | 1 +
> .../coresight/coresight-etm4x-sysfs.c | 34 +++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 38508e507d73a..263a7bee06f9a 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1146,6 +1146,7 @@
> #define SYS_MPIDR_SAFE_VAL (BIT(31))
>
> #define TRFCR_ELx_TS_SHIFT 5
> +#define TRFCR_ELx_TS_MASK ((0x3UL) << TRFCR_ELx_TS_SHIFT)
> #define TRFCR_ELx_TS_VIRTUAL ((0x1UL) << TRFCR_ELx_TS_SHIFT)
> #define TRFCR_ELx_TS_GUEST_PHYSICAL ((0x2UL) << TRFCR_ELx_TS_SHIFT)
> #define TRFCR_ELx_TS_PHYSICAL ((0x3UL) << TRFCR_ELx_TS_SHIFT)
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a0640fa5c55bd..c0c375c0cfde2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2264,6 +2264,39 @@ static ssize_t cpu_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(cpu);
>
> +static int etmv4_to_ts_source(struct etmv4_drvdata *drvdata)
> +{
> + int val;
> +
> + if (!drvdata->trfcr)
> + return -1;
> +
> + switch (drvdata->trfcr & TRFCR_ELx_TS_MASK) {
> + case TRFCR_ELx_TS_VIRTUAL:
> + case TRFCR_ELx_TS_GUEST_PHYSICAL:
> + case TRFCR_ELx_TS_PHYSICAL:
> + val = FIELD_GET(TRFCR_ELx_TS_MASK, drvdata->trfcr);
> + break;
> + default:
> + val = -1;
> + break;
> + }

I don't think the above is big and/or complex enough to mandate a separate
function - please move to ts_source_show().

> +
> + return val;
> +}
> +
> +static ssize_t ts_source_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + int val;
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> + val = etmv4_to_ts_source(drvdata);
> + return sysfs_emit(buf, "%d\n", val);
> +}
> +static DEVICE_ATTR_RO(ts_source);
> +
> static struct attribute *coresight_etmv4_attrs[] = {
> &dev_attr_nr_pe_cmp.attr,
> &dev_attr_nr_addr_cmp.attr,
> @@ -2318,6 +2351,7 @@ static struct attribute *coresight_etmv4_attrs[] = {
> &dev_attr_vmid_val.attr,
> &dev_attr_vmid_masks.attr,
> &dev_attr_cpu.attr,
> + &dev_attr_ts_source.attr,
> NULL,
> };
>
> --
> 2.25.1