linux-kernel - Re: [PATCH v1] RISC-V: hwprobe: sort EXT_KEY()s in hwprobe_isa

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: <cba0c880-a1b2-4bb8-bef0-d280d87ec308@rivosinc.com>
Date: Wed, 17 Jul 2024 15:34:06 +0200
From: Clément Léger <cleger@...osinc.com>
To: Conor Dooley <conor@...nel.org>, linux-riscv@...ts.infradead.org
Cc: Conor Dooley <conor.dooley@...rochip.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] RISC-V: hwprobe: sort EXT_KEY()s in hwprobe_isa_ext0()
 alphabetically



On 17/07/2024 10:54, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
> 
> Currently the entries appear to be in a random order (although according
> to Palmer he has tried to sort them by key value) which makes it harder
> to find entries in a growing list, and more likely to have conflicts as
> all patches are adding to the end of the list. Sort them alphabetically
> instead.
> 
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
> CC: Paul Walmsley <paul.walmsley@...ive.com>
> CC: Palmer Dabbelt <palmer@...belt.com>
> CC: linux-riscv@...ts.infradead.org
> CC: linux-kernel@...r.kernel.org
> ---
>  arch/riscv/kernel/sys_hwprobe.c | 43 ++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 685594769535..8a1c9ce170e8 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -93,44 +93,45 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		 * regardless of the kernel's configuration, as no other checks, besides
>  		 * presence in the hart_isa bitmap, are made.
>  		 */
> +		EXT_KEY(ZACAS);
> +		EXT_KEY(ZAWRS);
>  		EXT_KEY(ZBA);
>  		EXT_KEY(ZBB);
> -		EXT_KEY(ZBS);
> -		EXT_KEY(ZICBOZ);
>  		EXT_KEY(ZBC);
> -
>  		EXT_KEY(ZBKB);
>  		EXT_KEY(ZBKC);
>  		EXT_KEY(ZBKX);
> +		EXT_KEY(ZBS);
> +		EXT_KEY(ZCA);
> +		EXT_KEY(ZCB);
> +		EXT_KEY(ZCMOP);
> +		EXT_KEY(ZICBOZ);
> +		EXT_KEY(ZICOND);
> +		EXT_KEY(ZIHINTNTL);
> +		EXT_KEY(ZIHINTPAUSE);
> +		EXT_KEY(ZIMOP);
>  		EXT_KEY(ZKND);
>  		EXT_KEY(ZKNE);
>  		EXT_KEY(ZKNH);
>  		EXT_KEY(ZKSED);
>  		EXT_KEY(ZKSH);
>  		EXT_KEY(ZKT);
> -		EXT_KEY(ZIHINTNTL);
>  		EXT_KEY(ZTSO);
> -		EXT_KEY(ZACAS);
> -		EXT_KEY(ZICOND);
> -		EXT_KEY(ZIHINTPAUSE);
> -		EXT_KEY(ZIMOP);
> -		EXT_KEY(ZCA);
> -		EXT_KEY(ZCB);
> -		EXT_KEY(ZCMOP);
> -		EXT_KEY(ZAWRS);
>  
>  		/*
>  		 * All the following extensions must depend on the kernel
>  		 * support of V.
>  		 */
>  		if (has_vector()) {
> -			EXT_KEY(ZVE32X);
> -			EXT_KEY(ZVE32F);
> -			EXT_KEY(ZVE64X);
> -			EXT_KEY(ZVE64F);
> -			EXT_KEY(ZVE64D);
>  			EXT_KEY(ZVBB);
>  			EXT_KEY(ZVBC);
> +			EXT_KEY(ZVE32F);
> +			EXT_KEY(ZVE32X);
> +			EXT_KEY(ZVE64D);
> +			EXT_KEY(ZVE64F);
> +			EXT_KEY(ZVE64X);
> +			EXT_KEY(ZVFH);
> +			EXT_KEY(ZVFHMIN);
>  			EXT_KEY(ZVKB);
>  			EXT_KEY(ZVKG);
>  			EXT_KEY(ZVKNED);
> @@ -139,16 +140,14 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  			EXT_KEY(ZVKSED);
>  			EXT_KEY(ZVKSH);
>  			EXT_KEY(ZVKT);
> -			EXT_KEY(ZVFH);
> -			EXT_KEY(ZVFHMIN);
>  		}
>  
>  		if (has_fpu()) {
> -			EXT_KEY(ZFH);
> -			EXT_KEY(ZFHMIN);
> -			EXT_KEY(ZFA);
>  			EXT_KEY(ZCD);
>  			EXT_KEY(ZCF);
> +			EXT_KEY(ZFA);
> +			EXT_KEY(ZFH);
> +			EXT_KEY(ZFHMIN);
>  		}
>  #undef EXT_KEY
>  	}

I'd prefer that to be done after removing the "if
(has_vector()/has_fpu()) by using the .validate callback for ISA
extension. This way, you'll have only a single commit reordering everything.

Thanks,

Clément