All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: Borislav Petkov <bp@alien8.de>, "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
	Aristeu Rozanski <aris@redhat.com>,
	Mauro Carvalho Chehab <mchehab@s-opensource.com>,
	linux-edac@vger.kernel.org
Subject: [4/5] EDAC, dsm_edac: Wrap ACPI DSM methods for address translation
Date: Fri, 5 Oct 2018 15:25:57 -0700	[thread overview]
Message-ID: <20181005222557.GA23762@agluck-desk> (raw)

Rafael: You'd become the deciding maintainer if we follow this
new direction.

On Thu, Oct 04, 2018 at 11:31:01AM +0200, Borislav Petkov wrote:
> So this sounds like drivers/acpi/acpi_extlog.c which you guys did. Is
> that thing being abandoned now too, for yet another thing?

It's like acpi_extlog.c in that it uses an ACPI DSM. Also related
to error reporting.  No, we aren't abandoning extlog, but it seems
that fewer OEMs than we hoped are adopting eMCA and implementing
the extended log in their BIOS.

> And that extlog thing is under drivers/acpi/. So even if the DSM stuff
> is *maybe* going to be part of the ACPI spec, I believe drivers/acpi/ is
> where it should go. As in: preparing to become part of ACPI :-)

That's a good argment for moving it there.

> But, there's another aspect: is this DSM interface going to be used only
> by EDAC or some other subsystems? (It seems so.)

Unclear who else might want to use this.  I spoke to an internal
validation guy (who is the person who mostly came up with this
DSM idea). He has some out-of-tree driver that can call into any
DSM. So it is unclear whether he'd want to use whatever we write.
But if there are going to be other users someday, then the interface
should be less EDAC focussed.

> Because if it is going to be used by EDAC only and EDAC drivers will
> reg/unreg with it, then I guess making it part of the EDAC core, would
> probably be more fitting. Something like drivers/edac/dsm.c and then you
> don't have to have another module which needs to sync with the *actual*
> EDAC module...

The module syning part is painful/ugly.  Making this built-in would
be far prettier (more maintainable).

> Practically, this is going to be enabled on 99.9% of the boxes so making
> it part of the core is, I guess, just as well.

So I dropped Qiuxu's code into a blender and ran it on high
for a few minutes.  Below is an RFC patch (compiles, but
otherwise untested) to see what things would looks like with
a built in ACPI block with no registrations to EDAC core, or
other stuff that is specific to the EDAC usage model.

When it finds the BIOS support this, it provides three EXPORT
objects:

1) A count of how many address components are provided.
2) A "char **" pointer to the strings for those components
3) A function that takes a system address, and fills in an
   array of component values.

The final patch to update skx_edac.c would have to be updated
for these new interface options. But that shouldn't be too hard.

Here's the patch for the new ACPI ADXL code:
---

 drivers/acpi/Kconfig     |  10 +++
 drivers/acpi/Makefile    |   3 +
 drivers/acpi/acpi_adxl.c | 156 +++++++++++++++++++++++++++++++++++++++
 include/linux/adxl.h     |  18 +++++
 4 files changed, 187 insertions(+)
 create mode 100644 drivers/acpi/acpi_adxl.c
 create mode 100644 include/linux/adxl.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index dd1eea90f67f..327c93b51cb7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -498,6 +498,16 @@ config ACPI_EXTLOG
 	  driver adds support for that functionality with corresponding
 	  tracepoint which carries that information to userspace.
 
+config ACPI_ADXL
+	bool "Physical address to DIMM address translation"
+	def_bool n
+	help
+	  Enable interface that calls into BIOS using a DSM (device
+	  specific method) to convert system physical addresses
+	  to DIMM (socket, channel, rank, dimm, etc.).
+	  Only available on some servers.
+	  Used by newer EDAC drivers.
+
 menuconfig PMIC_OPREGION
 	bool "PMIC (Power Management Integrated Circuit) operation region support"
 	help
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 6d59aa109a91..edc039313cd6 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -61,6 +61,9 @@ acpi-$(CONFIG_ACPI_LPIT)	+= acpi_lpit.o
 acpi-$(CONFIG_ACPI_GENERIC_GSI) += irq.o
 acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
 
