Skip to content

Commit

Permalink
blocklevel: smart_write: Fix unaligned writes to ECC partitions
Browse files Browse the repository at this point in the history
Currently trying to clear a gard record results in errors:

    $ ./opal-gard -pef part create /sys0/node0/proc1
    $ ./opal-gard -pef part list
     ID       | Error    | Type       | Path
    ---------------------------------------------------------
     00000001 | 00000000 | Manual     | /Sys0/Node0/Proc1
    =========================================================
    $ ./opal-gard -pef part clear 00000001
    Clearing gard record 0x00000001...done
    ECC: uncorrectable error: ffffff00ffffffff ff
    libflash ecc invalid
    $

A little wrapper around hexdump(1) helps show where the error lies by grouping
output in blocks of nine such that the last byte is the ECC byte:

    $ declare -f ecchd
    ecchd ()
    {
        hexdump -e '"%08_ax""\t"' -e '9/1 "%02x ""\t""|"' -e '9/1 "%_p""|\n"' "$@"
    }

A clean GARD partition displays as:

    $ ecchd part
    0002c000        ff ff ff ff ff ff ff ff 00      |.........|
    *
    00030ffb        ff ff ff ff ff                  |.....|
    $

Dumping the corrupt partition shows:

    $ ecchd part
    0002c000        ff ff ff ff ff ff ff ff 00      |.........|
    *
    0002c024        ff ff ff ff ff ff ff ff ff      |.........|
    0002c02d        ff ff ff 00 ff ff ff ff ff      |.........|
    *
    0002c051        ff ff ff 00 ff ff ff ff 00      |.........|
    0002c05a        ff ff ff ff ff ff ff ff 00      |.........|
    *
    00030ffb        ff ff ff ff ff                  |.....|
    $

blocklevel_smart_write() turned out to not be quite as smart as it thought it
was in that any unaligned write to ECC protected partitions aligned the
calculated ECC values to the start of the write buffer and not relative to the
start of the partition.

Fixes: 29d1e6f ("libflash/blocklevel: add a smart write function which wraps up eraseing and writing")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
  • Loading branch information
amboar authored and oohal committed Oct 14, 2019
1 parent a950fd7 commit 96ddf4b
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 7 deletions.
Empty file.
13 changes: 13 additions & 0 deletions external/gard/test/results/11-clear-first.out
@@ -0,0 +1,13 @@
Clearing the entire gard partition...done
ID | Error | Type | Path
---------------------------------------------------------
00000001 | 00000000 | Manual | /Sys0/Node0/Proc0
00000002 | 00000000 | Manual | /Sys0/Node0/Proc1
=========================================================
Clearing gard record 0x00000001...done
ID | Error | Type | Path
---------------------------------------------------------
00000002 | 00000000 | Manual | /Sys0/Node0/Proc1
=========================================================
Clearing gard record 0x00000002...done
No GARD entries to display
26 changes: 26 additions & 0 deletions external/gard/test/tests/11-clear-first
@@ -0,0 +1,26 @@
#!/bin/sh

set -e

DATA=$(mktemp)

cleanup() {
rm -f $DATA
}

trap cleanup EXIT

dd if=/dev/zero of=$DATA bs=$((0x1000)) count=5 2>/dev/null

run_binary "./opal-gard" "-p -e -f $DATA clear all"
run_binary "./opal-gard" "-p -e -f $DATA create /sys0/node0/proc0"
run_binary "./opal-gard" "-p -e -f $DATA create /sys0/node0/proc1"
run_binary "./opal-gard" "-p -e -f $DATA list"
run_binary "./opal-gard" "-p -e -f $DATA clear 00000001"
run_binary "./opal-gard" "-p -e -f $DATA list"
run_binary "./opal-gard" "-p -e -f $DATA clear 00000002"
run_binary "./opal-gard" "-p -e -f $DATA list"

diff_with_result

pass_test
33 changes: 26 additions & 7 deletions libflash/blocklevel.c
Expand Up @@ -513,7 +513,7 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint64_t pos, const voi
void *erase_buf = NULL;
uint32_t erase_size;

const void *write_buf = buf;
const void *write_buf;
uint64_t write_len;
uint64_t write_pos;

Expand Down Expand Up @@ -543,27 +543,46 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint64_t pos, const voi
}

if (ecc_protection) {
uint64_t ecc_pos, ecc_align, ecc_diff, ecc_len;

FL_DBG("%s: region has ECC\n", __func__);

len = ecc_buffer_size(len);
ecc_pos = with_ecc_pos(ecc_start, pos);
ecc_align = ecc_buffer_align(ecc_start, ecc_pos);
ecc_diff = ecc_pos - ecc_align;
ecc_len = ecc_buffer_size(len + ecc_diff);

ecc_buf = malloc(len);
ecc_buf = malloc(ecc_len);
if (!ecc_buf) {
errno = ENOMEM;
return FLASH_ERR_MALLOC_FAILED;
}

if (memcpy_to_ecc(ecc_buf, buf, ecc_buffer_size_minus_ecc(len))) {
if (ecc_diff) {
rc = blocklevel_read(bl, ecc_align, ecc_buf, ecc_diff);
if (rc) {
errno = EBADF;
rc = FLASH_ERR_ECC_INVALID;
goto out;
}
}

rc = memcpy_to_ecc_unaligned(ecc_buf, buf, len, ecc_diff);
if (rc) {
free(ecc_buf);
errno = EBADF;
return FLASH_ERR_ECC_INVALID;
}

write_buf = ecc_buf;
write_len = ecc_len;
write_pos = ecc_pos;
} else {
write_buf = buf;
write_len = len;
write_pos = pos;
}

write_pos = pos;
write_len = len;

erase_buf = malloc(erase_size);
if (!erase_buf) {
errno = ENOMEM;
Expand Down

0 comments on commit 96ddf4b

Please sign in to comment.