[Skiboot] [PATCH] external/pflash: Add --mtd

Cyril Bur cyril.bur at au1.ibm.com
Wed Feb 17 14:08:08 AEDT 2016


The current behaviour of --bmc is to take over the flash controller. This
flag was written for very early bring-up when the BMC stack would run
entirely out of RAM. This is no longer the case and using --bmc on a BMC
running out of the very flash --bmc will read is extremely likely to brick
the system.

It has come to light that there is some requirement to read BMC flash and
some thinking that pflash is the appropriate tool. As AMI BMC firmware
exposes the flash through /dev/mtd and pflash can be easily taught to read
from MTD pflash can be upgraded to be the tool for the job.

In order to preserve the current behavior of the --bmc a new flag --mtd
has been introduced to have pflash access BMC flash the 'safe' way.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
 external/common/arch_flash.h     |  18 ++++--
 external/common/arch_flash_arm.c | 115 +++++++++++++++++++++++++++++++++------
 external/pflash/pflash.c         |  52 +++++++++++++-----
 3 files changed, 147 insertions(+), 38 deletions(-)

diff --git a/external/common/arch_flash.h b/external/common/arch_flash.h
index 918ffa9..aae9e10 100644
--- a/external/common/arch_flash.h
+++ b/external/common/arch_flash.h
@@ -20,6 +20,14 @@
 #include <getopt.h>
 #include <libflash/blocklevel.h>
 
+enum bmc_access {
+	PNOR_DIRECT,
+	PNOR_MTD,
+	BMC_DIRECT,
+	BMC_MTD,
+	ACCESS_INVAL
+};
+
 int arch_flash_init(struct blocklevel_device **bl, const char *file,
 		bool keep_alive);
 
@@ -28,12 +36,12 @@ void arch_flash_close(struct blocklevel_device *bl, const char *file);
 /* Low level functions that an architecture may support */
 
 /*
- * If called BEFORE init, then the behaviour is to set that on init the BMC
- * flash will be opened.
- * If called AFTER init, then the behaviour is to return wether or not BMC
- * flash has been opened
+ * If called BEFORE init, then this dictates how the flash will be
+ * accessed.
+ * If called AFTER init, then this returns how the flash is being accessed.
  */
-int arch_flash_bmc(struct blocklevel_device *bl, int bmc);
+enum bmc_access arch_flash_bmc(struct blocklevel_device *bl,
+		enum bmc_access access);
 
 int arch_flash_erase_chip(struct blocklevel_device *bl);
 int arch_flash_4b_mode(struct blocklevel_device *bl, int set_4b);
diff --git a/external/common/arch_flash_arm.c b/external/common/arch_flash_arm.c
index f65bddc..4e71dd9 100644
--- a/external/common/arch_flash_arm.c
+++ b/external/common/arch_flash_arm.c
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
+#define _GNU_SOURCE
 #include <stdlib.h>
 #include <stdio.h>
 #include <limits.h>
@@ -43,7 +43,7 @@ static struct arch_arm_data {
 	size_t ahb_flash_base;
 	size_t ahb_flash_size;
 	void *ahb_flash_map;
-	int bmc;
+	enum bmc_access access;
 	struct flash_chip *flash_chip;
 	struct blocklevel_device *init_bl;
 } arch_data;
@@ -156,8 +156,11 @@ static void close_devs(void)
 	 */
 }
 
