* [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac @ 2018-02-22 19:58 Tony Luck [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown This series was posted as RFC and got some review back in December: https://marc.info/?l=linux-edac&m=151257213825419&w=2 It couldn't go upstream back then because I was waiting for some updates to the ACPICA headers for NFIT support. Those patches are complete now, so here is the series again. The "partial support" part is that the changes here only do the detection of the non-volatile DIMMs. Coming later will be another patch/series to teach skx_edac how to decode system addresses to NVDIMM addresses. But this part stands on its own and is a useful first step. Tony Luck (5): EDAC: Drop duplicated array of strings for memory type names edac: Add new memory type for non-volatile DIMMs acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle firmware: dmi: Add function to look up a handle and return DIMM size EDAC, skx_edac: Detect non-volatile DIMMs drivers/acpi/nfit/core.c | 27 ++++++++++++++++++ drivers/edac/Kconfig | 5 +++- drivers/edac/edac_mc.c | 41 +++++++++++++------------- drivers/edac/edac_mc_sysfs.c | 26 ++--------------- drivers/edac/skx_edac.c | 68 ++++++++++++++++++++++++++++++++++++++++---- drivers/firmware/dmi_scan.c | 31 ++++++++++++++++++++ include/acpi/nfit.h | 26 +++++++++++++++++ include/linux/dmi.h | 2 ++ include/linux/edac.h | 3 ++ 9 files changed, 178 insertions(+), 51 deletions(-) create mode 100644 include/acpi/nfit.h -- 2.14.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-02-22 19:58 ` Tony Luck 2018-02-22 19:58 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck ` (4 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown Somehow we ended up with two separate arrays of strings to describe the "enum mem_type" values. In edac_mc.c we have an exported list edac_mem_types[] that is used by a couple of drivers in debug messaged. In edac_mc_sysfs.c we have a private list that is used to display values in: /sys/devices/system/edac/mc/mc*/dimm*/dimm_mem_type /sys/devices/system/edac/mc/mc*/csrow*/mem_type this list was missing a value for MEM_LRDDR3 The string values in the two lists were different :-( Combining the lists, I kept the values so that the sysfs output will be unchanged as some scripts may depend on that. Reported-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> Acked-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/edac_mc.c | 40 ++++++++++++++++++++-------------------- drivers/edac/edac_mc_sysfs.c | 26 ++------------------------ 2 files changed, 22 insertions(+), 44 deletions(-) diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 48193f5f3b56..1f61b736e075 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -195,26 +195,26 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci) #endif /* CONFIG_EDAC_DEBUG */ const char * const edac_mem_types[] = { - [MEM_EMPTY] = "Empty csrow", - [MEM_RESERVED] = "Reserved csrow type", - [MEM_UNKNOWN] = "Unknown csrow type", - [MEM_FPM] = "Fast page mode RAM", - [MEM_EDO] = "Extended data out RAM", - [MEM_BEDO] = "Burst Extended data out RAM", - [MEM_SDR] = "Single data rate SDRAM", - [MEM_RDR] = "Registered single data rate SDRAM", - [MEM_DDR] = "Double data rate SDRAM", - [MEM_RDDR] = "Registered Double data rate SDRAM", - [MEM_RMBS] = "Rambus DRAM", - [MEM_DDR2] = "Unbuffered DDR2 RAM", - [MEM_FB_DDR2] = "Fully buffered DDR2", - [MEM_RDDR2] = "Registered DDR2 RAM", - [MEM_XDR] = "Rambus XDR", - [MEM_DDR3] = "Unbuffered DDR3 RAM", - [MEM_RDDR3] = "Registered DDR3 RAM", - [MEM_LRDDR3] = "Load-Reduced DDR3 RAM", - [MEM_DDR4] = "Unbuffered DDR4 RAM", - [MEM_RDDR4] = "Registered DDR4 RAM", + [MEM_EMPTY] = "Empty", + [MEM_RESERVED] = "Reserved", + [MEM_UNKNOWN] = "Unknown", + [MEM_FPM] = "FPM", + [MEM_EDO] = "EDO", + [MEM_BEDO] = "BEDO", + [MEM_SDR] = "Unbuffered-SDR", + [MEM_RDR] = "Registered-SDR", + [MEM_DDR] = "Unbuffered-DDR", + [MEM_RDDR] = "Registered-DDR", + [MEM_RMBS] = "RMBS", + [MEM_DDR2] = "Unbuffered-DDR2", + [MEM_FB_DDR2] = "FullyBuffered-DDR2", + [MEM_RDDR2] = "Registered-DDR2", + [MEM_XDR] = "XDR", + [MEM_DDR3] = "Unbuffered-DDR3", + [MEM_RDDR3] = "Registered-DDR3", + [MEM_LRDDR3] = "Load-Reduced-DDR3-RAM", + [MEM_DDR4] = "Unbuffered-DDR4", + [MEM_RDDR4] = "Registered-DDR4" }; EXPORT_SYMBOL_GPL(edac_mem_types); diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index c70ea82c815c..7481955160a4 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -91,28 +91,6 @@ static struct device *mci_pdev; /* * various constants for Memory Controllers */ -static const char * const mem_types[] = { - [MEM_EMPTY] = "Empty", - [MEM_RESERVED] = "Reserved", - [MEM_UNKNOWN] = "Unknown", - [MEM_FPM] = "FPM", - [MEM_EDO] = "EDO", - [MEM_BEDO] = "BEDO", - [MEM_SDR] = "Unbuffered-SDR", - [MEM_RDR] = "Registered-SDR", - [MEM_DDR] = "Unbuffered-DDR", - [MEM_RDDR] = "Registered-DDR", - [MEM_RMBS] = "RMBS", - [MEM_DDR2] = "Unbuffered-DDR2", - [MEM_FB_DDR2] = "FullyBuffered-DDR2", - [MEM_RDDR2] = "Registered-DDR2", - [MEM_XDR] = "XDR", - [MEM_DDR3] = "Unbuffered-DDR3", - [MEM_RDDR3] = "Registered-DDR3", - [MEM_DDR4] = "Unbuffered-DDR4", - [MEM_RDDR4] = "Registered-DDR4" -}; - static const char * const dev_types[] = { [DEV_UNKNOWN] = "Unknown", [DEV_X1] = "x1", @@ -196,7 +174,7 @@ static ssize_t csrow_mem_type_show(struct device *dev, { struct csrow_info *csrow = to_csrow(dev); - return sprintf(data, "%s\n", mem_types[csrow->channels[0]->dimm->mtype]); + return sprintf(data, "%s\n", edac_mem_types[csrow->channels[0]->dimm->mtype]); } static ssize_t csrow_dev_type_show(struct device *dev, @@ -549,7 +527,7 @@ static ssize_t dimmdev_mem_type_show(struct device *dev, { struct dimm_info *dimm = to_dimm(dev); - return sprintf(data, "%s\n", mem_types[dimm->mtype]); + return sprintf(data, "%s\n", edac_mem_types[dimm->mtype]); } static ssize_t dimmdev_dev_type_show(struct device *dev, -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-22 19:58 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck @ 2018-02-22 19:58 ` Tony Luck 2018-02-22 19:58 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck ` (3 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown There are now non-volatile versions of DIMMs. Add a new entry to "enum mem_type" and a new string in edac_mem_types[]. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/edac_mc.c | 3 ++- include/linux/edac.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 1f61b736e075..3bb82e511eca 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -214,7 +214,8 @@ const char * const edac_mem_types[] = { [MEM_RDDR3] = "Registered-DDR3", [MEM_LRDDR3] = "Load-Reduced-DDR3-RAM", [MEM_DDR4] = "Unbuffered-DDR4", - [MEM_RDDR4] = "Registered-DDR4" + [MEM_RDDR4] = "Registered-DDR4", + [MEM_NVDIMM] = "Non-volatile-RAM", }; EXPORT_SYMBOL_GPL(edac_mem_types); diff --git a/include/linux/edac.h b/include/linux/edac.h index cd75c173fd00..bffb97828ed6 100644 --- a/include/linux/edac.h +++ b/include/linux/edac.h @@ -186,6 +186,7 @@ static inline char *mc_event_error_type(const unsigned int err_type) * @MEM_RDDR4: Registered DDR4 RAM * This is a variant of the DDR4 memories. * @MEM_LRDDR4: Load-Reduced DDR4 memory. + * @MEM_NVDIMM: Non-volatile RAM */ enum mem_type { MEM_EMPTY = 0, @@ -209,6 +210,7 @@ enum mem_type { MEM_DDR4, MEM_RDDR4, MEM_LRDDR4, + MEM_NVDIMM, }; #define MEM_FLAG_EMPTY BIT(MEM_EMPTY) @@ -231,6 +233,7 @@ enum mem_type { #define MEM_FLAG_DDR4 BIT(MEM_DDR4) #define MEM_FLAG_RDDR4 BIT(MEM_RDDR4) #define MEM_FLAG_LRDDR4 BIT(MEM_LRDDR4) +#define MEM_FLAG_NVDIMM BIT(MEM_NVDIMM) /** * enum edac-type - Error Detection and Correction capabilities and mode -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-22 19:58 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck 2018-02-22 19:58 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck @ 2018-02-22 19:58 ` Tony Luck [not found] ` <20180222195811.27237-4-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-22 19:58 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck ` (2 subsequent siblings) 5 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown EDAC driver needs to look up attributes of NVDIMMs provided in SMBIOS. Provide a function that looks up an acpi_nfit_memory_map from a device handle (node/socket/mc/channel/dimm) and returns the SMBIOS handle. Also pass back the "flags" so we can see if the NVDIMM is OK. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/acpi/nfit/core.c | 27 +++++++++++++++++++++++++++ include/acpi/nfit.h | 26 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 include/acpi/nfit.h diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index bbe48ad20886..4d6eeb1793e6 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -23,6 +23,7 @@ #include <linux/io.h> #include <linux/nd.h> #include <asm/cacheflush.h> +#include <acpi/nfit.h> #include "nfit.h" /* @@ -690,6 +691,32 @@ static bool add_memdev(struct acpi_nfit_desc *acpi_desc, return true; } +int nfit_get_smbios_id(u32 device_handle, u16 *flags) +{ + struct acpi_nfit_memory_map *memdev; + struct acpi_nfit_desc *acpi_desc; + struct nfit_mem *nfit_mem; + + mutex_lock(&acpi_desc_lock); + list_for_each_entry(acpi_desc, &acpi_descs, list) { + mutex_lock(&acpi_desc->init_mutex); + list_for_each_entry(nfit_mem, &acpi_desc->dimms, list) { + memdev = __to_nfit_memdev(nfit_mem); + if (memdev->device_handle == device_handle) { + mutex_unlock(&acpi_desc->init_mutex); + mutex_unlock(&acpi_desc_lock); + *flags = memdev->flags; + return memdev->physical_id; + } + } + mutex_unlock(&acpi_desc->init_mutex); + } + mutex_unlock(&acpi_desc_lock); + + return -ENODEV; +} +EXPORT_SYMBOL_GPL(nfit_get_smbios_id); + /* * An implementation may provide a truncated control region if no block windows * are defined. diff --git a/include/acpi/nfit.h b/include/acpi/nfit.h new file mode 100644 index 000000000000..6ccc6eacd855 --- /dev/null +++ b/include/acpi/nfit.h @@ -0,0 +1,26 @@ +/* + * Copyright(c) 2017 Intel Corporation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#ifndef __ACPI_NFIT_H +#define __ACPI_NFIT_H + +#if IS_ENABLED(CONFIG_ACPI_NFIT) +int nfit_get_smbios_id(u32 device_handle, u16 *flags); +#else +static inline int nfit_get_smbios_id(u32 device_handle, u16 *flags) +{ + return -EOPNOTSUPP; +} +#endif + +#endif /* __ACPI_NFIT_H */ -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <20180222195811.27237-4-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle [not found] ` <20180222195811.27237-4-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-02-28 17:36 ` Ross Zwisler [not found] ` <20180228173621.GA12883-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 26+ messages in thread From: Ross Zwisler @ 2018-02-28 17:36 UTC (permalink / raw) To: Tony Luck Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mauro Carvalho Chehab, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown, Lv Zheng On Thu, Feb 22, 2018 at 11:58:09AM -0800, Tony Luck wrote: > EDAC driver needs to look up attributes of NVDIMMs provided in SMBIOS. > > Provide a function that looks up an acpi_nfit_memory_map from a device > handle (node/socket/mc/channel/dimm) and returns the SMBIOS handle. > Also pass back the "flags" so we can see if the NVDIMM is OK. > > Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- <> > diff --git a/include/acpi/nfit.h b/include/acpi/nfit.h > new file mode 100644 > index 000000000000..6ccc6eacd855 > --- /dev/null > +++ b/include/acpi/nfit.h > @@ -0,0 +1,26 @@ > +/* > + * Copyright(c) 2017 Intel Corporation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of version 2 of the GNU General Public License as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + */ Just a quick nit - I think we're supposed to use SPDX license identifiers for new files? ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <20180228173621.GA12883-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle [not found] ` <20180228173621.GA12883-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2018-02-28 18:48 ` Borislav Petkov 0 siblings, 0 replies; 26+ messages in thread From: Borislav Petkov @ 2018-02-28 18:48 UTC (permalink / raw) To: Ross Zwisler Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown, Lv Zheng On Wed, Feb 28, 2018 at 10:36:21AM -0700, Ross Zwisler wrote: > Just a quick nit - I think we're supposed to use SPDX license identifiers for > new files? One would think checkpatch would warn about that but nope. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (2 preceding siblings ...) 2018-02-22 19:58 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck @ 2018-02-22 19:58 ` Tony Luck [not found] ` <20180222195811.27237-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-22 19:58 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck 2018-02-28 13:04 ` [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Borislav Petkov 5 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown When we first scan the SMBIOS table, save the size of the DIMM. Provide a function for other code (EDAC driver) to look up the size of a DIMM from its SMBIOS handle. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/firmware/dmi_scan.c | 31 +++++++++++++++++++++++++++++++ include/linux/dmi.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index e763e1484331..9947f1f6ef7b 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -32,6 +32,7 @@ static char dmi_ids_string[128] __initdata; static struct dmi_memdev_info { const char *device; const char *bank; + u64 size; u16 handle; } *dmi_memdev; static int dmi_memdev_nr; @@ -386,6 +387,8 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) { const char *d = (const char *)dm; static int nr; + u64 bytes; + u16 size; if (dm->type != DMI_ENTRY_MEM_DEVICE || dm->length < 0x12) return; @@ -396,6 +399,20 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) dmi_memdev[nr].handle = get_unaligned(&dm->handle); dmi_memdev[nr].device = dmi_string(dm, d[0x10]); dmi_memdev[nr].bank = dmi_string(dm, d[0x11]); + + size = get_unaligned((u16 *)&d[0xC]); + if (size == 0) + bytes = 0; + else if (size == 0xffff) + bytes = ~0ul; + else if (size & 0x8000) + bytes = (u64)(size & 0x7fff) << 10; + else if (size != 0x7fff) + bytes = (u64)size << 20; + else + bytes = (u64)get_unaligned((u32 *)&d[0x1C]) << 20; + + dmi_memdev[nr].size = bytes; nr++; } @@ -1065,3 +1082,17 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device) } } EXPORT_SYMBOL_GPL(dmi_memdev_name); + +u64 dmi_memdev_size(u16 handle) +{ + int n; + + if (dmi_memdev) { + for (n = 0; n < dmi_memdev_nr; n++) { + if (handle == dmi_memdev[n].handle) + return dmi_memdev[n].size; + } + } + return ~0ul; +} +EXPORT_SYMBOL_GPL(dmi_memdev_size); diff --git a/include/linux/dmi.h b/include/linux/dmi.h index 46e151172d95..7f5929123b69 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -113,6 +113,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *), void *private_data); extern bool dmi_match(enum dmi_field f, const char *str); extern void dmi_memdev_name(u16 handle, const char **bank, const char **device); +extern u64 dmi_memdev_size(u16 handle); #else @@ -142,6 +143,7 @@ static inline bool dmi_match(enum dmi_field f, const char *str) { return false; } static inline void dmi_memdev_name(u16 handle, const char **bank, const char **device) { } +static inline u64 dmi_memdev_size(u16 handle) { return ~0ul; } static inline const struct dmi_system_id * dmi_first_match(const struct dmi_system_id *list) { return NULL; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <20180222195811.27237-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size [not found] ` <20180222195811.27237-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-03-09 10:20 ` Jean Delvare 2018-03-09 23:03 ` Luck, Tony 0 siblings, 1 reply; 26+ messages in thread From: Jean Delvare @ 2018-03-09 10:20 UTC (permalink / raw) To: Tony Luck Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Lv Zheng, Borislav Petkov, Len Brown Hi Tony, On Thu, 22 Feb 2018 11:58:10 -0800, Tony Luck wrote: > When we first scan the SMBIOS table, save the size of the DIMM. > > Provide a function for other code (EDAC driver) to look up the size > of a DIMM from its SMBIOS handle. On the principle I'm OK with the idea. My comments on the implementation details are inline below. > Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- > drivers/firmware/dmi_scan.c | 31 +++++++++++++++++++++++++++++++ > include/linux/dmi.h | 2 ++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c > index e763e1484331..9947f1f6ef7b 100644 > --- a/drivers/firmware/dmi_scan.c > +++ b/drivers/firmware/dmi_scan.c > @@ -32,6 +32,7 @@ static char dmi_ids_string[128] __initdata; > static struct dmi_memdev_info { > const char *device; > const char *bank; > + u64 size; A comment stating in which unit the size is stored would be appreciated. > u16 handle; > } *dmi_memdev; > static int dmi_memdev_nr; > @@ -386,6 +387,8 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) > { > const char *d = (const char *)dm; > static int nr; > + u64 bytes; > + u16 size; > > if (dm->type != DMI_ENTRY_MEM_DEVICE || dm->length < 0x12) > return; > @@ -396,6 +399,20 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) > dmi_memdev[nr].handle = get_unaligned(&dm->handle); > dmi_memdev[nr].device = dmi_string(dm, d[0x10]); > dmi_memdev[nr].bank = dmi_string(dm, d[0x11]); > + > + size = get_unaligned((u16 *)&d[0xC]); > + if (size == 0) > + bytes = 0; > + else if (size == 0xffff) > + bytes = ~0ul; Should this not be ~0ull? On 32-bit systems ~0ul would be 0xffffffff not 0xffffffffffffffff. Also it would be nice to document somewhere that 0 means memory module not installed and all fs means size is unknown. > + else if (size & 0x8000) > + bytes = (u64)(size & 0x7fff) << 10; > + else if (size != 0x7fff) > + bytes = (u64)size << 20; > + else > + bytes = (u64)get_unaligned((u32 *)&d[0x1C]) << 20; > + > + dmi_memdev[nr].size = bytes; I'm curious, do you really need to know the amount of memory to the byte? The SMBIOS specification itself does not support granularity under 1 kB. I would be very surprised if any machine running Linux today has memory modules under 1 MB. If storing in MB you wouldn't need a u64... > nr++; > } > > @@ -1065,3 +1082,17 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device) > } > } > EXPORT_SYMBOL_GPL(dmi_memdev_name); > + > +u64 dmi_memdev_size(u16 handle) > +{ > + int n; > + > + if (dmi_memdev) { > + for (n = 0; n < dmi_memdev_nr; n++) { > + if (handle == dmi_memdev[n].handle) > + return dmi_memdev[n].size; > + } > + } > + return ~0ul; ~0ull? > +} > +EXPORT_SYMBOL_GPL(dmi_memdev_size); > diff --git a/include/linux/dmi.h b/include/linux/dmi.h > index 46e151172d95..7f5929123b69 100644 > --- a/include/linux/dmi.h > +++ b/include/linux/dmi.h > @@ -113,6 +113,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *), > void *private_data); > extern bool dmi_match(enum dmi_field f, const char *str); > extern void dmi_memdev_name(u16 handle, const char **bank, const char **device); > +extern u64 dmi_memdev_size(u16 handle); > > #else > > @@ -142,6 +143,7 @@ static inline bool dmi_match(enum dmi_field f, const char *str) > { return false; } > static inline void dmi_memdev_name(u16 handle, const char **bank, > const char **device) { } > +static inline u64 dmi_memdev_size(u16 handle) { return ~0ul; } ~0ull? > static inline const struct dmi_system_id * > dmi_first_match(const struct dmi_system_id *list) { return NULL; } > -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size 2018-03-09 10:20 ` Jean Delvare @ 2018-03-09 23:03 ` Luck, Tony 2018-03-10 13:22 ` Jean Delvare 0 siblings, 1 reply; 26+ messages in thread From: Luck, Tony @ 2018-03-09 23:03 UTC (permalink / raw) To: Jean Delvare Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Lv Zheng, Borislav Petkov, Len Brown On Fri, Mar 09, 2018 at 11:20:53AM +0100, Jean Delvare wrote: > > + else if (size & 0x8000) > > + bytes = (u64)(size & 0x7fff) << 10; > > + else if (size != 0x7fff) > > + bytes = (u64)size << 20; > > + else > > + bytes = (u64)get_unaligned((u32 *)&d[0x1C]) << 20; > > + > > + dmi_memdev[nr].size = bytes; > > I'm curious, do you really need to know the amount of memory to the > byte? The SMBIOS specification itself does not support granularity > under 1 kB. I would be very surprised if any machine running Linux > today has memory modules under 1 MB. If storing in MB you wouldn't need > a u64... I got side-tracked reading the standard with the ancient ways used to report size back in the day when kilobytes was a plausible unit. So I wrote code that covers all the crazy cases. Persistant DIMMs have sizes measured in gigabytes. I should stop worrying about "0" vs. "fffffff" and just treat the old cases as errors and simplify the code to be: u32 mbytes; if (size == 0 || (size & 0x8000)) mbytes = 0; if (size != 0x7fff) mbytes = size; else mbytes = get_unaligned((u32 *)&d[0x1C]); Then I can use 32-bit throughout this and patch 5. Thanks for the review. -Tony ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size 2018-03-09 23:03 ` Luck, Tony @ 2018-03-10 13:22 ` Jean Delvare 2018-03-12 16:46 ` Luck, Tony 0 siblings, 1 reply; 26+ messages in thread From: Jean Delvare @ 2018-03-10 13:22 UTC (permalink / raw) To: Luck, Tony Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Lv Zheng, Borislav Petkov, Len Brown Hi Tony, On Fri, 9 Mar 2018 15:03:58 -0800, Luck, Tony wrote: > I got side-tracked reading the standard with the ancient ways > used to report size back in the day when kilobytes was a > plausible unit. So I wrote code that covers all the crazy > cases. Persistant DIMMs have sizes measured in gigabytes. > I should stop worrying about "0" vs. "fffffff" and just treat > the old cases as errors and simplify the code to be: > > u32 mbytes; > > if (size == 0 || (size & 0x8000)) > mbytes = 0; > if (size != 0x7fff) "else" missing. > mbytes = size; > else > mbytes = get_unaligned((u32 *)&d[0x1C]); > > Then I can use 32-bit throughout this and patch 5. Note that it is possible to store MB values (up to 16 MB) using kB as the unit. The specification allows for it, and a few systems use that option. For example [1], the DMI data of a Supermicro X8STi board looks like: Handle 0x0038, DMI type 17, 28 bytes Memory Device (...) Size: 4096 kB and it is encoded as 0x9000. I understand you don't care about such "small" memory modules for persistent DIMMs, however the API is not specific to these, and it could be confusing for callers that modules between 1 MB and 16 MB are sometimes reported as 0 and sometimes not. So I believe that your code should handle this case. > Thanks for the review. You're welcome. [1] Let's be honest, that was the only instance out of the 180 DMI dumps I collected. So it's fairly rare. But it did happen. -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size 2018-03-10 13:22 ` Jean Delvare @ 2018-03-12 16:46 ` Luck, Tony 0 siblings, 0 replies; 26+ messages in thread From: Luck, Tony @ 2018-03-12 16:46 UTC (permalink / raw) To: Jean Delvare Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown On Sat, Mar 10, 2018 at 02:22:17PM +0100, Jean Delvare wrote: > Note that it is possible to store MB values (up to 16 MB) using kB as > the unit. The specification allows for it, and a few systems use that > option. For example [1], the DMI data of a Supermicro X8STi board looks > like: > > Handle 0x0038, DMI type 17, 28 bytes > Memory Device > (...) > Size: 4096 kB > > and it is encoded as 0x9000. > > I understand you don't care about such "small" memory modules for > persistent DIMMs, however the API is not specific to these, and it > could be confusing for callers that modules between 1 MB and 16 MB are > sometimes reported as 0 and sometimes not. So I believe that your code > should handle this case. Then I might as well go with a fully standards compliant version (just in case somebody in the future has a 512KB module encoded as 0x8200, they would also be confused by a return of 0 MB). That means I should stick with u64 as the type of the "size" field (using KB in a u32 would max out with 4TB modules, which may happen before I retire). -Tony ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (3 preceding siblings ...) 2018-02-22 19:58 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck @ 2018-02-22 19:58 ` Tony Luck [not found] ` <20180222195811.27237-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-28 13:04 ` [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Borislav Petkov 5 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-02-22 19:58 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Lv Zheng, Mauro Carvalho Chehab, Len Brown This just covers the topology function of the EDAC driver. We locate which DIMM slots are populated with NVDIMMs and query the NFIT and SMBIOS tables to get the size. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/Kconfig | 5 +++- drivers/edac/skx_edac.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index 3c4017007647..c12e34564557 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -232,9 +232,12 @@ config EDAC_SBRIDGE config EDAC_SKX tristate "Intel Skylake server Integrated MC" depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG + select DMI help Support for error detection and correction the Intel - Skylake server Integrated Memory Controllers. + Skylake server Integrated Memory Controllers. If your + system has non-volatile DIMMs you should also manually + select CONFIG_ACPI_NFIT config EDAC_PND2 tristate "Intel Pondicherry2" diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c index 912c4930c9ef..f34c60638c4b 100644 --- a/drivers/edac/skx_edac.c +++ b/drivers/edac/skx_edac.c @@ -14,6 +14,8 @@ #include <linux/module.h> #include <linux/init.h> +#include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/pci.h> #include <linux/pci_ids.h> #include <linux/slab.h> @@ -24,6 +26,7 @@ #include <linux/bitmap.h> #include <linux/math64.h> #include <linux/mod_devicetable.h> +#include <acpi/nfit.h> #include <asm/cpu_device_id.h> #include <asm/intel-family.h> #include <asm/processor.h> @@ -302,6 +305,7 @@ static int get_dimm_attr(u32 reg, int lobit, int hibit, int add, int minval, } #define IS_DIMM_PRESENT(mtr) GET_BITFIELD((mtr), 15, 15) +#define IS_NVDIMM_PRESENT(mcddrtcfg, i) GET_BITFIELD((mcddrtcfg), (i), (i)) #define numrank(reg) get_dimm_attr((reg), 12, 13, 0, 0, 2, "ranks") #define numrow(reg) get_dimm_attr((reg), 2, 4, 12, 1, 6, "rows") @@ -350,8 +354,6 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, int banks = 16, ranks, rows, cols, npages; u64 size; - if (!IS_DIMM_PRESENT(mtr)) - return 0; ranks = numrank(mtr); rows = numrow(mtr); cols = numcol(mtr); @@ -383,6 +385,55 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, return 1; } +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, + int chan, int dimmno) +{ + int smbios_handle; + u32 dev_handle; + u16 flags; + u64 size = 0; + + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc, + imc->src_id, 0); + + smbios_handle = nfit_get_smbios_id(dev_handle, &flags); + if (smbios_handle == -EOPNOTSUPP) { + pr_warn_once("skx_edac: can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n"); + goto unknown_size; + } + if (smbios_handle < 0) { + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle); + return 0; + } + + if (flags & ACPI_NFIT_MEM_MAP_FAILED) { + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle); + return 0; + } + + size = dmi_memdev_size(smbios_handle); + if (size == ~0ul) { + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n", + dev_handle, smbios_handle); + return 0; + } + +unknown_size: + edac_dbg(0, "mc#%d: channel %d, dimm %d, %lld Mb (%lld pages)\n", + imc->mc, chan, dimmno, size >> 20, size >> PAGE_SHIFT); + + dimm->nr_pages = size >> PAGE_SHIFT; + dimm->grain = 32; + dimm->dtype = DEV_UNKNOWN; + dimm->mtype = MEM_NVDIMM; + dimm->edac_mode = EDAC_SECDED; /* likely better than this */ + + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", + imc->src_id, imc->lmc, chan, dimmno); + + return 1; +} + #define SKX_GET_MTMTR(dev, reg) \ pci_read_config_dword((dev), 0x87c, ®) @@ -399,20 +450,24 @@ static int skx_get_dimm_config(struct mem_ctl_info *mci) { struct skx_pvt *pvt = mci->pvt_info; struct skx_imc *imc = pvt->imc; + u32 mtr, amap, mcddrtcfg; struct dimm_info *dimm; int i, j; - u32 mtr, amap; int ndimms; for (i = 0; i < NUM_CHANNELS; i++) { ndimms = 0; pci_read_config_dword(imc->chan[i].cdev, 0x8C, &amap); + pci_read_config_dword(imc->chan[i].cdev, 0x400, &mcddrtcfg); for (j = 0; j < NUM_DIMMS; j++) { dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, mci->n_layers, i, j, 0); pci_read_config_dword(imc->chan[i].cdev, 0x80 + 4*j, &mtr); - ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + if (IS_DIMM_PRESENT(mtr)) + ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + else if (IS_NVDIMM_PRESENT(mcddrtcfg, j)) + ndimms += get_nvdimm_info(dimm, imc, i, j); } if (ndimms && !skx_check_ecc(imc->chan[0].cdev)) { skx_printk(KERN_ERR, "ECC is disabled on imc %d\n", imc->mc); @@ -468,13 +523,14 @@ static int skx_register_mci(struct skx_imc *imc) pvt = mci->pvt_info; pvt->imc = imc; - mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", imc->node_id, imc->lmc); + mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", + imc->node_id, imc->lmc); if (!mci->ctl_name) { rc = -ENOMEM; goto fail0; } - mci->mtype_cap = MEM_FLAG_DDR4; + mci->mtype_cap = MEM_FLAG_DDR4 | MEM_FLAG_NVDIMM; mci->edac_ctl_cap = EDAC_FLAG_NONE; mci->edac_cap = EDAC_FLAG_NONE; mci->mod_name = EDAC_MOD_STR; -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <20180222195811.27237-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs [not found] ` <20180222195811.27237-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-03-09 10:38 ` Jean Delvare 0 siblings, 0 replies; 26+ messages in thread From: Jean Delvare @ 2018-03-09 10:38 UTC (permalink / raw) To: Tony Luck Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Lv Zheng, Borislav Petkov, Len Brown Hi Tony, On Thu, 22 Feb 2018 11:58:11 -0800, Tony Luck wrote: > This just covers the topology function of the EDAC driver. > We locate which DIMM slots are populated with NVDIMMs and > query the NFIT and SMBIOS tables to get the size. > > Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- > drivers/edac/Kconfig | 5 +++- > drivers/edac/skx_edac.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 66 insertions(+), 7 deletions(-) > (...) > +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, > + int chan, int dimmno) > +{ > + int smbios_handle; > + u32 dev_handle; > + u16 flags; > + u64 size = 0; > + > + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc, > + imc->src_id, 0); > + > + smbios_handle = nfit_get_smbios_id(dev_handle, &flags); > + if (smbios_handle == -EOPNOTSUPP) { > + pr_warn_once("skx_edac: can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n"); > + goto unknown_size; I'm curious why you continue in this (worse) error case, but stop on all other (some presumably less critical) error cases? Specifically I can't see how an unknown size returned by the dmi subsystem can be worse than not being able to query the size at all. > + } > + if (smbios_handle < 0) { > + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle); > + return 0; > + } > + > + if (flags & ACPI_NFIT_MEM_MAP_FAILED) { > + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle); > + return 0; > + } > + > + size = dmi_memdev_size(smbios_handle); > + if (size == ~0ul) { If you agree with my comment on previous patch then this becomes ~0ull. > + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n", > + dev_handle, smbios_handle); > + return 0; > + } > + > +unknown_size: > + edac_dbg(0, "mc#%d: channel %d, dimm %d, %lld Mb (%lld pages)\n", %llu instead of %lld (twice)? > + imc->mc, chan, dimmno, size >> 20, size >> PAGE_SHIFT); > + > + dimm->nr_pages = size >> PAGE_SHIFT; If you moved the debug print after, you wouldn't have to do the shift twice. > + dimm->grain = 32; > + dimm->dtype = DEV_UNKNOWN; > + dimm->mtype = MEM_NVDIMM; > + dimm->edac_mode = EDAC_SECDED; /* likely better than this */ > + > + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", > + imc->src_id, imc->lmc, chan, dimmno); > + > + return 1; > +} -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (4 preceding siblings ...) 2018-02-22 19:58 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck @ 2018-02-28 13:04 ` Borislav Petkov [not found] ` <20180228130410.GC3769-fF5Pk5pvG8Y@public.gmane.org> 5 siblings, 1 reply; 26+ messages in thread From: Borislav Petkov @ 2018-02-28 13:04 UTC (permalink / raw) To: Tony Luck, Dan Williams, Jean Delvare Cc: Qiuxu Zhuo, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Aristeu Rozanski, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Lv Zheng, Mauro Carvalho Chehab, Len Brown On Thu, Feb 22, 2018 at 11:58:06AM -0800, Tony Luck wrote: > This series was posted as RFC and got some review back in December: > > https://marc.info/?l=linux-edac&m=151257213825419&w=2 > > It couldn't go upstream back then because I was waiting for some updates > to the ACPICA headers for NFIT support. Those patches are complete now, > so here is the series again. > > The "partial support" part is that the changes here only do the detection of > the non-volatile DIMMs. Coming later will be another patch/series to teach > skx_edac how to decode system addresses to NVDIMM addresses. But this part > stands on its own and is a useful first step. > > Tony Luck (5): > EDAC: Drop duplicated array of strings for memory type names > edac: Add new memory type for non-volatile DIMMs > acpi, nfit: Add function to look up nvdimm device and provide SMBIOS > handle > firmware: dmi: Add function to look up a handle and return DIMM size > EDAC, skx_edac: Detect non-volatile DIMMs > > drivers/acpi/nfit/core.c | 27 ++++++++++++++++++ > drivers/edac/Kconfig | 5 +++- > drivers/edac/edac_mc.c | 41 +++++++++++++------------- > drivers/edac/edac_mc_sysfs.c | 26 ++--------------- > drivers/edac/skx_edac.c | 68 ++++++++++++++++++++++++++++++++++++++++---- > drivers/firmware/dmi_scan.c | 31 ++++++++++++++++++++ > include/acpi/nfit.h | 26 +++++++++++++++++ > include/linux/dmi.h | 2 ++ > include/linux/edac.h | 3 ++ > 9 files changed, 178 insertions(+), 51 deletions(-) > create mode 100644 include/acpi/nfit.h Series looks ok to me. Dan, I could route patch 3 through the EDAC tree if you'd prefer. Jean, ditto for patch 4. Thanks. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <20180228130410.GC3769-fF5Pk5pvG8Y@public.gmane.org>]
* Re: [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac [not found] ` <20180228130410.GC3769-fF5Pk5pvG8Y@public.gmane.org> @ 2018-02-28 13:59 ` Dan Williams [not found] ` <CAPcyv4iHr9nUAS0SiFeqqDkU8NXZ_x=rkD3YS010Dy-nbHJXmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-02-28 13:59 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm, Aristeu Rozanski, Rafael J. Wysocki, Qiuxu Zhuo, Linux ACPI, Tony Luck, Lv Zheng, Mauro Carvalho Chehab, Len Brown On Wed, Feb 28, 2018 at 5:04 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote: > On Thu, Feb 22, 2018 at 11:58:06AM -0800, Tony Luck wrote: >> This series was posted as RFC and got some review back in December: >> >> https://marc.info/?l=linux-edac&m=151257213825419&w=2 >> >> It couldn't go upstream back then because I was waiting for some updates >> to the ACPICA headers for NFIT support. Those patches are complete now, >> so here is the series again. >> >> The "partial support" part is that the changes here only do the detection of >> the non-volatile DIMMs. Coming later will be another patch/series to teach >> skx_edac how to decode system addresses to NVDIMM addresses. But this part >> stands on its own and is a useful first step. >> >> Tony Luck (5): >> EDAC: Drop duplicated array of strings for memory type names >> edac: Add new memory type for non-volatile DIMMs >> acpi, nfit: Add function to look up nvdimm device and provide SMBIOS >> handle >> firmware: dmi: Add function to look up a handle and return DIMM size >> EDAC, skx_edac: Detect non-volatile DIMMs >> >> drivers/acpi/nfit/core.c | 27 ++++++++++++++++++ >> drivers/edac/Kconfig | 5 +++- >> drivers/edac/edac_mc.c | 41 +++++++++++++------------- >> drivers/edac/edac_mc_sysfs.c | 26 ++--------------- >> drivers/edac/skx_edac.c | 68 ++++++++++++++++++++++++++++++++++++++++---- >> drivers/firmware/dmi_scan.c | 31 ++++++++++++++++++++ >> include/acpi/nfit.h | 26 +++++++++++++++++ >> include/linux/dmi.h | 2 ++ >> include/linux/edac.h | 3 ++ >> 9 files changed, 178 insertions(+), 51 deletions(-) >> create mode 100644 include/acpi/nfit.h > > Series looks ok to me. > > Dan, I could route patch 3 through the EDAC tree if you'd prefer. Yes, please, and you can add: Acked-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <CAPcyv4iHr9nUAS0SiFeqqDkU8NXZ_x=rkD3YS010Dy-nbHJXmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* [PATCH 0/5 V3] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac [not found] ` <CAPcyv4iHr9nUAS0SiFeqqDkU8NXZ_x=rkD3YS010Dy-nbHJXmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-03-12 18:24 ` Tony Luck [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown Add support for non-volatile DIMMS [Repost with fixes from Ross Zwisler and Jean Delvare's comments] Changes since previous version: Parts 1-2 unchanged since last post Part 3 Use new SPDX license header for new file include/acpi/nfit.h Part 4 Fix some ~0ul that should be ~0ull Part 5 Rationalize error handling for various different reasons that we didn't find the size of a non-volatile DIMM Fix some printk formats. Fix test for ~0ul that should be ~0ull Re-order code to avoid computing number of pages twice Dan gave the thumb's up to run part 3 through the EDAC tree. Tony Luck (5): EDAC: Drop duplicated array of strings for memory type names edac: Add new memory type for non-volatile DIMMs acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle firmware: dmi: Add function to look up a handle and return DIMM size EDAC, skx_edac: Detect non-volatile DIMMs drivers/acpi/nfit/core.c | 27 ++++++++++++++++++ drivers/edac/Kconfig | 5 +++- drivers/edac/edac_mc.c | 41 +++++++++++++-------------- drivers/edac/edac_mc_sysfs.c | 26 ++--------------- drivers/edac/skx_edac.c | 66 ++++++++++++++++++++++++++++++++++++++++---- drivers/firmware/dmi_scan.c | 31 +++++++++++++++++++++ include/acpi/nfit.h | 18 ++++++++++++ include/linux/dmi.h | 2 ++ include/linux/edac.h | 3 ++ 9 files changed, 168 insertions(+), 51 deletions(-) create mode 100644 include/acpi/nfit.h -- 2.14.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-03-12 18:24 ` Tony Luck 2018-03-12 18:24 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck ` (3 subsequent siblings) 4 siblings, 0 replies; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown Somehow we ended up with two separate arrays of strings to describe the "enum mem_type" values. In edac_mc.c we have an exported list edac_mem_types[] that is used by a couple of drivers in debug messaged. In edac_mc_sysfs.c we have a private list that is used to display values in: /sys/devices/system/edac/mc/mc*/dimm*/dimm_mem_type /sys/devices/system/edac/mc/mc*/csrow*/mem_type this list was missing a value for MEM_LRDDR3 The string values in the two lists were different :-( Combining the lists, I kept the values so that the sysfs output will be unchanged as some scripts may depend on that. Reported-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> Acked-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/edac_mc.c | 40 ++++++++++++++++++++-------------------- drivers/edac/edac_mc_sysfs.c | 26 ++------------------------ 2 files changed, 22 insertions(+), 44 deletions(-) diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 48193f5f3b56..1f61b736e075 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -195,26 +195,26 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci) #endif /* CONFIG_EDAC_DEBUG */ const char * const edac_mem_types[] = { - [MEM_EMPTY] = "Empty csrow", - [MEM_RESERVED] = "Reserved csrow type", - [MEM_UNKNOWN] = "Unknown csrow type", - [MEM_FPM] = "Fast page mode RAM", - [MEM_EDO] = "Extended data out RAM", - [MEM_BEDO] = "Burst Extended data out RAM", - [MEM_SDR] = "Single data rate SDRAM", - [MEM_RDR] = "Registered single data rate SDRAM", - [MEM_DDR] = "Double data rate SDRAM", - [MEM_RDDR] = "Registered Double data rate SDRAM", - [MEM_RMBS] = "Rambus DRAM", - [MEM_DDR2] = "Unbuffered DDR2 RAM", - [MEM_FB_DDR2] = "Fully buffered DDR2", - [MEM_RDDR2] = "Registered DDR2 RAM", - [MEM_XDR] = "Rambus XDR", - [MEM_DDR3] = "Unbuffered DDR3 RAM", - [MEM_RDDR3] = "Registered DDR3 RAM", - [MEM_LRDDR3] = "Load-Reduced DDR3 RAM", - [MEM_DDR4] = "Unbuffered DDR4 RAM", - [MEM_RDDR4] = "Registered DDR4 RAM", + [MEM_EMPTY] = "Empty", + [MEM_RESERVED] = "Reserved", + [MEM_UNKNOWN] = "Unknown", + [MEM_FPM] = "FPM", + [MEM_EDO] = "EDO", + [MEM_BEDO] = "BEDO", + [MEM_SDR] = "Unbuffered-SDR", + [MEM_RDR] = "Registered-SDR", + [MEM_DDR] = "Unbuffered-DDR", + [MEM_RDDR] = "Registered-DDR", + [MEM_RMBS] = "RMBS", + [MEM_DDR2] = "Unbuffered-DDR2", + [MEM_FB_DDR2] = "FullyBuffered-DDR2", + [MEM_RDDR2] = "Registered-DDR2", + [MEM_XDR] = "XDR", + [MEM_DDR3] = "Unbuffered-DDR3", + [MEM_RDDR3] = "Registered-DDR3", + [MEM_LRDDR3] = "Load-Reduced-DDR3-RAM", + [MEM_DDR4] = "Unbuffered-DDR4", + [MEM_RDDR4] = "Registered-DDR4" }; EXPORT_SYMBOL_GPL(edac_mem_types); diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index c70ea82c815c..7481955160a4 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -91,28 +91,6 @@ static struct device *mci_pdev; /* * various constants for Memory Controllers */ -static const char * const mem_types[] = { - [MEM_EMPTY] = "Empty", - [MEM_RESERVED] = "Reserved", - [MEM_UNKNOWN] = "Unknown", - [MEM_FPM] = "FPM", - [MEM_EDO] = "EDO", - [MEM_BEDO] = "BEDO", - [MEM_SDR] = "Unbuffered-SDR", - [MEM_RDR] = "Registered-SDR", - [MEM_DDR] = "Unbuffered-DDR", - [MEM_RDDR] = "Registered-DDR", - [MEM_RMBS] = "RMBS", - [MEM_DDR2] = "Unbuffered-DDR2", - [MEM_FB_DDR2] = "FullyBuffered-DDR2", - [MEM_RDDR2] = "Registered-DDR2", - [MEM_XDR] = "XDR", - [MEM_DDR3] = "Unbuffered-DDR3", - [MEM_RDDR3] = "Registered-DDR3", - [MEM_DDR4] = "Unbuffered-DDR4", - [MEM_RDDR4] = "Registered-DDR4" -}; - static const char * const dev_types[] = { [DEV_UNKNOWN] = "Unknown", [DEV_X1] = "x1", @@ -196,7 +174,7 @@ static ssize_t csrow_mem_type_show(struct device *dev, { struct csrow_info *csrow = to_csrow(dev); - return sprintf(data, "%s\n", mem_types[csrow->channels[0]->dimm->mtype]); + return sprintf(data, "%s\n", edac_mem_types[csrow->channels[0]->dimm->mtype]); } static ssize_t csrow_dev_type_show(struct device *dev, @@ -549,7 +527,7 @@ static ssize_t dimmdev_mem_type_show(struct device *dev, { struct dimm_info *dimm = to_dimm(dev); - return sprintf(data, "%s\n", mem_types[dimm->mtype]); + return sprintf(data, "%s\n", edac_mem_types[dimm->mtype]); } static ssize_t dimmdev_dev_type_show(struct device *dev, -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-12 18:24 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck @ 2018-03-12 18:24 ` Tony Luck 2018-03-12 18:24 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck ` (2 subsequent siblings) 4 siblings, 0 replies; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown There are now non-volatile versions of DIMMs. Add a new entry to "enum mem_type" and a new string in edac_mem_types[]. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/edac_mc.c | 3 ++- include/linux/edac.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 1f61b736e075..3bb82e511eca 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -214,7 +214,8 @@ const char * const edac_mem_types[] = { [MEM_RDDR3] = "Registered-DDR3", [MEM_LRDDR3] = "Load-Reduced-DDR3-RAM", [MEM_DDR4] = "Unbuffered-DDR4", - [MEM_RDDR4] = "Registered-DDR4" + [MEM_RDDR4] = "Registered-DDR4", + [MEM_NVDIMM] = "Non-volatile-RAM", }; EXPORT_SYMBOL_GPL(edac_mem_types); diff --git a/include/linux/edac.h b/include/linux/edac.h index cd75c173fd00..bffb97828ed6 100644 --- a/include/linux/edac.h +++ b/include/linux/edac.h @@ -186,6 +186,7 @@ static inline char *mc_event_error_type(const unsigned int err_type) * @MEM_RDDR4: Registered DDR4 RAM * This is a variant of the DDR4 memories. * @MEM_LRDDR4: Load-Reduced DDR4 memory. + * @MEM_NVDIMM: Non-volatile RAM */ enum mem_type { MEM_EMPTY = 0, @@ -209,6 +210,7 @@ enum mem_type { MEM_DDR4, MEM_RDDR4, MEM_LRDDR4, + MEM_NVDIMM, }; #define MEM_FLAG_EMPTY BIT(MEM_EMPTY) @@ -231,6 +233,7 @@ enum mem_type { #define MEM_FLAG_DDR4 BIT(MEM_DDR4) #define MEM_FLAG_RDDR4 BIT(MEM_RDDR4) #define MEM_FLAG_LRDDR4 BIT(MEM_LRDDR4) +#define MEM_FLAG_NVDIMM BIT(MEM_NVDIMM) /** * enum edac-type - Error Detection and Correction capabilities and mode -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-12 18:24 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck 2018-03-12 18:24 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck @ 2018-03-12 18:24 ` Tony Luck 2018-03-12 18:24 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck 2018-03-12 18:24 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck 4 siblings, 0 replies; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown EDAC driver needs to look up attributes of NVDIMMs provided in SMBIOS. Provide a function that looks up an acpi_nfit_memory_map from a device handle (node/socket/mc/channel/dimm) and returns the SMBIOS handle. Also pass back the "flags" so we can see if the NVDIMM is OK. Acked-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/acpi/nfit/core.c | 27 +++++++++++++++++++++++++++ include/acpi/nfit.h | 18 ++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 include/acpi/nfit.h diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index bbe48ad20886..4d6eeb1793e6 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -23,6 +23,7 @@ #include <linux/io.h> #include <linux/nd.h> #include <asm/cacheflush.h> +#include <acpi/nfit.h> #include "nfit.h" /* @@ -690,6 +691,32 @@ static bool add_memdev(struct acpi_nfit_desc *acpi_desc, return true; } +int nfit_get_smbios_id(u32 device_handle, u16 *flags) +{ + struct acpi_nfit_memory_map *memdev; + struct acpi_nfit_desc *acpi_desc; + struct nfit_mem *nfit_mem; + + mutex_lock(&acpi_desc_lock); + list_for_each_entry(acpi_desc, &acpi_descs, list) { + mutex_lock(&acpi_desc->init_mutex); + list_for_each_entry(nfit_mem, &acpi_desc->dimms, list) { + memdev = __to_nfit_memdev(nfit_mem); + if (memdev->device_handle == device_handle) { + mutex_unlock(&acpi_desc->init_mutex); + mutex_unlock(&acpi_desc_lock); + *flags = memdev->flags; + return memdev->physical_id; + } + } + mutex_unlock(&acpi_desc->init_mutex); + } + mutex_unlock(&acpi_desc_lock); + + return -ENODEV; +} +EXPORT_SYMBOL_GPL(nfit_get_smbios_id); + /* * An implementation may provide a truncated control region if no block windows * are defined. diff --git a/include/acpi/nfit.h b/include/acpi/nfit.h new file mode 100644 index 000000000000..86ed07c1200d --- /dev/null +++ b/include/acpi/nfit.h @@ -0,0 +1,18 @@ +/* + * SPDX-License-Identifier: GPL-2.0 + * Copyright (C) 2018 Intel Corporation + */ + +#ifndef __ACPI_NFIT_H +#define __ACPI_NFIT_H + +#if IS_ENABLED(CONFIG_ACPI_NFIT) +int nfit_get_smbios_id(u32 device_handle, u16 *flags); +#else +static inline int nfit_get_smbios_id(u32 device_handle, u16 *flags) +{ + return -EOPNOTSUPP; +} +#endif + +#endif /* __ACPI_NFIT_H */ -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (2 preceding siblings ...) 2018-03-12 18:24 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck @ 2018-03-12 18:24 ` Tony Luck [not found] ` <20180312182430.10335-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-12 18:24 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck 4 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown When we first scan the SMBIOS table, save the size of the DIMM. Provide a function for other code (EDAC driver) to look up the size of a DIMM from its SMBIOS handle. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/firmware/dmi_scan.c | 31 +++++++++++++++++++++++++++++++ include/linux/dmi.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index e763e1484331..35c6c74c9304 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -32,6 +32,7 @@ static char dmi_ids_string[128] __initdata; static struct dmi_memdev_info { const char *device; const char *bank; + u64 size; /* bytes */ u16 handle; } *dmi_memdev; static int dmi_memdev_nr; @@ -386,6 +387,8 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) { const char *d = (const char *)dm; static int nr; + u64 bytes; + u16 size; if (dm->type != DMI_ENTRY_MEM_DEVICE || dm->length < 0x12) return; @@ -396,6 +399,20 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) dmi_memdev[nr].handle = get_unaligned(&dm->handle); dmi_memdev[nr].device = dmi_string(dm, d[0x10]); dmi_memdev[nr].bank = dmi_string(dm, d[0x11]); + + size = get_unaligned((u16 *)&d[0xC]); + if (size == 0) + bytes = 0; + else if (size == 0xffff) + bytes = ~0ull; + else if (size & 0x8000) + bytes = (u64)(size & 0x7fff) << 10; + else if (size != 0x7fff) + bytes = (u64)size << 20; + else + bytes = (u64)get_unaligned((u32 *)&d[0x1C]) << 20; + + dmi_memdev[nr].size = bytes; nr++; } @@ -1065,3 +1082,17 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device) } } EXPORT_SYMBOL_GPL(dmi_memdev_name); + +u64 dmi_memdev_size(u16 handle) +{ + int n; + + if (dmi_memdev) { + for (n = 0; n < dmi_memdev_nr; n++) { + if (handle == dmi_memdev[n].handle) + return dmi_memdev[n].size; + } + } + return ~0ull; +} +EXPORT_SYMBOL_GPL(dmi_memdev_size); diff --git a/include/linux/dmi.h b/include/linux/dmi.h index 46e151172d95..7f5929123b69 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -113,6 +113,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *), void *private_data); extern bool dmi_match(enum dmi_field f, const char *str); extern void dmi_memdev_name(u16 handle, const char **bank, const char **device); +extern u64 dmi_memdev_size(u16 handle); #else @@ -142,6 +143,7 @@ static inline bool dmi_match(enum dmi_field f, const char *str) { return false; } static inline void dmi_memdev_name(u16 handle, const char **bank, const char **device) { } +static inline u64 dmi_memdev_size(u16 handle) { return ~0ul; } static inline const struct dmi_system_id * dmi_first_match(const struct dmi_system_id *list) { return NULL; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <20180312182430.10335-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size [not found] ` <20180312182430.10335-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-03-13 9:43 ` Jean Delvare 0 siblings, 0 replies; 26+ messages in thread From: Jean Delvare @ 2018-03-13 9:43 UTC (permalink / raw) To: Tony Luck Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown On Mon, 12 Mar 2018 11:24:29 -0700, Tony Luck wrote: > When we first scan the SMBIOS table, save the size of the DIMM. > > Provide a function for other code (EDAC driver) to look up the size > of a DIMM from its SMBIOS handle. > > Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- > drivers/firmware/dmi_scan.c | 31 +++++++++++++++++++++++++++++++ > include/linux/dmi.h | 2 ++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c > index e763e1484331..35c6c74c9304 100644 > --- a/drivers/firmware/dmi_scan.c > +++ b/drivers/firmware/dmi_scan.c > @@ -32,6 +32,7 @@ static char dmi_ids_string[128] __initdata; > static struct dmi_memdev_info { > const char *device; > const char *bank; > + u64 size; /* bytes */ > u16 handle; > } *dmi_memdev; > static int dmi_memdev_nr; > @@ -386,6 +387,8 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) > { > const char *d = (const char *)dm; > static int nr; > + u64 bytes; > + u16 size; > > if (dm->type != DMI_ENTRY_MEM_DEVICE || dm->length < 0x12) > return; > @@ -396,6 +399,20 @@ static void __init save_mem_devices(const struct dmi_header *dm, void *v) > dmi_memdev[nr].handle = get_unaligned(&dm->handle); > dmi_memdev[nr].device = dmi_string(dm, d[0x10]); > dmi_memdev[nr].bank = dmi_string(dm, d[0x11]); > + > + size = get_unaligned((u16 *)&d[0xC]); > + if (size == 0) > + bytes = 0; > + else if (size == 0xffff) > + bytes = ~0ull; > + else if (size & 0x8000) > + bytes = (u64)(size & 0x7fff) << 10; > + else if (size != 0x7fff) > + bytes = (u64)size << 20; > + else > + bytes = (u64)get_unaligned((u32 *)&d[0x1C]) << 20; > + > + dmi_memdev[nr].size = bytes; > nr++; > } > > @@ -1065,3 +1082,17 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device) > } > } > EXPORT_SYMBOL_GPL(dmi_memdev_name); > + > +u64 dmi_memdev_size(u16 handle) > +{ > + int n; > + > + if (dmi_memdev) { > + for (n = 0; n < dmi_memdev_nr; n++) { > + if (handle == dmi_memdev[n].handle) > + return dmi_memdev[n].size; > + } > + } > + return ~0ull; > +} > +EXPORT_SYMBOL_GPL(dmi_memdev_size); > diff --git a/include/linux/dmi.h b/include/linux/dmi.h > index 46e151172d95..7f5929123b69 100644 > --- a/include/linux/dmi.h > +++ b/include/linux/dmi.h > @@ -113,6 +113,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *), > void *private_data); > extern bool dmi_match(enum dmi_field f, const char *str); > extern void dmi_memdev_name(u16 handle, const char **bank, const char **device); > +extern u64 dmi_memdev_size(u16 handle); > > #else > > @@ -142,6 +143,7 @@ static inline bool dmi_match(enum dmi_field f, const char *str) > { return false; } > static inline void dmi_memdev_name(u16 handle, const char **bank, > const char **device) { } > +static inline u64 dmi_memdev_size(u16 handle) { return ~0ul; } > static inline const struct dmi_system_id * > dmi_first_match(const struct dmi_system_id *list) { return NULL; } > Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (3 preceding siblings ...) 2018-03-12 18:24 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck @ 2018-03-12 18:24 ` Tony Luck [not found] ` <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 4 siblings, 1 reply; 26+ messages in thread From: Tony Luck @ 2018-03-12 18:24 UTC (permalink / raw) To: Borislav Petkov Cc: Jean Delvare, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, Qiuxu Zhuo, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Tony Luck, Borislav Petkov, Aristeu Rozanski, Mauro Carvalho Chehab, Len Brown This just covers the topology function of the EDAC driver. We locate which DIMM slots are populated with NVDIMMs and query the NFIT and SMBIOS tables to get the size. Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- drivers/edac/Kconfig | 5 +++- drivers/edac/skx_edac.c | 66 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index 3c4017007647..c12e34564557 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -232,9 +232,12 @@ config EDAC_SBRIDGE config EDAC_SKX tristate "Intel Skylake server Integrated MC" depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG + select DMI help Support for error detection and correction the Intel - Skylake server Integrated Memory Controllers. + Skylake server Integrated Memory Controllers. If your + system has non-volatile DIMMs you should also manually + select CONFIG_ACPI_NFIT config EDAC_PND2 tristate "Intel Pondicherry2" diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c index 912c4930c9ef..84c18bb1e0cd 100644 --- a/drivers/edac/skx_edac.c +++ b/drivers/edac/skx_edac.c @@ -14,6 +14,8 @@ #include <linux/module.h> #include <linux/init.h> +#include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/pci.h> #include <linux/pci_ids.h> #include <linux/slab.h> @@ -24,6 +26,7 @@ #include <linux/bitmap.h> #include <linux/math64.h> #include <linux/mod_devicetable.h> +#include <acpi/nfit.h> #include <asm/cpu_device_id.h> #include <asm/intel-family.h> #include <asm/processor.h> @@ -302,6 +305,7 @@ static int get_dimm_attr(u32 reg, int lobit, int hibit, int add, int minval, } #define IS_DIMM_PRESENT(mtr) GET_BITFIELD((mtr), 15, 15) +#define IS_NVDIMM_PRESENT(mcddrtcfg, i) GET_BITFIELD((mcddrtcfg), (i), (i)) #define numrank(reg) get_dimm_attr((reg), 12, 13, 0, 0, 2, "ranks") #define numrow(reg) get_dimm_attr((reg), 2, 4, 12, 1, 6, "rows") @@ -350,8 +354,6 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, int banks = 16, ranks, rows, cols, npages; u64 size; - if (!IS_DIMM_PRESENT(mtr)) - return 0; ranks = numrank(mtr); rows = numrow(mtr); cols = numcol(mtr); @@ -383,6 +385,53 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, return 1; } +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, + int chan, int dimmno) +{ + int smbios_handle; + u32 dev_handle; + u16 flags; + u64 size = 0; + + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc, + imc->src_id, 0); + + smbios_handle = nfit_get_smbios_id(dev_handle, &flags); + if (smbios_handle == -EOPNOTSUPP) { + pr_warn_once("skx_edac: can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n"); + goto unknown_size; + } + if (smbios_handle < 0) { + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle); + goto unknown_size; + } + + if (flags & ACPI_NFIT_MEM_MAP_FAILED) { + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle); + goto unknown_size; + } + + size = dmi_memdev_size(smbios_handle); + if (size == ~0ull) + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n", + dev_handle, smbios_handle); + +unknown_size: + dimm->nr_pages = size >> PAGE_SHIFT; + dimm->grain = 32; + dimm->dtype = DEV_UNKNOWN; + dimm->mtype = MEM_NVDIMM; + dimm->edac_mode = EDAC_SECDED; /* likely better than this */ + + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n", + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages); + + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", + imc->src_id, imc->lmc, chan, dimmno); + + return 1; +} + #define SKX_GET_MTMTR(dev, reg) \ pci_read_config_dword((dev), 0x87c, ®) @@ -399,20 +448,24 @@ static int skx_get_dimm_config(struct mem_ctl_info *mci) { struct skx_pvt *pvt = mci->pvt_info; struct skx_imc *imc = pvt->imc; + u32 mtr, amap, mcddrtcfg; struct dimm_info *dimm; int i, j; - u32 mtr, amap; int ndimms; for (i = 0; i < NUM_CHANNELS; i++) { ndimms = 0; pci_read_config_dword(imc->chan[i].cdev, 0x8C, &amap); + pci_read_config_dword(imc->chan[i].cdev, 0x400, &mcddrtcfg); for (j = 0; j < NUM_DIMMS; j++) { dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, mci->n_layers, i, j, 0); pci_read_config_dword(imc->chan[i].cdev, 0x80 + 4*j, &mtr); - ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + if (IS_DIMM_PRESENT(mtr)) + ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + else if (IS_NVDIMM_PRESENT(mcddrtcfg, j)) + ndimms += get_nvdimm_info(dimm, imc, i, j); } if (ndimms && !skx_check_ecc(imc->chan[0].cdev)) { skx_printk(KERN_ERR, "ECC is disabled on imc %d\n", imc->mc); @@ -468,13 +521,14 @@ static int skx_register_mci(struct skx_imc *imc) pvt = mci->pvt_info; pvt->imc = imc; - mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", imc->node_id, imc->lmc); + mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", + imc->node_id, imc->lmc); if (!mci->ctl_name) { rc = -ENOMEM; goto fail0; } - mci->mtype_cap = MEM_FLAG_DDR4; + mci->mtype_cap = MEM_FLAG_DDR4 | MEM_FLAG_NVDIMM; mci->edac_ctl_cap = EDAC_FLAG_NONE; mci->edac_cap = EDAC_FLAG_NONE; mci->mod_name = EDAC_MOD_STR; -- 2.14.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs [not found] ` <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2018-03-13 9:59 ` Jean Delvare 2018-03-13 15:59 ` Luck, Tony 0 siblings, 1 reply; 26+ messages in thread From: Jean Delvare @ 2018-03-13 9:59 UTC (permalink / raw) To: Tony Luck Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown Hi Tony, On Mon, 12 Mar 2018 11:24:30 -0700, Tony Luck wrote: > This just covers the topology function of the EDAC driver. > We locate which DIMM slots are populated with NVDIMMs and > query the NFIT and SMBIOS tables to get the size. > > Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- > drivers/edac/Kconfig | 5 +++- > drivers/edac/skx_edac.c | 66 ++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 64 insertions(+), 7 deletions(-) > > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig > index 3c4017007647..c12e34564557 100644 > --- a/drivers/edac/Kconfig > +++ b/drivers/edac/Kconfig > @@ -232,9 +232,12 @@ config EDAC_SBRIDGE > config EDAC_SKX > tristate "Intel Skylake server Integrated MC" > depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG > + select DMI > help > Support for error detection and correction the Intel > - Skylake server Integrated Memory Controllers. > + Skylake server Integrated Memory Controllers. If your > + system has non-volatile DIMMs you should also manually > + select CONFIG_ACPI_NFIT Nitpicking: trailing dot is missing. > > config EDAC_PND2 > tristate "Intel Pondicherry2" > diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c > index 912c4930c9ef..84c18bb1e0cd 100644 > --- a/drivers/edac/skx_edac.c > +++ b/drivers/edac/skx_edac.c > (...) > +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, > + int chan, int dimmno) > +{ > + int smbios_handle; > + u32 dev_handle; > + u16 flags; > + u64 size = 0; > + > + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc, > + imc->src_id, 0); > + > + smbios_handle = nfit_get_smbios_id(dev_handle, &flags); > + if (smbios_handle == -EOPNOTSUPP) { > + pr_warn_once("skx_edac: can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n"); > + goto unknown_size; > + } > + if (smbios_handle < 0) { > + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle); > + goto unknown_size; > + } > + > + if (flags & ACPI_NFIT_MEM_MAP_FAILED) { > + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle); > + goto unknown_size; > + } > + > + size = dmi_memdev_size(smbios_handle); > + if (size == ~0ull) > + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n", > + dev_handle, smbios_handle); > + > +unknown_size: > + dimm->nr_pages = size >> PAGE_SHIFT; > + dimm->grain = 32; > + dimm->dtype = DEV_UNKNOWN; > + dimm->mtype = MEM_NVDIMM; > + dimm->edac_mode = EDAC_SECDED; /* likely better than this */ > + > + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n", I did not notice on previous review, but I think "b" in general means bit not byte, so "MB" would be better. > + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages); > + > + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", > + imc->src_id, imc->lmc, chan, dimmno); > + > + return 1; > +} Now this function always return 1, that doesn't make a lot of sense? Other than these details, the dmi-related code looks good to me now. Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> # for DMI -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs 2018-03-13 9:59 ` Jean Delvare @ 2018-03-13 15:59 ` Luck, Tony 2018-03-13 16:16 ` Jean Delvare 2018-03-14 12:00 ` Borislav Petkov 0 siblings, 2 replies; 26+ messages in thread From: Luck, Tony @ 2018-03-13 15:59 UTC (permalink / raw) To: Jean Delvare Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown On Tue, Mar 13, 2018 at 10:59:01AM +0100, Jean Delvare wrote: > > + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n", > > I did not notice on previous review, but I think "b" in general means > bit not byte, so "MB" would be better. Heh! We've been cut & pasting that line from EDAC drivers since the i7core_edac.c I think the origin is in 2.6.35. So you are far from the only person to not notice it in a review :-) > > > + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages); > > + > > + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", > > + imc->src_id, imc->lmc, chan, dimmno); > > + > > + return 1; > > +} > > Now this function always return 1, that doesn't make a lot of sense? I could fix the return to be: diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c index 84c18bb1e0cd..1a66e145c0bd 100644 --- a/drivers/edac/skx_edac.c +++ b/drivers/edac/skx_edac.c @@ -429,7 +429,7 @@ static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", imc->src_id, imc->lmc, chan, dimmno); - return 1; + return (size == 0 || size == ~0ull) ? 0 : 1; } #define SKX_GET_MTMTR(dev, reg) \ > Other than these details, the dmi-related code looks good to me now. > > Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> # for DMI Thanks. Boris ... ready to go now? Or do you have some other comments? -Tony ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs 2018-03-13 15:59 ` Luck, Tony @ 2018-03-13 16:16 ` Jean Delvare 2018-03-14 12:00 ` Borislav Petkov 1 sibling, 0 replies; 26+ messages in thread From: Jean Delvare @ 2018-03-13 16:16 UTC (permalink / raw) To: Luck, Tony Cc: Mauro Carvalho Chehab, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo, Borislav Petkov, Aristeu Rozanski, Borislav Petkov, Len Brown On Tue, 13 Mar 2018 08:59:56 -0700, Luck, Tony wrote: > On Tue, Mar 13, 2018 at 10:59:01AM +0100, Jean Delvare wrote: > > > + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n", > > > > I did not notice on previous review, but I think "b" in general means > > bit not byte, so "MB" would be better. > > Heh! We've been cut & pasting that line from EDAC drivers since the i7core_edac.c > I think the origin is in 2.6.35. So you are far from the only person to not > notice it in a review :-) Hehe :-D Then I guess a patch fixing all drivers are once is in order, after your patch set it accepted. > > > + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages); > > > + > > > + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", > > > + imc->src_id, imc->lmc, chan, dimmno); > > > + > > > + return 1; > > > +} > > > > Now this function always return 1, that doesn't make a lot of sense? > > I could fix the return to be: > > > diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c > index 84c18bb1e0cd..1a66e145c0bd 100644 > --- a/drivers/edac/skx_edac.c > +++ b/drivers/edac/skx_edac.c > @@ -429,7 +429,7 @@ static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, > snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", > imc->src_id, imc->lmc, chan, dimmno); > > - return 1; > + return (size == 0 || size == ~0ull) ? 0 : 1; > } Yes, I think it makes sense, thanks. -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs 2018-03-13 15:59 ` Luck, Tony 2018-03-13 16:16 ` Jean Delvare @ 2018-03-14 12:00 ` Borislav Petkov 1 sibling, 0 replies; 26+ messages in thread From: Borislav Petkov @ 2018-03-14 12:00 UTC (permalink / raw) To: Luck, Tony Cc: Qiuxu Zhuo, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Rafael J. Wysocki, linux-acpi-u79uwXL29TY76Z2rM5mHXA, Aristeu Rozanski, Mauro Carvalho Chehab, Jean Delvare, Len Brown On Tue, Mar 13, 2018 at 08:59:56AM -0700, Luck, Tony wrote: > Thanks. Boris ... ready to go now? Or do you have some other comments? I've committed this. Running build smoke tests now. Thx. --- From: Tony Luck <tony.luck@intel.com> Date: Mon, 12 Mar 2018 11:24:30 -0700 Subject: [PATCH] EDAC, skx_edac: Detect non-volatile DIMMs This just covers the topology function of the EDAC driver. We locate which DIMM slots are populated with NVDIMMs and query the NFIT and SMBIOS tables to get the size. Reviewed-by: Jean Delvare <jdelvare@suse.de> Cc: Aristeu Rozanski <aris@redhat.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Len Brown <lenb@kernel.org> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> Cc: Qiuxu Zhuo <qiuxu.zhuo@intel.com> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: linux-acpi@vger.kernel.org Cc: linux-edac <linux-edac@vger.kernel.org> Cc: linux-nvdimm@lists.01.org Link: http://lkml.kernel.org/r/20180312182430.10335-6-tony.luck@intel.com Signed-off-by: Borislav Petkov <bp@suse.de> --- drivers/edac/Kconfig | 5 +++- drivers/edac/skx_edac.c | 67 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index cb4ff1cc6eb4..07d569d32b90 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -232,9 +232,12 @@ config EDAC_SBRIDGE config EDAC_SKX tristate "Intel Skylake server Integrated MC" depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG + select DMI help Support for error detection and correction the Intel - Skylake server Integrated Memory Controllers. + Skylake server Integrated Memory Controllers. If your + system has non-volatile DIMMs you should also manually + select CONFIG_ACPI_NFIT. config EDAC_PND2 tristate "Intel Pondicherry2" diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c index 912c4930c9ef..fae095162c01 100644 --- a/drivers/edac/skx_edac.c +++ b/drivers/edac/skx_edac.c @@ -14,6 +14,8 @@ #include <linux/module.h> #include <linux/init.h> +#include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/pci.h> #include <linux/pci_ids.h> #include <linux/slab.h> @@ -24,6 +26,7 @@ #include <linux/bitmap.h> #include <linux/math64.h> #include <linux/mod_devicetable.h> +#include <acpi/nfit.h> #include <asm/cpu_device_id.h> #include <asm/intel-family.h> #include <asm/processor.h> @@ -302,6 +305,7 @@ static int get_dimm_attr(u32 reg, int lobit, int hibit, int add, int minval, } #define IS_DIMM_PRESENT(mtr) GET_BITFIELD((mtr), 15, 15) +#define IS_NVDIMM_PRESENT(mcddrtcfg, i) GET_BITFIELD((mcddrtcfg), (i), (i)) #define numrank(reg) get_dimm_attr((reg), 12, 13, 0, 0, 2, "ranks") #define numrow(reg) get_dimm_attr((reg), 2, 4, 12, 1, 6, "rows") @@ -350,8 +354,6 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, int banks = 16, ranks, rows, cols, npages; u64 size; - if (!IS_DIMM_PRESENT(mtr)) - return 0; ranks = numrank(mtr); rows = numrow(mtr); cols = numcol(mtr); @@ -383,6 +385,54 @@ static int get_dimm_info(u32 mtr, u32 amap, struct dimm_info *dimm, return 1; } +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc, + int chan, int dimmno) +{ + int smbios_handle; + u32 dev_handle; + u16 flags; + u64 size = 0; + + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc, + imc->src_id, 0); + + smbios_handle = nfit_get_smbios_id(dev_handle, &flags); + if (smbios_handle == -EOPNOTSUPP) { + pr_warn_once(EDAC_MOD_STR ": Can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n"); + goto unknown_size; + } + + if (smbios_handle < 0) { + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle); + goto unknown_size; + } + + if (flags & ACPI_NFIT_MEM_MAP_FAILED) { + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle); + goto unknown_size; + } + + size = dmi_memdev_size(smbios_handle); + if (size == ~0ull) + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n", + dev_handle, smbios_handle); + +unknown_size: + dimm->nr_pages = size >> PAGE_SHIFT; + dimm->grain = 32; + dimm->dtype = DEV_UNKNOWN; + dimm->mtype = MEM_NVDIMM; + dimm->edac_mode = EDAC_SECDED; /* likely better than this */ + + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n", + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages); + + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u", + imc->src_id, imc->lmc, chan, dimmno); + + return (size == 0 || size == ~0ull) ? 0 : 1; +} + #define SKX_GET_MTMTR(dev, reg) \ pci_read_config_dword((dev), 0x87c, ®) @@ -399,20 +449,24 @@ static int skx_get_dimm_config(struct mem_ctl_info *mci) { struct skx_pvt *pvt = mci->pvt_info; struct skx_imc *imc = pvt->imc; + u32 mtr, amap, mcddrtcfg; struct dimm_info *dimm; int i, j; - u32 mtr, amap; int ndimms; for (i = 0; i < NUM_CHANNELS; i++) { ndimms = 0; pci_read_config_dword(imc->chan[i].cdev, 0x8C, &amap); + pci_read_config_dword(imc->chan[i].cdev, 0x400, &mcddrtcfg); for (j = 0; j < NUM_DIMMS; j++) { dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, mci->n_layers, i, j, 0); pci_read_config_dword(imc->chan[i].cdev, 0x80 + 4*j, &mtr); - ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + if (IS_DIMM_PRESENT(mtr)) + ndimms += get_dimm_info(mtr, amap, dimm, imc, i, j); + else if (IS_NVDIMM_PRESENT(mcddrtcfg, j)) + ndimms += get_nvdimm_info(dimm, imc, i, j); } if (ndimms && !skx_check_ecc(imc->chan[0].cdev)) { skx_printk(KERN_ERR, "ECC is disabled on imc %d\n", imc->mc); @@ -468,13 +522,14 @@ static int skx_register_mci(struct skx_imc *imc) pvt = mci->pvt_info; pvt->imc = imc; - mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", imc->node_id, imc->lmc); + mci->ctl_name = kasprintf(GFP_KERNEL, "Skylake Socket#%d IMC#%d", + imc->node_id, imc->lmc); if (!mci->ctl_name) { rc = -ENOMEM; goto fail0; } - mci->mtype_cap = MEM_FLAG_DDR4; + mci->mtype_cap = MEM_FLAG_DDR4 | MEM_FLAG_NVDIMM; mci->edac_ctl_cap = EDAC_FLAG_NONE; mci->edac_cap = EDAC_FLAG_NONE; mci->mod_name = EDAC_MOD_STR; -- 2.13.0 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-03-14 12:00 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-22 19:58 [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Tony Luck [not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-22 19:58 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck 2018-02-22 19:58 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck 2018-02-22 19:58 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck [not found] ` <20180222195811.27237-4-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-02-28 17:36 ` Ross Zwisler [not found] ` <20180228173621.GA12883-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2018-02-28 18:48 ` Borislav Petkov 2018-02-22 19:58 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck [not found] ` <20180222195811.27237-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-09 10:20 ` Jean Delvare 2018-03-09 23:03 ` Luck, Tony 2018-03-10 13:22 ` Jean Delvare 2018-03-12 16:46 ` Luck, Tony 2018-02-22 19:58 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck [not found] ` <20180222195811.27237-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-09 10:38 ` Jean Delvare 2018-02-28 13:04 ` [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Borislav Petkov [not found] ` <20180228130410.GC3769-fF5Pk5pvG8Y@public.gmane.org> 2018-02-28 13:59 ` Dan Williams [not found] ` <CAPcyv4iHr9nUAS0SiFeqqDkU8NXZ_x=rkD3YS010Dy-nbHJXmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-03-12 18:24 ` [PATCH 0/5 V3] " Tony Luck [not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-12 18:24 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck 2018-03-12 18:24 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck 2018-03-12 18:24 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck 2018-03-12 18:24 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck [not found] ` <20180312182430.10335-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-13 9:43 ` Jean Delvare 2018-03-12 18:24 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck [not found] ` <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-03-13 9:59 ` Jean Delvare 2018-03-13 15:59 ` Luck, Tony 2018-03-13 16:16 ` Jean Delvare 2018-03-14 12:00 ` Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).