On 09/18/2020 04:35 PM, Mike Leach wrote:

Hi Suzuki,

On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose [off-list ref] wrote:

quoted

ETM v4.4 onwards adds support for system instruction access
to the ETM. Detect the support on an ETM and switch to using the
mode when available.

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
  drivers/hwtracing/coresight/coresight-etm4x.c | 31 +++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 0fce9fb12cff..dc5ac171db35 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c

@@ -693,11 +693,39 @@ static void etm_detect_lock_status(struct etmv4_drvdata *drvdata,
         drvdata->os_lock_model = TRCOSLSR_OSM(os_lsr);
  }

+static inline bool cpu_supports_sysreg_trace(void)
+{
+       u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
+
+       return ((dfr0 >> ID_AA64DFR0_TRACEVER_SHIFT) & 0xfUL) > 0;
+}
+

This will be an issue if you have an aarch32 device (eg Cortex-A32 or
similar, with ETM support but no aarch64)

Agreed and in fact this was part of the header file in my initial versions.
I could move it back there. However, the ETM4x even without this series doesn't
support aarch32. The compilation would fail for various reasons (e.g, readq()).

quoted

  static inline bool trace_unit_supported(u32 devarch)
  {
         return (devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH;
  }

+static bool etm_init_sysreg_access(struct etmv4_drvdata *drvdata,
+                                  struct csdev_access *csa)
+{
+       u32 devarch;
+
+       if (!cpu_supports_sysreg_trace())
+               return false;
+
+       devarch = read_etm4x_sysreg_const_offset(TRCDEVARCH);
+       if (!trace_unit_supported(devarch))
+               return false;
+       *csa = (struct csdev_access) {
+               .io_mem = false,
+               .read   = etm4x_sysreg_read,
+               .write  = etm4x_sysreg_write,
+       };
+
+       drvdata->arch = devarch;
+       return true;
+}
+
  static bool etm_init_iomem_access(struct etmv4_drvdata *drvdata,
                                   struct csdev_access *csa)
  {

@@ -716,6 +744,9 @@ static bool etm_init_iomem_access(struct etmv4_drvdata *drvdata,
  static bool etm_init_csdev_access(struct etmv4_drvdata *drvdata,
                                   struct csdev_access *csa)
  {
+       if (etm_init_sysreg_access(drvdata, csa))
+

Don't think we should enforce system instruction access if the device
tree has defined memory access. The driver cannot possibly know if
this is a mistake or deliberate (e.g. test / implementation bug fix).>
+               return true;

Agreed, will fix it.

quoted

         if (drvdata->base)
                 return etm_init_iomem_access(drvdata, csa);

--
2.24.1

The device tree bindings define the access support intended - and
there is access specific probing. i.e. the next patch splits amba (mem
access) / platform (sys access) driver probes, followed by the common
probe section. The  register / memory access support used should be
made there, and the detection of a compatible device for the register
access i.e. check TRCDEVARCH should be in the platform probe path too
- possibly simplifying things and ensuring the common code changes are
reduced.

I will address this in the next version. I believe we could work around
the problem of missing TRCDEVARCH on older platforms.

Thanks a lot for the review !

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel