linux-kernel - Re: [PATCH] check-dbam1-if-there-is-only-one-DIMM-on-dbam0 in unganged mode

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: <20090806144240.GB19878@aftab>
Date:	Thu, 6 Aug 2009 16:42:40 +0200
From:	Borislav Petkov <borislav.petkov@....com>
To:	wan wei <onewayforever@...il.com>
CC:	dougthompson@...ssion.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] check-dbam1-if-there-is-only-one-DIMM-on-dbam0 in
 unganged mode

On Thu, Aug 06, 2009 at 06:44:34PM +0800, wan wei wrote:
> 
>     When I populated just one DIMM for each of the two channels  of
> the cpu node in unganged mode,   there was only one channel found by
> edac.
>     In counting the channels, if there is only one DIMM found by
> dbam0,  i think dbam1 should be checked for the other one  also.
>     Below is the patch
> 
> Signed-off-by: Wan Wei<wanwei@...ning.com.cn>
> 
> ---
>  drivers/edac/amd64_edac.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index e2a10bc..afd210a 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -1248,7 +1248,7 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
>         /* If more than 2 DIMMs are present, then we have 2 channels */
>         if (channels > 2)
>                 channels = 2;
> -       else if (channels == 0) {
> +       else if (channels <= 1) {

This is correct, we're missing the one DIMM per DCT case. Can you try
the patch below instead - it should be equivalent to yours while making
the code a bit more understandable.

Thanks.

--
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index e2a10bc..71c215b 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1245,11 +1245,8 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
 	if (DBAM_DIMM(3, dbam) > 0)
 		channels++;
 
-	/* If more than 2 DIMMs are present, then we have 2 channels */
-	if (channels > 2)
-		channels = 2;
-	else if (channels == 0) {
-		/* No DIMMs on DCT0, so look at DCT1 */
+	/* check whether DCT1 has also a channel enabled */
+	if (channels < 2) {
 		err = pci_read_config_dword(pvt->dram_f2_ctl, DBAM1, &dbam);
 		if (err)
 			goto err_reg;
@@ -1262,11 +1259,11 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
 			channels++;
 		if (DBAM_DIMM(3, dbam) > 0)
 			channels++;
-
-		if (channels > 2)
-			channels = 2;
 	}
 
+	if (channels > 2)
+		channels = 2;
+
 	/* If we found ALL 0 values, then assume just ONE DIMM-ONE Channel */
 	if (channels == 0)
 		channels = 1;

-- 
Regards/Gruss,
Boris.

Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/