Re: [PATCH v2 1/3] ACPI / adxl: Address translation interface using ACPI DSM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux