[PATCH 00/19] coresight: Support for ETMv4.4 system instructions

[Mike Leach]

Hi Suzuki,

I've looked at the set and have only one real gripe - the
implementation and timing of component detection on the sysreg path.
I've summarised my thoughts here, but as the changes are found across
multiple patches I may well have repeated myself a little in the
individual places.

On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose <suzuki.poulose at arm.com> wrote:
>
> CoreSight ETMv4.4 introduced system instructions for accessing
> the ETM. This also implies that they may not be on the amba bus.

System instructions have always been an option - but we have never
supported them up to now. In fact both memory access and system
instructions can live side by side - but the driver really needs to
choose just one!
What did happen is that a PE that supports Arm Trace 8.4 mandates ETM
4.4, and ETM 4.4 mandates system instruction access for PEs with Arm
Trace 8.4, and deprecates memory access.

But there is nothing to stop other variants having the  system
instruction interface. So there is no need to describe this as a
purely 4.4. onwards support - it will support any version of the ETM
that has sysreg access.

The spec permits aarch32 / armv7 register access via CP14 - but I
assume this is omitted deliberately & not intended to be supported at
this time.

> Right now all the CoreSight components are accessed via memory
> map. Also, we have some common routines in coresight generic
> code driver (e.g, CS_LOCK, claim/disclaim), which assume the
> mmio. In order to preserve the generic algorithms at a single
> place and to allow dynamic switch for ETMs, this series introduces
> an abstraction layer for accessing a coresight device. It is
> designed such that the mmio access are fast tracked (i.e, without
> an indirect function call).
>
> This will also help us to get rid of the driver+attribute specific
> sysfs show/store routines and replace them with a single routine
> to access a given register offset (which can be embedded in the
> dev_ext_attribute). This is not currently implemented in the series,
> but can be achieved.
>
> Further we switch the generic routines to work with the abstraction.
> With this in place, we refactor the etm4x code a bit to allow for
> supporting the system instructions with very little new code. The
> changes also switch to using the system instructions by default
> even when we may have an MMIO.
>
> We use TRCDEVARCH for the detection of the ETM component, which
> is a standard register as per CoreSight architecture, rather than
> the etm specific id register TRCIDR3. This is for making sure
> that we are able to detect the ETM via system instructions accurately,
> when the the trace unit could be anything (etm or a custom trace unit).
>

I'm assuming you mean TRCIDR1 here -- which in part, defines the etm
architecture version. TRCIDR3 does something else entirely.
Not sure I agree with this though - the driver is designed to match
the ETM spec so there is no problem with using TRCIDR1 to spot
functional variants according to ETM version,

The etm4_init_arch_data() function is not about detecting the presence
of an ETMv4 component, but about exploring the capabilities it has. We
check 4 bits of the version as a sanity check, but at this point we
should be pretty sure we are dealing with an ETM of some kind.

TRCDEVARCH is already used for detection in the AMBA matching code -
assuming the table includes the optional CoreSIght UCI. I would
imagine that similar detection needs to go on for instruction access -
but once we have detected an ETM, then ETM architected registers are
sufficient. If the device is not an ETM then it should be detected and
rejected early - and the bindings examined to determine why this
driver was attached!

The act of adding in a check against TRCDEVARCH as part of the
etm4_init_arch_data() function adds new and hidden checks to AMBA
devices where it was sufficient to have an entry in the probe match
table before. Most recent additions include the UCI matching, but
older ones don't. I am concerned that this changes may trip up older
existing implementations which for some reason may not have
TRCDEVCARCH, or have set it to not present.

For this reason, I beleive that the TRCDEVARCH check for the sys reg
access should occur on the sysreg specific probe - balancing what
happens on the AMBA side.  That way the common code remains common.
Further the setup of the CSA for the device can happen immediately in
the common etm_probe() function, based on *base being NULL or not,
rather than as a side effect of the etm4_init_arch_data() call.


Regards

Mike


> The series has been mildly tested on a model. I would really
> appreciate any testing on real hardware.
>
> Applies on coresight/next
>
> Changes since V1:
>   - Flip the switch for iomem from no_iomem to io_mem in csdev_access.
>   - Split patches for claim/disclaim and CS_LOCK/UNLOCK conversions.
>   - Move device access initialisation for etm4x to the target CPU
>   - Cleanup secure exception level mask handling.
>   - Switch to use TRCDEVARCH for ETM component discovery. This
>     is for making
>   - Check the availability of OS/Software Locks before using them.
>
>
> Suzuki K Poulose (19):
>   coresight: Introduce device access abstraction
>   coresight: tpiu: Prepare for using coresight device access abstraction
>   coresight: Convert coresight_timeout to use access abstraction
>   coresight: Convert claim/disclaim operations to use access wrappers
>   coresight: Use device access layer for Software lock/unlock operations
>   coresight: etm4x: Always read the registers on the host CPU
>   coresight: etm4x: Convert all register accesses
>   coresight: etm4x: Add commentary on the registers
>   coresight: etm4x: Add sysreg access helpers
>   coresight: etm4x: Define DEVARCH register fields
>   coresight: etm4x: Check for OS and Software Lock
>   coresight: etm4x: Cleanup secure exception level masks
>   coresight: etm4x: Clean up exception level masks
>   coresight: etm4x: Detect access early on the target CPU
>   coresight: etm4x: Use TRCDEVARCH for component discovery
>   coresight: etm4x: Detect system instructions support
>   coresight: etm4x: Refactor probing routine
>   coresight: etm4x: Add support for sysreg only devices
>   dts: bindings: coresight: ETMv4.4 system register access only units
>
>  .../devicetree/bindings/arm/coresight.txt     |   6 +-
>  drivers/hwtracing/coresight/coresight-catu.c  |  24 +-
>  .../hwtracing/coresight/coresight-cpu-debug.c |  22 +-
>  .../hwtracing/coresight/coresight-cti-sysfs.c |   5 +-
>  drivers/hwtracing/coresight/coresight-cti.c   |  34 +-
>  drivers/hwtracing/coresight/coresight-etb10.c |  29 +-
>  .../coresight/coresight-etm3x-sysfs.c         |  10 +-
>  drivers/hwtracing/coresight/coresight-etm3x.c |  35 +-
>  .../coresight/coresight-etm4x-sysfs.c         |  44 +-
>  drivers/hwtracing/coresight/coresight-etm4x.c | 716 +++++++++++-------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 440 ++++++++++-
>  .../hwtracing/coresight/coresight-funnel.c    |  22 +-
>  drivers/hwtracing/coresight/coresight-priv.h  |   9 +-
>  .../coresight/coresight-replicator.c          |  31 +-
>  drivers/hwtracing/coresight/coresight-stm.c   |  50 +-
>  .../hwtracing/coresight/coresight-tmc-etf.c   |  38 +-
>  .../hwtracing/coresight/coresight-tmc-etr.c   |  20 +-
>  drivers/hwtracing/coresight/coresight-tmc.c   |  16 +-
>  drivers/hwtracing/coresight/coresight-tpiu.c  |  32 +-
>  drivers/hwtracing/coresight/coresight.c       | 130 +++-
>  include/linux/coresight.h                     | 230 +++++-
>  21 files changed, 1449 insertions(+), 494 deletions(-)
>
> --
> 2.24.1
>


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