> Subject: Re: [PATCH v2 1/3] ACPI / adxl: Address translation interface using ... Commit name needs a verb: "Add address translation..." On Thu, Oct 11, 2018 at 02:12:34PM -0700, Tony Luck wrote: > Some new servers provide an interface so that the OS can ask the > BIOS to translate a system physical address to a memory address > (socket, memory controller, channel, rank, dimm, etc.). This is > useful for EDAC drivers that want to take the address of an error > reported in a machine check bank and let the user know which > DIMM may need to be replaced. > > Specification for this interface is available at: > > https://cdrdv2.intel.com/v1/dl/getContent/603354 What do we do when that link dies? (And I've seen them die). Normally we upload those docs in bugzilla.kernel.org but that link wants me to "Accept". > [Based on earlier code by Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>] > > Tested-by: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx> > Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx> > --- > drivers/acpi/Kconfig | 3 + > drivers/acpi/Makefile | 3 + > drivers/acpi/acpi_adxl.c | 199 +++++++++++++++++++++++++++++++++++++++ > include/linux/adxl.h | 25 +++++ > 4 files changed, 230 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..09991cc91b89 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -498,6 +498,9 @@ config ACPI_EXTLOG > driver adds support for that functionality with corresponding > tracepoint which carries that information to userspace. > > +config ACPI_ADXL > + bool > + > 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..22b12822f25d > --- /dev/null > +++ b/drivers/acpi/acpi_adxl.c > @@ -0,0 +1,199 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Address translation interface via ACPI DSM. > + * Copyright (C) 2018 Intel Corporation > + * > + * Specification for this interface is available at: > + * > + * https://cdrdv2.intel.com/v1/dl/getContent/603354 > + */ > + > +#ifdef CONFIG_ACPI_ADXL This can go now. > +#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" > + > +/* > + * The specification doesn't provide a limit on how many > + * components are in a memory address. But since we allocate > + * memory based on the number the BIOS tells us, we should > + * defend against insane values. > + */ > +#define ADXL_MAX_COMPONENTS 500 > + > +#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 int adxl_count; > +static char **adxl_component_names; ... > +static bool adxl_detect(void) > +{ > + char *path = ACPI_ADXL_PATH; > + union acpi_object *p; > + acpi_status status; > + int i; > + > + status = acpi_get_handle(NULL, path, &handle); > + if (ACPI_FAILURE(status)) { > + pr_info("No ACPI handle for path %s\n", path); > + return false; > + } > + > + if (!acpi_has_method(handle, "_DSM")) { > + pr_info("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_info("No ADXL DSM methods\n"); > + return false; > + } > + > + params = adxl_dsm(ADXL_IDX_GET_ADDR_PARAMS, NULL); > + if (!params) { > + pr_info("Failed to get params\n"); > + return false; > + } > + > + p = params->package.elements + 1; > + adxl_count = p->package.count; > + if (adxl_count > ADXL_MAX_COMPONENTS) { > + pr_info("Insane number of address component names %d\n", adxl_count); > + ACPI_FREE(params); > + return false; > + } > + p = p->package.elements; > + /* * Allocate one more for NULL termination. */ > + adxl_component_names = kcalloc(adxl_count + 1, sizeof(char *), GFP_KERNEL); > + if (!adxl_component_names) { pr_err("Error allocating... " > + ACPI_FREE(params); > + return false; > + } > + > + for (i = 0; i < adxl_count; i++) > + adxl_component_names[i] = p[i].string.pointer; > + > + return true; > +} > + > +static int __init adxl_init(void) > +{ > + if (!adxl_detect()) I guess you don't need that adxl_detect() function and can move its body in here. > + 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..6023704e5d0b > --- /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 > +const char * const *adxl_get_component_names(void); > +int adxl_decode(u64 addr, u64 component_values[]); > +#else > +static inline const char * const *adxl_get_component_names(void) > +{ > + return NULL; > +} > + > +static inline int adxl_decode(u64 addr, u64 component_values[]) > +{ > + return -EOPNOTSUPP; > +} > +#endif > + > +#endif /* _LINUX_ADXL_H */ > -- > 2.17.1 > -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.