+# Address translation
+acpi-$(CONFIG_ACPI_ADXL)	+= acpi_adxl.o
+
 # These are (potentially) separate modules
 
 # IPMI may be used by other drivers, so it has to initialise before them
diff --git a/drivers/acpi/acpi_adxl.c b/drivers/acpi/acpi_adxl.c
new file mode 100644
index 000000000000..a0e026680c3d
--- /dev/null
+++ b/drivers/acpi/acpi_adxl.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Address translation interface via ACPI DSM.
+ * Copyright (C) 2018 Intel Corporation
+ */
+
+#ifdef CONFIG_ACPI_ADXL
+#include <linux/acpi.h>
+#include <linux/adxl.h>
+
+#define ADXL_REV				0x1
+#define ADXL_IDX_GET_ADDR_PARAMS		0x1
+#define ADXL_IDX_FORWARD_TRANSLATE	0x2
+#define ACPI_ADXL_PATH			"\\_SB.ADXL"
+
+#undef pr_fmt
+#define pr_fmt(fmt) "ADXL: " fmt
+
+static acpi_handle handle;
+static union acpi_object *params;
+static const guid_t adxl_guid =
+	GUID_INIT(0xAA3C050A, 0x7EA4, 0x4C1F,
+		  0xAF, 0xDA, 0x12, 0x67, 0xDF, 0xD3, 0xD4, 0x8D);
+
+int adxl_count;
+EXPORT_SYMBOL_GPL(adxl_count);
+
+char **adxl_component_names;
+EXPORT_SYMBOL_GPL(adxl_component_names);
+
+static union acpi_object *adxl_dsm(int cmd, union acpi_object argv[])
+{
+	union acpi_object *obj, *o;
+
+	obj = acpi_evaluate_dsm_typed(handle, &adxl_guid, ADXL_REV,
+				      cmd, argv, ACPI_TYPE_PACKAGE);
+	if (!obj) {
+		pr_debug("Empty obj\n");
+		goto out;
+	}
+
+	if (obj->package.count != 2) {
+		pr_debug("Bad pkg cnt %d\n", obj->package.count);
+		goto err;
+	}
+
+	o = obj->package.elements;
+	if (o->type != ACPI_TYPE_INTEGER) {
+		pr_debug("Bad 1st element type %d\n", o->type);
+		goto err;
+	}
+	if (o->integer.value) {
+		pr_debug("Bad ret val %llu\n", o->integer.value);
+		goto err;
+	}
+
+	o = obj->package.elements + 1;
+	if (o->type != ACPI_TYPE_PACKAGE) {
+		pr_debug("Bad 2nd element type %d\n", o->type);
+		goto err;
+	}
+	return obj;
+
+err:
+	ACPI_FREE(obj);
+out:
+	return obj;
+}
+
+int adxl_decode(u64 addr, u64 component_values[])
+{
+	union acpi_object argv4[2], *results, *r;
+	int i, cnt;
+
+	argv4[0].type = ACPI_TYPE_PACKAGE;
+	argv4[0].package.count = 1;
+	argv4[0].package.elements = &argv4[1];
+	argv4[1].integer.type = ACPI_TYPE_INTEGER;
+	argv4[1].integer.value = addr;
+
+	results = adxl_dsm(ADXL_IDX_FORWARD_TRANSLATE, argv4);
+
+	if (!results)
+		return -EINVAL;
+
+	r = results->package.elements + 1;
+	cnt = r->package.count;
+	r = r->package.elements;
+
+	for (i = 0; i < cnt; i++)
+		component_values[i] = r[i].integer.value;
+
+	ACPI_FREE(results);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(adxl_decode);
+
+static bool adxl_detect(void)
+{
+	char *path = ACPI_ADXL_PATH;
+	union acpi_object *p;
+	acpi_status status;
+	int i, cnt;
+
+	status = acpi_get_handle(NULL, path, &handle);
+	if (ACPI_FAILURE(status)) {
+		pr_debug("No ACPI handle for path %s\n", path);
+		return false;
+	}
+
+	if (!acpi_has_method(handle, "_DSM")) {
+		pr_debug("No DSM method\n");
+		return false;
+	}
+
+	if (!acpi_check_dsm(handle, &adxl_guid, ADXL_REV,
+			    ADXL_IDX_GET_ADDR_PARAMS |
+			    ADXL_IDX_FORWARD_TRANSLATE)) {
+		pr_debug("No ADXL DSM methods\n");
+		return false;
+	}
+
+	params = adxl_dsm(ADXL_IDX_GET_ADDR_PARAMS, NULL);
+	if (!params) {
+		pr_debug("Failed to get params\n");
+		return false;
+	}
+
+	p = params->package.elements + 1;
+	cnt = p->package.count;
+	p = p->package.elements;
+
+	adxl_component_names = kmalloc_array(cnt, sizeof(char *), GFP_KERNEL);
+	if (!adxl_component_names) {
+		ACPI_FREE(params);
+		return false;
+	}
+
+	for (i = 0; i < cnt; i++)
+		adxl_component_names[i] = p[i].string.pointer;
+
+	adxl_count = cnt;
+
+	return true;
+}
+
+static int __init adxl_init(void)
+{
+	if (!adxl_detect())
+		return -ENODEV;
+	return 0;
+}
+subsys_initcall(adxl_init);
+
+#endif /* CONFIG_ACPI_ADXL */
diff --git a/include/linux/adxl.h b/include/linux/adxl.h
new file mode 100644
index 000000000000..11d62bd9ca3c
--- /dev/null
+++ b/include/linux/adxl.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Address translation interface via ACPI DSM.
+ * Copyright (C) 2018 Intel Corporation
+ */
+
+#ifdef CONFIG_ACPI_ADXL
+int adxl_count;
+char **adxl_component_names;
+int adxl_decode(u64 addr, u64 component_values[]);
+#else
+#define adxl_count 0
+#define adxl_component_names NULL
+static inline int adxl_decode(u64 addr, u64 component_values[])
+{
+	return -EOPNOTSUPP;
+}
+#endif

             reply	other threads:[~2018-10-05 22:25 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05 22:25 Luck, Tony [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-10-09 18:33 [4/5] EDAC, dsm_edac: Wrap ACPI DSM methods for address translation Luck, Tony
2018-10-09 18:29 Luck, Tony
2018-10-09 16:17 Borislav Petkov
2018-10-09 15:29 Rafael J. Wysocki
2018-10-09 15:25 Rafael J. Wysocki
2018-10-09 15:22 Luck, Tony
2018-10-09 15:14 Borislav Petkov
2018-10-09 11:43 Qiuxu Zhuo
2018-10-09 10:28 Rafael J. Wysocki
2018-10-08 16:57 Luck, Tony
2018-10-06 20:44 Borislav Petkov
2018-10-04  9:31 Borislav Petkov
2018-10-03 17:58 Luck, Tony
2018-09-26 18:22 Luck, Tony
2018-09-26 17:33 Borislav Petkov
2018-09-24 20:16 Luck, Tony

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=20181005222557.GA23762@agluck-desk \
    --to=tony.luck@intel.com \
    --cc=aris@redhat.com \
    --cc=bp@alien8.de \
    --cc=linux-edac@vger.kernel.org \
    --cc=mchehab@s-opensource.com \
    --cc=qiuxu.zhuo@intel.com \
    --cc=rjw@rjwysocki.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.