On 09/18/2020 04:35 PM, Mike Leach wrote:
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
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)
Very good point and I agree. I could restrict this to system instruction based devices and use the TRCIDR1 for
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.
I agree that we have to read up the registers. However, for system instructions based devices, we shouldn't try to access random register space, if they are not ETM. Moreover, it kind of simplifies the logic, where you don't have to read up all the registers if this is not a supported device in the first place.
Or the other way around, I think the priority is to make sure we are dealing with a valid device we support, before we tread into reading the register space to avoid unknown side effects of the operations.
Thanks
Suzuki