linux-kernel - Re: [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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250414131101.GA65192@yaz-khff2.amd.com>
Date: Mon, 14 Apr 2025 09:11:01 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: "Naik, Avadhut" <avadnaik@....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: Re: [PATCH] EDAC/amd64: Fix size calculation for Non-Power-of-Two
 DIMMs

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?

Thanks,
Yazen