diff mbox series

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

Message ID 20250415213150.755255-1-avadhut.naik@amd.com (mailing list archive)
State New
Headers show
Series [v2] EDAC/amd64: Fix size calculation for Non-Power-of-Two DIMMs | expand

Commit Message

Avadhut Naik April 15, 2025, 9:25 p.m. UTC 
Each Chip-Select (CS) of a Unified Memory Controller (UMC) on AMD EPYC
SOCs has an Address Mask and a Secondary Address Mask register associated
with it. The amd64_edac module logs DIMM sizes on a per-UMC per-CS
granularity during init using these two registers.

Currently, the module primarily considers only the Address Mask register
for computing DIMM sizes. The Secondary Address Mask register is only
considered for odd CS. Additionally, if it has been considered, the
Address Mask register is ignored altogether for that CS. For
power-of-two DIMMs, this is not an issue since only the Address Mask
register is used.

For non-power-of-two DIMMs, however, the Secondary Address Mask register
is used in conjunction with the Address Mask register. However, since the
module only considers either of the two registers for a CS, the size
computed by the module is incorrect. The Secondary Address Mask register
is not considered for even CS, and the Address Mask register is not
considered for odd CS.

Introduce a new helper function so that both Address Mask and Secondary
Address Mask registers are considered, when valid, for computing DIMM
sizes. Furthermore, also rename some variables for greater clarity.

Fixes: 81f5090db843 ("EDAC/amd64: Support asymmetric dual-rank DIMMs")
Reported-by: Žilvinas Žaltiena <zilvinas@natrix.lt>
Tested-by: Žilvinas Žaltiena <zilvinas@natrix.lt>
Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
Cc: stable@vger.kernel.org
```
Changes in v2:
1. Avoid unnecessary variable initialization.
2. Modify commit message to accurately reflect the changes.
3. Move check for non-zero Address Mask register into the new helper.

Links:
v1: https://lore.kernel.org/linux-edac/9a33b6ff-9ce8-4abd-8629-3c9f6a546514@amd.com/T/#mc0b1101055f12ccb06e5a251d16f186597ed4133
---
 drivers/edac/amd64_edac.c | 57 ++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 21 deletions(-)


base-commit: b4d2bada09b17fcd68a0f00811ca7f900ec988e6

Comments

Yazen Ghannam April 16, 2025, 2:10 p.m. UTC | #1 
On Tue, Apr 15, 2025 at 09:25:58PM +0000, Avadhut Naik wrote:
> Each Chip-Select (CS) of a Unified Memory Controller (UMC) on AMD EPYC

This affects Zen-based systems in general, not just the EPYC line.

> SOCs has an Address Mask and a Secondary Address Mask register associated
> with it. The amd64_edac module logs DIMM sizes on a per-UMC per-CS
> granularity during init using these two registers.
> 
> Currently, the module primarily considers only the Address Mask register
> for computing DIMM sizes. The Secondary Address Mask register is only
> considered for odd CS. Additionally, if it has been considered, the
> Address Mask register is ignored altogether for that CS. For
> power-of-two DIMMs, this is not an issue since only the Address Mask
> register is used.
> 
> For non-power-of-two DIMMs, however, the Secondary Address Mask register
> is used in conjunction with the Address Mask register. However, since the
> module only considers either of the two registers for a CS, the size
> computed by the module is incorrect. The Secondary Address Mask register
> is not considered for even CS, and the Address Mask register is not
> considered for odd CS.
> 
> Introduce a new helper function so that both Address Mask and Secondary
> Address Mask registers are considered, when valid, for computing DIMM
> sizes. Furthermore, also rename some variables for greater clarity.
> 
> Fixes: 81f5090db843 ("EDAC/amd64: Support asymmetric dual-rank DIMMs")
> Reported-by: Žilvinas Žaltiena <zilvinas@natrix.lt>

Checkpatch says this should also have a 'Closes' tag. I never thought
about this before, but it is mentioned in 'Documentation'. 

Closes: https://lore.kernel.org/dbec22b6-00f2-498b-b70d-ab6f8a5ec87e@natrix.lt

> Tested-by: Žilvinas Žaltiena <zilvinas@natrix.lt>

Minor nit: TIP tree handbook say 'Tested-by' goes after your SoB. I
should check my submissions too.

> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> Cc: stable@vger.kernel.org
> ```
> Changes in v2:
> 1. Avoid unnecessary variable initialization.
> 2. Modify commit message to accurately reflect the changes.
> 3. Move check for non-zero Address Mask register into the new helper.
> 
> Links:
> v1: https://lore.kernel.org/linux-edac/9a33b6ff-9ce8-4abd-8629-3c9f6a546514@amd.com/T/#mc0b1101055f12ccb06e5a251d16f186597ed4133

