From cddf02f17414c5b284a361424456943cad8dee30 Mon Sep 17 00:00:00 2001 From: Uwe Bonnes Date: Tue, 20 Apr 2021 16:43:37 +0200 Subject: [PATCH 1/4] samx5: Verbose error reports on protected devices. --- src/target/samx5x.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/target/samx5x.c b/src/target/samx5x.c index 3604663a..4569ae4d 100644 --- a/src/target/samx5x.c +++ b/src/target/samx5x.c @@ -519,7 +519,7 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr, target *t = f->t; uint16_t errs = samx5x_read_nvm_error(t); if (errs) { - DEBUG_INFO(NVM_ERROR_BITS_MSG, "erase", addr, len); + DEBUG_WARN(NVM_ERROR_BITS_MSG, "erase", addr, len); samx5x_print_nvm_error(errs); samx5x_clear_nvm_error(t); } @@ -533,11 +533,15 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr, SAMX5X_PAGE_SIZE; lock_region_size = flash_size >> 5; - if (addr < (15 - bootprot) * 8192) - return -1; + if (addr < (15 - bootprot) * 8192) { + DEBUG_WARN("Bootprot\n"); + return -1; + } - if (~runlock & (1 << addr / lock_region_size)) - return -1; + if (~runlock & (1 << addr / lock_region_size)) { + DEBUG_WARN("runlock\n"); + return -1; + } while (len) { target_mem_write32(t, SAMX5X_NVMC_ADDRESS, addr); @@ -553,11 +557,15 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr, /* Poll for NVM Ready */ while ((target_mem_read32(t, SAMX5X_NVMC_STATUS) & SAMX5X_STATUS_READY) == 0) - if (target_check_error(t) || samx5x_check_nvm_error(t)) - return -1; + if (target_check_error(t) || samx5x_check_nvm_error(t)) { + DEBUG_WARN("NVM Ready\n"); + return -1; + } - if (target_check_error(t) || samx5x_check_nvm_error(t)) - return -1; + if (target_check_error(t) || samx5x_check_nvm_error(t)) { + DEBUG_WARN("Error\n"); + return -1; + } /* Lock */ samx5x_lock_current_address(t); From ac7c1057cc72cf6abc64a846ec24a9886e990e28 Mon Sep 17 00:00:00 2001 From: Uwe Bonnes Date: Wed, 21 Apr 2021 12:37:14 +0200 Subject: [PATCH 2/4] efm32/samd/samx5x: Remove static allocates strings. Allocate in priv_storage. Static allocated variables in the different targets eat up common RAM and will collide in chains with multiple similar targets. --- src/target/efm32.c | 23 +++++++++++++++++------ src/target/samd.c | 14 ++++++++++---- src/target/samx5x.c | 16 ++++++++++++---- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/target/efm32.c b/src/target/efm32.c index f8218786..6ad4f32f 100644 --- a/src/target/efm32.c +++ b/src/target/efm32.c @@ -584,7 +584,10 @@ static efm32_device_t const * efm32_get_device(size_t index) /** * Probe */ -static char efm32_variant_string[60]; +struct efm32_priv_s { + char efm32_variant_string[60]; +}; + bool efm32_probe(target *t) { uint8_t di_version = 1; @@ -636,13 +639,17 @@ bool efm32_probe(target *t) uint32_t ram_size = ram_kib * 0x400; uint32_t flash_page_size = device->flash_page_size; - snprintf(efm32_variant_string, sizeof(efm32_variant_string), "%c\b%c\b%s %d F%d %s", + struct efm32_priv_s *priv_storage = calloc(1, sizeof(*priv_storage)); + t->target_storage = (void*)priv_storage; + + snprintf(priv_storage->efm32_variant_string, + sizeof(priv_storage->efm32_variant_string), "%c\b%c\b%s %d F%d %s", di_version + 48, (uint8_t)device_index + 32, device->name, part_number, flash_kib, device->description); /* Setup Target */ t->target_options |= CORTEXM_TOPT_INHIBIT_SRST; - t->driver = efm32_variant_string; + t->driver = priv_storage->efm32_variant_string; tc_printf(t, "flash size %d page size %d\n", flash_size, flash_page_size); target_add_ram (t, SRAM_BASE, ram_size); efm32_add_flash(t, 0x00000000, flash_size, flash_page_size); @@ -980,7 +987,10 @@ static bool nop_function(void) /** * AAP Probe */ -char aap_driver_string[42]; +struct efm32_aap_priv_s { + char aap_driver_string[42]; +}; + void efm32_aap_probe(ADIv5_AP_t *ap) { if ((ap->idr & EFM32_APP_IDR_MASK) == EFM32_AAP_IDR) { @@ -1004,10 +1014,11 @@ void efm32_aap_probe(ADIv5_AP_t *ap) /* Read status */ DEBUG_INFO("EFM32: AAP STATUS=%08"PRIx32"\n", adiv5_ap_read(ap, AAP_STATUS)); - sprintf(aap_driver_string, + struct efm32_aap_priv_s *priv_storage = calloc(1, sizeof(*priv_storage)); + sprintf(priv_storage->aap_driver_string, "EFM32 Authentication Access Port rev.%d", aap_revision); - t->driver = aap_driver_string; + t->driver = priv_storage->aap_driver_string; t->attach = (void*)nop_function; t->detach = (void*)nop_function; t->check_error = (void*)nop_function; diff --git a/src/target/samd.c b/src/target/samd.c index 525a43b7..02879fbd 100644 --- a/src/target/samd.c +++ b/src/target/samd.c @@ -441,7 +441,10 @@ static void samd_add_flash(target *t, uint32_t addr, size_t length) target_add_flash(t, f); } -static char samd_variant_string[60]; +struct samd_priv_s { + char samd_variant_string[60]; +}; + bool samd_probe(target *t) { ADIv5_AP_t *ap = cortexm_ap(t); @@ -460,6 +463,9 @@ bool samd_probe(target *t) if ((did & SAMD_DID_MASK) != SAMD_DID_CONST_VALUE) return false; + struct samd_priv_s *priv_storage = calloc(1, sizeof(*priv_storage)); + t->target_storage = (void*)priv_storage; + uint32_t ctrlstat = target_mem_read32(t, SAMD_DSU_CTRLSTAT); struct samd_descr samd = samd_parse_device_id(did); @@ -468,14 +474,14 @@ bool samd_probe(target *t) /* Part String */ if (protected) { - sprintf(samd_variant_string, + sprintf(priv_storage->samd_variant_string, "Atmel SAM%c%d%c%d%c%s (rev %c) (PROT=1)", samd.family, samd.series, samd.pin, samd.mem, samd.variant, samd.package, samd.revision); } else { - sprintf(samd_variant_string, + sprintf(priv_storage->samd_variant_string, "Atmel SAM%c%d%c%d%c%s (rev %c)", samd.family, samd.series, samd.pin, samd.mem, @@ -484,7 +490,7 @@ bool samd_probe(target *t) } /* Setup Target */ - t->driver = samd_variant_string; + t->driver = priv_storage->samd_variant_string; t->reset = samd_reset; if (samd.series == 20 && samd.revision == 'B') { diff --git a/src/target/samx5x.c b/src/target/samx5x.c index 4569ae4d..d0abff60 100644 --- a/src/target/samx5x.c +++ b/src/target/samx5x.c @@ -344,7 +344,10 @@ static void samx5x_add_flash(target *t, uint32_t addr, size_t length, target_add_flash(t, f); } -static char samx5x_variant_string[60]; +struct samx5x_priv_s { + char samx5x_variant_string[60]; +}; + bool samx5x_probe(target *t) { ADIv5_AP_t *ap = cortexm_ap(t); @@ -370,20 +373,25 @@ bool samx5x_probe(target *t) bool protected = (ctrlstat & SAMX5X_STATUSB_PROT); /* Part String */ + struct samx5x_priv_s *priv_storage = calloc(1, sizeof(*priv_storage)); + t->target_storage = (void*)priv_storage; + if (protected) { - snprintf(samx5x_variant_string, sizeof(samx5x_variant_string), + snprintf(priv_storage->samx5x_variant_string, + sizeof(priv_storage->samx5x_variant_string), "Microchip SAM%c%d%c%dA (rev %c) (PROT=1)", samx5x.series_letter, samx5x.series_number, samx5x.pin, samx5x.mem, samx5x.revision); } else { - snprintf(samx5x_variant_string, sizeof(samx5x_variant_string), + snprintf(priv_storage->samx5x_variant_string, + sizeof(priv_storage->samx5x_variant_string), "Microchip SAM%c%d%c%dA (rev %c)", samx5x.series_letter, samx5x.series_number, samx5x.pin, samx5x.mem, samx5x.revision); } /* Setup Target */ - t->driver = samx5x_variant_string; + t->driver = priv_storage->samx5x_variant_string; t->reset = samx5x_reset; if (protected) { From f87dff8d830531e5cbba4cbde173458c8f806c1c Mon Sep 17 00:00:00 2001 From: Uwe Bonnes Date: Tue, 20 Apr 2021 13:06:57 +0200 Subject: [PATCH 3/4] kinetis: Remove static variables, add word union to target_s. Add a word union to target structure to hold a single word variable. --- src/target/kinetis.c | 20 +++++++++----------- src/target/target_internal.h | 4 ++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/target/kinetis.c b/src/target/kinetis.c index 98f9f764..22551580 100644 --- a/src/target/kinetis.c +++ b/src/target/kinetis.c @@ -74,7 +74,6 @@ #define K64_WRITE_LEN 8 static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]); -static bool unsafe_enabled; const struct command_s kinetis_cmd_list[] = { {"unsafe", (cmd_handler)kinetis_cmd_unsafe, "Allow programming security byte (enable|disable)"}, @@ -85,9 +84,9 @@ static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]) { if (argc == 1) { tc_printf(t, "Allow programming security byte: %s\n", - unsafe_enabled ? "enabled" : "disabled"); + t->unsafe_enabled ? "enabled" : "disabled"); } else { - parse_enable_or_disable(argv[1], &unsafe_enabled); + parse_enable_or_disable(argv[1], &t->unsafe_enabled); } return true; } @@ -336,7 +335,7 @@ bool kinetis_probe(target *t) default: return false; } - unsafe_enabled = false; + t->unsafe_enabled = false; target_add_commands(t, kinetis_cmd_list, t->driver); return true; } @@ -403,7 +402,7 @@ static int kl_gen_flash_write(struct target_flash *f, struct kinetis_flash *kf = (struct kinetis_flash *)f; /* Ensure we don't write something horrible over the security byte */ - if (!unsafe_enabled && + if (!f->t->unsafe_enabled && (dest <= FLASH_SECURITY_BYTE_ADDRESS) && ((dest + len) > FLASH_SECURITY_BYTE_ADDRESS)) { ((uint8_t*)src)[FLASH_SECURITY_BYTE_ADDRESS - dest] = @@ -437,7 +436,7 @@ static int kl_gen_flash_done(struct target_flash *f) { struct kinetis_flash *kf = (struct kinetis_flash *)f; - if (unsafe_enabled) + if (f->t->unsafe_enabled) return 0; if (target_mem_read8(f->t, FLASH_SECURITY_BYTE_ADDRESS) == @@ -527,13 +526,12 @@ void kinetis_mdm_probe(ADIv5_AP_t *ap) /* This is needed as a separate command, as there's no way to * * tell a KE04 from other kinetis in kinetis_mdm_probe() */ -static bool ke04_mode = false; static bool kinetis_mdm_cmd_ke04_mode(target *t, int argc, const char **argv) { (void)argc; (void)argv; /* Set a flag to ignore part of the status and assert reset */ - ke04_mode = true; + t->ke04_mode = true; tc_printf(t, "Mass erase for KE04 now allowed\n"); return true; } @@ -544,7 +542,7 @@ static bool kinetis_mdm_cmd_erase_mass(target *t, int argc, const char **argv) ADIv5_AP_t *ap = t->priv; /* Keep the MCU in reset as stated in KL25PxxM48SF0RM */ - if(ke04_mode) + if(t->ke04_mode) adiv5_ap_write(ap, MDM_CONTROL, MDM_CONTROL_SYS_RESET); uint32_t status, control; @@ -553,13 +551,13 @@ static bool kinetis_mdm_cmd_erase_mass(target *t, int argc, const char **argv) tc_printf(t, "Requesting mass erase (status = 0x%"PRIx32")\n", status); /* This flag does not exist on KE04 */ - if (!(status & MDM_STATUS_MASS_ERASE_ENABLED) && !ke04_mode) { + if (!(status & MDM_STATUS_MASS_ERASE_ENABLED) && !t->ke04_mode) { tc_printf(t, "ERROR: Mass erase disabled!\n"); return false; } /* Flag is not persistent */ - ke04_mode = false; + t->ke04_mode = false; if (!(status & MDM_STATUS_FLASH_READY)) { tc_printf(t, "ERROR: Flash not ready!\n"); diff --git a/src/target/target_internal.h b/src/target/target_internal.h index ebc791f9..17f95800 100644 --- a/src/target/target_internal.h +++ b/src/target/target_internal.h @@ -114,6 +114,10 @@ struct target_s { uint16_t t_designer; uint16_t idcode; void *target_storage; + union { + bool unsafe_enabled; + bool ke04_mode; + }; struct target_ram *ram; struct target_flash *flash; From 1f67bab475cd9dca40336bd233f8ba00730b6eff Mon Sep 17 00:00:00 2001 From: Uwe Bonnes Date: Wed, 21 Apr 2021 12:25:18 +0200 Subject: [PATCH 4/4] nxpke04: Move unsafe_enables to target bool union. --- src/target/nxpke04.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/target/nxpke04.c b/src/target/nxpke04.c index 19fad37a..c577fc2c 100644 --- a/src/target/nxpke04.c +++ b/src/target/nxpke04.c @@ -126,7 +126,6 @@ static int ke04_flash_done(struct target_flash *f); static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]); static bool ke04_cmd_sector_erase(target *t, int argc, char *argv[]); static bool ke04_cmd_mass_erase(target *t, int argc, char *argv[]); -static bool unsafe_enabled; const struct command_s ke_cmd_list[] = { {"unsafe", (cmd_handler)kinetis_cmd_unsafe, "Allow programming security byte (enable|disable)"}, @@ -173,9 +172,9 @@ static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]) { if (argc == 1) { tc_printf(t, "Allow programming security byte: %s\n", - unsafe_enabled ? "enabled" : "disabled"); + t->unsafe_enabled ? "enabled" : "disabled"); } else { - parse_enable_or_disable(argv[1], &unsafe_enabled); + parse_enable_or_disable(argv[1], &t->unsafe_enabled); } return true; } @@ -253,7 +252,6 @@ bool ke04_probe(target *t) target_add_flash(t, f); /* Add target specific commands */ - unsafe_enabled = false; target_add_commands(t, ke_cmd_list, t->driver); return true; @@ -343,7 +341,8 @@ static int ke04_flash_write(struct target_flash *f, target_addr dest, const void *src, size_t len) { /* Ensure we don't write something horrible over the security byte */ - if (!unsafe_enabled && + target *t = f->t; + if (!t->unsafe_enabled && (dest <= FLASH_SECURITY_BYTE_ADDRESS) && ((dest + len) > FLASH_SECURITY_BYTE_ADDRESS)) { ((uint8_t*)src)[FLASH_SECURITY_BYTE_ADDRESS - dest] = @@ -364,7 +363,8 @@ static int ke04_flash_write(struct target_flash *f, static int ke04_flash_done(struct target_flash *f) { - if (unsafe_enabled) + target *t = f->t; + if (t->unsafe_enabled) return 0; if (target_mem_read8(f->t, FLASH_SECURITY_BYTE_ADDRESS) ==