From 49d04c589c669401bc45a491a64aa68762b20efe Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Fri, 18 Jan 2019 21:19:03 +0200 Subject: env: common: Return specific error code on bad CRC Callers of env_import*() functions might want to check the case when we have incorrect environment (with bad CRC). For example, when environment location is being defined in env_load(), call chain may look like this: env_load() -> drv->load() = env_mmc_load() -> env_import() Return code will be passed from env_import() all way up to env_load(). Right now both env_mmc_load() and env_import() return -EIO error code, so env_load() can't differentiate between two cases: 1. Driver reports the error, because device is not accessible 2. Device is actually accessible, but environment is broken Let's return -ENOMSG in env_import(), so we can distinguish two cases mentioned above. It will make it possible to continue working with "bad CRC" environment (like doing "env save"), instead of considering it not functional (implemented in subsequent patch). Signed-off-by: Sam Protsenko Reviewed-by: Simon Goldschmidt --- env/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'env') diff --git a/env/common.c b/env/common.c index d1a6a528601..324502ed829 100644 --- a/env/common.c +++ b/env/common.c @@ -115,7 +115,7 @@ int env_import(const char *buf, int check) if (crc32(0, ep->data, ENV_SIZE) != crc) { set_default_env("bad CRC", 0); - return -EIO; + return -ENOMSG; /* needed for env_load() */ } } @@ -169,7 +169,7 @@ int env_import_redund(const char *buf1, int buf1_read_fail, if (!crc1_ok && !crc2_ok) { set_default_env("bad CRC", 0); - return -EIO; + return -ENOMSG; /* needed for env_load() */ } else if (crc1_ok && !crc2_ok) { gd->env_valid = ENV_VALID; } else if (!crc1_ok && crc2_ok) { -- cgit v1.2.3 From 293a172b67230e14c43f2b7a8b6f302b1d03ca4e Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Fri, 18 Jan 2019 21:19:04 +0200 Subject: env: Fix saving environment to "bad CRC" location In case when the environment on some location is malformed (CRC isn't matching), there is a chance we won't be able to save the environment to that location. For example, consider the case when we only have the environment on eMMC, but it's zeroed out. In that case, we won't be able to "env save" to it, because of "bad CRC" error. That's happening because in env_load() function we consider malformed environment as incorrect one, and defaulting to the location with highest (0) priority, which can be different from one we are dealing with right now (e.g., highest priority can be ENV_FAT on SD card, which is not inserted, but we want to use ENV_MMC on eMMC, where we were booted from). This issue began to reproduce after commit d30ba2315ae3 ("u-boot: remove driver lookup loop from env_save()") on BeagleBone Black, but that commit didn't introduce the wrong logic, it just changed the behavior for default location to use, merely revealing this issue. To fix that, let's implement next logic in env_load(): 1. Try to find out correct environment; if found -- use it 2. If working environment wasn't found, but we found malformed one (with bad CRC), let's use it for further "env save". But make sure to use malformed environment location with highest priority. 3. If neither correct nor malformed environment was found, let's default to environment location with highest priority (0) Steps to reproduce mentioned issue on BeagleBone Black (fixed in this patch): 1. Boot from SD card and erase eMMC in U-Boot shell: => mmc dev 1 => mmc erase 0 100000 => gpt write mmc 1 $partitions 2. Write new SPL and U-Boot to eMMC; the rest of eMMC will stay filled with zeroes 3. Boot from eMMC; try to do: => env save 4. Observe the error (incorrect behavior). Correct behavior: environment should be stored correctly on eMMC, in spite of it has "bad CRC" Fixes: d30ba2315ae3 ("u-boot: remove driver lookup loop from env_save()") Signed-off-by: Sam Protsenko Reviewed-by: Simon Goldschmidt --- env/env.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'env') diff --git a/env/env.c b/env/env.c index 003509d3424..4b417b90a29 100644 --- a/env/env.c +++ b/env/env.c @@ -177,6 +177,7 @@ int env_get_char(int index) int env_load(void) { struct env_driver *drv; + int best_prio = -1; int prio; for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { @@ -195,20 +196,32 @@ int env_load(void) * one message. */ ret = drv->load(); - if (ret) { - debug("Failed (%d)\n", ret); - } else { + if (!ret) { printf("OK\n"); return 0; + } else if (ret == -ENOMSG) { + /* Handle "bad CRC" case */ + if (best_prio == -1) + best_prio = prio; + } else { + debug("Failed (%d)\n", ret); } } /* * In case of invalid environment, we set the 'default' env location - * to the highest priority. In this way, next calls to env_save() - * will restore the environment at the right place. + * to the best choice, i.e.: + * 1. Environment location with bad CRC, if such location was found + * 2. Otherwise use the location with highest priority + * + * This way, next calls to env_save() will restore the environment + * at the right place. */ - env_get_location(ENVOP_LOAD, 0); + if (best_prio >= 0) + debug("Selecting environment with bad CRC\n"); + else + best_prio = 0; + env_get_location(ENVOP_LOAD, best_prio); return -ENODEV; } -- cgit v1.2.3