[PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Suzuki K Poulose]

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@os.amperecomputing.com>
Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 104333c2c8a3..c1b72d892d7d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1070,25 +1070,17 @@ 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
 	 */
-	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;
 }
-- 
2.34.1

[PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Suzuki K Poulose]

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@os.amperecomputing.com>
Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 104333c2c8a3..c1b72d892d7d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1070,25 +1070,17 @@ 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
 	 */
-	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;
 }
-- 
2.34.1


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

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Mike Leach]

On Fri, 17 Mar 2023 at 11:57, Suzuki K Poulose <suzuki.poulose@arm.com> 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@os.amperecomputing.com>
> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 104333c2c8a3..c1b72d892d7d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1070,25 +1070,17 @@ 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
>          */
> -       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;
>  }
> --
> 2.34.1
>
Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Mike Leach]

On Fri, 17 Mar 2023 at 11:57, Suzuki K Poulose <suzuki.poulose@arm.com> 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@os.amperecomputing.com>
> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 104333c2c8a3..c1b72d892d7d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1070,25 +1070,17 @@ 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
>          */
> -       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;
>  }
> --
> 2.34.1
>
Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

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

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Anshuman Khandual]

On 3/17/23 17:27, 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@os.amperecomputing.com>
> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 104333c2c8a3..c1b72d892d7d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1070,25 +1070,17 @@ 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
>  	 */
> -	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);

etm_trcidr_to_arch() does not seem to be used else where, should be dropped ?

> +	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;
>  }

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Anshuman Khandual]

On 3/17/23 17:27, 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@os.amperecomputing.com>
> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 104333c2c8a3..c1b72d892d7d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1070,25 +1070,17 @@ 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
>  	 */
> -	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);

etm_trcidr_to_arch() does not seem to be used else where, should be dropped ?

> +	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;
>  }

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

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Suzuki K Poulose]

On 21/03/2023 10:17, Anshuman Khandual wrote:
> 
> 
> On 3/17/23 17:27, 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@os.amperecomputing.com>
>> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>>   1 file changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 104333c2c8a3..c1b72d892d7d 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1070,25 +1070,17 @@ 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
>>   	 */
>> -	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);
> 
> etm_trcidr_to_arch() does not seem to be used else where, should be dropped ?

Good point, will drop it. Thanks for the review.

Suzuki

Re: [PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

[Suzuki K Poulose]

On 21/03/2023 10:17, Anshuman Khandual wrote:
> 
> 
> On 3/17/23 17:27, 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@os.amperecomputing.com>
>> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.1677881753.git.scclevenger@os.amperecomputing.com/
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery")
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
>>   1 file changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 104333c2c8a3..c1b72d892d7d 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1070,25 +1070,17 @@ 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
>>   	 */
>> -	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);
> 
> etm_trcidr_to_arch() does not seem to be used else where, should be dropped ?

Good point, will drop it. Thanks for the review.

Suzuki


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