From: Borislav Petkov <bp@alien8.de>
To: James Morse <james.morse@arm.com>
Cc: Zhengqiang <zhengqiang10@huawei.com>,
mchehab@kernel.org, toshi.kani@hpe.com,
linux-edac@vger.kernel.org, linuxarm@huawei.com,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: ghes_edac: enable HIP08 platform edac driver
Date: Wed, 16 May 2018 20:29:58 +0200 [thread overview]
Message-ID: <20180516182958.GB17092@pd.tnic> (raw)
On Wed, May 16, 2018 at 02:38:38PM +0100, James Morse wrote:
> XGene has its own edac driver, but it doesn't probe when booted via ACPI so
> won't conflict with ghes_edac.
Actually it will. EDAC core can have only one EDAC driver loaded. Don't
ask me why - it has been that way since forever. We can change it some
day but frankly, I don't see reasoning for it. One driver can easily
manage *all* error sources on a system, I'd say.
> ... The thing has 4 dimm slots, but only two are populated. I swapped
> them round and the table was regenerated, so I don't think its faking
> it.
Ok.
> So I think we're good to make the whitelist x86 only.
> Your diff-hunk makes 'idx=-1', so we always get the 'Unfortunately' warning. I'd
> like to suppress this unless force_load has been used.
Yeah, we should handle that differently for ARM. Toshi added the idx
thing in
5deed6b6a479 ("EDAC, ghes: Add platform check")
to dump this when the platform is not whitelisted. So let's do that:
---
---
> What is the history behind the fake thing here? It predates 32fa1f53c2d
> "ghes_edac: do a better job of filling EDAC DIMM info", was it to support a
> valid system, or just to ease merging the driver when not all systems had the
> dmi table?
I wouldn't be surprised if there were some, TBH.
Looks to me like it used to fake DIMMs, see
- /* FIXME: FAKE DATA */
- dimm->nr_pages = 1000;
- dimm->grain = 128;
- dimm->mtype = MEM_UNKNOWN;
- dimm->dtype = DEV_UNKNOWN;
- dimm->edac_mode = EDAC_SECDED;
which 32fa1f53c2d removes.
$ git annotate drivers/edac/ghes_edac.c 32fa1f53c2d~1
shows you the driver before the DMI scanning so it looks like initially
it was faking stuff to satisfy EDAC core when it creates sysfs entries
using struct dimm_info descriptors.
> It looks like even the oldest Arm64 ACPI systems have dmi tables, so we can
> probably require DMI or the 'force' flag.
Well, with the hunk above it would still do ghes_edac_count_dimms() on
ARM and if it fails to find something, it will set fake, which is a good
sanity-check as it screams loudly. :)
Thx.
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 863fbf3db29f..473aeec4b1da 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -440,12 +440,16 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
struct mem_ctl_info *mci;
struct edac_mc_layer layers[1];
struct ghes_edac_dimm_fill dimm_fill;
- int idx;
+ int idx = -1;
- /* Check if safe to enable on this system */
- idx = acpi_match_platform_list(plat_list);
- if (!force_load && idx < 0)
- return -ENODEV;
+ if (IS_ENABLED(CONFIG_X86)) {
+ /* Check if safe to enable on this system */
+ idx = acpi_match_platform_list(plat_list);
+ if (!force_load && idx < 0)
+ return -ENODEV;
+ } else {
+ idx = 0;
+ }
/*
* We have only one logical memory controller to which all DIMMs belong.
next reply other threads:[~2018-05-16 18:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-16 18:29 Borislav Petkov [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-05-18 11:11 ghes_edac: enable HIP08 platform edac driver Borislav Petkov
2018-05-18 7:13 Zhengqiang
2018-05-17 18:02 James Morse
2018-05-16 13:38 James Morse
2018-05-14 16:47 Borislav Petkov
2018-05-14 15:12 James Morse
2018-05-14 9:47 Borislav Petkov
2018-05-14 4:11 Zhengqiang
2018-05-11 12:19 Borislav Petkov
2018-05-11 11:52 Zhengqiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180516182958.GB17092@pd.tnic \
--to=bp@alien8.de \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-edac@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=mchehab@kernel.org \
--cc=toshi.kani@hpe.com \
--cc=zhengqiang10@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).