| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c1b23a12-5fb7-4e70-f4c3-4cdf2b13f392@arm.com>
Date: Tue, 21 Mar 2023 17:26:42 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Suzuki K Poulose <suzuki.poulose@....com>,
coresight@...ts.linaro.org
Cc: mike.leach@...aro.org, james.clark@....com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Steve Clevenger <scclevenger@...amperecomputing.com>
Subject: Re: [PATCH v2] coresight: etm4x: Do not access TRCIDR1 for
identification
On 3/21/23 16:15, Suzuki K Poulose wrote:
> CoreSight ETM4x architecture clearly provides ways to identify a device
> via registers in the "Management" class, TRCDEVARCH and TRCDEVTYPE. These
> registers can be accessed without the Trace domain being powered on.
> We additionally added TRCIDR1 as fallback in order to cover for any
> ETMs that may not have implemented TRCDEVARCH. So far, nobody has
> reported hitting a WARNING we placed to catch such systems.
>
> Also, more importantly it is problematic to access TRCIDR1, which is a
> "Trace" register via MMIO access, without clearing the OSLK. But we cannot
> mess with the OSLK until we know for sure that this is an ETMv4 device.
> Thus, this kind of creates a chicken and egg problem unnecessarily for
> systems "which are compliant" to the ETMv4 architecture.
>
> Let us remove the TRCIDR1 fall back check and rely only on TRCDEVARCH.
>
> Reported-by: Steve Clevenger <scclevenger@...amperecomputing.com>
> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
> Cc: Mike Leach <mike.leach@...aro.org>
> Cc: James Clark <james.clark@....com>
> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
> Reviewed-by: Mike Leach <mike.leach@...aro.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
>
> Changes since v2:
> - Remove unused etm_tridr_to_arch() helper
> - Add comment to explain why TRCIDR1 cannot be used.
>
> ---
> .../coresight/coresight-etm4x-core.c | 22 ++++++++-----------
> drivers/hwtracing/coresight/coresight-etm4x.h | 20 +++++------------
> 2 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 104333c2c8a3..4c15fae534f3 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1070,25 +1070,21 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata,
> struct csdev_access *csa)
> {
> u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH);
> - u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1);
>
> /*
> * All ETMs must implement TRCDEVARCH to indicate that
> - * the component is an ETMv4. To support any broken
> - * implementations we fall back to TRCIDR1 check, which
> - * is not really reliable.
> + * the component is an ETMv4. Even though TRCIDR1 also
> + * contains the information, it is part of the "Trace"
> + * register and must be accessed with the OSLK cleared,
> + * with MMIO. But we cannot touch the OSLK until we are
> + * sure this is an ETM. So rely only on the TRCDEVARCH.
> */
> - if ((devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH) {
> - drvdata->arch = etm_devarch_to_arch(devarch);
> - } else {
> - pr_warn("CPU%d: ETM4x incompatible TRCDEVARCH: %x, falling back to TRCIDR1\n",
> - smp_processor_id(), devarch);
> -
> - if (ETM_TRCIDR1_ARCH_MAJOR(idr1) != ETM_TRCIDR1_ARCH_ETMv4)
> - return false;
> - drvdata->arch = etm_trcidr_to_arch(idr1);
> + if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
> + pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n");
> + return false;
> }
>
> + drvdata->arch = etm_devarch_to_arch(devarch);
> *csa = CSDEV_ACCESS_IOMEM(drvdata->base);
> return true;
> }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 434f4e95ee17..27c8a9901868 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -753,14 +753,12 @@
> * TRCDEVARCH - CoreSight architected register
> * - Bits[15:12] - Major version
> * - Bits[19:16] - Minor version
> - * TRCIDR1 - ETM architected register
> - * - Bits[11:8] - Major version
> - * - Bits[7:4] - Minor version
> - * We must rely on TRCDEVARCH for the version information,
> - * however we don't want to break the support for potential
> - * old implementations which might not implement it. Thus
> - * we fall back to TRCIDR1 if TRCDEVARCH is not implemented
> - * for memory mapped components.
> + *
> + * We must rely only on TRCDEVARCH for the version information. Even though,
> + * TRCIDR1 also provides the architecture version, it is a "Trace" register
> + * and as such must be accessed only with Trace power domain ON. This may
> + * not be available at probe time.
> + *
> * Now to make certain decisions easier based on the version
> * we use an internal representation of the version in the
> * driver, as follows :
> @@ -786,12 +784,6 @@ static inline u8 etm_devarch_to_arch(u32 devarch)
> ETM_DEVARCH_REVISION(devarch));
> }
>
> -static inline u8 etm_trcidr_to_arch(u32 trcidr1)
> -{
> - return ETM_ARCH_VERSION(ETM_TRCIDR1_ARCH_MAJOR(trcidr1),
> - ETM_TRCIDR1_ARCH_MINOR(trcidr1));
> -}
> -
> enum etm_impdef_type {
> ETM4_IMPDEF_HISI_CORE_COMMIT,
> ETM4_IMPDEF_FEATURE_MAX,
Powered by blists - more mailing lists