linux-kernel - Re: [PATCH v3 2/7] coresight: Only check bottom two claim bits

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40e30ec8-a7ea-46b9-b687-aa7efa8cf0e1@arm.com>
Date: Fri, 21 Mar 2025 12:10:47 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: James Clark <james.clark@...aro.org>, Mike Leach <mike.leach@...aro.org>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>, leo.yan@....com
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH v3 2/7] coresight: Only check bottom two claim bits

On 20/03/2025 14:34, James Clark wrote:
> The use of the whole register and == could break the claim mechanism if
> any of the other bits are used in the future. The referenced doc "PSCI -
> ARM DEN 0022D" also says to only read and clear the bottom two bits.
> 
> Use FIELD_GET() to extract only the relevant part.
> 
> Reviewed-by: Leo Yan <leo.yan@....com>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 3 ++-
>   drivers/hwtracing/coresight/coresight-priv.h | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 8471aefeac76..26d149a4c579 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -131,7 +131,8 @@ coresight_find_out_connection(struct coresight_device *csdev,
>   
>   static inline u32 coresight_read_claim_tags(struct coresight_device *csdev)
>   {
> -	return csdev_access_relaxed_read32(&csdev->access, CORESIGHT_CLAIMCLR);
> +	return FIELD_GET(CORESIGHT_CLAIM_MASK,
> +			 csdev_access_relaxed_read32(&csdev->access, CORESIGHT_CLAIMCLR));
>   }
>   
>   static inline bool coresight_is_claimed_self_hosted(struct coresight_device *csdev)
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 82644aff8d2b..38bb4e8b50ef 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -35,6 +35,7 @@ extern const struct device_type coresight_dev_type[];
>    * Coresight device CLAIM protocol.
>    * See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
>    */
> +#define CORESIGHT_CLAIM_MASK		GENMASK(1, 0)
>   #define CORESIGHT_CLAIM_SELF_HOSTED	BIT(1)

I am checking with the Arm CoreSight architects on this. This is
problematic, if another agent is assigned, say BIT(2) and we wouldn't
forward compatible.

Suzuki


>   
>   #define TIMEOUT_US		100
>