linux-kernel - Re: [PATCH 3/3] EDAC, ghes: Make it a proper module

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

Message-ID: <1501713103.2042.107.camel@hpe.com>
Date:   Wed, 2 Aug 2017 22:41:14 +0000
From:   "Kani, Toshimitsu" <toshi.kani@....com>
To:     "bp@...en8.de" <bp@...en8.de>
CC:     "mchehab@...radead.org" <mchehab@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>
Subject: Re: [PATCH 3/3] EDAC, ghes: Make it a proper module

On Wed, 2017-08-02 at 05:18 +0200, Borislav Petkov wrote:
> On Wed, Aug 02, 2017 at 12:19:29AM +0000, Kani, Toshimitsu wrote:
> > 1. Device-probing-logic should belong to a driver, and should
> > remain private to a driver.  When we add the white-list, it should
> > be added to ghes_edac.
> 
> Nonsense. There are a lot of examples where driver probing depends on
> outside modalities like built-in quirks and such.
> 
> > 2. ghes_edac is an extension to the ghes driver as they both are
> > specific to ghes.  ghes_edac is merely ghes driver's edac error-
> > reporting wrapper than an independent edac driver.  It looks OK to
> > let ghes_edac get registered as part of ghes_probe() and leave it
> > as an unconventional edac driver.
> 
> Except that GHES wants to report into the EDAC infrastructure so it
> better has a wrapper for it.
> 
> One of the directions I explored when looking at this is to stick
> ghes_edac functionality into ghes.c or so and make it completely
> independent from EDAC. Would've been much cleaner.

Agreed. I think the current model aimed at this direction while it was
needed to depend on EDAC.

> > 3. EDAC does not have its managed probe-chain.  All edac drivers
> > are called from module_init list.  They independently probe the
> > hardware and get unloaded when not needed.  The core edac is simply
> > a set of library to them.  I think it's good to keep them
> > independent, and not to introduce a new central mechanism for a
> > special case like ghes_edac.
> 
> They're independent because before GHES we needed to load one driver
> per system. Until the bolted-on thing came. And it is bolted on
> because the already overwhelmed firmware decided to do error
> reporting too.
> 
> So the only real reason why I'm fine with keeping the current
> situation is the whitelist. Because then, we can at least control
> what loads and what not.
> 
> But then we need:
> 
> 1. A clean mechanism for the platform drivers to query whether
> another agent is loaded (ghes_edac) and not do any probing then.
> 
> 2. ghes_edac needs to drop that multiple probing thing as its
> dmi_walk(ghes_edac_count_dimms, &num_dimm) already probes *all* DIMMs
> on the system so no need to do that multiple times.

Sounds good. I will keep the current model and address the above
points.

Thanks,
-Toshi