linux-kernel - RE: [PATCH v3 0/5] AMD64 EDAC: Check for nodes without memory, etc.

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]

Date:   Thu, 7 Nov 2019 19:20:25 +0000
From:   "Ghannam, Yazen" <Yazen.Ghannam@....com>
To:     Borislav Petkov <bp@...en8.de>
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 0/5] AMD64 EDAC: Check for nodes without memory, etc.

> -----Original Message-----
> From: linux-kernel-owner@...r.kernel.org <linux-kernel-owner@...r.kernel.org> On Behalf Of Borislav Petkov
> Sent: Thursday, November 7, 2019 10:40 AM
> To: Ghannam, Yazen <Yazen.Ghannam@....com>
> Cc: linux-edac@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v3 0/5] AMD64 EDAC: Check for nodes without memory, etc.
> 
> On Thu, Nov 07, 2019 at 01:47:53PM +0000, Ghannam, Yazen wrote:
> > BTW, what do you think about loading based on PCI devices? The module
> > used to do this. I ask because I'm starting to see that future systems may
> > re-use PCI IDs, and this indicates the same level of hardware support.
> 
> The reason we switched to family-based autoloading was that almost
> every new platform would add a new PCI device ID, which would require
> enablement work...
> 

Yes, that's right. But it looks like future systems will re-use PCI IDs even
across families and models. And the PCI IDs will be more closely related to
hardware capabilities than family and model.

In any case, we can address that when we get there.

> > Just a nit, but this else seems unnecessary right?
> 
> Maybe it is easier if you look at the function end in the .c file directly as
> diffs can be confusing:
> 
> static bool ecc_enabled(struct amd64_pvt *pvt)
> {
> 
> 	...
> 
>         amd64_info("Node %d: DRAM ECC %s.\n",
>                    nid, (ecc_en ? "enabled" : "disabled"));
> 
>         if (!ecc_en || !nb_mce_en)
>                 return false;
>         else

Right, I meant you can drop this else and just return true.

>                 return true;
> }
> 

Thanks,
Yazen