linux-kernel - Re: [PATCH 2/4] EDAC/amd64: Remove NUM

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] [day] [month] [year] [list]

Message-ID: <20251021104426.GJaPdkCgb5JjXqvSZz@fat_crate.local>
Date: Tue, 21 Oct 2025 12:44:26 +0200
From: Borislav Petkov <bp@...en8.de>
To: Avadhut Naik <avadhut.naik@....com>
Cc: linux-edac@...r.kernel.org, yazen.ghannam@....com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] EDAC/amd64: Remove NUM_CONTROLLERS macro

On Mon, Oct 13, 2025 at 05:30:41PM +0000, Avadhut Naik wrote:
> Currently, the NUM_CONTROLLERS macro is only used to statically allocate
> the csels array of struct chip_select in struct amd64_pvt.

"... is used to limit the amount of memory controllers available per node."

You don't need to explain the code - think big picture.

> The size of this array, however, will never exceed the number of UMCs on
> the SOC.

Not on the SOC - the thing is per node instance.

> Since, max_mcs variable in struct amd64_pvt already stores the
> number of UMCs on the SOC, the macro can be removed and the static array

Please describe your changes in imperative mood.

Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details.

> can be dynamically allocated instead.
> 
> The max_mcs variable and the csels array are used for legacy systems too.
> These systems have a max of 2 controllers (DCTs). Since the default value

DCTs are DRAM controllers. Do not confuse the reader.

> of max_mcs, set in per_family_init(), is 2, these legacy system are also
> covered by this change.

...

> @@ -347,8 +346,8 @@ struct amd64_pvt {
>  	u32 dbam0;		/* DRAM Base Address Mapping reg for DCT0 */
>  	u32 dbam1;		/* DRAM Base Address Mapping reg for DCT1 */
>  
> -	/* one for each DCT/UMC */
> -	struct chip_select csels[NUM_CONTROLLERS];
> +	/* Allocate one for each DCT/UMC */

You're not allocating here anything. Just explain what this variable
represents - IOW, the comment was fine.

> +	struct chip_select *csels;
>  
>  	/* DRAM base and limit pairs F1x[78,70,68,60,58,50,48,40] */
>  	struct dram_range ranges[DRAM_RANGES];
> -- 
> 2.43.0
> 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette