[Skiboot] [PATCH 9/9] blocklevel: smart_write: Fix unaligned writes to ECC partitions
Andrew Jeffery
andrew at aj.id.au
Thu Oct 3 15:53:42 AEST 2019
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: 29d1e6f78109 ("libflash/blocklevel: add a smart write function which wraps up eraseing and writing")
Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
---
external/gard/test/results/11-clear-first.err | 0
external/gard/test/results/11-clear-first.out | 13 ++++++++
external/gard/test/tests/11-clear-first | 26 +++++++++++++++
libflash/blocklevel.c | 33 +++++++++++++++----
4 files changed, 65 insertions(+), 7 deletions(-)
create mode 100644 external/gard/test/results/11-clear-first.err
create mode 100644 external/gard/test/results/11-clear-first.out
create mode 100644 external/gard/test/tests/11-clear-first
diff --git a/external/gard/test/results/11-clear-first.err b/external/gard/test/results/11-clear-first.err
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/external/gard/test/results/11-clear-first.out b/external/gard/test/results/11-clear-first.out
new file mode 100644
index 000000000000..3b67689ce30d
--- /dev/null
+++ b/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
diff --git a/external/gard/test/tests/11-clear-first b/external/gard/test/tests/11-clear-first
new file mode 100644
index 000000000000..c59fcc8f0969
--- /dev/null
+++ b/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
diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
index 6cc4a8122701..b1b8f904880c 100644
--- a/libflash/blocklevel.c
+++ b/libflash/blocklevel.c
@@ -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;
@@ -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;
--
2.20.1
More information about the Skiboot
mailing list