linux-kernel - [PATCH] EDAC/amd64: Fix size calculation for Non-Power-of-Two DIMMs

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: <9a33b6ff-9ce8-4abd-8629-3c9f6a546514@amd.com>
Date: Mon, 14 Apr 2025 11:36:41 -0500
From: "Naik, Avadhut" <avadnaik@....com>
To: Yazen Ghannam <yazen.ghannam@....com>
Cc: linux-edac@...r.kernel.org, bp@...en8.de, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org, Avadhut Naik <avadhut.naik@....com>
Subject: [PATCH] EDAC/amd64: Fix size calculation for Non-Power-of-Two DIMMs



On 4/14/2025 08:11, Yazen Ghannam wrote:
> On Wed, Apr 09, 2025 at 12:48:57AM -0500, Naik, Avadhut wrote:
> 
> [...]
> 
>>>>  
>>>>  	edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
>>>> -	edac_dbg(1, "  Original AddrMask: 0x%x\n", addr_mask_orig);
>>>> -	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
>>>> +	edac_dbg(1, "  Primary AddrMask: 0x%x\n", addr_mask);
>>>>  
>>>>  	/* Register [31:1] = Address [39:9]. Size is in kBs here. */
>>>> -	size = (addr_mask_deinterleaved >> 2) + 1;
>>>> +	size = calculate_cs_size(addr_mask, cs_mode);
>>>> +
>>>> +	if (addr_mask_sec) {
>>>
>>> I think we can skip this check.
>>>
>> Commented below on this.
>>
>>> For debug messages, it doesn't hurt to be more explicit. So printing a
>>> 'mask: 0x0' message is more helpful/reassuring than 'no message'.
>>>
>>>> +		edac_dbg(1, "  Secondary AddrMask: 0x%x\n", addr_mask);
>>>
>>> addr_mask -> addr_mask_sec
>>>
>>>> +		size += calculate_cs_size(addr_mask_sec, cs_mode);
>>>
>>> Maybe add a "!mask" check to return early if you want to save some
>>> cycles in this helper function.
>>>
>> In a way, this is the reason why I had added the above condition check.
>> To avoid unnecessary function calls.
>>
>> AFAIK, power-of-2 DIMMs are predominantly used, so the Secondary Address
>> Mask register will seldom be used.
>>
>> Would you agree?
> 
> I don't know. It seems non-power-of-2 (24G, 48G, 96G) is becoming more
> common for individual DIMMs and sets of DIMMs.
> 
> Thinking on it more, I don't think we need to be concerned about saving
> cycles here. These functions are only called during module init,
> correct?
> 
Yes, this should only be executed during module init.
Get your point. Will remove the condition check and move it to the
new helper function.

> Thanks,
> Yazen

-- 
Thanks,
Avadhut Naik