[Skiboot] [PATCH 0/2] Endian fixups for ecc code in libflash

Cyril Bur cyril.bur at au1.ibm.com
Tue Mar 3 17:21:27 AEDT 2015


The ecc code (originally taken from hostboot) is not endian safe. Here is a
simple fix to make it work on little endian cpu machines.

After consultation with Michael Neuling, we think perhaps we should change the
interface to eccmemcpy() because its second argument is misleading. Currently
it looks like: uint8_t eccmemcpy(uint64_t *dst, uint64_t *src, uint32_t len)

What it actually wants is to read 9 byte chunks from src, 8 data bytes and 1
ecc byte and fill dst with 8 bytes of ecc corrected data. It makes more sense
for type of src to reflect that.

As we see it there are two types which could work:
1. Using a packed struct
Using something like
struct ecc64 {
	beint64_t data;
	uint8_t  ecc;
} __attribute__((__packed__));

eccmemcpy(uint64_t *dst, struct ecc64 *src, uint32_t len);

This creates a nice separation for the caller so that there can be no
misunderstanding that the data is expected to be big endian and the presence
of the ecc byte.

2. Using a 9 byte array
eccmemcpy(uint64_t *dst, uint8_t *src[9], uint32_t len);

This ensures that the user understands that multiples of 9 bytes must be
passed for every 8 bytes into dest however it doesn't make the breakdown of 8
of data and 1 of ecc quite so clearly as option 1. Nor does it highlight the
endian problem as nicely.

Option 1 seems best, it would make the code in eccmemcpy much clearer, it is
essentually doing that at the moment but with messy pointer arithmetic.

For now it makes most sense to just fix the endian issue and once theres a
consensus, I'll be happy to improve eccmemcpy.

Cyril Bur (2):
  libflash: whitespace fixups for ecc.c
  libflash: endian fixups for ecc.c

 libflash/ecc.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

-- 
1.9.1



More information about the Skiboot mailing list