From df803bf2942747457c7275439c0b4da4e3e287f1 Mon Sep 17 00:00:00 2001 From: Lionel Debroux Date: Sun, 21 May 2023 21:31:29 +0200 Subject: [PATCH] Fix memory error addresses displayed by badram reporting mode (#308). --- app/badram.c | 49 +++++++++++++++++++++++++------------------------ app/badram.h | 4 +++- app/error.c | 19 ++++++++----------- app/test.h | 28 +++++++++++++++++++++------- 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/app/badram.c b/app/badram.c index 0c5e552..ef9a3cd 100644 --- a/app/badram.c +++ b/app/badram.c @@ -28,6 +28,7 @@ #include "display.h" #include "badram.h" +#include "memsize.h" //------------------------------------------------------------------------------ // Constants @@ -38,9 +39,9 @@ // DEFAULT_MASK covers a uintptr_t, since that is the testing granularity. #ifdef __x86_64__ -#define DEFAULT_MASK (UINTPTR_MAX << 3) +#define DEFAULT_MASK (UINT64_MAX << 3) #else -#define DEFAULT_MASK (UINTPTR_MAX << 2) +#define DEFAULT_MASK (UINT64_MAX << 2) #endif //------------------------------------------------------------------------------ @@ -48,8 +49,8 @@ //------------------------------------------------------------------------------ typedef struct { - uintptr_t addr; - uintptr_t mask; + uint64_t addr; + uint64_t mask; } pattern_t; //------------------------------------------------------------------------------ @@ -70,7 +71,7 @@ static int num_patterns = 0; /* * Combine two addr/mask pairs to one addr/mask pair. */ -static void combine(uintptr_t addr1, uintptr_t mask1, uintptr_t addr2, uintptr_t mask2, uintptr_t *addr, uintptr_t *mask) +static void combine(uint64_t addr1, uint64_t mask1, uint64_t addr2, uint64_t mask2, uint64_t *addr, uint64_t *mask) { *mask = COMBINE_MASK(addr1, mask1, addr2, mask2); @@ -81,10 +82,10 @@ static void combine(uintptr_t addr1, uintptr_t mask1, uintptr_t addr2, uintptr_t /* * Count the number of addresses covered with a mask. */ -static uintptr_t addresses(uintptr_t mask) +static uint64_t addresses(uint64_t mask) { - uintptr_t ctr = 1; - int i = 8*sizeof(uintptr_t); + uint64_t ctr = 1; + int i = 8*sizeof(uint64_t); while (i-- > 0) { if (! (mask & 1)) { ctr += ctr; @@ -98,10 +99,10 @@ static uintptr_t addresses(uintptr_t mask) * Count how many more addresses would be covered by addr1/mask1 when combined * with addr2/mask2. */ -static uintptr_t combi_cost(uintptr_t addr1, uintptr_t mask1, uintptr_t addr2, uintptr_t mask2) +static uint64_t combi_cost(uint64_t addr1, uint64_t mask1, uint64_t addr2, uint64_t mask2) { - uintptr_t cost1 = addresses(mask1); - uintptr_t tmp, mask; + uint64_t cost1 = addresses(mask1); + uint64_t tmp, mask; combine(addr1, mask1, addr2, mask2, &tmp, &mask); return addresses(mask) - cost1; } @@ -130,13 +131,13 @@ static int cheapest_pair() // This is guaranteed to be overwritten with >= 0 as long as num_patterns > 1 int merge_idx = -1; - uintptr_t min_cost = UINTPTR_MAX; + uint64_t min_cost = UINT64_MAX; for (int i = 0; i < num_patterns - 1; i++) { - uintptr_t tmp_cost = combi_cost( - patterns[i].addr, - patterns[i].mask, - patterns[i+1].addr, - patterns[i+1].mask + uint64_t tmp_cost = combi_cost( + patterns[i].addr, + patterns[i].mask, + patterns[i+1].addr, + patterns[i+1].mask ); if (tmp_cost <= min_cost) { min_cost = tmp_cost; @@ -227,11 +228,11 @@ void badram_init(void) } } -bool badram_insert(uintptr_t addr) +bool badram_insert(testword_t page, testword_t offset) { pattern_t pattern = { - .addr = addr, - .mask = DEFAULT_MASK + .addr = ((uint64_t)page << PAGE_SHIFT) + offset, + .mask = DEFAULT_MASK }; // If covered by existing entry we return immediately @@ -277,14 +278,14 @@ void badram_display(void) display_scrolled_message(col, ","); col++; } - int text_width = 2 * (TESTWORD_DIGITS + 2) + 1; + int text_width = 2 * (16 + 2) + 1; if (col > (SCREEN_WIDTH - text_width)) { scroll(); col = 7; } - display_scrolled_message(col, "0x%0*x,0x%0*x", - TESTWORD_DIGITS, patterns[i].addr, - TESTWORD_DIGITS, patterns[i].mask); + display_scrolled_message(col, "0x%08x%08x,0x%08x%08x", + (uintptr_t)(patterns[i].addr >> 32), (uintptr_t)(patterns[i].addr & 0xFFFFFFFFU), + (uintptr_t)(patterns[i].mask >> 32), (uintptr_t)(patterns[i].mask & 0xFFFFFFFFU)); col += text_width; } } diff --git a/app/badram.h b/app/badram.h index db450cf..184a29e 100644 --- a/app/badram.h +++ b/app/badram.h @@ -13,6 +13,8 @@ #include #include +#include "test.h" + /** * Initialises the pattern array. */ @@ -22,7 +24,7 @@ void badram_init(void); * Inserts a single faulty address into the pattern array. Returns * true iff the array was changed. */ -bool badram_insert(uintptr_t addr); +bool badram_insert(testword_t page, testword_t offset); /** * Displays the pattern array in the scrollable display region in the diff --git a/app/error.c b/app/error.c index a47f313..c557731 100644 --- a/app/error.c +++ b/app/error.c @@ -77,15 +77,12 @@ uint64_t error_count = 0; // Private Functions //------------------------------------------------------------------------------ -static bool update_error_info(uintptr_t addr, testword_t xor) +static bool update_error_info(testword_t page, testword_t offset, uintptr_t addr, testword_t xor) { bool update_stats = false; // Update address range. - testword_t page = page_of((void *)addr); - testword_t offset = addr & 0xFFF; - if (error_info.min_addr.page > page) { error_info.min_addr.page = page; error_info.min_addr.offset = offset; @@ -163,12 +160,15 @@ static void common_err(error_type_t type, uintptr_t addr, testword_t good, testw testword_t xor = good ^ bad; bool new_stats = false; + testword_t page = page_of((void *)addr); + testword_t offset = addr & (PAGE_SIZE - 1); + switch (type) { case ADDR_ERROR: - new_stats = update_error_info(addr, 0); + new_stats = update_error_info(page, offset, addr, 0); break; case DATA_ERROR: - new_stats = update_error_info(addr, xor); + new_stats = update_error_info(page, offset, addr, xor); break; case NEW_MODE: new_stats = (error_count > 0); @@ -180,7 +180,7 @@ static void common_err(error_type_t type, uintptr_t addr, testword_t good, testw bool new_badram = false; if (error_mode == ERROR_MODE_BADRAM && use_for_badram) { - new_badram = badram_insert(addr); + new_badram = badram_insert(page, offset); } if (new_address) { @@ -267,9 +267,6 @@ static void common_err(error_type_t type, uintptr_t addr, testword_t good, testw check_input(); scroll(); - uintptr_t page = page_of((void *)addr); - uintptr_t offset = addr & 0xFFF; - set_foreground_colour(YELLOW); display_scrolled_message(0, " %2i %4i %2i %09x%03x (%kB)", smp_my_cpu_num(), pass_num, test_num, page, offset, page << 2); @@ -313,7 +310,7 @@ static void common_err(error_type_t type, uintptr_t addr, testword_t good, testw void error_init(void) { error_info.min_addr.page = UINTPTR_MAX; - error_info.min_addr.offset = 0xfff; + error_info.min_addr.offset = PAGE_SIZE - 1; error_info.max_addr.page = 0; error_info.max_addr.offset = 0; error_info.bad_bits = 0; diff --git a/app/test.h b/app/test.h index a592194..a81bfae 100644 --- a/app/test.h +++ b/app/test.h @@ -47,19 +47,33 @@ extern barrier_t *run_barrier; */ extern spinlock_t *error_mutex; +#ifdef __x86_64__ /** * The word width (in bits) used for memory testing. */ -#ifdef __x86_64__ -#define TESTWORD_WIDTH 64 -#else -#define TESTWORD_WIDTH 32 -#endif - +#define TESTWORD_WIDTH 64 /** * The number of hex digits needed to display a memory test word. */ -#define TESTWORD_DIGITS (TESTWORD_WIDTH / 4) +#define TESTWORD_DIGITS 16 +/** + * The string representation of TESTWORDS_DIGITS + */ +#define TESTWORD_DIGITS_STR "16" +#else +/** + * The word width (in bits) used for memory testing. + */ +#define TESTWORD_WIDTH 32 +/** + * The number of hex digits needed to display a memory test word. + */ +#define TESTWORD_DIGITS 8 +/** + * The string representation of TESTWORDS_DIGITS + */ +#define TESTWORD_DIGITS_STR "8" +#endif /** * The word type used for memory testing.