[Skiboot] [RFC] external/pflash: Change --bmc behaviour and add --direct

Cyril Bur cyril.bur at au1.ibm.com
Fri Jan 15 16:11:09 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 --direct
is called which will use the flash controller directly. This option now
includes a large warning so as to prevent any accidental bricking.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
Before anyone mentions it, this could well be two patches but trying to
split them up is tricky, I could put the effort in if deemed absolutely
necessary, RFC away.

 external/common/arch_flash.h     | 18 ++++++--
 external/common/arch_flash_arm.c | 99 ++++++++++++++++++++++++++++++++--------
 external/pflash/pflash.c         | 41 +++++++++++++----
 3 files changed, 127 insertions(+), 31 deletions(-)

diff --git a/external/common/arch_flash.h b/external/common/arch_flash.h
index 60c4de8..7d0f4e0 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_MTD = 0,
+	PNOR_DIRECT,
+	BMC_MTD,
+	BMC_DIRECT,
+	ACCESS_INVAL
+};
+
 int arch_flash_init(struct blocklevel_device **bl, const char *file);
 
 void arch_flash_close(struct blocklevel_device *bl, const char *file);
@@ -27,12 +35,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 13d2946..654808d 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,61 @@ static struct blocklevel_device *flash_setup(int bmc)
 	return bl;
 }
 
-int arch_flash_bmc(struct blocklevel_device *bl, int bmc)
+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 && strstr(line, "fullpart")) {
+			pos = strchr(line, ':');
+			if (!pos)
+				break;
+
+		} else if (access == PNOR_MTD && strstr(line, "PNOR")) {
+			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 +317,30 @@ int arch_flash_set_wrprotect(struct blocklevel_device *bl, int set)
 int arch_flash_init(struct blocklevel_device **r_bl, const char *file)
 {
 	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, &new_bl);
+		rc = file_init_path(file, NULL, &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, &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 +349,7 @@ int arch_flash_init(struct blocklevel_device **r_bl, const char *file)
 
 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 8231708..b79d612 100644
--- a/external/pflash/pflash.c
+++ b/external/pflash/pflash.c
@@ -453,10 +453,10 @@ 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--direct\n");
+	printf("\t\tAccess flash directly. High chance of bricking a BMC\n");
+	printf("\t\tWarning: Only use this option if you are sure you know what ");
+	printf("\t\tdoing.\nThis will access your flash controller directly\n\n");
 	printf("\t-b, --bmc\n");
 	printf("\t\tTarget BMC flash instead of host flash\n\n");
 	printf("\t-S, --side\n");
@@ -524,14 +524,16 @@ int main(int argc, char *argv[])
 	bool no_action = false, tune = false;
 	char *write_file = NULL, *read_file = NULL, *part_name = NULL;
 	bool ffs_toc_seen = false;
+	int direct = 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'},
+			{"direct",		no_argument,	&direct,	true},
 			{"enable-4B",	no_argument,		NULL,	'4'},
 			{"disable-4B",	no_argument,		NULL,	'3'},
 			{"read",	required_argument,	NULL,	'r'},
@@ -620,6 +622,9 @@ int main(int argc, char *argv[])
 		case 'c':
 			do_clear = true;
 			break;
+		case 0:
+			/* Long opt was recognised */
+			break;
 		default:
 			exit(1);
 		}
@@ -716,14 +721,34 @@ 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 flash directly.
+		 * This helps give a more meaningful error messages.
+		 */
+
+		if (arch_flash_bmc(NULL, BMC_MTD) == ACCESS_INVAL) {
+			fprintf(stderr, "Can't access BMC flash on this architecture\n");
 			exit(1);
 		}
 	}
 
-	if (arch_flash_init(&bl, NULL))
+	if (direct) {
+		fprintf(stderr, "--direct flag detected. Are you sure you want to continue?\n");
+		check_confirm();
+		/* Still confirm later on */
+		must_confirm = true;
+		if (arch_flash_bmc(NULL, bmc_flash ? BMC_DIRECT : PNOR_DIRECT) == ACCESS_INVAL) {
+			fprintf(stderr, "Can't access %s flash directly on this architecture\n",
+			        bmc_flash ? "BMC" : "PNOR");
+			exit(1);
+		}
+	}
+
+	if (arch_flash_init(&bl, NULL)) {
+		fprintf(stderr, "Couldn't initialise architecture flash structures\n");
 		exit(1);
+	}
 
 	on_exit(exiting, NULL);
 
-- 
2.7.0



More information about the Skiboot mailing list