linux-kernel - Re: [PATCH v2 3/6] coresight: Repack struct etmv4

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: <20250930144153.GI7985@e132581.arm.com>
Date: Tue, 30 Sep 2025 15:41:53 +0100
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jonathan Corbet <corbet@....net>, coresight@...ts.linaro.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 3/6] coresight: Repack struct etmv4_drvdata

On Thu, Aug 14, 2025 at 11:49:54AM +0100, James Clark wrote:
> Fix holes and convert the long list of bools to single bits to save
> some space because there's one of these for each ETM.
> 
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x.h | 39 ++++++++++++++-------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index a355a1e9606d..1c67b263b01b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -1020,29 +1020,30 @@ struct etmv4_drvdata {
>  	u8				ns_ex_level;
>  	u8				q_support;
>  	u8				os_lock_model;
> -	bool				sticky_enable;
> -	bool				boot_enable;
> -	bool				os_unlock;
> -	bool				instrp0;
> -	bool				q_filt;
> -	bool				trcbb;
> -	bool				trccond;
> -	bool				retstack;
> -	bool				trccci;
> -	bool				trc_error;
> -	bool				syncpr;
> -	bool				stallctl;
> -	bool				sysstall;
> -	bool				nooverflow;
> -	bool				atbtrig;
> -	bool				lpoverride;
> +	bool				sticky_enable : 1;
> +	bool				boot_enable : 1;
> +	bool				os_unlock : 1;
> +	bool				instrp0 : 1;
> +	bool				q_filt : 1;
> +	bool				trcbb : 1;
> +	bool				trccond : 1;
> +	bool				retstack : 1;
> +	bool				trccci : 1;
> +	bool				trc_error : 1;
> +	bool				syncpr : 1;
> +	bool				stallctl : 1;
> +	bool				sysstall : 1;
> +	bool				nooverflow : 1;
> +	bool				atbtrig : 1;
> +	bool				lpoverride : 1;
> +	bool				state_needs_restore : 1;
> +	bool				skip_power_up : 1;
> +	bool				paused : 1;

I used pahole to check the structure layout. It is good to see that
bool fields are packed into single cache line (and we don't expect
these fields to modified frequently so no concern for false sharing).

  /* XXX 1 byte hole, try to pack */

  u16                        ccitmin;              /*   120     2 */
  u8                         s_ex_level;           /*   122     1 */
  u8                         ns_ex_level;          /*   123     1 */
  u8                         q_support;            /*   124     1 */
  u8                         os_lock_model;        /*   125     1 */
  bool                       sticky_enable:1;      /*   126: 0  1 */
  bool                       boot_enable:1;        /*   126: 1  1 */
  bool                       os_unlock:1;          /*   126: 2  1 */
  bool                       instrp0:1;            /*   126: 3  1 */
  bool                       q_filt:1;             /*   126: 4  1 */
  bool                       trcbb:1;              /*   126: 5  1 */
  bool                       trccond:1;            /*   126: 6  1 */
  bool                       retstack:1;           /*   126: 7  1 */
  bool                       trccci:1;             /*   127: 0  1 */
  bool                       trc_error:1;          /*   127: 1  1 */
  bool                       syncpr:1;             /*   127: 2  1 */
  bool                       stallctl:1;           /*   127: 3  1 */
  bool                       sysstall:1;           /*   127: 4  1 */
  bool                       nooverflow:1;         /*   127: 5  1 */
  bool                       atbtrig:1;            /*   127: 6  1 */
  bool                       lpoverride:1;         /*   127: 7  1 */
  /* --- cacheline 2 boundary (128 bytes) --- */
  bool                       state_needs_restore:1; /*   128: 0  1 */
  bool                       skip_power_up:1;      /*   128: 1  1 */
  bool                       paused:1;             /*   128: 2  1 */

Reviewed-by: Leo Yan <leo.yan@....com>

>  	u64				trfcr;
>  	struct etmv4_config		config;
>  	u64				save_trfcr;
>  	struct etmv4_save_state		*save_state;
> -	bool				state_needs_restore;
> -	bool				skip_power_up;
> -	bool				paused;
> +
>  	DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
>  };
>  
> 
> -- 
> 2.34.1
>