linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.

             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).