linux-kernel - Re: [PATCH v3 7/8] EDAC/amd64: Support Asymmetric Dual-Rank 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190823112628.GA28379@zn.tnic>
Date:   Fri, 23 Aug 2019 13:26:28 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Ghannam, Yazen" <Yazen.Ghannam@....com>
Cc:     "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 7/8] EDAC/amd64: Support Asymmetric Dual-Rank DIMMs

On Thu, Aug 22, 2019 at 12:00:02AM +0000, Ghannam, Yazen wrote:
> From: Yazen Ghannam <yazen.ghannam@....com>
> 
> Future AMD systems will support "Asymmetric" Dual-Rank DIMMs. These are
> DIMMs where the ranks are of different sizes.
> 
> The even rank will use the Primary Even Chip Select registers and the
> odd rank will use the Secondary Odd Chip Select registers.
> 
> Recognize if a Secondary Odd Chip Select is being used. Use the
> Secondary Odd Address Mask when calculating the chip select size.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> ---
> Link:
> https://lkml.kernel.org/r/20190709215643.171078-8-Yazen.Ghannam@amd.com
> 
> v2->v3:
> * Add check of csrow_nr before using secondary mask.
> 
> v1->v2:
> * No change.
> 
>  drivers/edac/amd64_edac.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 26ce48fcaf00..4d1e6daa7ec4 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -790,9 +790,13 @@ static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan)
>  
>  #define CS_EVEN_PRIMARY		BIT(0)
>  #define CS_ODD_PRIMARY		BIT(1)
> +#define CS_EVEN_SECONDARY	BIT(2)
> +#define CS_ODD_SECONDARY	BIT(3)
>  
> -#define CS_EVEN			CS_EVEN_PRIMARY
> -#define CS_ODD			CS_ODD_PRIMARY
> +#define CS_EVEN			(CS_EVEN_PRIMARY | CS_EVEN_SECONDARY)
> +#define CS_ODD			(CS_ODD_PRIMARY | CS_EVEN_SECONDARY)

That's just my urge to have stuff ballanced but shouldn't that last line be:

#define CS_ODD			(CS_ODD_PRIMARY | CS_ODD_SECONDARY)

i.e., not have "even" as in CS_EVEN_SECONDARY in there but only "odd"s? :)

> +#define csrow_sec_enabled(i, dct, pvt)	((pvt)->csels[(dct)].csbases_sec[(i)] & DCSB_CS_ENABLE)

I moved that to the header, under csrow_enabled().

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.