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: <5b4c5e2d-75c3-4236-81ad-ddc94945a54b@amd.com>
Date: Wed, 22 Oct 2025 01:07:21 -0500
From: "Naik, Avadhut" <avadnaik@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-edac@...r.kernel.org, yazen.ghannam@....com,
 linux-kernel@...r.kernel.org, Avadhut Naik <avadhut.naik@....com>
Subject: Re: [PATCH 2/4] EDAC/amd64: Remove NUM_CONTROLLERS macro



On 10/21/2025 05:44, Borislav Petkov wrote:
> 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.
> 
Okay!

>> 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.
>
Will change this!
 
>> 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.
> 
Will do!

>> 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.
> 
Will remove the *DCTs* word altogether.

>> 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.
> 
Will revert this!

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

-- 
Thanks,
Avadhut Naik