-static int open_devs(int bmc)
+static int open_devs(enum bmc_access access)
 {
+	if (access != BMC_DIRECT && access != PNOR_DIRECT)
+		return -1;
+
 	arch_data.fd = open("/dev/mem", O_RDWR | O_SYNC);
 	if (arch_data.fd < 0) {
 		perror("can't open /dev/mem");
@@ -176,8 +179,8 @@ static int open_devs(int bmc)
 		perror("can't map GPIO control via /dev/mem");
 		return -1;
 	}
-	arch_data.ahb_flash_base = bmc ? BMC_FLASH_BASE : PNOR_FLASH_BASE;
-	arch_data.ahb_flash_size = bmc ? BMC_FLASH_SIZE : PNOR_FLASH_SIZE;
+	arch_data.ahb_flash_base = access == BMC_DIRECT ? BMC_FLASH_BASE : PNOR_FLASH_BASE;
+	arch_data.ahb_flash_size = access == BMC_DIRECT ? BMC_FLASH_SIZE : PNOR_FLASH_SIZE;
 	arch_data.ahb_flash_map = mmap(0, arch_data.ahb_flash_size, PROT_READ |
 			PROT_WRITE, MAP_SHARED, arch_data.fd, arch_data.ahb_flash_base);
 	if (arch_data.ahb_flash_map == MAP_FAILED) {
@@ -187,17 +190,22 @@ static int open_devs(int bmc)
 	return 0;
 }
 
-static struct blocklevel_device *flash_setup(int bmc)
+static struct blocklevel_device *flash_setup(enum bmc_access access)
 {
 	int rc;
 	struct blocklevel_device *bl;
 	struct spi_flash_ctrl *fl;
 
+	if (access != BMC_DIRECT && access != PNOR_DIRECT)
+		return NULL;
+
 	/* Open and map devices */
-	open_devs(bmc);
+	rc = open_devs(access);
+	if (rc)
+		return NULL;
 
 	/* Create the AST flash controller */
-	rc = ast_sf_open(bmc ? AST_SF_TYPE_BMC : AST_SF_TYPE_PNOR, &fl);
+	rc = ast_sf_open(access == BMC_DIRECT ? AST_SF_TYPE_BMC : AST_SF_TYPE_PNOR, &fl);
 	if (rc) {
 		fprintf(stderr, "Failed to open controller\n");
 		return NULL;
@@ -213,18 +221,77 @@ static struct blocklevel_device *flash_setup(int bmc)
 	return bl;
 }
 
-int arch_flash_bmc(struct blocklevel_device *bl, int bmc)
+static bool is_bmc_part(const char *str) {
+	/*
+	 * On AMI firmmware "fullpart" is what they called the BMC partition
+	 * On OpenBMC "bmc" is what they called the BMC partition
+	 */
+	return strstr(str, "fullpart") || strstr(str, "bmc");
+}
+
+static bool is_pnor_part(const char *str) {
+	/*
+	 * On AMI firmware "PNOR" is what they called the full PNOR
+	 * On OpenBMC "pnor" is what they called the full PNOR
+	 */
+	return strcasestr(str, "pnor");
+}
+
+static char *get_dev_mtd(enum bmc_access access)
+{
+	FILE *f;
+	char *ret = NULL, *pos = NULL;
+	char line[50];
+
+	if (access != BMC_MTD && access != PNOR_MTD)
+		return NULL;
+
+	f = fopen("/proc/mtd", "r");
+	if (!f)
+		return NULL;
+
+	while (!pos && fgets(line, sizeof(line), f) != NULL) {
+		/* Going to have issues if we didn't get the full line */
+		if (line[strlen(line) - 1] != '\n')
+			break;
+
+		if (access == BMC_MTD && is_bmc_part(line)) {
+			pos = strchr(line, ':');
+			if (!pos)
+				break;
+
+		} else if (access == PNOR_MTD && is_pnor_part(line)) {
+			pos = strchr(line, ':');
+			if (!pos)
+				break;
+		}
+	}
+	if (pos) {
+		*pos = '\0';
+		if (asprintf(&ret, "/dev/%s", line) == -1)
+			ret = NULL;
+	}
+
+	fclose(f);
+	return ret;
+}
+
+enum bmc_access arch_flash_bmc(struct blocklevel_device *bl,
+		enum bmc_access access)
 {
+	if (access == ACCESS_INVAL)
+		return ACCESS_INVAL;
+
 	if (!arch_data.init_bl) {
-		arch_data.bmc = bmc;
-		return 0;
+		arch_data.access = access;
+		return access;
 	}
 
 	/* Called with a BL not inited here, bail */
 	if (arch_data.init_bl != bl)
-		return -1;
+		return ACCESS_INVAL;
 
-	return arch_data.flash_chip ? arch_data.bmc : -1;
+	return arch_data.flash_chip ? arch_data.access : ACCESS_INVAL;
 }
 
 int arch_flash_erase_chip(struct blocklevel_device *bl)
@@ -266,18 +333,30 @@ int arch_flash_set_wrprotect(struct blocklevel_device *bl, int set)
 int arch_flash_init(struct blocklevel_device **r_bl, const char *file, bool keep_alive)
 {
 	struct blocklevel_device *new_bl;
+	int rc = 0;
 
 	/* Check we haven't already inited */
 	if (arch_data.init_bl)
 		return -1;
 
 	if (file) {
-		file_init_path(file, NULL, keep_alive, &new_bl);
+		rc = file_init_path(file, NULL, keep_alive, &new_bl);
+	} else if (arch_data.access == BMC_MTD || arch_data.access == PNOR_MTD) {
+		char *mtd_dev;
+
+		mtd_dev = get_dev_mtd(arch_data.access);
+		if (!mtd_dev) {
+			return -1;
+		}
+		rc = file_init_path(mtd_dev, NULL, keep_alive, &new_bl);
+		free(mtd_dev);
 	} else {
-		new_bl = flash_setup(arch_data.bmc);
+		new_bl = flash_setup(arch_data.access);
+		if (!new_bl)
+			rc = -1;
 	}
-	if (!new_bl)
-		return -1;
+	if (rc)
+		return rc;
 
 	arch_data.init_bl = new_bl;
 	*r_bl = new_bl;
@@ -286,7 +365,7 @@ int arch_flash_init(struct blocklevel_device **r_bl, const char *file, bool keep
 
 void arch_flash_close(struct blocklevel_device *bl, const char *file)
 {
-	if (file) {
+	if (file || arch_data.access == BMC_MTD || arch_data.access == PNOR_MTD) {
 		file_exit_close(bl);
 	} else {
 		flash_exit_close(bl, &ast_sf_close);
diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
index c1d4949..7fc0de8 100644
--- a/external/pflash/pflash.c
+++ b/external/pflash/pflash.c
@@ -90,16 +90,16 @@ static void print_ffs_info(uint32_t toc_offset)
 	int rc;
 	uint32_t i;
 
+	printf("\n");
+	printf("TOC at 0x%08x Partitions:\n", toc_offset);
+	printf("-----------\n");
+
 	rc = ffs_init(toc_offset, fl_total_size, bl, &ffs_handle, 0);
 	if (rc) {
 		fprintf(stderr, "Error %d opening ffs !\n", rc);
 		return;
 	}
 
-	printf("\n");
-	printf("TOC at 0x%08x Partitions:\n", toc_offset);
-	printf("-----------\n");
-
 	for (i = 0;; i++) {
 		uint32_t start, size, act, end;
 		bool ecc;
@@ -453,12 +453,14 @@ static void print_help(const char *pname)
 	printf("\t\tDon't ask for confirmation before erasing or flashing\n\n");
 	printf("\t-d, --dummy\n");
 	printf("\t\tDon't write to flash\n\n");
-#ifdef __powerpc__
-	printf("\t-l, --lpc\n");
-	printf("\t\tUse LPC accesses instead of PCI\n\n");
-#endif
+	printf("\t-m, --mtd\n");
+	printf("\t\tAvoid accessing the flash directly if the BMC supports it.\n");
+	printf("\t\tThis will access the flash through the kernel MTD layer and\n");
+	printf("\t\tnot the flash directly\n");
 	printf("\t-b, --bmc\n");
-	printf("\t\tTarget BMC flash instead of host flash\n\n");
+	printf("\t\tTarget BMC flash instead of host flash.\n");
+	printf("\t\tNote: This carries a high chance of bricking your BMC if you\n");
+	printf("\t\tdon't know what you're doing. Consider --mtd to be safe(r)\n\n");
 	printf("\t-S, --side\n");
 	printf("\t\tSide of the flash on which to operate, 0 (default) or 1\n\n");
 	printf("\t-T, --toc\n");
@@ -523,15 +525,16 @@ int main(int argc, char *argv[])
 	bool show_help = false, show_version = false;
 	bool no_action = false, tune = false;
 	char *write_file = NULL, *read_file = NULL, *part_name = NULL;
-	bool ffs_toc_seen = false;
+	bool ffs_toc_seen = false, mtd = false;
 	int rc;
 
 	while(1) {
-		static struct option long_opts[] = {
+		struct option long_opts[] = {
 			{"address",	required_argument,	NULL,	'a'},
 			{"size",	required_argument,	NULL,	's'},
 			{"partition",	required_argument,	NULL,	'P'},
 			{"bmc",		no_argument,		NULL,	'b'},
+			{"mtd",		no_argument,		NULL,	'm'},
 			{"enable-4B",	no_argument,		NULL,	'4'},
 			{"disable-4B",	no_argument,		NULL,	'3'},
 			{"read",	required_argument,	NULL,	'r'},
@@ -551,7 +554,7 @@ int main(int argc, char *argv[])
 		};
 		int c, oidx = 0;
 
-		c = getopt_long(argc, argv, "a:s:P:r:43Eep:fdihvbtgS:T:c",
+		c = getopt_long(argc, argv, "a:s:P:r:43Eemp:fdihvbtgS:T:c",
 				long_opts, &oidx);
 		if (c == EOF)
 			break;
@@ -581,6 +584,9 @@ int main(int argc, char *argv[])
 		case 'e':
 			erase = true;
 			break;
+		case 'm':
+			mtd = true;
+			break;
 		case 'p':
 			program = true;
 			write_file = strdup(optarg);
@@ -716,14 +722,30 @@ int main(int argc, char *argv[])
 	}
 
 	if (bmc_flash) {
-		if (arch_flash_bmc(NULL, 1) == -1) {
-			fprintf(stderr, "Can't switch to BMC flash on this architecture\n");
+		/*
+		 * Try to see if we can access BMC flash on this arch at all...
+		 * even if what we really want to do is use MTD.
+		 * This helps give a more meaningful error messages.
+		 */
+
+		if (arch_flash_bmc(NULL, BMC_DIRECT) == ACCESS_INVAL) {
+			fprintf(stderr, "Can't access BMC flash on this architecture\n");
+			exit(1);
+		}
+	}
+
+	if (mtd) {
+		if (arch_flash_bmc(NULL, bmc_flash ? BMC_MTD : PNOR_MTD) == ACCESS_INVAL) {
+			fprintf(stderr, "Can't access %s flash through MTD on this architecture\n",
+			        bmc_flash ? "BMC" : "PNOR");
 			exit(1);
 		}
 	}
 
-	if (arch_flash_init(&bl, NULL, true))
+	if (arch_flash_init(&bl, NULL, true)) {
+		fprintf(stderr, "Couldn't initialise architecture flash structures\n");
 		exit(1);
+	}
 
 	on_exit(exiting, NULL);
 
-- 
2.7.1



More information about the Skiboot mailing list