[4/5] EDAC, dsm_edac: Wrap ACPI DSM methods for address translation

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Monday, October 8, 2018 6:57:06 PM CEST Luck, Tony wrote:
> Added linux-acpi list to Cc:
> 
> On Sat, Oct 06, 2018 at 10:44:56PM +0200, Borislav Petkov wrote:
> > On Fri, Oct 05, 2018 at 03:25:57PM -0700, Luck, Tony wrote:
> > > 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.
> > 
> > Ok, but what is the difference with this DSM thing and why would OEMs
> > use that? They get it for free from you? Or?
> 
> This shouldn't conflict with other BIOS "value add" code for firmware
> first mode, etc. 
> 
> Yes, they get it for free with the reference/example BIOS code.
> 
> > > 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.
> > 
> > Looks nice and clean to me.
> 
> Thanks!
> 
> > > +int adxl_count;
> > > +EXPORT_SYMBOL_GPL(adxl_count);
> > 
> > That's the length of the address components adxl_component_names array?
> 
> Yes. But see below.
> 
> > > +char **adxl_component_names;
> > > +EXPORT_SYMBOL_GPL(adxl_component_names);
> > 
> > I guess I'm still unclear on how this is going to be used...
> 
> Comments added in new version to save people from mailing list
> archeology.
> 
> > aha, the DSM replies with a bunch of components which map to the names
> > and they'll get filled up by BIOS and the driver simply dumps them.
> > Something along those lines at least.
> > 
> > So you don't need count if you NULL-terminate the names array, right?
> > 
> > So you can simply do
> > 
> > 	kcalloc(cnt + 1, ...
> > 
> > and the last element will be NULL.
> 
> Good idea. Done in new version, and the EXPORT_SYMBOL_GPL(adxl_count) is
> gone.
> 
> Still untested ... Qiuxu, can you try this out please.
> 
> Rafael: Does this look like something you can merge (once we
> clean up any minor style stuff you find).

Well, it looks reasonable, one question though (below).

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

I wonder if the extra config option is really needed.  I guess it is useful
for the "tinification" people, but then can it simply depend on something
else we already have?

Distros will set it to "Y" anyway.

>  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..01d2b4d06430
> --- /dev/null
> +++ b/drivers/acpi/acpi_adxl.c
> @@ -0,0 +1,179 @@
> +// 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_REVISION			0x1
> +#define ADXL_IDX_GET_ADDR_PARAMS	0x1
> +#define ADXL_IDX_FORWARD_TRANSLATE	0x2
> +#define ACPI_ADXL_PATH			"\\_SB.ADXL"

It would be good to add a comment to say where the ADXL object is
defined.

> +
> +#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);
> +
> +static char **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_REVISION,
> +				      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;
> +}
> +
> +/**
> + * adxl_get_component_names - get list of memory component names
> + * Returns NULL terminated list of string names
> + *
> + * Give the caller a pointer to the list of memory component names
> + * e.g. { "SystemAddress", "ProcessorSocketId", "ChannelId", ... NULL }
> + * Caller should count how many strings in order to allocate a buffer
> + * for the return from adxl_decode()
> + */
> +char **adxl_get_component_names(void)
> +{
> +	return adxl_component_names;
> +}
> +EXPORT_SYMBOL_GPL(adxl_get_component_names);
> +
> +/**
> + * adxl_decode - ask BIOS to decode a system address to memory address
> + * @addr: the address to decode
> + * @component_values: pointer to array of values for each component
> + * Returns 0 on success, negative error code otherwise
> + *
> + * The index of each value returned in the array matches the index of
> + * each component name returned by adxl_get_component_names().
> + * Components that are not defined for this address translation (e.g.
> + * mirror channel number for a non-mirrored address) are set to ~0ull
> + */
> +int adxl_decode(u64 addr, u64 component_values[])
> +{
> +	union acpi_object argv4[2], *results, *r;
> +	int i, cnt;
> +
> +	if (!adxl_component_names)
> +		return -EOPNOTSUPP;
> +
> +	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_REVISION,
> +			    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 = kcalloc(cnt + 1, 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;
> +
> +	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..96d356a06e8b
> --- /dev/null
> +++ b/include/linux/adxl.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Address translation interface via ACPI DSM.
> + * Copyright (C) 2018 Intel Corporation
> + */
> +
> +#ifndef _LINUX_ADXL_H
> +#define _LINUX_ADXL_H
> +
> +#ifdef CONFIG_ACPI_ADXL
> +char **adxl_get_component_names(void);
> +int adxl_decode(u64 addr, u64 component_values[]);
> +#else
> +static inline char **adxl_get_component_names(void)
> +{
> +	return NULL;
> +}
> +
> +static inline int adxl_decode(u64 addr, u64 component_values[])
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
> +
> +#endif /* _LINUX_ADXL_H */
>