- Create distinct functions for mapping and unmapping for flash chips. - Map only when needed: map before probing and unmap immediately after it. Map again when a single chip was probed successfully before taking any actual actions and clean up afterwards. - Map special function chip registers centrally together with flash space instead of within (some) probing methods after successful probes. - Save the used base addresses of the mappings in struct flashctx as well. - Do not try to (un)map the zero-sized chip definitions that are merely hacks. This also fixes the printing of wrong warnings for these chip definitions introduced in r1765.
Signed-off-by: Stefan Tauner <[email protected]> --- After some more discussions on IRC I came up with this... It was actually a nice exercise to get more familiar with the issue - too bad it is so annoying ;) 82802ab.c | 3 -- cli_classic.c | 15 +++++++++- flash.h | 17 +++++++---- flashrom.c | 93 ++++++++++++++++++++++++++++++++++++++++++++--------------- jedec.c | 6 ---- 5 files changed, 96 insertions(+), 38 deletions(-) diff --git a/82802ab.c b/82802ab.c index 70c6af0..1436f8a 100644 --- a/82802ab.c +++ b/82802ab.c @@ -83,9 +83,6 @@ int probe_82802ab(struct flashctx *flash) if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) return 0; - if (flash->chip->feature_bits & FEATURE_REGISTERMAP) - map_flash_registers(flash); - return 1; } diff --git a/cli_classic.c b/cli_classic.c index 945ad7b..a693596 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -482,7 +482,9 @@ int main(int argc, char *argv[]) goto out_shutdown; } msg_cinfo("Please note that forced reads most likely contain garbage.\n"); + map_flash(&flashes[0]); ret = read_flash_to_file(&flashes[0], filename); + unmap_flash(&flashes[0]); free(flashes[0].chip); goto out_shutdown; } @@ -516,9 +518,18 @@ int main(int argc, char *argv[]) goto out_shutdown; } + msg_pdbg2("Successfully mapped flash chip \"%s\" at physical address 0x%0*" PRIxPTR ".\n", + fill_flash->chip->name, PRIxPTR_WIDTH, fill_flash->physical_memory); + + /* Map the selected flash chip again. */ + if (map_flash(fill_flash) != 0) { + ret = 1; + goto out_shutdown; + } + if (!(read_it | write_it | verify_it | erase_it)) { msg_ginfo("No operations were specified.\n"); - goto out_shutdown; + goto out_unmap; } /* Always verify write operations unless -n is used. */ @@ -532,6 +543,8 @@ int main(int argc, char *argv[]) programmer_delay(100000); ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); +out_unmap: + unmap_flash(fill_flash); out_shutdown: programmer_shutdown(); out: diff --git a/flash.h b/flash.h index 301eb7a..a5d3a55 100644 --- a/flash.h +++ b/flash.h @@ -48,9 +48,9 @@ typedef uintptr_t chipaddr; /* Types and macros regarding the maximum flash space size supported by generic code. */ typedef uint32_t chipoff_t; /* Able to store any addressable offset within a supported flash memory. */ typedef uint32_t chipsize_t; /* Able to store the number of bytes of any supported flash memory. */ -#define FL_MAX_CHIPADDR_BITS (24) -#define FL_MAX_CHIPADDR ((chipoff_t)(1ULL<<FL_MAX_CHIPADDR_BITS)-1) -#define PRIxCHIPADDR "06"PRIx32 +#define FL_MAX_CHIPOFF_BITS (24) +#define FL_MAX_CHIPOFF ((chipoff_t)(1ULL<<FL_MAX_CHIPOFF_BITS)-1) +#define PRIxCHIPOFF "06"PRIx32 #define PRIuCHIPSIZE PRIu32 int register_shutdown(int (*function) (void *data), void *data); @@ -209,8 +209,14 @@ struct flashchip { struct flashctx { struct flashchip *chip; + /* FIXME: The memory mappings should be saved in a more structured way. */ + /* The physical_* fields store the respective addresses in the physical address space of the CPU. */ + uintptr_t physical_memory; + /* The virtual_* fields store where the respective physical address is mapped into flashrom's address + * space. A value equivalent to (chipaddr)ERROR_PTR indicates an invalid mapping (or none at all). */ chipaddr virtual_memory; - /* Some flash devices have an additional register space. */ + /* Some flash devices have an additional register space; semantics are like above. */ + uintptr_t physical_registers; chipaddr virtual_registers; struct registered_master *mst; }; @@ -252,7 +258,8 @@ void tolower_string(char *str); /* flashrom.c */ extern const char flashrom_version[]; extern const char *chip_to_probe; -void map_flash_registers(struct flashctx *flash); +int map_flash(struct flashctx *flash); +void unmap_flash(struct flashctx *flash); int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int erase_flash(struct flashctx *flash); int probe_flash(struct registered_master *mst, int startchip, struct flashctx *fill_flash, int force); diff --git a/flashrom.c b/flashrom.c index 93b292b..7471cac 100644 --- a/flashrom.c +++ b/flashrom.c @@ -479,6 +479,7 @@ void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t void programmer_unmap_flash_region(void *virt_addr, size_t len) { programmer_table[programmer].unmap_flash_region(virt_addr, len); + msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr); } void chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) @@ -528,14 +529,6 @@ void programmer_delay(unsigned int usecs) programmer_table[programmer].delay(usecs); } -void map_flash_registers(struct flashctx *flash) -{ - size_t size = flash->chip->total_size * 1024; - /* Flash registers live 4 MByte below the flash. */ - /* FIXME: This is incorrect for nonstandard flashbase. */ - flash->virtual_registers = (chipaddr)programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size); -} - int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, int unsigned len) { @@ -1052,12 +1045,64 @@ unsigned int count_max_decode_exceedings(const struct flashctx *flash) return limitexceeded; } +void unmap_flash(struct flashctx *flash) +{ + if (flash->virtual_registers != (chipaddr)ERROR_PTR) { + programmer_unmap_flash_region((void *)flash->virtual_registers, flash->chip->total_size * 1024); + flash->physical_registers = 0; + flash->virtual_registers = (chipaddr)ERROR_PTR; + } + + if (flash->virtual_memory != (chipaddr)ERROR_PTR) { + programmer_unmap_flash_region((void *)flash->virtual_memory, flash->chip->total_size * 1024); + flash->physical_memory = 0; + flash->virtual_memory = (chipaddr)ERROR_PTR; + } +} + +int map_flash(struct flashctx *flash) +{ + /* Init pointers to the fail-safe state to distinguish them later from legit values. */ + flash->virtual_memory = (chipaddr)ERROR_PTR; + flash->virtual_registers = (chipaddr)ERROR_PTR; + + /* FIXME: This avoids mapping (and unmapping) of flash chip definitions with size 0. + * These are used for various probing-related hacks that would not map successfully anyway and should be + * removed ASAP. */ + if (flash->chip->total_size == 0) + return 0; + + const chipsize_t size = flash->chip->total_size * 1024; + uintptr_t base = flashbase ? flashbase : (0xffffffff - size + 1); + void *addr = programmer_map_flash_region(flash->chip->name, base, size); + if (addr == ERROR_PTR) { + msg_perr("Could not map flash chip %s at 0x%0*" PRIxPTR ".\n", + flash->chip->name, PRIxPTR_WIDTH, base); + return 1; + } + flash->physical_memory = base; + flash->virtual_memory = (chipaddr)addr; + + /* FIXME: Special function registers normally live 4 MByte below flash space, but it might be somewhere + * completely different on some chips and programmers, or not mappable at all. + * Ignore these problems for now and always report success. */ + if (flash->chip->feature_bits & FEATURE_REGISTERMAP) { + base = 0xffffffff - size - 0x400000 + 1; + addr = programmer_map_flash_region("flash chip registers", base, size); + if (addr == ERROR_PTR) { + msg_pdbg2("Could not map flash chip registers %s at 0x%0*" PRIxPTR ".\n", + flash->chip->name, PRIxPTR_WIDTH, base); + return 0; + } + flash->physical_registers = base; + flash->virtual_registers = (chipaddr)addr; + } + return 0; +} + int probe_flash(struct registered_master *mst, int startchip, struct flashctx *flash, int force) { const struct flashchip *chip; - unsigned long base = 0; - char location[64]; - uint32_t size; enum chipbustype buses_common; char *tmp; @@ -1082,9 +1127,8 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f memcpy(flash->chip, chip, sizeof(struct flashchip)); flash->mst = mst; - size = flash->chip->total_size * 1024; - base = flashbase ? flashbase : (0xffffffff - size + 1); - flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size); + if (map_flash(flash) != 0) + return -1; /* We handle a forced match like a real match, we just avoid probing. Note that probe_flash() * is only called with force=1 after normal probing failed. @@ -1133,8 +1177,7 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f break; /* Not the first flash chip detected on this bus, and it's just a generic match. Ignore it. */ notfound: - programmer_unmap_flash_region((void *)flash->virtual_memory, size); - flash->virtual_memory = (chipaddr)NULL; + unmap_flash(flash); free(flash->chip); flash->chip = NULL; } @@ -1142,17 +1185,18 @@ notfound: if (!flash->chip) return -1; + + tmp = flashbuses_to_text(flash->chip->bustype); + msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", + flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp); + free(tmp); #if CONFIG_INTERNAL == 1 if (programmer_table[programmer].map_flash_region == physmap) - snprintf(location, sizeof(location), "at physical address 0x%lx", base); + msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n", + PRIxPTR_WIDTH, flash->physical_memory); else #endif - snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name); - - tmp = flashbuses_to_text(flash->chip->bustype); - msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) %s.\n", force ? "Assuming" : "Found", - flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp, location); - free(tmp); + msg_cinfo("on %s.\n", programmer_table[programmer].name); /* Flash registers will not be mapped if the chip was forced. Lock info * may be stored in registers, so avoid lock info printing. @@ -1161,6 +1205,9 @@ notfound: if (flash->chip->printlock) flash->chip->printlock(flash); + /* Get out of the way for later runs. */ + unmap_flash(flash); + /* Return position of matching chip. */ return chip - flashchips; } diff --git a/jedec.c b/jedec.c index 358b850..1345b89 100644 --- a/jedec.c +++ b/jedec.c @@ -166,9 +166,6 @@ int probe_jedec_29gl(struct flashctx *flash) if (man_id != chip->manufacture_id || dev_id != chip->model_id) return 0; - if (chip->feature_bits & FEATURE_REGISTERMAP) - map_flash_registers(flash); - return 1; } @@ -287,9 +284,6 @@ static int probe_jedec_common(struct flashctx *flash, unsigned int mask) if (largeid1 != chip->manufacture_id || largeid2 != chip->model_id) return 0; - if (chip->feature_bits & FEATURE_REGISTERMAP) - map_flash_registers(flash); - return 1; } -- Kind regards, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
