Fix memory error addresses displayed by badram reporting mode (#308).

This commit is contained in:
Lionel Debroux 2023-05-21 21:31:29 +02:00 committed by Lionel Debroux
parent 03b6cbe4e4
commit df803bf294
4 changed files with 57 additions and 43 deletions

View File

@ -28,6 +28,7 @@
#include "display.h" #include "display.h"
#include "badram.h" #include "badram.h"
#include "memsize.h"
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// Constants // Constants
@ -38,9 +39,9 @@
// DEFAULT_MASK covers a uintptr_t, since that is the testing granularity. // DEFAULT_MASK covers a uintptr_t, since that is the testing granularity.
#ifdef __x86_64__ #ifdef __x86_64__
#define DEFAULT_MASK (UINTPTR_MAX << 3) #define DEFAULT_MASK (UINT64_MAX << 3)
#else #else
#define DEFAULT_MASK (UINTPTR_MAX << 2) #define DEFAULT_MASK (UINT64_MAX << 2)
#endif #endif
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
@ -48,8 +49,8 @@
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
typedef struct { typedef struct {
uintptr_t addr; uint64_t addr;
uintptr_t mask; uint64_t mask;
} pattern_t; } pattern_t;
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
@ -70,7 +71,7 @@ static int num_patterns = 0;
/* /*
* Combine two addr/mask pairs to one addr/mask pair. * 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); *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. * 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; uint64_t ctr = 1;
int i = 8*sizeof(uintptr_t); int i = 8*sizeof(uint64_t);
while (i-- > 0) { while (i-- > 0) {
if (! (mask & 1)) { if (! (mask & 1)) {
ctr += ctr; 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 * Count how many more addresses would be covered by addr1/mask1 when combined
* with addr2/mask2. * 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); uint64_t cost1 = addresses(mask1);
uintptr_t tmp, mask; uint64_t tmp, mask;
combine(addr1, mask1, addr2, mask2, &tmp, &mask); combine(addr1, mask1, addr2, mask2, &tmp, &mask);
return addresses(mask) - cost1; 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 // This is guaranteed to be overwritten with >= 0 as long as num_patterns > 1
int merge_idx = -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++) { for (int i = 0; i < num_patterns - 1; i++) {
uintptr_t tmp_cost = combi_cost( uint64_t tmp_cost = combi_cost(
patterns[i].addr, patterns[i].addr,
patterns[i].mask, patterns[i].mask,
patterns[i+1].addr, patterns[i+1].addr,
patterns[i+1].mask patterns[i+1].mask
); );
if (tmp_cost <= min_cost) { if (tmp_cost <= min_cost) {
min_cost = tmp_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 = { pattern_t pattern = {
.addr = addr, .addr = ((uint64_t)page << PAGE_SHIFT) + offset,
.mask = DEFAULT_MASK .mask = DEFAULT_MASK
}; };
// If covered by existing entry we return immediately // If covered by existing entry we return immediately
@ -277,14 +278,14 @@ void badram_display(void)
display_scrolled_message(col, ","); display_scrolled_message(col, ",");
col++; col++;
} }
int text_width = 2 * (TESTWORD_DIGITS + 2) + 1; int text_width = 2 * (16 + 2) + 1;
if (col > (SCREEN_WIDTH - text_width)) { if (col > (SCREEN_WIDTH - text_width)) {
scroll(); scroll();
col = 7; col = 7;
} }
display_scrolled_message(col, "0x%0*x,0x%0*x", display_scrolled_message(col, "0x%08x%08x,0x%08x%08x",
TESTWORD_DIGITS, patterns[i].addr, (uintptr_t)(patterns[i].addr >> 32), (uintptr_t)(patterns[i].addr & 0xFFFFFFFFU),
TESTWORD_DIGITS, patterns[i].mask); (uintptr_t)(patterns[i].mask >> 32), (uintptr_t)(patterns[i].mask & 0xFFFFFFFFU));
col += text_width; col += text_width;
} }
} }

View File

@ -13,6 +13,8 @@
#include <stdbool.h> #include <stdbool.h>
#include <stdint.h> #include <stdint.h>
#include "test.h"
/** /**
* Initialises the pattern array. * Initialises the pattern array.
*/ */
@ -22,7 +24,7 @@ void badram_init(void);
* Inserts a single faulty address into the pattern array. Returns * Inserts a single faulty address into the pattern array. Returns
* true iff the array was changed. * 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 * Displays the pattern array in the scrollable display region in the

View File

@ -77,15 +77,12 @@ uint64_t error_count = 0;
// Private Functions // 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; bool update_stats = false;
// Update address range. // Update address range.
testword_t page = page_of((void *)addr);
testword_t offset = addr & 0xFFF;
if (error_info.min_addr.page > page) { if (error_info.min_addr.page > page) {
error_info.min_addr.page = page; error_info.min_addr.page = page;
error_info.min_addr.offset = offset; 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; testword_t xor = good ^ bad;
bool new_stats = false; bool new_stats = false;
testword_t page = page_of((void *)addr);
testword_t offset = addr & (PAGE_SIZE - 1);
switch (type) { switch (type) {
case ADDR_ERROR: case ADDR_ERROR:
new_stats = update_error_info(addr, 0); new_stats = update_error_info(page, offset, addr, 0);
break; break;
case DATA_ERROR: case DATA_ERROR:
new_stats = update_error_info(addr, xor); new_stats = update_error_info(page, offset, addr, xor);
break; break;
case NEW_MODE: case NEW_MODE:
new_stats = (error_count > 0); 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; bool new_badram = false;
if (error_mode == ERROR_MODE_BADRAM && use_for_badram) { if (error_mode == ERROR_MODE_BADRAM && use_for_badram) {
new_badram = badram_insert(addr); new_badram = badram_insert(page, offset);
} }
if (new_address) { if (new_address) {
@ -267,9 +267,6 @@ static void common_err(error_type_t type, uintptr_t addr, testword_t good, testw
check_input(); check_input();
scroll(); scroll();
uintptr_t page = page_of((void *)addr);
uintptr_t offset = addr & 0xFFF;
set_foreground_colour(YELLOW); set_foreground_colour(YELLOW);
display_scrolled_message(0, " %2i %4i %2i %09x%03x (%kB)", display_scrolled_message(0, " %2i %4i %2i %09x%03x (%kB)",
smp_my_cpu_num(), pass_num, test_num, page, offset, page << 2); 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) void error_init(void)
{ {
error_info.min_addr.page = UINTPTR_MAX; 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.page = 0;
error_info.max_addr.offset = 0; error_info.max_addr.offset = 0;
error_info.bad_bits = 0; error_info.bad_bits = 0;

View File

@ -47,19 +47,33 @@ extern barrier_t *run_barrier;
*/ */
extern spinlock_t *error_mutex; extern spinlock_t *error_mutex;
#ifdef __x86_64__
/** /**
* The word width (in bits) used for memory testing. * The word width (in bits) used for memory testing.
*/ */
#ifdef __x86_64__ #define TESTWORD_WIDTH 64
#define TESTWORD_WIDTH 64
#else
#define TESTWORD_WIDTH 32
#endif
/** /**
* The number of hex digits needed to display a memory test word. * 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. * The word type used for memory testing.