linux-kernel - Re: [PATCH v2] EDAC/amd64: Fix possible module load failure on some UMC usage combinations

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

Message-ID: <28fb96f2-726c-4a86-a72b-cdfdcac9bce0@amd.com>
Date: Wed, 11 Dec 2024 13:18:39 -0600
From: "Naik, Avadhut" <avadnaik@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: Avadhut Naik <avadhut.naik@....com>, linux-edac@...r.kernel.org,
 linux-kernel@...r.kernel.org, Yazen Ghannam <yazen.ghannam@....com>
Subject: Re: [PATCH v2] EDAC/amd64: Fix possible module load failure on some
 UMC usage combinations



On 12/11/2024 12:51, Borislav Petkov wrote:
> On Wed, Dec 11, 2024 at 10:46:37AM -0500, Yazen Ghannam wrote:
>> Looks good overall. We can even remove the "nid" variable and just use
>> "pvt->mc_node_id" directly in the debug message. This is another remnant
>> from when this function did register accesses.
> 
> Ok, done.
> 
> Avadhut, can you pls verify this fixes your issue too?
> 
Yes, this fixes the issue of module not loading with some UMC
configurations.

If relevant, then for the below patch:

Tested-by: Avadhut Naik <avadhut.naik@....com>
Reviewed-by: Avadhut Naik <avadhut.naik@....com>

> I'll run it on my boxes too, to make sure nothing breaks.
> 
> Thx.
> 
> ---
> From: "Borislav Petkov (AMD)" <bp@...en8.de>
> Date: Wed, 11 Dec 2024 12:07:42 +0100
> Subject: [PATCH] EDAC/amd64: Simplify ECC check on unified memory controllers
> 
> The intent of the check is to see whether at least one UMC has ECC
> enabled. So do that instead of tracking which ones are enabled in masks
> which are too small in size anyway and lead to not loading the driver on
> Zen4 machines with UMCs enabled over UMC8.
> 
> Fixes: e2be5955a886 ("EDAC/amd64: Add support for AMD Family 19h Models 10h-1Fh and A0h-AFh")
> Reported-by: Avadhut Naik <avadhut.naik@....com>
> Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>
> Cc: <stable@...nel.org>
> Link: https://lore.kernel.org/r/20241210212054.3895697-1-avadhut.naik@amd.com
> ---
>  drivers/edac/amd64_edac.c | 32 ++++++++++----------------------
>  1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index ddfbdb66b794..5d356b7c4589 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3362,36 +3362,24 @@ static bool dct_ecc_enabled(struct amd64_pvt *pvt)
>  
>  static bool umc_ecc_enabled(struct amd64_pvt *pvt)
>  {
> -	u8 umc_en_mask = 0, ecc_en_mask = 0;
> -	u16 nid = pvt->mc_node_id;
>  	struct amd64_umc *umc;
> -	u8 ecc_en = 0, i;
> +	bool ecc_en = false;
> +	int i;
>  
> +	/* Check whether at least one UMC is enabled: */
>  	for_each_umc(i) {
>  		umc = &pvt->umc[i];
>  
> -		/* Only check enabled UMCs. */
> -		if (!(umc->sdp_ctrl & UMC_SDP_INIT))
> -			continue;
> -
> -		umc_en_mask |= BIT(i);
> -
> -		if (umc->umc_cap_hi & UMC_ECC_ENABLED)
> -			ecc_en_mask |= BIT(i);
> +		if (umc->sdp_ctrl & UMC_SDP_INIT &&
> +		    umc->umc_cap_hi & UMC_ECC_ENABLED) {
> +			ecc_en = true;
> +			break;
> +		}
>  	}
>  
> -	/* Check whether at least one UMC is enabled: */
> -	if (umc_en_mask)
> -		ecc_en = umc_en_mask == ecc_en_mask;
> -	else
> -		edac_dbg(0, "Node %d: No enabled UMCs.\n", nid);
> -
> -	edac_dbg(3, "Node %d: DRAM ECC %s.\n", nid, (ecc_en ? "enabled" : "disabled"));
> +	edac_dbg(3, "Node %d: DRAM ECC %s.\n", pvt->mc_node_id, (ecc_en ? "enabled" : "disabled"));
>  
> -	if (!ecc_en)
> -		return false;
> -	else
> -		return true;
> +	return ecc_en;
>  }
>  
>  static inline void

-- 
Thanks,
Avadhut Naik