[Skiboot] [PATCH] core/flash: Don't do anything clever for OPAL_FLASH_{READ, WRITE, ERASE}

Cyril Bur cyril.bur at au1.ibm.com
Wed Nov 2 17:46:21 AEDT 2016


These OPAL calls are exposing the host flash chip to linux as a flash
device. The host is expected to understand that it is raw flash and use
it appropriately, it isn't skiboots place to strip ecc bytes or perform
erase before writes.

Over the course of other developments blocklevel has gotten quite clever
but in this case the cleverness is inappropriate.

Fixes: https://github.com/open-power/skiboot/issues/44
Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
 core/flash.c          | 15 ++++++++-----
 libflash/blocklevel.c | 62 ++++++++++++++++++++++++++++++++++++---------------
 libflash/blocklevel.h |  3 ++-
 3 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/core/flash.c b/core/flash.c
index 83cd6c8..5f08021 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -348,16 +348,19 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset,
 		goto err;
 	}
 
+	/*
+	 * These ops intentionally have no smarts (ecc correction or erase
+	 * before write) to them.
+	 * Skiboot is simply exposing the PNOR flash to the host.
+	 * The host is expected to understand that this is a raw flash
+	 * device and treat it as such.
+	 */
 	switch (op) {
 	case FLASH_OP_READ:
-		rc = blocklevel_read(flash->bl, offset, (void *)buf, size);
+		rc = blocklevel_raw_read(flash->bl, offset, (void *)buf, size);
 		break;
 	case FLASH_OP_WRITE:
-		/*
-		 * Note: blocklevel_write() uses flash_smart_write(), this call used to
-		 * be flash_write()
-		 */
-		rc = blocklevel_write(flash->bl, offset, (void *)buf, size);
+		rc = blocklevel_raw_write(flash->bl, offset, (void *)buf, size);
 		break;
 	case FLASH_OP_ERASE:
 		rc = blocklevel_erase(flash->bl, offset, size);
diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
index b827045..c18de1f 100644
--- a/libflash/blocklevel.c
+++ b/libflash/blocklevel.c
@@ -78,11 +78,9 @@ static int release(struct blocklevel_device *bl)
 	return rc;
 }
 
-int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len)
+int blocklevel_raw_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len)
 {
 	int rc;
-	struct ecc64 *buffer;
-	uint64_t ecc_len = ecc_buffer_size(len);
 
 	if (!bl || !bl->read || !buf) {
 		errno = EINVAL;
@@ -93,12 +91,27 @@ int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint6
 	if (rc)
 		return rc;
 
-	if (!ecc_protected(bl, pos, len)) {
-		rc = bl->read(bl, pos, buf, len);
-		release(bl);
-		return rc;
+	rc = bl->read(bl, pos, buf, len);
+
+	release(bl);
+
+	return rc;
+}
+
+int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len)
+{
+	int rc;
+	struct ecc64 *buffer;
+	uint64_t ecc_len = ecc_buffer_size(len);
+
+	if (!bl || !buf) {
+		errno = EINVAL;
+		return FLASH_ERR_PARM_ERROR;
 	}
 
+	if (!ecc_protected(bl, pos, len))
+		return blocklevel_raw_read(bl, pos, buf, len);
+
 	buffer = malloc(ecc_len);
 	if (!buffer) {
 		errno = ENOMEM;
@@ -106,7 +119,7 @@ int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint6
 		goto out;
 	}
 
-	rc = bl->read(bl, pos, buffer, ecc_len);
+	rc = blocklevel_raw_read(bl, pos, buffer, ecc_len);
 	if (rc)
 		goto out;
 
@@ -116,16 +129,14 @@ int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint6
 	}
 
 out:
-	release(bl);
 	free(buffer);
 	return rc;
 }
 
-int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf, uint64_t len)
+int blocklevel_raw_write(struct blocklevel_device *bl, uint64_t pos,
+		const void *buf, uint64_t len)
 {
 	int rc;
-	struct ecc64 *buffer;
-	uint64_t ecc_len = ecc_buffer_size(len);
 
 	if (!bl || !bl->write || !buf) {
 		errno = EINVAL;
@@ -136,12 +147,28 @@ int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf
 	if (rc)
 		return rc;
 
-	if (!ecc_protected(bl, pos, len)) {
-		rc =  bl->write(bl, pos, buf, len);
-		release(bl);
-		return rc;
+	rc = bl->write(bl, pos, buf, len);
+
+	release(bl);
+
+	return rc;
+}
+
+int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf,
+		uint64_t len)
+{
+	int rc;
+	struct ecc64 *buffer;
+	uint64_t ecc_len = ecc_buffer_size(len);
+
+	if (!bl || !buf) {
+		errno = EINVAL;
+		return FLASH_ERR_PARM_ERROR;
 	}
 
+	if (!ecc_protected(bl, pos, len))
+		return blocklevel_raw_write(bl, pos, buf, len);
+
 	buffer = malloc(ecc_len);
 	if (!buffer) {
 		errno = ENOMEM;
@@ -155,10 +182,9 @@ int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf
 		goto out;
 	}
 
-	rc = bl->write(bl, pos, buffer, ecc_len);
+	rc = blocklevel_raw_write(bl, pos, buffer, ecc_len);
 
 out:
-	release(bl);
 	free(buffer);
 	return rc;
 }
diff --git a/libflash/blocklevel.h b/libflash/blocklevel.h
index ddec1dd..09f0096 100644
--- a/libflash/blocklevel.h
+++ b/libflash/blocklevel.h
@@ -57,8 +57,9 @@ struct blocklevel_device {
 
 	struct blocklevel_range ecc_prot;
 };
-
+int blocklevel_raw_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len);
 int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len);
+int blocklevel_raw_write(struct blocklevel_device *bl, uint64_t pos, const void *buf, uint64_t len);
 int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf, uint64_t len);
 int blocklevel_erase(struct blocklevel_device *bl, uint64_t pos, uint64_t len);
 int blocklevel_get_info(struct blocklevel_device *bl, const char **name, uint64_t *total_size,
-- 
2.10.1



More information about the Skiboot mailing list