Use a 'permalink' rather than this threaded link. Search for
'permalink' on the web page.

Better option is to use this format:
  https://lore.kernel.org/<Message-ID>

As shown in 'Documentation/process/submitting-patches.rst'

Ex:
  https://lore.kernel.org/20250327210718.1640762-1-avadhut.naik@amd.com

Side note: I've been adding 'r/' between lore and Message-ID out of
habit. But it seems that this is no longer needed.

> ---
>  drivers/edac/amd64_edac.c | 57 ++++++++++++++++++++++++---------------
>  1 file changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 58b1482a0fbb..91d22e63bdb1 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -1209,7 +1209,9 @@ static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
>  	if (csrow_enabled(2 * dimm + 1, ctrl, pvt))
>  		cs_mode |= CS_ODD_PRIMARY;
>  
> -	/* Asymmetric dual-rank DIMM support. */
> +	if (csrow_sec_enabled(2 * dimm, ctrl, pvt))
> +		cs_mode |= CS_EVEN_SECONDARY;
> +
>  	if (csrow_sec_enabled(2 * dimm + 1, ctrl, pvt))
>  		cs_mode |= CS_ODD_SECONDARY;
>  
> @@ -1230,12 +1232,13 @@ static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
>  	return cs_mode;
>  }
>  
> -static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
> -				  int csrow_nr, int dimm)
> +static int calculate_cs_size(u32 mask, unsigned int cs_mode)
>  {
> -	u32 msb, weight, num_zero_bits;
> -	u32 addr_mask_deinterleaved;
> -	int size = 0;
> +	int msb, weight, num_zero_bits;
> +	u32 deinterleaved_mask;
> +
> +	if (!mask)
> +		return 0;
>  
>  	/*
>  	 * The number of zero bits in the mask is equal to the number of bits
> @@ -1248,19 +1251,30 @@ static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
>  	 * without swapping with the most significant bit. This can be handled
>  	 * by keeping the MSB where it is and ignoring the single zero bit.
>  	 */
> -	msb = fls(addr_mask_orig) - 1;
> -	weight = hweight_long(addr_mask_orig);
> +	msb = fls(mask) - 1;
> +	weight = hweight_long(mask);
>  	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
>  
>  	/* Take the number of zero bits off from the top of the mask. */
> -	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
> +	deinterleaved_mask = GENMASK(msb - num_zero_bits, 1);
> +	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", deinterleaved_mask);
> +
> +	return (deinterleaved_mask >> 2) + 1;
> +}
> +
> +static int __addr_mask_to_cs_size(u32 addr_mask, u32 addr_mask_sec,
> +				  unsigned int cs_mode, int csrow_nr, int dimm)
> +{
> +	int size;
>  
>  	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);
> +
> +	edac_dbg(1, "  Secondary AddrMask: 0x%x\n", addr_mask_sec);
> +	size += calculate_cs_size(addr_mask_sec, cs_mode);
>  
>  	/* Return size in MBs. */
>  	return size >> 10;
> @@ -1270,7 +1284,7 @@ static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  				    unsigned int cs_mode, int csrow_nr)
>  {
>  	int cs_mask_nr = csrow_nr;
> -	u32 addr_mask_orig;
> +	u32 addr_mask = 0, addr_mask_sec = 0;
>  	int dimm, size = 0;
>  
>  	/* No Chip Selects are enabled. */
> @@ -1308,13 +1322,13 @@ static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  	if (!pvt->flags.zn_regs_v2)
>  		cs_mask_nr >>= 1;
>  
> -	/* Asymmetric dual-rank DIMM support. */
> -	if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
> -		addr_mask_orig = pvt->csels[umc].csmasks_sec[cs_mask_nr];
> -	else
> -		addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
> +	if (cs_mode & (CS_EVEN_PRIMARY | CS_ODD_PRIMARY))
> +		addr_mask = pvt->csels[umc].csmasks[cs_mask_nr];
> +
> +	if (cs_mode & (CS_EVEN_SECONDARY | CS_ODD_SECONDARY))
> +		addr_mask_sec = pvt->csels[umc].csmasks_sec[cs_mask_nr];
>  
> -	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, dimm);
> +	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, dimm);
>  }
>  
>  static void umc_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
> @@ -3512,9 +3526,10 @@ static void gpu_get_err_info(struct mce *m, struct err_info *err)
>  static int gpu_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  				    unsigned int cs_mode, int csrow_nr)
>  {
> -	u32 addr_mask_orig = pvt->csels[umc].csmasks[csrow_nr];
> +	u32 addr_mask		= pvt->csels[umc].csmasks[csrow_nr];
> +	u32 addr_mask_sec	= pvt->csels[umc].csmasks_sec[csrow_nr];
>  
> -	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, csrow_nr >> 1);
> +	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, csrow_nr >> 1);
>  }
>  
>  static void gpu_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
> 
> base-commit: b4d2bada09b17fcd68a0f00811ca7f900ec988e6

