EDAC, dsm_edac: Wrap ACPI DSM methods for address translation
PATCH 4/5 EDAC, dsm_edac: Wrap ACPI DSM methods for address translation
Quoted 54 lines · Tony
Added linux-acpi list to Cc:
Quoted 8 lines · Borislav Petkov
Quoted 4 lines · Tony
It's like
acpi_extlog.cin 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.
Quoted 7 lines
Quoted 5 lines
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!
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+int adxl_count; +EXPORT_SYMBOL_GPL(adxl_count);That's the length of the address components
adxl_component_namesarray?Yes. But see below.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+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.
Quoted 11 lines
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.
Quoted 7 lines
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).
Quoted 8 lines
drivers/acpi/Kconfig +0
-0@@ -498,6 +498,16 @@ config ACPI_EXTLOG driver adds support for that functionality with corresponding tracepoint which carries that information to userspace.
drivers/acpi/Kconfig config ACPI_EXTLOG · line 510
Quoted 10 lines
drivers/acpi/Kconfig +10
-0+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.
Quoted 27 lines
drivers/acpi/Kconfig +0
-0menuconfig PMIC_OPREGION bool "PMIC (Power Management Integrated Circuit) operation region support" helpdrivers/acpi/Makefile +3
-0@@ -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 themdrivers/acpi/acpi_adxl.c new file +4
-0@@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Address translation interface via ACPI DSM. +* Copyright (C) 2018 Intel Corporation
drivers/acpi/acpi_adxl.c line 14
Quoted 10 lines
drivers/acpi/acpi_adxl.c new file +10
-0+*/ + +#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.
Quoted 197 lines
drivers/acpi/acpi_adxl.c new file +165
-0+ +#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 */ include/linux/adxl.h new file +25
-0@@ -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 */