Re: [PATCH 15/19] coresight: etm4x: Use TRCDEVARCH for component discovery

Hi Suzuki

On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose suzuki.poulose@arm.com wrote:

We have been using TRCIDR1 for detecting the ETM version. As we are about to discover the trace unit on a CPU, let us use a CoreSight architected register, instead of an ETM architected register for accurate detection on a CPU. e.g, a CPU might implement a custom trace unit, not an ETM.

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

drivers/hwtracing/coresight/coresight-etm4x.c | 50 ++++++++++--------- drivers/hwtracing/coresight/coresight-etm4x.h | 4 +- 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 5880f105268f..0fce9fb12cff 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -61,6 +61,17 @@ struct etm_init_arg { struct csdev_access *csa; };

+static inline u8 etm_devarch_arch(u32 devarch) +{

•   return (devarch >> ETM_DEVARCH_ARCHID_ARCH_VER_SHIFT) & 0xfUL;
  

+}

+static inline u8 etm_devarch_rev(u32 devarch) +{

•   return (devarch & ETM_DEVARCH_REVISION_MASK) >>
  

•           ETM_DEVARCH_REVISION_SHIFT;
  

+}

u64 etm4x_sysreg_read(struct csdev_access *csa, u32 offset, bool _relaxed, @@ -149,18 +160,6 @@ static void etm4_cs_unlock(struct etmv4_drvdata *drvdata, CS_UNLOCK(csa); }

-static bool etm4_arch_supported(u8 arch) -{

•   /* Mask out the minor version number */
  

•   switch (arch & 0xf0) {
  

•   case ETM_ARCH_V4:
  

•           break;
  

•   default:
  

•           return false;
  

•   }
  

•   return true;
  

-}

static int etm4_cpu_id(struct coresight_device *csdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -694,10 +693,23 @@ static void etm_detect_lock_status(struct etmv4_drvdata *drvdata, drvdata->os_lock_model = TRCOSLSR_OSM(os_lsr); }

+static inline bool trace_unit_supported(u32 devarch) +{

•   return (devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH;
  

+}

This is fine for system reg access - but imposes additional constraints on memory mapped devices that previously may have worked just by matching CID/PID. Can you be certain there are no devices out there that have omitted this register (it does have a present bit after all)

static bool etm_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) {

•   u32 devarch;
  

•   devarch = readl_relaxed(drvdata->base + TRCDEVARCH);
  

•   if (!trace_unit_supported(devarch))
  

•           return false;
  

•   *csa = CSDEV_ACCESS_IOMEM(drvdata->base);
  

•   drvdata->arch = devarch;
  

•   return true;
  

}

@@ -713,7 +725,6 @@ static bool etm_init_csdev_access(struct etmv4_drvdata *drvdata, static void etm4_init_arch_data(void *info)

The primary task of this routine is to read all the ID registers and set up the data in regards to resources etc. We should not be mixing in a secondary task of detection of a valid device.

{ u32 etmidr0;

•   u32 etmidr1;
    u32 etmidr2;
    u32 etmidr3;
    u32 etmidr4;
  

@@ -784,14 +795,6 @@ static void etm4_init_arch_data(void *info) /* TSSIZE, bits[28:24] Global timestamp size field */ drvdata->ts_size = BMVAL(etmidr0, 24, 28);

•   /* base architecture of trace unit */
  

•   etmidr1 = etm4x_relaxed_read32(csa, TRCIDR1);
  

•   /*
  

•    * TRCARCHMIN, bits[7:4] architecture the minor version number
  

•    * TRCARCHMAJ, bits[11:8] architecture major versin number
  

•    */
  

•   drvdata->arch = BMVAL(etmidr1, 4, 11);
  

•   /* maximum size of resources */
    etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);
    /* CIDSIZE, bits[9:5] Indicates the Context ID size */
  

@@ -1618,7 +1621,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) etm4_init_arch_data, &init_arg, 1)) dev_err(dev, "ETM arch init failed\n");

•   if (etm4_arch_supported(drvdata->arch) == false) {
  

•   if (!drvdata->arch) {
            ret = -EINVAL;
            goto err_arch_supported;
    }
  

@@ -1653,7 +1656,8 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)

    pm_runtime_put(&adev->dev);
    dev_info(&drvdata->csdev->dev, "CPU%d: ETM v%d.%d initialized\n",

•            drvdata->cpu, drvdata->arch >> 4, drvdata->arch & 0xf);
  

•            drvdata->cpu, etm_devarch_arch(drvdata->arch),
  

•            etm_devarch_rev(drvdata->arch));
  
    if (boot_enable) {
            coresight_enable(drvdata->csdev);
  

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 277c22540db6..b217f16ad921 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -730,7 +730,7 @@ struct etmv4_save_state {

• @spinlock: Only one at a time pls.
• @mode: This tracer's mode, i.e sysFS, Perf or disabled.
• @cpu: The cpu this component is affined to.

• • @arch: ETM version number.

• • @arch: ETM device architecture register.
  • @nr_pe: The number of processing entity available for tracing.
  • @nr_pe_cmp: The number of processing entity comparator inputs that are

  •         available for tracing.
    

@@ -793,7 +793,7 @@ struct etmv4_drvdata { spinlock_t spinlock; local_t mode; int cpu;

•   u8                              arch;
  

•   u32                             arch;
    u8                              nr_pe;
    u8                              nr_pe_cmp;
    u8                              nr_addr_cmp;
  

-- 2.24.1

Regards

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