This is a tip branch/commit. However, you should use a 'ras' tree branch,
since this patch is totally within EDAC.

Repo:   https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git
Branch: edac-for-next

Besides the process notes, the patch looks good to me.

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen
Naik, Avadhut April 16, 2025, 5:26 p.m. UTC | #2 
On 4/16/2025 09:10, Yazen Ghannam wrote:
> On Tue, Apr 15, 2025 at 09:25:58PM +0000, Avadhut Naik wrote:
>> Each Chip-Select (CS) of a Unified Memory Controller (UMC) on AMD EPYC
> 
> This affects Zen-based systems in general, not just the EPYC line.
> 
Will change this to something like:

Each Chip-Select (CS) of a Unified Memory Controller (UMC) on AMD's
modern Zen-based SOCs has an Address Mask and a Secondary Address Mask
register associated with it.

>> SOCs has an Address Mask and a Secondary Address Mask register associated
>> with it. The amd64_edac module logs DIMM sizes on a per-UMC per-CS
>> granularity during init using these two registers.
>>
>> Currently, the module primarily considers only the Address Mask register
>> for computing DIMM sizes. The Secondary Address Mask register is only
>> considered for odd CS. Additionally, if it has been considered, the
>> Address Mask register is ignored altogether for that CS. For
>> power-of-two DIMMs, this is not an issue since only the Address Mask
>> register is used.
>>
>> For non-power-of-two DIMMs, however, the Secondary Address Mask register
>> is used in conjunction with the Address Mask register. However, since the
>> module only considers either of the two registers for a CS, the size
>> computed by the module is incorrect. The Secondary Address Mask register
>> is not considered for even CS, and the Address Mask register is not
>> considered for odd CS.
>>
>> Introduce a new helper function so that both Address Mask and Secondary
>> Address Mask registers are considered, when valid, for computing DIMM
>> sizes. Furthermore, also rename some variables for greater clarity.
>>
>> Fixes: 81f5090db843 ("EDAC/amd64: Support asymmetric dual-rank DIMMs")
>> Reported-by: Žilvinas Žaltiena <zilvinas@natrix.lt>
> 
> Checkpatch says this should also have a 'Closes' tag. I never thought
> about this before, but it is mentioned in 'Documentation'. 
> 
> Closes: https://lore.kernel.org/dbec22b6-00f2-498b-b70d-ab6f8a5ec87e@natrix.lt
> 
Will add this.

>> Tested-by: Žilvinas Žaltiena <zilvinas@natrix.lt>
> 
> Minor nit: TIP tree handbook say 'Tested-by' goes after your SoB. I
> should check my submissions too.
> 
Had noticed that in some recent commits Tested-by immediately follows
Reported-by. And, SoB follows Tested-by.
Nonetheless, thanks for pointing this out. Will follow the tip handbook.

>> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
>> Cc: stable@vger.kernel.org
>> ```
>> Changes in v2:
>> 1. Avoid unnecessary variable initialization.
>> 2. Modify commit message to accurately reflect the changes.
>> 3. Move check for non-zero Address Mask register into the new helper.
>>
>> Links:
>> v1: https://lore.kernel.org/linux-edac/9a33b6ff-9ce8-4abd-8629-3c9f6a546514@amd.com/T/#mc0b1101055f12ccb06e5a251d16f186597ed4133
> 
> Use a 'permalink' rather than this threaded link. Search for
> 'permalink' on the web page.
> 
> Better option is to use this format:
>   https://lore.kernel.org/<Message-ID>
> 
> As shown in 'Documentation/process/submitting-patches.rst'
> 
> Ex:
>   https://lore.kernel.org/20250327210718.1640762-1-avadhut.naik@amd.com
> 
> Side note: I've been adding 'r/' between lore and Message-ID out of
> habit. But it seems that this is no longer needed.
> 
Will use this henceforth. Thanks!

>> ---
>>  drivers/edac/amd64_edac.c | 57 ++++++++++++++++++++++++---------------
>>  1 file changed, 36 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
>> index 58b1482a0fbb..91d22e63bdb1 100644
>> --- a/drivers/edac/amd64_edac.c
>> +++ b/drivers/edac/amd64_edac.c
>> @@ -1209,7 +1209,9 @@ static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
>>  	if (csrow_enabled(2 * dimm + 1, ctrl, pvt))
>>  		cs_mode |= CS_ODD_PRIMARY;
>>  
>> -	/* Asymmetric dual-rank DIMM support. */
>> +	if (csrow_sec_enabled(2 * dimm, ctrl, pvt))
>> +		cs_mode |= CS_EVEN_SECONDARY;
>> +
>>  	if (csrow_sec_enabled(2 * dimm + 1, ctrl, pvt))
>>  		cs_mode |= CS_ODD_SECONDARY;
>>  
>> @@ -1230,12 +1232,13 @@ static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
>>  	return cs_mode;
>>  }
>>  
>> -static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
>> -				  int csrow_nr, int dimm)
>> +static int calculate_cs_size(u32 mask, unsigned int cs_mode)
>>  {
>> -	u32 msb, weight, num_zero_bits;
>> -	u32 addr_mask_deinterleaved;
>> -	int size = 0;
>> +	int msb, weight, num_zero_bits;
>> +	u32 deinterleaved_mask;
>> +
>> +	if (!mask)
>> +		return 0;
>>  
>>  	/*
>>  	 * The number of zero bits in the mask is equal to the number of bits
>> @@ -1248,19 +1251,30 @@ static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
>>  	 * without swapping with the most significant bit. This can be handled
>>  	 * by keeping the MSB where it is and ignoring the single zero bit.
>>  	 */
>> -	msb = fls(addr_mask_orig) - 1;
>> -	weight = hweight_long(addr_mask_orig);
>> +	msb = fls(mask) - 1;
>> +	weight = hweight_long(mask);
>>  	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
>>  
>>  	/* Take the number of zero bits off from the top of the mask. */
>> -	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
>> +	deinterleaved_mask = GENMASK(msb - num_zero_bits, 1);
>> +	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", deinterleaved_mask);
>> +
>> +	return (deinterleaved_mask >> 2) + 1;
>> +}
>> +
>> +static int __addr_mask_to_cs_size(u32 addr_mask, u32 addr_mask_sec,
>> +				  unsigned int cs_mode, int csrow_nr, int dimm)
>> +{
>> +	int size;
>>  
>>  	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);
>> +
>> +	edac_dbg(1, "  Secondary AddrMask: 0x%x\n", addr_mask_sec);
>> +	size += calculate_cs_size(addr_mask_sec, cs_mode);
>>  
>>  	/* Return size in MBs. */
>>  	return size >> 10;
>> @@ -1270,7 +1284,7 @@ static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>>  				    unsigned int cs_mode, int csrow_nr)
>>  {
>>  	int cs_mask_nr = csrow_nr;
>> -	u32 addr_mask_orig;
>> +	u32 addr_mask = 0, addr_mask_sec = 0;
>>  	int dimm, size = 0;
>>  
>>  	/* No Chip Selects are enabled. */
>> @@ -1308,13 +1322,13 @@ static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>>  	if (!pvt->flags.zn_regs_v2)
>>  		cs_mask_nr >>= 1;
>>  
>> -	/* Asymmetric dual-rank DIMM support. */
>> -	if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
>> -		addr_mask_orig = pvt->csels[umc].csmasks_sec[cs_mask_nr];
>> -	else
>> -		addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
>> +	if (cs_mode & (CS_EVEN_PRIMARY | CS_ODD_PRIMARY))
>> +		addr_mask = pvt->csels[umc].csmasks[cs_mask_nr];
>> +
>> +	if (cs_mode & (CS_EVEN_SECONDARY | CS_ODD_SECONDARY))
>> +		addr_mask_sec = pvt->csels[umc].csmasks_sec[cs_mask_nr];
>>  
>> -	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, dimm);
>> +	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, dimm);
>>  }
>>  
>>  static void umc_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
>> @@ -3512,9 +3526,10 @@ static void gpu_get_err_info(struct mce *m, struct err_info *err)
>>  static int gpu_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>>  				    unsigned int cs_mode, int csrow_nr)
>>  {
>> -	u32 addr_mask_orig = pvt->csels[umc].csmasks[csrow_nr];
>> +	u32 addr_mask		= pvt->csels[umc].csmasks[csrow_nr];
>> +	u32 addr_mask_sec	= pvt->csels[umc].csmasks_sec[csrow_nr];
>>  
>> -	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, csrow_nr >> 1);
>> +	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, csrow_nr >> 1);
>>  }
>>  
>>  static void gpu_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
>>
>> base-commit: b4d2bada09b17fcd68a0f00811ca7f900ec988e6
> 
> This is a tip branch/commit. However, you should use a 'ras' tree branch,
> since this patch is totally within EDAC.
> 
> Repo:   https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git
> Branch: edac-for-next
> 
Had initially encountered this on tip kernel and stuck with it.
Will rebase though.

> Besides the process notes, the patch looks good to me.
> 
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> Thanks,
> Yazen
diff mbox series

Patch

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 58b1482a0fbb..91d22e63bdb1 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1209,7 +1209,9 @@  static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
 	if (csrow_enabled(2 * dimm + 1, ctrl, pvt))
 		cs_mode |= CS_ODD_PRIMARY;
 
-	/* Asymmetric dual-rank DIMM support. */
+	if (csrow_sec_enabled(2 * dimm, ctrl, pvt))
+		cs_mode |= CS_EVEN_SECONDARY;
+
 	if (csrow_sec_enabled(2 * dimm + 1, ctrl, pvt))
 		cs_mode |= CS_ODD_SECONDARY;
 
@@ -1230,12 +1232,13 @@  static int umc_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
 	return cs_mode;
 }
 
-static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
-				  int csrow_nr, int dimm)
+static int calculate_cs_size(u32 mask, unsigned int cs_mode)
 {
-	u32 msb, weight, num_zero_bits;
-	u32 addr_mask_deinterleaved;
-	int size = 0;
+	int msb, weight, num_zero_bits;
+	u32 deinterleaved_mask;
+
+	if (!mask)
+		return 0;
 
 	/*
 	 * The number of zero bits in the mask is equal to the number of bits
@@ -1248,19 +1251,30 @@  static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
 	 * without swapping with the most significant bit. This can be handled
 	 * by keeping the MSB where it is and ignoring the single zero bit.
 	 */
-	msb = fls(addr_mask_orig) - 1;
-	weight = hweight_long(addr_mask_orig);
+	msb = fls(mask) - 1;
+	weight = hweight_long(mask);
 	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
 
 	/* Take the number of zero bits off from the top of the mask. */
-	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
+	deinterleaved_mask = GENMASK(msb - num_zero_bits, 1);
+	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", deinterleaved_mask);
+
+	return (deinterleaved_mask >> 2) + 1;
+}
+
+static int __addr_mask_to_cs_size(u32 addr_mask, u32 addr_mask_sec,
+				  unsigned int cs_mode, int csrow_nr, int dimm)
+{
+	int size;
 
 	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);
+
+	edac_dbg(1, "  Secondary AddrMask: 0x%x\n", addr_mask_sec);
+	size += calculate_cs_size(addr_mask_sec, cs_mode);
 
 	/* Return size in MBs. */
 	return size >> 10;
@@ -1270,7 +1284,7 @@  static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 				    unsigned int cs_mode, int csrow_nr)
 {
 	int cs_mask_nr = csrow_nr;
-	u32 addr_mask_orig;
+	u32 addr_mask = 0, addr_mask_sec = 0;
 	int dimm, size = 0;
 
 	/* No Chip Selects are enabled. */
@@ -1308,13 +1322,13 @@  static int umc_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 	if (!pvt->flags.zn_regs_v2)
 		cs_mask_nr >>= 1;
 
-	/* Asymmetric dual-rank DIMM support. */
-	if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
-		addr_mask_orig = pvt->csels[umc].csmasks_sec[cs_mask_nr];
-	else
-		addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
+	if (cs_mode & (CS_EVEN_PRIMARY | CS_ODD_PRIMARY))
+		addr_mask = pvt->csels[umc].csmasks[cs_mask_nr];
+
+	if (cs_mode & (CS_EVEN_SECONDARY | CS_ODD_SECONDARY))
+		addr_mask_sec = pvt->csels[umc].csmasks_sec[cs_mask_nr];
 
-	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, dimm);
+	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, dimm);
 }
 
 static void umc_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
@@ -3512,9 +3526,10 @@  static void gpu_get_err_info(struct mce *m, struct err_info *err)
 static int gpu_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 				    unsigned int cs_mode, int csrow_nr)
 {
-	u32 addr_mask_orig = pvt->csels[umc].csmasks[csrow_nr];
+	u32 addr_mask		= pvt->csels[umc].csmasks[csrow_nr];
+	u32 addr_mask_sec	= pvt->csels[umc].csmasks_sec[csrow_nr];
 
-	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, csrow_nr >> 1);
+	return __addr_mask_to_cs_size(addr_mask, addr_mask_sec, cs_mode, csrow_nr, csrow_nr >> 1);
 }
 
 static void gpu_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)