From 6d8e294bf5f0e85c34e8b14b064e2965f53f38b0 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 12:53:05 +0200 Subject: RAS/CEC: Fix pfn insertion When inserting random PFNs for debugging the CEC through (debugfs)/ras/cec/pfn, depending on the return value of pfn_set(), multiple values get inserted per a single write. That is because simple_attr_write() interprets a retval of 0 as success and claims the whole input. However, pfn_set() returns the cec_add_elem() value, which, if > 0 and smaller than the whole input length, makes glibc continue issuing the write syscall until there's input left: pfn_set simple_attr_write debugfs_attr_write full_proxy_write vfs_write ksys_write do_syscall_64 entry_SYSCALL_64_after_hwframe leading to those repeated calls. Return 0 to fix that. Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac --- drivers/ras/cec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index 673f8a128397..f5795adc5a6e 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -369,7 +369,9 @@ static int pfn_set(void *data, u64 val) { *(u64 *)data = val; - return cec_add_elem(val); + cec_add_elem(val); + + return 0; } DEFINE_DEBUGFS_ATTRIBUTE(pfn_ops, u64_get, pfn_set, "0x%llx\n"); -- cgit v1.2.3 From de0e0624d86ff9fc512dedb297f8978698abf21a Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 14:06:37 +0200 Subject: RAS/CEC: Check count_threshold unconditionally The count_threshold should be checked unconditionally, after insertion too, so that a count_threshold value of 1 can cause an immediate offlining. I.e., offline the page on the *first* error encountered. Add comments to make it clear what cec_add_elem() does, while at it. Reported-by: WANG Chao Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac@vger.kernel.org Link: https://lkml.kernel.org/r/20190418034115.75954-3-chao.wang@ucloud.cn --- drivers/ras/cec.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index f5795adc5a6e..73a975c26f9f 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -294,6 +294,7 @@ int cec_add_elem(u64 pfn) ca->ces_entered++; + /* Array full, free the LRU slot. */ if (ca->n == MAX_ELEMS) WARN_ON(!del_lru_elem_unlocked(ca)); @@ -306,24 +307,17 @@ int cec_add_elem(u64 pfn) (void *)&ca->array[to], (ca->n - to) * sizeof(u64)); - ca->array[to] = (pfn << PAGE_SHIFT) | - (DECAY_MASK << COUNT_BITS) | 1; - + ca->array[to] = pfn << PAGE_SHIFT; ca->n++; - - ret = 0; - - goto decay; } - count = COUNT(ca->array[to]); - - if (count < count_threshold) { - ca->array[to] |= (DECAY_MASK << COUNT_BITS); - ca->array[to]++; + /* Add/refresh element generation and increment count */ + ca->array[to] |= DECAY_MASK << COUNT_BITS; + ca->array[to]++; - ret = 0; - } else { + /* Check action threshold and soft-offline, if reached. */ + count = COUNT(ca->array[to]); + if (count >= count_threshold) { u64 pfn = ca->array[to] >> PAGE_SHIFT; if (!pfn_valid(pfn)) { @@ -338,15 +332,14 @@ int cec_add_elem(u64 pfn) del_elem(ca, to); /* - * Return a >0 value to denote that we've reached the offlining - * threshold. + * Return a >0 value to callers, to denote that we've reached + * the offlining threshold. */ ret = 1; goto unlock; } -decay: ca->decay_count++; if (ca->decay_count >= CLEAN_ELEMS) -- cgit v1.2.3 From 5cc6b16ea1313d05956b55e83a1f753c604282a8 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 21:33:08 +0200 Subject: RAS/CEC: Do not set decay value on error When the value requested doesn't match the allowed (min,max) range, the @data buffer should not be modified with the invalid value because reading "decay_interval" shows it otherwise as if the previous write succeeded. Move the data write after the check. Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac --- drivers/ras/cec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index 73a975c26f9f..31868bd99e8d 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -371,17 +371,17 @@ DEFINE_DEBUGFS_ATTRIBUTE(pfn_ops, u64_get, pfn_set, "0x%llx\n"); static int decay_interval_set(void *data, u64 val) { - *(u64 *)data = val; - if (val < CEC_DECAY_MIN_INTERVAL) return -EINVAL; if (val > CEC_DECAY_MAX_INTERVAL) return -EINVAL; + *(u64 *)data = val; decay_interval = val; cec_mod_work(decay_interval); + return 0; } DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, u64_get, decay_interval_set, "%lld\n"); -- cgit v1.2.3 From d0e375e8f26edd2e577e3afa9d952f91037cbd87 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 21:39:24 +0200 Subject: RAS/CEC: Fix potential memory leak Free the array page if a failure is encountered while creating the debugfs nodes. Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac --- drivers/ras/cec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index 31868bd99e8d..f57e869dfea2 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -504,8 +504,10 @@ void __init cec_init(void) return; } - if (create_debugfs_nodes()) + if (create_debugfs_nodes()) { + free_page((unsigned long)ce_arr.array); return; + } INIT_DELAYED_WORK(&cec_work, cec_work_fn); schedule_delayed_work(&cec_work, CEC_DECAY_DEFAULT_INTERVAL); -- cgit v1.2.3 From 9632a3299bb1897f01c6a485ff035b20e61d7ae1 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 21 Apr 2019 21:41:45 +0200 Subject: RAS/CEC: Sanity-check array on every insertion Check the elements order in the array after every insertion. Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac --- drivers/ras/cec.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index f57e869dfea2..da5797c38051 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -276,11 +276,39 @@ static u64 __maybe_unused del_lru_elem(void) return pfn; } +static bool sanity_check(struct ce_array *ca) +{ + bool ret = false; + u64 prev = 0; + int i; + + for (i = 0; i < ca->n; i++) { + u64 this = PFN(ca->array[i]); + + if (WARN(prev > this, "prev: 0x%016llx <-> this: 0x%016llx\n", prev, this)) + ret = true; + + prev = this; + } + + if (!ret) + return ret; + + pr_info("Sanity check dump:\n{ n: %d\n", ca->n); + for (i = 0; i < ca->n; i++) { + u64 this = PFN(ca->array[i]); + + pr_info(" %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i])); + } + pr_info("}\n"); + + return ret; +} int cec_add_elem(u64 pfn) { struct ce_array *ca = &ce_arr; - unsigned int to; + unsigned int to = 0; int count, ret = 0; /* @@ -345,6 +373,8 @@ int cec_add_elem(u64 pfn) if (ca->decay_count >= CLEAN_ELEMS) do_spring_cleaning(ca); + WARN_ON_ONCE(sanity_check(ca)); + unlock: mutex_unlock(&ce_mutex); @@ -402,7 +432,6 @@ DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, u64_get, count_threshold_set, "%ll static int array_dump(struct seq_file *m, void *v) { struct ce_array *ca = &ce_arr; - u64 prev = 0; int i; mutex_lock(&ce_mutex); @@ -412,10 +441,6 @@ static int array_dump(struct seq_file *m, void *v) u64 this = PFN(ca->array[i]); seq_printf(m, " %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i])); - - WARN_ON(prev > this); - - prev = this; } seq_printf(m, "}\n"); -- cgit v1.2.3 From b8b5ca6600dec2a4f1e50ca9d3cf9e1d032870cd Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 21:30:11 +0200 Subject: RAS/CEC: Rename count_threshold to action_threshold ... which is the better, more-fitting name anyway. Tony: - make action_threshold u64 due to debugfs accessors expecting u64. - rename the remaining: s/count_threshold/action_threshold/g Co-developed-by: Tony Luck Signed-off-by: Tony Luck Signed-off-by: Borislav Petkov Cc: linux-edac --- drivers/ras/cec.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index da5797c38051..364f7e1a6bad 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -37,9 +37,9 @@ * thus emulate an an LRU-like behavior when deleting elements to free up space * in the page. * - * When an element reaches it's max count of count_threshold, we try to poison - * it by assuming that errors triggered count_threshold times in a single page - * are excessive and that page shouldn't be used anymore. count_threshold is + * When an element reaches it's max count of action_threshold, we try to poison + * it by assuming that errors triggered action_threshold times in a single page + * are excessive and that page shouldn't be used anymore. action_threshold is * initialized to COUNT_MASK which is the maximum. * * That error event entry causes cec_add_elem() to return !0 value and thus @@ -122,7 +122,7 @@ static DEFINE_MUTEX(ce_mutex); static u64 dfs_pfn; /* Amount of errors after which we offline */ -static unsigned int count_threshold = COUNT_MASK; +static u64 action_threshold = COUNT_MASK; /* Each element "decays" each decay_interval which is 24hrs by default. */ #define CEC_DECAY_DEFAULT_INTERVAL 24 * 60 * 60 /* 24 hrs */ @@ -345,7 +345,7 @@ int cec_add_elem(u64 pfn) /* Check action threshold and soft-offline, if reached. */ count = COUNT(ca->array[to]); - if (count >= count_threshold) { + if (count >= action_threshold) { u64 pfn = ca->array[to] >> PAGE_SHIFT; if (!pfn_valid(pfn)) { @@ -416,18 +416,18 @@ static int decay_interval_set(void *data, u64 val) } DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, u64_get, decay_interval_set, "%lld\n"); -static int count_threshold_set(void *data, u64 val) +static int action_threshold_set(void *data, u64 val) { *(u64 *)data = val; if (val > COUNT_MASK) val = COUNT_MASK; - count_threshold = val; + action_threshold = val; return 0; } -DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, u64_get, count_threshold_set, "%lld\n"); +DEFINE_DEBUGFS_ATTRIBUTE(action_threshold_ops, u64_get, action_threshold_set, "%lld\n"); static int array_dump(struct seq_file *m, void *v) { @@ -453,7 +453,7 @@ static int array_dump(struct seq_file *m, void *v) seq_printf(m, "Decay interval: %lld seconds\n", decay_interval); seq_printf(m, "Decays: %lld\n", ca->decays_done); - seq_printf(m, "Action threshold: %d\n", count_threshold); + seq_printf(m, "Action threshold: %lld\n", action_threshold); mutex_unlock(&ce_mutex); @@ -502,10 +502,10 @@ static int __init create_debugfs_nodes(void) goto err; } - count = debugfs_create_file("count_threshold", S_IRUSR | S_IWUSR, d, - &count_threshold, &count_threshold_ops); + count = debugfs_create_file("action_threshold", S_IRUSR | S_IWUSR, d, + &action_threshold, &action_threshold_ops); if (!count) { - pr_warn("Error creating count_threshold debugfs node!\n"); + pr_warn("Error creating action_threshold debugfs node!\n"); goto err; } -- cgit v1.2.3 From f57518cd56e2919afbcef3839122a75e291c7f85 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 23:01:03 +0200 Subject: RAS/CEC: Dump the different array element sections When dumping the array elements, print them in the following format: [ PFN | generation in binary | count ] to be perfectly clear what all those sections are. Signed-off-by: Borislav Petkov Cc: Tony Luck Cc: linux-edac --- drivers/ras/cec.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index 364f7e1a6bad..dc08c705b493 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -429,6 +429,8 @@ static int action_threshold_set(void *data, u64 val) } DEFINE_DEBUGFS_ATTRIBUTE(action_threshold_ops, u64_get, action_threshold_set, "%lld\n"); +static const char * const bins[] = { "00", "01", "10", "11" }; + static int array_dump(struct seq_file *m, void *v) { struct ce_array *ca = &ce_arr; @@ -440,7 +442,8 @@ static int array_dump(struct seq_file *m, void *v) for (i = 0; i < ca->n; i++) { u64 this = PFN(ca->array[i]); - seq_printf(m, " %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i])); + seq_printf(m, " %3d: [%016llx|%s|%03llx]\n", + i, this, bins[DECAY(ca->array[i])], COUNT(ca->array[i])); } seq_printf(m, "}\n"); -- cgit v1.2.3 From 60fd42d26cc7ec8847598da50ebf27e3c9647d7b Mon Sep 17 00:00:00 2001 From: Tony Luck Date: Mon, 6 May 2019 13:13:22 +0200 Subject: RAS/CEC: Add CONFIG_RAS_CEC_DEBUG and move CEC debug features there The pfn and array files in (debugfs)/ras/cec are intended for debugging the CEC code itself. They are not needed on production systems, so the default setting for this CONFIG option is "n". [ bp: Have it with less ifdeffery by using IS_ENABLED(). ] Signed-off-by: Tony Luck Signed-off-by: Borislav Petkov --- arch/x86/ras/Kconfig | 10 ++++++++++ drivers/ras/cec.c | 26 ++++++++++++++------------ 2 files changed, 24 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/arch/x86/ras/Kconfig b/arch/x86/ras/Kconfig index a9c3db125222..9ad6842de4b4 100644 --- a/arch/x86/ras/Kconfig +++ b/arch/x86/ras/Kconfig @@ -11,3 +11,13 @@ config RAS_CEC Bear in mind that this is absolutely useless if your platform doesn't have ECC DIMMs and doesn't have DRAM ECC checking enabled in the BIOS. + +config RAS_CEC_DEBUG + bool "CEC debugging machinery" + default n + depends on RAS_CEC + help + Add extra files to (debugfs)/ras/cec to test the correctable error + collector feature. "pfn" is a writable file that allows user to + simulate an error in a particular page frame. "array" is a read-only + file that dumps out the current state of all pages logged so far. diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index dc08c705b493..0907dc6f4afe 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -486,18 +486,6 @@ static int __init create_debugfs_nodes(void) return -1; } - pfn = debugfs_create_file("pfn", S_IRUSR | S_IWUSR, d, &dfs_pfn, &pfn_ops); - if (!pfn) { - pr_warn("Error creating pfn debugfs node!\n"); - goto err; - } - - array = debugfs_create_file("array", S_IRUSR, d, NULL, &array_ops); - if (!array) { - pr_warn("Error creating array debugfs node!\n"); - goto err; - } - decay = debugfs_create_file("decay_interval", S_IRUSR | S_IWUSR, d, &decay_interval, &decay_interval_ops); if (!decay) { @@ -512,6 +500,20 @@ static int __init create_debugfs_nodes(void) goto err; } + if (!IS_ENABLED(CONFIG_RAS_CEC_DEBUG)) + return 0; + + pfn = debugfs_create_file("pfn", S_IRUSR | S_IWUSR, d, &dfs_pfn, &pfn_ops); + if (!pfn) { + pr_warn("Error creating pfn debugfs node!\n"); + goto err; + } + + array = debugfs_create_file("array", S_IRUSR, d, NULL, &array_ops); + if (!array) { + pr_warn("Error creating array debugfs node!\n"); + goto err; + } return 0; -- cgit v1.2.3 From 09afc797f3629f722df6a90ca6eb944013133c7a Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sat, 20 Apr 2019 22:06:43 +0200 Subject: RAS/CEC: Add copyright Signed-off-by: Borislav Petkov --- drivers/ras/cec.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c index 0907dc6f4afe..5d545806d930 100644 --- a/drivers/ras/cec.c +++ b/drivers/ras/cec.c @@ -1,4 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2017-2019 Borislav Petkov, SUSE Labs. + */ #include #include #include -- cgit v1.2.3