Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac

From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"mchehab@kernel.org" <mchehab@kernel.org>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"srinivas.pandruvada@linux.intel.com"
	<srinivas.pandruvada@linux.intel.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"lenb@kernel.org" <lenb@kernel.org>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Subject: Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac
Date: Wed, 19 Jul 2017 16:40:25 +0000	[thread overview]
Message-ID: <1500481869.2042.29.camel@hpe.com> (raw)
In-Reply-To: <20170718181545.32bd9181@vento.lan>

On Tue, 2017-07-18 at 18:15 -0300, Mauro Carvalho Chehab wrote:
> Em Tue, 18 Jul 2017 19:58:54 +0000
 :
> We had a similar discussion several years ago when I wrote this
> driver. On that time, I talked with Red Hat, HP, Dell, Intel people
> and with some customers with large clusters.
> 
> The way it is, ghes_edac is a poor man's driver. What it hopefully
> provide is a detection that an error happened, without really telling
> the user what component should be replaced.

"poor man's driver" is a bit misleading, but yes, firmware-first
platforms have RAS features built-into the platforms, and they do not
need intelligence in EDAC drivers, which may conflict with the
platform's RAS features.  I cannot speak for other vendors, but HPE
platforms log errors and provide FRU info.  ghes_edac allows to report
errors to OS management tools like rasdaemon in addition to platform-
specific managements.

> Ok, on machines with their own error reporting mechanism (like
> HP servers), a sys admin can look on some proprietary software
> (or bios), in order to identify what happened.
> 
> Yet, BIOS doesn't provide any glue about what's the memory
> architecture, as it maps memory as if it was a single DIMM memory:
> 
> (from ghes_edac_register)
> 
> 	layers[0].type = EDAC_MC_LAYER_ALL_MEM;
> 	layers[0].size = num_dimm;
> 	layers[0].is_virt_csrow = true;
> 
> So, even on systems where the BIOS actually knows how the memory
> cards are wired, it will mask the memory controller data.
> 
> Now, the EDAC driver can also be used to identify what
> channels are used. That helps the sys admin to know if the
> memories are connected in a way that it will be using multiple
> channels, or not, helping to setup the machine to obtain
> the maximum possible performance.
> 
> So, for example, on my Intel-based HP server, I can check
> such info with:
> 
> $ ras-mc-ctl --mainboard
> ras-mc-ctl: mainboard: HP model ProLiant ML350 Gen9
> $ ras-mc-ctl --layout
>        +-------------------------------------------------------------
> ----------+
>        |                mc0                |                mc1      
>           |
>        | channel0  | channel1  | channel2  | channel0  | channel1  |
> channel2  |
> -------+-------------------------------------------------------------
> ----------+
> slot2: |     0 MB  |     0 MB  |     0 MB  |     0 MB  |     0
> MB  |     0 MB  |
> slot1: |     0 MB  |     0 MB  |     0 MB  |     0 MB  |     0
> MB  |     0 MB  |
> slot0: |  16384 MB  |     0 MB  |  16384 MB  |  16384 MB  |     0
> MB  |  16384 MB  |
> -------+-------------------------------------------------------------
> --------------+
> 
> So, I know that both CPUs will be connected to my memories, and,
> on both, it is using 2 channels.
> 
> If I was using the ghes driver, that information would be hidden.
> 
> So, due to all problems with ghes, it is enabled only if there are no
> better solution, e. g. on systems where there's no way to talk
> directly to the hardware (like on E7 Xeon machines, where the memory
> controller is actually on a separate chip that are controlled only by
> the BIOS).

Thanks for the info!  That's very helpful.  I will check to see if
ghes_edac provides enough info that we need.
-Toshi