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

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

 



On Tue, Oct 9, 2018 at 8:35 PM Tony Luck <tony.luck@xxxxxxxxx> 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
>
> [Based on earlier code by Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>]
>
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
>
> ---
> Comments addressed since last version:
>    Rafael:
>         Added URL for specification (to commit & header comment)
>         Fixed NULL error return from error case of adxl_dsm()
>
>    Boris:
>         Added sanity check on number of address components
>         (also added check that number of values returned by
>          decode operation matches number of strings)
>         g/pr_debug/s//pr_info/
>         Added "." at end of sentences in comments
>         Make return from adxl_get_component_names() immutable
>
>  drivers/acpi/Kconfig     |  10 ++
>  drivers/acpi/Makefile    |   3 +
>  drivers/acpi/acpi_adxl.c | 199 +++++++++++++++++++++++++++++++++++++++
>  include/linux/adxl.h     |  25 +++++
>  4 files changed, 237 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.

I'm still not sure why this has to be a user-selectable option.  If
the plan is to select it from the drivers that need it, why do we need
to ask about it?  And what if it is enabled, but no drivers use it?
Will it just be dead code then?

> +
>  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..a58bd8ec396e
> --- /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
> +#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 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_info("Empty obj\n");

pr_debug() perhaps?  It is somewhat devoid of details anyway.

Thanks,
Rafael



[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