From 495311d2067e85ee1fb53593a4671ef0ec09c6f1 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sun, 7 Apr 2024 12:41:48 +0200 Subject: [PATCH 01/27] doc: style: report indentation of multi-line condition To help readability and discriminate the 'then' block from the multi-line condition, suggest to increase the indentation of the condition. Change-Id: I02e3834be3001e7ecf24349ad3cefe94b27b79c8 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8199 Reviewed-by: Tomas Vanek Tested-by: jenkins Reviewed-by: Oleksij Rempel --- doc/manual/style.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/doc/manual/style.txt b/doc/manual/style.txt index 1d3ec6748d..a1e6b8f016 100644 --- a/doc/manual/style.txt +++ b/doc/manual/style.txt @@ -97,6 +97,21 @@ OpenOCD project. x = 0; } @endcode +- on if statements where the condition is split among multiple + lines, increase the indentation of the condition to prevent it to match + to the indentation of the then block due to length of 'if (' + being same of the TAB width of 4 characters. Use: + @code + if (CMD_ARGC < 3 + || CMD_ARGC > 8) + return ERROR_COMMAND_SYNTAX_ERROR; + @endcode + instead of: + @code + if (CMD_ARGC < 3 || + CMD_ARGC > 8) + return ERROR_COMMAND_SYNTAX_ERROR; + @endcode Finally, try to avoid lines of code that are longer than 72-80 columns: From a9e8ca55a6f44cf6edef7ea5ca66927fa02f3cca Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sun, 7 Apr 2024 12:28:55 +0200 Subject: [PATCH 02/27] jtag: linuxgpiod: minor alignment to coding style Avoid double TAB in 'then' block by increasing indentation of the multi-line condition. Change-Id: I7f5a4437fe4f74228f1b0d98e5c5921af4fd36b8 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8200 Tested-by: jenkins Reviewed-by: Oleksij Rempel --- src/jtag/drivers/linuxgpiod.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/jtag/drivers/linuxgpiod.c b/src/jtag/drivers/linuxgpiod.c index 2f3d644542..1926ed9ae6 100644 --- a/src/jtag/drivers/linuxgpiod.c +++ b/src/jtag/drivers/linuxgpiod.c @@ -378,12 +378,12 @@ static int linuxgpiod_init(void) goto out_error; } - if (helper_get_line(ADAPTER_GPIO_IDX_TDO) != ERROR_OK || - helper_get_line(ADAPTER_GPIO_IDX_TDI) != ERROR_OK || - helper_get_line(ADAPTER_GPIO_IDX_TCK) != ERROR_OK || - helper_get_line(ADAPTER_GPIO_IDX_TMS) != ERROR_OK || - helper_get_line(ADAPTER_GPIO_IDX_TRST) != ERROR_OK) - goto out_error; + if (helper_get_line(ADAPTER_GPIO_IDX_TDO) != ERROR_OK + || helper_get_line(ADAPTER_GPIO_IDX_TDI) != ERROR_OK + || helper_get_line(ADAPTER_GPIO_IDX_TCK) != ERROR_OK + || helper_get_line(ADAPTER_GPIO_IDX_TMS) != ERROR_OK + || helper_get_line(ADAPTER_GPIO_IDX_TRST) != ERROR_OK) + goto out_error; } if (transport_is_swd()) { @@ -413,9 +413,9 @@ static int linuxgpiod_init(void) goto out_error; } - if (helper_get_line(ADAPTER_GPIO_IDX_SRST) != ERROR_OK || - helper_get_line(ADAPTER_GPIO_IDX_LED) != ERROR_OK) - goto out_error; + if (helper_get_line(ADAPTER_GPIO_IDX_SRST) != ERROR_OK + || helper_get_line(ADAPTER_GPIO_IDX_LED) != ERROR_OK) + goto out_error; return ERROR_OK; From ac6b00c3cae5def9c1b3f08fae68703abc494109 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 6 Apr 2024 00:26:38 +0200 Subject: [PATCH 03/27] target: cortex_m: fix display of DWT registers Commit 16b4b8cf5453 ("Cortex-M3: expose most DWT registers") added the DWT registers to the list of CPU registers. The commit message from 2009 reports the reason behind this odd mixing of CPU and DWT registers. This feature got broken in 2017 with the introduction of the field struct reg::exist and its further use in the code. As result, the command 'reg' on a target Cortex-M reports only the core registers and then the header line ===== Cortex-M DWT registers not anymore followed by the DWT registers. Fix it by tagging each DWT registers as existing. Change-Id: Iab026e7da8d6b8ba052514c3fd3b5cdfe301f330 Signed-off-by: Antonio Borneo Fixes: b5964191f0d2 ("register: support non-existent registers") Reviewed-on: https://review.openocd.org/c/openocd/+/8198 Reviewed-by: Tomas Vanek Tested-by: jenkins --- src/target/cortex_m.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c index 4894cabf8b..a26df2e5a2 100644 --- a/src/target/cortex_m.c +++ b/src/target/cortex_m.c @@ -2351,6 +2351,7 @@ static void cortex_m_dwt_addreg(struct target *t, struct reg *r, const struct dw r->value = state->value; r->arch_info = state; r->type = &dwt_reg_type; + r->exist = true; } static void cortex_m_dwt_setup(struct cortex_m_common *cm, struct target *target) From 8667a726532604afab099881779a4b898539dabf Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Sat, 18 Nov 2023 12:48:33 +0100 Subject: [PATCH 04/27] target/target: Add 'debug_reason' to current target Change-Id: Ie35b13b3e06411b4866ffeada47b3262493dbf2e Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8021 Tested-by: jenkins Reviewed-by: Antonio Borneo --- doc/openocd.texi | 12 ++++++++++++ src/target/target.c | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/doc/openocd.texi b/doc/openocd.texi index 52a51c196f..55e6e76808 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9191,6 +9191,18 @@ read_memory 0x20000000 32 2 @end example @end deffn +@deffn {Command} {debug_reason} +Displays the current debug reason: +@code{debug-request}, +@code{breakpoint}, +@code{watchpoint}, +@code{watchpoint-and-breakpoint}, +@code{single-step}, +@code{target-not-halted}, +@code{program-exit}, +@code{exception-catch} or @code{undefined}. +@end deffn + @deffn {Command} {halt} [ms] @deffnx {Command} {wait_halt} [ms] The @command{halt} command first sends a halt request to the target, diff --git a/src/target/target.c b/src/target/target.c index 45698a66c5..5168305dee 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -6736,6 +6736,13 @@ static const struct command_registration target_exec_command_handlers[] = { .help = "Write Tcl list of 8/16/32/64 bit numbers to target memory", .usage = "address width data ['phys']", }, + { + .name = "debug_reason", + .mode = COMMAND_EXEC, + .handler = handle_target_debug_reason, + .help = "displays the debug reason of this target", + .usage = "", + }, { .name = "reset_nag", .handler = handle_target_reset_nag, From 89d881c19ae89712652f17962b8b7d2e79e25ca3 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Mon, 8 Apr 2024 17:40:09 +0200 Subject: [PATCH 05/27] cortex_m: don't try to halt not-examined targets Prevent a segmentation fault by preventing to try to halt a target that has not been examined yet. Change-Id: I5d344e7fbdb5422f7c5e2c39bdd48cbc6c2a3e58 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8213 Tested-by: jenkins --- src/target/cortex_m.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c index a26df2e5a2..c225b1aa9d 100644 --- a/src/target/cortex_m.c +++ b/src/target/cortex_m.c @@ -1109,6 +1109,11 @@ static int cortex_m_halt_one(struct target *target) int retval; LOG_TARGET_DEBUG(target, "target->state: %s", target_state_name(target)); + if (!target_was_examined(target)) { + LOG_TARGET_ERROR(target, "target non examined yet"); + return ERROR_TARGET_NOT_EXAMINED; + } + if (target->state == TARGET_HALTED) { LOG_TARGET_DEBUG(target, "target was already halted"); return ERROR_OK; From 42e31d75b443e369597669b5ff7901de902ad35e Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 13 Apr 2024 18:46:11 +0200 Subject: [PATCH 06/27] target: aarch64: fix regs invalidation when -defer-examine The code for aarch64 allocates the register cache during the very first examine of the target. To prevent a segmentation fault in assert_reset(), the call to register_cache_invalidate() is guarded by target_was_examined(). But for targets with -defer-examine, the target is set as not examined in handle_target_reset() just before entering in assert_reset(). This causes registers to not be invalidated while reset a target examined but with -defer-examine. Change the condition and invalidate the register cache if it has been already allocated. Change-Id: Ie13abb0ae2cc28fc3295d678c4ad1691024eb7b8 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8214 Tested-by: jenkins --- src/target/aarch64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/aarch64.c b/src/target/aarch64.c index 2e4d0b5c03..6a70b2ddf8 100644 --- a/src/target/aarch64.c +++ b/src/target/aarch64.c @@ -1983,7 +1983,7 @@ static int aarch64_assert_reset(struct target *target) } /* registers are now invalid */ - if (target_was_examined(target)) { + if (armv8->arm.core_cache) { register_cache_invalidate(armv8->arm.core_cache); register_cache_invalidate(armv8->arm.core_cache->next); } From c72afedce794a7251fd9c822e3bfc89f870b9fc1 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 13 Apr 2024 18:54:12 +0200 Subject: [PATCH 07/27] target: cortex_a: fix regs invalidation when -defer-examine The code for cortex_a allocates the register cache during the very first examine of the target. To prevent a segmentation fault in assert_reset(), the call to register_cache_invalidate() is guarded by target_was_examined(). But for targets with -defer-examine, the target is set as not examined in handle_target_reset() just before entering in assert_reset(). This causes registers to not be invalidated while reset a target examined but with -defer-examine. Change the condition and invalidate the register cache if it has been already allocated. Change-Id: I81ae782ddce07431d5f2c1bea3e2f19dfcd6d1ce Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8215 Tested-by: jenkins --- src/target/cortex_a.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/cortex_a.c b/src/target/cortex_a.c index 7fa0c4e8b7..78fd4482c3 100644 --- a/src/target/cortex_a.c +++ b/src/target/cortex_a.c @@ -1932,7 +1932,7 @@ static int cortex_a_assert_reset(struct target *target) } /* registers are now invalid */ - if (target_was_examined(target)) + if (armv7a->arm.core_cache) register_cache_invalidate(armv7a->arm.core_cache); target->state = TARGET_RESET; From 3eba7b53bf067508197e2455b81fc1375b1d945e Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Mon, 15 Apr 2024 17:42:12 +0200 Subject: [PATCH 08/27] smp: fix SIGSEGV for "smp off" during target examine The gdb subsystem is initialized after the first target examine, so the field struct target::gdb_service is NULL during examine. A command "smp off" in the examine event handler causes a SIGSEGV during OpenOCD startup. Check for pointer not NULL before dereferencing it. Change-Id: Id115e28be23a957fef1b97ab66d7273f0ea0dce4 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8216 Tested-by: jenkins --- src/target/smp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/smp.c b/src/target/smp.c index 50b19d01a0..41ca880d43 100644 --- a/src/target/smp.c +++ b/src/target/smp.c @@ -119,7 +119,7 @@ COMMAND_HANDLER(default_handle_smp_command) head->target->smp = 0; /* fixes the target display to the debugger */ - if (!list_empty(target->smp_targets)) + if (!list_empty(target->smp_targets) && target->gdb_service) target->gdb_service->target = target; return ERROR_OK; From a84d1b5f5e2754680c12c1595db9d296eec7d45c Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Thu, 22 Feb 2024 12:18:36 -0500 Subject: [PATCH 09/27] tcl/target: Add helpers for booting Xilinx ZynqMP from JTAG Add some helpers for booting ZynqMPs over JTAG. Normally, the CSU ROM will load boot.bin from the boot medium. However, when booting from JTAG we have to do this ourselves. There are generally two parts to this. First, we need to load the PMU firmware. Xilinx's tools do this by attaching to the PMU (a Microblaze CPU) over JTAG. However, the TAP is undocumented and we don't have any microblaze support in-tree. So instead we do it the same way FSBL does it: - We ask the PMU to halt - We load the firmware into the PMU RAM - We ask the PMU to resume The second thing we need to do is start one of the APU cores. When an APU is released from reset, it starts executing at the value of its RVBARADDR. While we could load the APU firmware over the AXI target, it is faster to load it over the APU target. To do this, we put the APU into an infinite loop before halting it. As an aside, I chose to use the "APU" terminology as opposed to "core" to make it clear that these commands operate on the A53 cores and not the R5F cores. Typical usage of these commands could look something like targets uscale.axi boot_pmu /path/to/pmu-firmware.bin boot_apu /path/to/u-boot-spl.bin But of course there is always the option to call lower-level commands individually if your boot process is more unusual. Signed-off-by: Sean Anderson Change-Id: I816940c2022ccca0fabb489aa75d682edd0f6138 Reviewed-on: https://review.openocd.org/c/openocd/+/8133 Reviewed-by: Antonio Borneo Tested-by: jenkins --- tcl/target/xilinx_zynqmp.cfg | 134 +++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/tcl/target/xilinx_zynqmp.cfg b/tcl/target/xilinx_zynqmp.cfg index 9734a18376..e8224be043 100644 --- a/tcl/target/xilinx_zynqmp.cfg +++ b/tcl/target/xilinx_zynqmp.cfg @@ -104,3 +104,137 @@ proc core_up { args } { $_TARGETNAME.$core arp_examine } } + +proc BIT {n} { + return [expr {1 << $n}] +} + +set IPI_BASE 0xff300000 +set IPI_PMU_0_TRIG [expr {$IPI_BASE + 0x30000}] +set IPI_PMU_0_IER [expr {$IPI_BASE + 0x30018}] +set IPI_PMU_0 [BIT 16] + +set CRF_APB_BASE 0xfd1a0000 +set CRF_APB_RST_FPD_APU [expr {$CRF_APB_BASE + 0x104}] +set CRF_APB_RST_FPD_APU_ACPU0_PWRON_RESET [BIT 10] +set CRF_APB_RST_FPD_APU_L2_RESET [BIT 8] +set CRF_APB_RST_FPD_APU_ACPU0_RESET [BIT 0] + +set APU_BASE 0xfd5c0000 +set APU_RVBARADDR_BASE [expr {$APU_BASE + 0x40}] + +set PMU_BASE 0xffd80000 +set PMU_GLOBAL $PMU_BASE +set PMU_GLOBAL_MB_SLEEP [BIT 16] +set PMU_GLOBAL_FW_IS_PRESENT [BIT 4] +set PMU_GLOBAL_DONT_SLEEP [BIT 0] + +set PMU_RAM_BASE 0xffdc0000 + +set OCM_RAM_BASE 0xfffc0000 + +rename BIT {} + +add_help_text halt_pmu "Halt the PMU in preparation for loading new firmware.\ + This should be matched with a call to resume_pmu." +proc halt_pmu {} { + set axi $::_CHIPNAME.axi + set val [$axi read_memory $::IPI_PMU_0_IER 32 1] + $axi write_memory $::IPI_PMU_0_IER 32 [expr {$val | $::IPI_PMU_0}] + + set val [$axi read_memory $::IPI_PMU_0_TRIG 32 1] + $axi write_memory $::IPI_PMU_0_TRIG 32 [expr {$val | $::IPI_PMU_0}] + + set start [ms] + while {!([$axi read_memory $::PMU_GLOBAL 32 1] & $::PMU_GLOBAL_MB_SLEEP)} { + if {[ms] - $start > 1000} { + error "Timed out waiting for PMU to halt" + } + } +} + +add_help_text resume_pmu "Resume the PMU after loading new firmware. This\ + should be matched with a call to halt_pmu." +proc resume_pmu {} { + set axi $::_CHIPNAME.axi + set val [$axi read_memory $::PMU_GLOBAL 32 1] + $axi write_memory $::PMU_GLOBAL 32 [expr {$val | $::PMU_GLOBAL_DONT_SLEEP}] + + set start [ms] + while {!([$axi read_memory $::PMU_GLOBAL 32 1] & $::PMU_GLOBAL_FW_IS_PRESENT)} { + if {[ms] - $start > 5000} { + error "Timed out waiting for PMU firmware" + } + } +} + +add_usage_text release_apu {apu} +add_help_text release_apu "Release an APU from reset. It will start executing\ + at RVBARADDR. You probably want resume_apu or start_apu instead." +proc release_apu {apu} { + set axi $::_CHIPNAME.axi + set val [$axi read_memory $::CRF_APB_RST_FPD_APU 32 1] + set mask [expr { + (($::CRF_APB_RST_FPD_APU_ACPU0_PWRON_RESET | \ + $::CRF_APB_RST_FPD_APU_ACPU0_RESET) << $apu) | \ + $::CRF_APB_RST_FPD_APU_L2_RESET + }] + $axi write_memory $::CRF_APB_RST_FPD_APU 32 [expr {$val & ~$mask}] + + core_up $apu + $::_TARGETNAME.$apu aarch64 dbginit +} + +proc _rvbaraddr {apu} { + return [expr {$::APU_RVBARADDR_BASE + 8 * $apu}] +} + +add_usage_text resume_apu {apu addr} +add_help_text resume_apu "Resume an APU at a given address." +proc resume_apu {apu addr} { + set addrl [expr {$addr & 0xffffffff}] + set addrh [expr {$addr >> 32}] + $::_CHIPNAME.axi write_memory [_rvbaraddr $apu] 32 [list $addrl $addrh] + + release_apu $apu +} + +add_usage_text start_apu {apu} +add_help_text start_apu "Start an APU and put it into an infinite loop at\ + RVBARADDR. This can be convenient if you just want to halt the APU\ + (since it won't execute anything unusual)." +proc start_apu {apu} { + set axi $::_CHIPNAME.axi + foreach {addrl addrh} [$axi read_memory [_rvbaraddr $apu] 32 2] { + set addr [expr {($addrh << 32) | $addrl}] + } + # write the infinite loop instruction + $axi write_memory $addr 32 0x14000000 + + release_apu $apu +} + +add_usage_text boot_pmu {image} +add_help_text boot_pmu "Boot the PMU with a given firmware image, loading it\ + to the beginning of PMU RAM. The PMU ROM will jump to this location\ + after we resume it." +proc boot_pmu {image} { + halt_pmu + echo "Info : Loading PMU firmware $image to $::PMU_RAM_BASE" + load_image $image $::PMU_RAM_BASE + resume_pmu +} + +add_usage_text boot_apu "image \[apu=0 \[addr=$OCM_RAM_BASE\]\]" +add_help_text boot_apu "Boot an APU with a given firmware image. The default\ + address is the beginning of OCM RAM. Upon success, the default target\ + will be changed to the (running) apu." +proc boot_apu [list image {apu 0} [list addr $OCM_RAM_BASE]] { + start_apu $apu + targets $::_TARGETNAME.$apu + halt + + echo "Info : Loading APU$apu firmware $image to $addr" + load_image $image $addr + resume $addr +} From bc9ca5f4a82ccbbdbe07108a83f7979b53e89889 Mon Sep 17 00:00:00 2001 From: Daniel Anselmi Date: Sun, 17 Mar 2024 01:15:16 +0100 Subject: [PATCH 10/27] ipdbg: fix double free of virtual-ir data Fix possible double free and possible memory leak while creating an ipdbg hub. Change-Id: I6254663c27c4f38d46008c4dbff11aa27b84f399 Signed-off-by: Daniel Anselmi Reviewed-on: https://review.openocd.org/c/openocd/+/8085 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/server/ipdbg.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/server/ipdbg.c b/src/server/ipdbg.c index e7eb96e13c..859fdb035e 100644 --- a/src/server/ipdbg.c +++ b/src/server/ipdbg.c @@ -285,6 +285,7 @@ static struct ipdbg_hub *ipdbg_allocate_hub(uint8_t data_register_length, struct { struct ipdbg_hub *new_hub = calloc(1, sizeof(struct ipdbg_hub)); if (!new_hub) { + free(virtual_ir); LOG_ERROR("Out of memory"); return NULL; } @@ -292,6 +293,7 @@ static struct ipdbg_hub *ipdbg_allocate_hub(uint8_t data_register_length, struct new_hub->name = strdup(name); if (!new_hub->name) { free(new_hub); + free(virtual_ir); LOG_ERROR("Out of memory"); return NULL; } @@ -304,8 +306,10 @@ static struct ipdbg_hub *ipdbg_allocate_hub(uint8_t data_register_length, struct new_hub->scratch_memory.fields = calloc(IPDBG_SCRATCH_MEMORY_SIZE, sizeof(struct scan_field)); new_hub->connections = calloc(max_tools, sizeof(struct connection *)); - if (virtual_ir) + if (virtual_ir) { + new_hub->virtual_ir = virtual_ir; new_hub->scratch_memory.vir_out_val = calloc(1, DIV_ROUND_UP(virtual_ir->length, 8)); + } if (!new_hub->scratch_memory.dr_out_vals || !new_hub->scratch_memory.dr_in_vals || !new_hub->scratch_memory.fields || (virtual_ir && !new_hub->scratch_memory.vir_out_val) || @@ -997,7 +1001,6 @@ static int ipdbg_create_hub(struct jtag_tap *tap, uint32_t user_instruction, uin new_hub->xoff_mask = BIT(data_register_length - 2); new_hub->tool_mask = (new_hub->xoff_mask - 1) >> 8; new_hub->last_dn_tool = new_hub->tool_mask; - new_hub->virtual_ir = virtual_ir; new_hub->max_tools = ipdbg_max_tools_from_data_register_length(data_register_length); new_hub->using_queue_size = IPDBG_SCRATCH_MEMORY_SIZE; @@ -1123,11 +1126,7 @@ COMMAND_HANDLER(handle_ipdbg_create_hub_command) return ERROR_FAIL; } - int retval = ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, hub_name, cmd); - if (retval != ERROR_OK) - free(virtual_ir); - - return retval; + return ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, hub_name, cmd); } static const struct command_registration ipdbg_config_command_handlers[] = { From dbef02789fb1f30ae77020754b1468549fd16510 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Thu, 2 May 2024 15:24:57 +0200 Subject: [PATCH 11/27] cortex_a: drop useless cache invalidate on mem write The initial OpenOCD code for Cortex-A (ARMv7a) [1] was merged in 2009 but, due to lack of public documentation for ARMv7a, it was almost a simple copy/paste from the existing code for Cortex-M (ARMv7m). On Cortex-M the same AP provides access to both CPU debug and CPU memory. This feature is not present on ARMv7a. To still keep some communality with ARMv7m code, the change [2] splits the CPU debug access from the CPU memory access by using two independent AP; this is copied from the system architecture of TI OMAP3530 which provides to DAP a direct AHB-AP memory bus on AP#0, separated from AP#1 for the APB-AP CPU debug. But the direct memory access through the system bus breaks the coherency between memory and CPU caches, so change [3] added some cache invalidation to avoid issues. The code to allow ARMv7a CPU to really read/write in CPU memory was added by change [4] in 2011. Such still not optimized implementation was very slow, so it did not replace the access through the system bus. A selection through DAP's 'apsel" command was used to select between the two modes. Only in 2015, with change [5], the speed of CPU read/write was improved using the DCC_FAST_MODE. But the direct access to the memory through the system bus remained. Finally, with change [6] in 2018 the system bus access was dropped for good, as the new virtual target "mem_ap" could implement such access in a more clean way. Only memory access through CPU remained for ARMv7a. Nevertheless, a useless cache invalidation remained in the code, decreasing the speed of the write access. Drop the useless cache invalidate on CPU memory write and the associated comment, not anymore valid. Drop the now unused function armv7a_cache_auto_flush_on_write(). This provides a speedup of between 4 and 8, depending on adapter and JTAG/SWD speed. Link: [1] 7a93100c2dfe ("Add minimalist Cortex A8 file") Link: [2] 1d0b276c9f75 ("The rest of the Cortex-A8 support from Magnus: ...") Link: [3] d4e4d65d284f ("Cache invalidation when writing to memory") Link: [4] 05ab8bdb813a ("cortex_a9: implement read/write memory through APB-AP") Link: [5] 0228f8e8274d ("Cortex A: fix extra memory read and non-word sizes") Link: [6] fac9be64d944 ("target/cortex_a: remove buggy memory AP accesses") Change-Id: Ifa3c7ddf2698b2c87037fb48f783844034a7140e Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8229 Tested-by: jenkins Reviewed-by: Oleksij Rempel --- src/target/armv7a_cache.c | 21 --------------------- src/target/armv7a_cache.h | 2 -- src/target/cortex_a.c | 3 --- 3 files changed, 26 deletions(-) diff --git a/src/target/armv7a_cache.c b/src/target/armv7a_cache.c index e1f0dfafb0..681c06affc 100644 --- a/src/target/armv7a_cache.c +++ b/src/target/armv7a_cache.c @@ -391,27 +391,6 @@ int armv7a_cache_flush_virt(struct target *target, uint32_t virt, return ERROR_OK; } -/* - * We assume that target core was chosen correctly. It means if same data - * was handled by two cores, other core will loose the changes. Since it - * is impossible to know (FIXME) which core has correct data, keep in mind - * that some kind of data lost or corruption is possible. - * Possible scenario: - * - core1 loaded and changed data on 0x12345678 - * - we halted target and modified same data on core0 - * - data on core1 will be lost. - */ -int armv7a_cache_auto_flush_on_write(struct target *target, uint32_t virt, - uint32_t size) -{ - struct armv7a_common *armv7a = target_to_armv7a(target); - - if (!armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled) - return ERROR_OK; - - return armv7a_cache_flush_virt(target, virt, size); -} - COMMAND_HANDLER(arm7a_l1_cache_info_cmd) { struct target *target = get_current_target(CMD_CTX); diff --git a/src/target/armv7a_cache.h b/src/target/armv7a_cache.h index 17ec5e6de1..3e3eae55c5 100644 --- a/src/target/armv7a_cache.h +++ b/src/target/armv7a_cache.h @@ -20,8 +20,6 @@ int armv7a_l1_d_cache_flush_virt(struct target *target, uint32_t virt, int armv7a_l1_i_cache_inval_all(struct target *target); int armv7a_l1_i_cache_inval_virt(struct target *target, uint32_t virt, uint32_t size); -int armv7a_cache_auto_flush_on_write(struct target *target, uint32_t virt, - uint32_t size); int armv7a_cache_auto_flush_all_data(struct target *target); int armv7a_cache_flush_virt(struct target *target, uint32_t virt, uint32_t size); diff --git a/src/target/cortex_a.c b/src/target/cortex_a.c index 78fd4482c3..f90c02a958 100644 --- a/src/target/cortex_a.c +++ b/src/target/cortex_a.c @@ -2791,9 +2791,6 @@ static int cortex_a_write_memory(struct target *target, target_addr_t address, LOG_DEBUG("Writing memory at address " TARGET_ADDR_FMT "; size %" PRIu32 "; count %" PRIu32, address, size, count); - /* memory writes bypass the caches, must flush before writing */ - armv7a_cache_auto_flush_on_write(target, address, size * count); - cortex_a_prep_memaccess(target, 0); retval = cortex_a_write_cpu_memory(target, address, size, count, buffer); cortex_a_post_memaccess(target, 0); From caabdd4a6627a86be43cd91e09f127173eb9e692 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 4 May 2024 20:09:51 +0200 Subject: [PATCH 12/27] cortex_a: drop the command 'cache auto' The command 'cache auto' was introduced with commit cd440bd32a12 ("add armv7a_cache handlers") in 2015 to allow disabling the cache handling done automatically by OpenOCD. This was probably a way to test the cache handling when there were still the two independent accesses for APB-AP CPU debug and for AHB-AP memory bus. The handling of cache for cortex_a is robust and there is no more reason to disable it. The command 'cache auto' is not used in any upstream script. On target aarch64 this command has never been introduced as the cache is always handled automatically by OpenOCD. Drop the command 'cache auto' and add it in the deprecated list. Drop the flag 'auto_cache_enabled' by considering it as true. Rename the function 'armv7a_cache_auto_flush_all_data()' as 'armv7a_cache_flush_all_data()' and, while there, fix the error propagation in SMP case. Change-Id: I0399f1081b08c4929e0795b76f4a686630f41d56 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8230 Tested-by: jenkins Reviewed-by: Oleksij Rempel --- src/target/armv7a.c | 3 +-- src/target/armv7a.h | 2 -- src/target/armv7a_cache.c | 42 ++++++--------------------------------- src/target/armv7a_cache.h | 2 +- src/target/cortex_a.c | 8 ++------ src/target/startup.tcl | 22 ++++++++++++++++++++ 6 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/target/armv7a.c b/src/target/armv7a.c index 82f4be5a0b..dc3752e0b0 100644 --- a/src/target/armv7a.c +++ b/src/target/armv7a.c @@ -473,7 +473,7 @@ int armv7a_identify_cache(struct target *target) /* if no l2 cache initialize l1 data cache flush function function */ if (!armv7a->armv7a_mmu.armv7a_cache.flush_all_data_cache) { armv7a->armv7a_mmu.armv7a_cache.flush_all_data_cache = - armv7a_cache_auto_flush_all_data; + armv7a_cache_flush_all_data; } armv7a->armv7a_mmu.armv7a_cache.info = 1; @@ -525,7 +525,6 @@ int armv7a_init_arch_info(struct target *target, struct armv7a_common *armv7a) armv7a->armv7a_mmu.armv7a_cache.info = -1; armv7a->armv7a_mmu.armv7a_cache.outer_cache = NULL; armv7a->armv7a_mmu.armv7a_cache.flush_all_data_cache = NULL; - armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled = 1; return ERROR_OK; } diff --git a/src/target/armv7a.h b/src/target/armv7a.h index 6b9c2a68f4..8943f1c697 100644 --- a/src/target/armv7a.h +++ b/src/target/armv7a.h @@ -65,8 +65,6 @@ struct armv7a_cache_common { struct armv7a_arch_cache arch[6]; /* cache info, L1 - L7 */ int i_cache_enabled; int d_u_cache_enabled; - int auto_cache_enabled; /* openocd automatic - * cache handling */ /* outer unified cache if some */ void *outer_cache; int (*flush_all_data_cache)(struct target *target); diff --git a/src/target/armv7a_cache.c b/src/target/armv7a_cache.c index 681c06affc..f2f1097c5a 100644 --- a/src/target/armv7a_cache.c +++ b/src/target/armv7a_cache.c @@ -118,20 +118,19 @@ static int armv7a_l1_d_cache_clean_inval_all(struct target *target) return retval; } -int armv7a_cache_auto_flush_all_data(struct target *target) +int armv7a_cache_flush_all_data(struct target *target) { int retval = ERROR_FAIL; - struct armv7a_common *armv7a = target_to_armv7a(target); - - if (!armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled) - return ERROR_OK; if (target->smp) { struct target_list *head; foreach_smp_target(head, target->smp_targets) { struct target *curr = head->target; - if (curr->state == TARGET_HALTED) - retval = armv7a_l1_d_cache_clean_inval_all(curr); + if (curr->state == TARGET_HALTED) { + int retval1 = armv7a_l1_d_cache_clean_inval_all(curr); + if (retval1 != ERROR_OK) + retval = retval1; + } } } else retval = armv7a_l1_d_cache_clean_inval_all(target); @@ -472,28 +471,6 @@ COMMAND_HANDLER(arm7a_l1_i_cache_inval_virt_cmd) return armv7a_l1_i_cache_inval_virt(target, virt, size); } -COMMAND_HANDLER(arm7a_cache_disable_auto_cmd) -{ - struct target *target = get_current_target(CMD_CTX); - struct armv7a_common *armv7a = target_to_armv7a(target); - - if (CMD_ARGC == 0) { - command_print(CMD, "auto cache is %s", - armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled ? "enabled" : "disabled"); - return ERROR_OK; - } - - if (CMD_ARGC == 1) { - uint32_t set; - - COMMAND_PARSE_ENABLE(CMD_ARGV[0], set); - armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled = !!set; - return ERROR_OK; - } - - return ERROR_COMMAND_SYNTAX_ERROR; -} - static const struct command_registration arm7a_l1_d_cache_commands[] = { { .name = "flush_all", @@ -563,13 +540,6 @@ static const struct command_registration arm7a_l1_di_cache_group_handlers[] = { }; static const struct command_registration arm7a_cache_group_handlers[] = { - { - .name = "auto", - .handler = arm7a_cache_disable_auto_cmd, - .mode = COMMAND_ANY, - .help = "disable or enable automatic cache handling.", - .usage = "(1|0)", - }, { .name = "l1", .mode = COMMAND_ANY, diff --git a/src/target/armv7a_cache.h b/src/target/armv7a_cache.h index 3e3eae55c5..c4637c5d30 100644 --- a/src/target/armv7a_cache.h +++ b/src/target/armv7a_cache.h @@ -20,7 +20,7 @@ int armv7a_l1_d_cache_flush_virt(struct target *target, uint32_t virt, int armv7a_l1_i_cache_inval_all(struct target *target); int armv7a_l1_i_cache_inval_virt(struct target *target, uint32_t virt, uint32_t size); -int armv7a_cache_auto_flush_all_data(struct target *target); +int armv7a_cache_flush_all_data(struct target *target); int armv7a_cache_flush_virt(struct target *target, uint32_t virt, uint32_t size); extern const struct command_registration arm7a_cache_command_handlers[]; diff --git a/src/target/cortex_a.c b/src/target/cortex_a.c index f90c02a958..a235404632 100644 --- a/src/target/cortex_a.c +++ b/src/target/cortex_a.c @@ -1362,10 +1362,8 @@ static int cortex_a_set_breakpoint(struct target *target, return retval; /* make sure data cache is cleaned & invalidated down to PoC */ - if (!armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled) { - armv7a_cache_flush_virt(target, breakpoint->address, + armv7a_cache_flush_virt(target, breakpoint->address, breakpoint->length); - } retval = target_write_memory(target, breakpoint->address & 0xFFFFFFFE, @@ -1600,10 +1598,8 @@ static int cortex_a_unset_breakpoint(struct target *target, struct breakpoint *b } else { /* make sure data cache is cleaned & invalidated down to PoC */ - if (!armv7a->armv7a_mmu.armv7a_cache.auto_cache_enabled) { - armv7a_cache_flush_virt(target, breakpoint->address, + armv7a_cache_flush_virt(target, breakpoint->address, breakpoint->length); - } /* restore original instruction (kept in target endianness) */ if (breakpoint->length == 4) { diff --git a/src/target/startup.tcl b/src/target/startup.tcl index 75e0edc77d..e9646097f0 100644 --- a/src/target/startup.tcl +++ b/src/target/startup.tcl @@ -294,3 +294,25 @@ proc "mips_m4k smp_off" {args} { echo "DEPRECATED! use 'mips_m4k smp off' not 'mips_m4k smp_off'" eval mips_m4k smp off $args } + +lappend _telnet_autocomplete_skip _post_init_target_cortex_a_cache_auto +proc _post_init_target_cortex_a_cache_auto {} { + set cortex_a_found 0 + + foreach t [target names] { + if { [$t cget -type] != "cortex_a" } { continue } + set cortex_a_found 1 + lappend ::_telnet_autocomplete_skip "$t cache auto" + proc "$t cache auto" { enable } { + echo "DEPRECATED! Don't use anymore '[dict get [info frame 0] proc] $enable' as it's always enabled" + } + } + + if { $cortex_a_found } { + lappend ::_telnet_autocomplete_skip "cache auto" + proc "cache auto" { enable } { + echo "DEPRECATED! Don't use anymore 'cache auto $enable' as it's always enabled" + } + } +} +lappend post_init_commands _post_init_target_cortex_a_cache_auto From 126d8a0972ab6031588eb2f0e1fc3957bd3ccc6b Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sat, 4 May 2024 19:13:51 +0200 Subject: [PATCH 13/27] cortex_a: drop cortex_a_dap_write_memap_register_u32() Historically, the function cortex_a_dap_write_memap_register_u32() was used to discriminate the register write in APB-AP CPU debug against the complex memory access in AHB-AP memory bus. It has no sense to keep the function and its comment. Plus, by forcing atomic write it impacts the debug performance. Drop it! A further rework to enqueue sequence of atomic writes is needed. Change-Id: I2f5e9015f0e27fa5a6d8337a1ae25e753e2e1d26 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8231 Reviewed-by: Oleksij Rempel Tested-by: jenkins --- src/target/cortex_a.c | 91 +++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 52 deletions(-) diff --git a/src/target/cortex_a.c b/src/target/cortex_a.c index a235404632..2de77c9602 100644 --- a/src/target/cortex_a.c +++ b/src/target/cortex_a.c @@ -314,19 +314,6 @@ static int cortex_a_exec_opcode(struct target *target, return retval; } -/* Write to memory mapped registers directly with no cache or mmu handling */ -static int cortex_a_dap_write_memap_register_u32(struct target *target, - uint32_t address, - uint32_t value) -{ - int retval; - struct armv7a_common *armv7a = target_to_armv7a(target); - - retval = mem_ap_write_atomic_u32(armv7a->debug_ap, address, value); - - return retval; -} - /* * Cortex-A implementation of Debug Programmer's Model * @@ -611,11 +598,11 @@ static int cortex_a_bpwp_enable(struct arm_dpm *dpm, unsigned index_t, LOG_DEBUG("A: bpwp enable, vr %08x cr %08x", (unsigned) vr, (unsigned) cr); - retval = cortex_a_dap_write_memap_register_u32(dpm->arm->target, + retval = mem_ap_write_atomic_u32(a->armv7a_common.debug_ap, vr, addr); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(dpm->arm->target, + retval = mem_ap_write_atomic_u32(a->armv7a_common.debug_ap, cr, control); return retval; } @@ -641,7 +628,7 @@ static int cortex_a_bpwp_disable(struct arm_dpm *dpm, unsigned index_t) LOG_DEBUG("A: bpwp disable, cr %08x", (unsigned) cr); /* clear control register */ - return cortex_a_dap_write_memap_register_u32(dpm->arm->target, cr, 0); + return mem_ap_write_atomic_u32(a->armv7a_common.debug_ap, cr, 0); } static int cortex_a_dpm_setup(struct cortex_a_common *a, uint32_t didr) @@ -1323,13 +1310,13 @@ static int cortex_a_set_breakpoint(struct target *target, brp_list[brp_i].used = true; brp_list[brp_i].value = (breakpoint->address & 0xFFFFFFFC); brp_list[brp_i].control = control; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].value); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].control); if (retval != ERROR_OK) return retval; @@ -1415,13 +1402,13 @@ static int cortex_a_set_context_breakpoint(struct target *target, brp_list[brp_i].used = true; brp_list[brp_i].value = (breakpoint->asid); brp_list[brp_i].control = control; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].value); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].control); if (retval != ERROR_OK) return retval; @@ -1481,13 +1468,13 @@ static int cortex_a_set_hybrid_breakpoint(struct target *target, struct breakpoi brp_list[brp_1].used = true; brp_list[brp_1].value = (breakpoint->asid); brp_list[brp_1].control = control_ctx; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_1].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_1].brpn, brp_list[brp_1].value); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_1].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_1].brpn, brp_list[brp_1].control); if (retval != ERROR_OK) return retval; @@ -1499,13 +1486,13 @@ static int cortex_a_set_hybrid_breakpoint(struct target *target, struct breakpoi brp_list[brp_2].used = true; brp_list[brp_2].value = (breakpoint->address & 0xFFFFFFFC); brp_list[brp_2].control = control_iva; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_2].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_2].brpn, brp_list[brp_2].value); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_2].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_2].brpn, brp_list[brp_2].control); if (retval != ERROR_OK) return retval; @@ -1538,13 +1525,13 @@ static int cortex_a_unset_breakpoint(struct target *target, struct breakpoint *b brp_list[brp_i].used = false; brp_list[brp_i].value = 0; brp_list[brp_i].control = 0; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].control); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].value); if (retval != ERROR_OK) return retval; @@ -1557,13 +1544,13 @@ static int cortex_a_unset_breakpoint(struct target *target, struct breakpoint *b brp_list[brp_j].used = false; brp_list[brp_j].value = 0; brp_list[brp_j].control = 0; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_j].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_j].brpn, brp_list[brp_j].control); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_j].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_j].brpn, brp_list[brp_j].value); if (retval != ERROR_OK) return retval; @@ -1582,13 +1569,13 @@ static int cortex_a_unset_breakpoint(struct target *target, struct breakpoint *b brp_list[brp_i].used = false; brp_list[brp_i].value = 0; brp_list[brp_i].control = 0; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BCR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].control); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_BVR_BASE + 4 * brp_list[brp_i].brpn, brp_list[brp_i].value); if (retval != ERROR_OK) return retval; @@ -1783,14 +1770,14 @@ static int cortex_a_set_watchpoint(struct target *target, struct watchpoint *wat wrp_list[wrp_i].value = address; wrp_list[wrp_i].control = control; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_WVR_BASE + 4 * wrp_list[wrp_i].wrpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_WVR_BASE + 4 * wrp_list[wrp_i].wrpn, wrp_list[wrp_i].value); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_WCR_BASE + 4 * wrp_list[wrp_i].wrpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_WCR_BASE + 4 * wrp_list[wrp_i].wrpn, wrp_list[wrp_i].control); if (retval != ERROR_OK) return retval; @@ -1832,13 +1819,13 @@ static int cortex_a_unset_watchpoint(struct target *target, struct watchpoint *w wrp_list[wrp_i].used = false; wrp_list[wrp_i].value = 0; wrp_list[wrp_i].control = 0; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_WCR_BASE + 4 * wrp_list[wrp_i].wrpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_WCR_BASE + 4 * wrp_list[wrp_i].wrpn, wrp_list[wrp_i].control); if (retval != ERROR_OK) return retval; - retval = cortex_a_dap_write_memap_register_u32(target, armv7a->debug_base - + CPUDBG_WVR_BASE + 4 * wrp_list[wrp_i].wrpn, + retval = mem_ap_write_atomic_u32(armv7a->debug_ap, + armv7a->debug_base + CPUDBG_WVR_BASE + 4 * wrp_list[wrp_i].wrpn, wrp_list[wrp_i].value); if (retval != ERROR_OK) return retval; From 22ddf62d759b10a89e9c4c3f3929a66c9d72572d Mon Sep 17 00:00:00 2001 From: Parshintsev Anatoly Date: Fri, 3 May 2024 00:28:33 +0300 Subject: [PATCH 14/27] gdb_server: enable keep-alive packets for qCRC packet Change-Id: Ia384179bb83ad6b70bf385cc9d575e9ec58f76c7 Signed-off-by: Parshintsev Anatoly Reviewed-on: https://review.openocd.org/c/openocd/+/8227 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/server/gdb_server.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index c1ee4aa2a1..dfd7cd5201 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -2845,7 +2845,9 @@ static int gdb_query_packet(struct connection *connection, len = strtoul(separator + 1, NULL, 16); + gdb_connection->output_flag = GDB_OUTPUT_NOTIF; retval = target_checksum_memory(target, addr, len, &checksum); + gdb_connection->output_flag = GDB_OUTPUT_NO; if (retval == ERROR_OK) { snprintf(gdb_reply, 10, "C%8.8" PRIx32 "", checksum); From 2c8376b79d104a855bd3a559e59edf330309bcad Mon Sep 17 00:00:00 2001 From: Ian Thompson Date: Wed, 31 Jan 2024 15:14:25 -0800 Subject: [PATCH 15/27] target/xtensa: avoid IHI for writes to non-executable memory For MPU configs, determine memory access rights by probing protection TLB. Issuing IHI without execute permissions can trigger an exception. No new clang static analyzer warnings. Change-Id: Iea8eab5c2113df3f954285c3b9a79e96d41aa941 Signed-off-by: Ian Thompson Reviewed-on: https://review.openocd.org/c/openocd/+/8080 Reviewed-by: Erhan Kurubas Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/target/xtensa/xtensa.c | 89 ++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 8 deletions(-) diff --git a/src/target/xtensa/xtensa.c b/src/target/xtensa/xtensa.c index fb7748aa2d..f7c82efed4 100644 --- a/src/target/xtensa/xtensa.c +++ b/src/target/xtensa/xtensa.c @@ -158,6 +158,12 @@ #define XT_INS_RFWU(X) (XT_ISBE(X) ? 0x005300 << 8 : 0x003500) #define XT_INS_RFWO_RFWU_MASK(X) (XT_ISBE(X) ? 0xFFFFFF << 8 : 0xFFFFFF) +/* Read Protection TLB Entry Info */ +#define XT_INS_PPTLB(X, S, T) _XT_INS_FORMAT_RRR(X, 0x500000, ((S) << 4) | (T), 0xD) + +#define XT_TLB1_ACC_SHIFT 8 +#define XT_TLB1_ACC_MSK 0xF + #define XT_WATCHPOINTS_NUM_MAX 2 /* Special register number macro for DDR, PS, WB, A3, A4 registers. @@ -298,6 +304,27 @@ enum xtensa_mem_region_type { XTENSA_MEM_REGS_NUM }; +/** + * Types of access rights for MPU option + * The first block is kernel RWX ARs; the second block is user rwx ARs. + */ +enum xtensa_mpu_access_type { + XTENSA_ACC_00X_000 = 0x2, + XTENSA_ACC_000_00X, + XTENSA_ACC_R00_000, + XTENSA_ACC_R0X_000, + XTENSA_ACC_RW0_000, + XTENSA_ACC_RWX_000, + XTENSA_ACC_0W0_0W0, + XTENSA_ACC_RW0_RWX, + XTENSA_ACC_RW0_R00, + XTENSA_ACC_RWX_R0X, + XTENSA_ACC_R00_R00, + XTENSA_ACC_R0X_R0X, + XTENSA_ACC_RW0_RW0, + XTENSA_ACC_RWX_RWX +}; + /* Register definition as union for list allocation */ union xtensa_reg_val_u { xtensa_reg_val_t val; @@ -521,6 +548,44 @@ static void xtensa_queue_exec_ins_wide(struct xtensa *xtensa, uint8_t *ops, uint } } +/* NOTE: Assumes A3 has already been saved and marked dirty; A3 will be clobbered */ +static inline bool xtensa_region_ar_exec(struct target *target, target_addr_t start, target_addr_t end) +{ + struct xtensa *xtensa = target_to_xtensa(target); + if (xtensa->core_config->mpu.enabled) { + /* For cores with the MPU option, issue PPTLB on start and end addresses. + * Parse access rights field, and confirm both have execute permissions. + */ + for (int i = 0; i <= 1; i++) { + uint32_t at, acc; + uint8_t at_buf[4]; + bool exec_acc; + target_addr_t addr = i ? end : start; + xtensa_queue_dbg_reg_write(xtensa, XDMREG_DDR, addr); + xtensa_queue_exec_ins(xtensa, XT_INS_RSR(xtensa, XT_SR_DDR, XT_REG_A3)); + xtensa_queue_exec_ins(xtensa, XT_INS_PPTLB(xtensa, XT_REG_A3, XT_REG_A3)); + xtensa_queue_exec_ins(xtensa, XT_INS_WSR(xtensa, XT_SR_DDR, XT_REG_A3)); + xtensa_queue_dbg_reg_read(xtensa, XDMREG_DDR, at_buf); + int res = xtensa_dm_queue_execute(&xtensa->dbg_mod); + if (res != ERROR_OK) + LOG_TARGET_ERROR(target, "Error queuing PPTLB: %d", res); + res = xtensa_core_status_check(target); + if (res != ERROR_OK) + LOG_TARGET_ERROR(target, "Error issuing PPTLB: %d", res); + at = buf_get_u32(at_buf, 0, 32); + acc = (at >> XT_TLB1_ACC_SHIFT) & XT_TLB1_ACC_MSK; + exec_acc = ((acc == XTENSA_ACC_00X_000) || (acc == XTENSA_ACC_R0X_000) || + (acc == XTENSA_ACC_RWX_000) || (acc == XTENSA_ACC_RWX_R0X) || + (acc == XTENSA_ACC_R0X_R0X) || (acc == XTENSA_ACC_RWX_RWX)); + LOG_TARGET_DEBUG(target, "PPTLB(" TARGET_ADDR_FMT ") -> 0x%08" PRIx32 " exec_acc %d", + addr, at, exec_acc); + if (!exec_acc) + return false; + } + } + return true; +} + static int xtensa_queue_pwr_reg_write(struct xtensa *xtensa, unsigned int reg, uint32_t data) { struct xtensa_debug_module *dm = &xtensa->dbg_mod; @@ -2176,11 +2241,13 @@ int xtensa_write_memory(struct target *target, } } else { /* Invalidate ICACHE, writeback DCACHE if present */ - uint32_t issue_ihi = xtensa_is_icacheable(xtensa, address); - uint32_t issue_dhwb = xtensa_is_dcacheable(xtensa, address); - if (issue_ihi || issue_dhwb) { + bool issue_ihi = xtensa_is_icacheable(xtensa, address) && + xtensa_region_ar_exec(target, addrstart_al, addrend_al); + bool issue_dhwbi = xtensa_is_dcacheable(xtensa, address); + LOG_TARGET_DEBUG(target, "Cache OPs: IHI %d, DHWBI %d", issue_ihi, issue_dhwbi); + if (issue_ihi || issue_dhwbi) { uint32_t ilinesize = issue_ihi ? xtensa->core_config->icache.line_size : UINT32_MAX; - uint32_t dlinesize = issue_dhwb ? xtensa->core_config->dcache.line_size : UINT32_MAX; + uint32_t dlinesize = issue_dhwbi ? xtensa->core_config->dcache.line_size : UINT32_MAX; uint32_t linesize = MIN(ilinesize, dlinesize); uint32_t off = 0; adr = addrstart_al; @@ -2193,7 +2260,7 @@ int xtensa_write_memory(struct target *target, } if (issue_ihi) xtensa_queue_exec_ins(xtensa, XT_INS_IHI(xtensa, XT_REG_A3, off)); - if (issue_dhwb) + if (issue_dhwbi) xtensa_queue_exec_ins(xtensa, XT_INS_DHWBI(xtensa, XT_REG_A3, off)); off += linesize; if (off > 1020) { @@ -2205,7 +2272,11 @@ int xtensa_write_memory(struct target *target, /* Execute cache WB/INV instructions */ res = xtensa_dm_queue_execute(&xtensa->dbg_mod); - xtensa_core_status_check(target); + if (res != ERROR_OK) + LOG_TARGET_ERROR(target, + "Error queuing cache writeback/invaldate instruction(s): %d", + res); + res = xtensa_core_status_check(target); if (res != ERROR_OK) LOG_TARGET_ERROR(target, "Error issuing cache writeback/invaldate instruction(s): %d", @@ -2367,7 +2438,8 @@ int xtensa_poll(struct target *target) static int xtensa_update_instruction(struct target *target, target_addr_t address, uint32_t size, const uint8_t *buffer) { struct xtensa *xtensa = target_to_xtensa(target); - unsigned int issue_ihi = xtensa_is_icacheable(xtensa, address); + unsigned int issue_ihi = xtensa_is_icacheable(xtensa, address) && + xtensa_region_ar_exec(target, address, address + size); unsigned int issue_dhwbi = xtensa_is_dcacheable(xtensa, address); uint32_t icache_line_size = issue_ihi ? xtensa->core_config->icache.line_size : UINT32_MAX; uint32_t dcache_line_size = issue_dhwbi ? xtensa->core_config->dcache.line_size : UINT32_MAX; @@ -2385,7 +2457,8 @@ static int xtensa_update_instruction(struct target *target, target_addr_t addres /* Write start address to A3 and invalidate */ xtensa_queue_dbg_reg_write(xtensa, XDMREG_DDR, address); xtensa_queue_exec_ins(xtensa, XT_INS_RSR(xtensa, XT_SR_DDR, XT_REG_A3)); - LOG_TARGET_DEBUG(target, "DHWBI, IHI for address "TARGET_ADDR_FMT, address); + LOG_TARGET_DEBUG(target, "IHI %d, DHWBI %d for address " TARGET_ADDR_FMT, + issue_ihi, issue_dhwbi, address); if (issue_dhwbi) { xtensa_queue_exec_ins(xtensa, XT_INS_DHWBI(xtensa, XT_REG_A3, 0)); if (!same_dc_line) { From 2506ccb5091504fa50bc71979852701a74246f06 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Sun, 28 Apr 2024 12:47:42 +0200 Subject: [PATCH 16/27] target/arm_tpiu_swo: Fix division by zero When external capturing is configured (default), the SWO pin frequency is required. Enforce this to avoid a division by zero error. While at it, ensure that the 'out_filename' variable always contains a valid string. This saves a few checks and makes the code more clean and readable. Change-Id: If8c1dae9549dd10e2f21d5b896414d47edac9fc2 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8224 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/target/arm_tpiu_swo.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/target/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c index 5cea682ec4..128e14702c 100644 --- a/src/target/arm_tpiu_swo.c +++ b/src/target/arm_tpiu_swo.c @@ -153,7 +153,7 @@ static int arm_tpiu_swo_poll_trace(void *priv) } } - if (obj->out_filename && obj->out_filename[0] == ':') + if (obj->out_filename[0] == ':') list_for_each_entry(c, &obj->connections, lh) if (connection_write(c->connection, buf, size) != (int)size) LOG_ERROR("Error writing to connection"); /* FIXME: which connection? */ @@ -201,7 +201,7 @@ static void arm_tpiu_swo_close_output(struct arm_tpiu_swo_object *obj) fclose(obj->file); obj->file = NULL; } - if (obj->out_filename && obj->out_filename[0] == ':') + if (obj->out_filename[0] == ':') remove_service(TCP_SERVICE_NAME, &obj->out_filename[1]); } @@ -475,12 +475,13 @@ static int arm_tpiu_swo_configure(struct jim_getopt_info *goi, struct arm_tpiu_s return JIM_ERR; } } - free(obj->out_filename); - obj->out_filename = strdup(s); - if (!obj->out_filename) { + char *out_filename = strdup(s); + if (!out_filename) { LOG_ERROR("Out of memory"); return JIM_ERR; } + free(obj->out_filename); + obj->out_filename = out_filename; } else { if (goi->argc) goto err_no_params; @@ -625,9 +626,18 @@ COMMAND_HANDLER(handle_arm_tpiu_swo_enable) return ERROR_FAIL; } - if (obj->pin_protocol == TPIU_SPPR_PROTOCOL_MANCHESTER || obj->pin_protocol == TPIU_SPPR_PROTOCOL_UART) - if (!obj->swo_pin_freq) + const bool output_external = !strcmp(obj->out_filename, "external"); + + if (obj->pin_protocol == TPIU_SPPR_PROTOCOL_MANCHESTER || obj->pin_protocol == TPIU_SPPR_PROTOCOL_UART) { + if (!obj->swo_pin_freq) { + if (output_external) { + command_print(CMD, "SWO pin frequency required when using external capturing"); + return ERROR_FAIL; + } + LOG_DEBUG("SWO pin frequency not set, will be autodetected by the adapter"); + } + } struct target *target = get_current_target(CMD_CTX); @@ -705,7 +715,7 @@ COMMAND_HANDLER(handle_arm_tpiu_swo_enable) uint16_t prescaler = 1; /* dummy value */ unsigned int swo_pin_freq = obj->swo_pin_freq; /* could be replaced */ - if (obj->out_filename && strcmp(obj->out_filename, "external") && obj->out_filename[0]) { + if (!output_external) { if (obj->out_filename[0] == ':') { struct arm_tpiu_swo_priv_connection *priv = malloc(sizeof(*priv)); if (!priv) { @@ -947,6 +957,12 @@ static int jim_arm_tpiu_swo_create(Jim_Interp *interp, int argc, Jim_Obj *const adiv5_mem_ap_spot_init(&obj->spot); obj->spot.base = TPIU_SWO_DEFAULT_BASE; obj->port_width = 1; + obj->out_filename = strdup("external"); + if (!obj->out_filename) { + LOG_ERROR("Out of memory"); + free(obj); + return JIM_ERR; + } Jim_Obj *n; jim_getopt_obj(&goi, &n); From c831758ea4f0baf6e4d1b2ee69ae3c895922fee5 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Fri, 3 May 2024 09:18:00 +0200 Subject: [PATCH 17/27] server/gdb: Use 'bool' data type where appropriate Change-Id: Ic23c5469334337963185b69fcabeedf70c2c7ae9 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8253 Reviewed-by: Tomas Vanek Reviewed-by: Antonio Borneo Tested-by: jenkins --- src/server/gdb_server.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index dfd7cd5201..5052bf43b2 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -128,9 +128,9 @@ static int gdb_actual_connections; /* set if we are sending a memory map to gdb * via qXfer:memory-map:read packet */ /* enabled by default*/ -static int gdb_use_memory_map = 1; +static bool gdb_use_memory_map = true; /* enabled by default*/ -static int gdb_flash_program = 1; +static bool gdb_flash_program = true; /* if set, data aborts cause an error to be reported in memory read packets * see the code in gdb_read_memory_packet() for further explanations. @@ -144,7 +144,7 @@ static int gdb_report_register_access_error; /* set if we are sending target descriptions to gdb * via qXfer:features:read packet */ /* enabled by default */ -static int gdb_use_target_description = 1; +static bool gdb_use_target_description = true; /* current processing free-run type, used by file-I/O */ static char gdb_running_type; @@ -2599,7 +2599,7 @@ static int gdb_get_target_description_chunk(struct target *target, struct target return ERROR_OK; } -static int gdb_target_description_supported(struct target *target, int *supported) +static int gdb_target_description_supported(struct target *target, bool *supported) { int retval = ERROR_OK; struct reg **reg_list = NULL; @@ -2631,9 +2631,9 @@ static int gdb_target_description_supported(struct target *target, int *supporte if (supported) { if (architecture || feature_list_size) - *supported = 1; + *supported = true; else - *supported = 0; + *supported = false; } error: @@ -2867,20 +2867,20 @@ static int gdb_query_packet(struct connection *connection, char *buffer = NULL; int pos = 0; int size = 0; - int gdb_target_desc_supported = 0; + bool gdb_target_desc_supported = false; /* we need to test that the target supports target descriptions */ retval = gdb_target_description_supported(target, &gdb_target_desc_supported); if (retval != ERROR_OK) { LOG_INFO("Failed detecting Target Description Support, disabling"); - gdb_target_desc_supported = 0; + gdb_target_desc_supported = false; } /* support may be disabled globally */ - if (gdb_use_target_description == 0) { + if (!gdb_use_target_description) { if (gdb_target_desc_supported) LOG_WARNING("Target Descriptions Supported, but disabled"); - gdb_target_desc_supported = 0; + gdb_target_desc_supported = false; } xml_printf(&retval, @@ -2889,8 +2889,8 @@ static int gdb_query_packet(struct connection *connection, &size, "PacketSize=%x;qXfer:memory-map:read%c;qXfer:features:read%c;qXfer:threads:read+;QStartNoAckMode+;vContSupported+", GDB_BUFFER_SIZE, - ((gdb_use_memory_map == 1) && (flash_get_bank_count() > 0)) ? '+' : '-', - (gdb_target_desc_supported == 1) ? '+' : '-'); + (gdb_use_memory_map && (flash_get_bank_count() > 0)) ? '+' : '-', + gdb_target_desc_supported ? '+' : '-'); if (retval != ERROR_OK) { gdb_send_error(connection, 01); @@ -3280,7 +3280,7 @@ static int gdb_v_packet(struct connection *connection, /* if flash programming disabled - send a empty reply */ - if (gdb_flash_program == 0) { + if (!gdb_flash_program) { gdb_put_packet(connection, "", 0); return ERROR_OK; } From be1aebd8183c5b6b3bbecfc065a84b7e10bd8f8e Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Wed, 1 May 2024 12:58:18 +0200 Subject: [PATCH 18/27] target/arm_tpiu_swo: Handle errors in pre/post-enable events Currently, errors in pre/post-enable events are ignored and capturing is always started, even if necessary device configuration fails. This behaviour is confusing to users. Also, the TPIU must be disabled before re-configuration is possible. Start capturing and enable TPIU only if no errors in pre/post-enable events occurred. Change-Id: I422033e36ca006e38aa4504d491b7947def1237a Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8254 Reviewed-by: Antonio Borneo Tested-by: jenkins --- src/target/arm_tpiu_swo.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/target/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c index 128e14702c..340d3323d4 100644 --- a/src/target/arm_tpiu_swo.c +++ b/src/target/arm_tpiu_swo.c @@ -161,7 +161,7 @@ static int arm_tpiu_swo_poll_trace(void *priv) return ERROR_OK; } -static void arm_tpiu_swo_handle_event(struct arm_tpiu_swo_object *obj, enum arm_tpiu_swo_event event) +static int arm_tpiu_swo_handle_event(struct arm_tpiu_swo_object *obj, enum arm_tpiu_swo_event event) { for (struct arm_tpiu_swo_event_action *ea = obj->event_action; ea; ea = ea->next) { if (ea->event != event) @@ -182,7 +182,7 @@ static void arm_tpiu_swo_handle_event(struct arm_tpiu_swo_object *obj, enum arm_ if (retval == JIM_RETURN) retval = ea->interp->returnCode; if (retval == JIM_OK || retval == ERROR_COMMAND_CLOSE_CONNECTION) - return; + return ERROR_OK; Jim_MakeErrorMessage(ea->interp); LOG_USER("Error executing event %s on TPIU/SWO %s:\n%s", @@ -191,8 +191,10 @@ static void arm_tpiu_swo_handle_event(struct arm_tpiu_swo_object *obj, enum arm_ Jim_GetString(Jim_GetResult(ea->interp), NULL)); /* clean both error code and stacktrace before return */ Jim_Eval(ea->interp, "error \"\" \"\""); - return; + return ERROR_FAIL; } + + return ERROR_OK; } static void arm_tpiu_swo_close_output(struct arm_tpiu_swo_object *obj) @@ -674,7 +676,9 @@ COMMAND_HANDLER(handle_arm_tpiu_swo_enable) } /* trigger the event before any attempt to R/W in the TPIU/SWO */ - arm_tpiu_swo_handle_event(obj, TPIU_SWO_EVENT_PRE_ENABLE); + retval = arm_tpiu_swo_handle_event(obj, TPIU_SWO_EVENT_PRE_ENABLE); + if (retval != ERROR_OK) + return retval; retval = wrap_read_u32(target, obj->ap, obj->spot.base + TPIU_DEVID_OFFSET, &value); if (retval != ERROR_OK) { @@ -800,7 +804,9 @@ COMMAND_HANDLER(handle_arm_tpiu_swo_enable) if (retval != ERROR_OK) goto error_exit; - arm_tpiu_swo_handle_event(obj, TPIU_SWO_EVENT_POST_ENABLE); + retval = arm_tpiu_swo_handle_event(obj, TPIU_SWO_EVENT_POST_ENABLE); + if (retval != ERROR_OK) + goto error_exit; /* START_DEPRECATED_TPIU */ target_handle_event(target, TARGET_EVENT_TRACE_CONFIG); From 1a00c67e101015ba23b8213604b9cc5725bc1d52 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Sun, 5 May 2024 15:20:07 +0200 Subject: [PATCH 19/27] tcl/target/nrf52: Use 'error' instead of 'echo' Use 'error' instead of 'echo' for error messages. Otherwise, capturing is always started, for example with an unsupported device. While at it, make the error messages more consistent and clear. Change-Id: I83c9abfb4514e6b638c4be14651e67f768af8bad Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8255 Tested-by: jenkins Reviewed-by: Antonio Borneo Reviewed-by: --- tcl/target/nrf52.cfg | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tcl/target/nrf52.cfg b/tcl/target/nrf52.cfg index 0c82c5758a..a9121d3561 100644 --- a/tcl/target/nrf52.cfg +++ b/tcl/target/nrf52.cfg @@ -133,8 +133,7 @@ proc _proc_pre_enable_$_CHIPNAME.tpiu {_targetname _chipname} { 0x52832 { if { [$_chipname.tpiu cget -protocol] eq "sync" } { if { [$_chipname.tpiu cget -port-width] != 4 } { - echo "Error. Device only supports 4-bit sync traces." - return + error "Device only supports 4-bit sync traces" } # Set TRACECONFIG.TRACEMUX to enable synchronous trace @@ -154,12 +153,10 @@ proc _proc_pre_enable_$_CHIPNAME.tpiu {_targetname _chipname} { 0x52811 - 0x52810 - 0x52805 { - echo "Error: Device does not support TPIU" - return + error "Device does not support TPIU" } default { - echo "Error: Unknown device" - return + error "Unknown device, cannot configure TPIU" } } } From edb14a02e9b698d015696a959ca81f821d49d4e3 Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Sun, 5 May 2024 15:28:20 +0200 Subject: [PATCH 20/27] tcl/target/nrf52: Configure trace port speed Configure the TRACECONFIG.TRACEPORTSPEED register depending on the trace clock speed. Also catch invalid trace clock speeds. Change-Id: I1ece1cc59da539732d2d71f296fd55799c195387 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8256 Reviewed-by: Antonio Borneo Tested-by: jenkins --- tcl/target/nrf52.cfg | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tcl/target/nrf52.cfg b/tcl/target/nrf52.cfg index a9121d3561..0703b18862 100644 --- a/tcl/target/nrf52.cfg +++ b/tcl/target/nrf52.cfg @@ -131,6 +131,26 @@ proc _proc_pre_enable_$_CHIPNAME.tpiu {_targetname _chipname} { 0x52840 - 0x52833 - 0x52832 { + # Configuration values for all supported trace port speeds, see + # TRACECONFIG.TRACEPORTSPEED + set trace_port_speeds { + 32000000 0 + 16000000 1 + 8000000 2 + 4000000 3 + } + + # Note that trace port clock stands for what is referred to as + # TRACECLKIN in the Arm CoreSight documentation. + set trace_port_clock [$_chipname.tpiu cget -traceclk] + + if { ![dict exists $trace_port_speeds $trace_port_clock] } { + error "Trace clock speed is not supported" + } + + # Set TRACECONFIG.TRACEPORTSPEED + mmw 0x4000055C [dict get $trace_port_speeds $trace_port_clock] 0x3 + if { [$_chipname.tpiu cget -protocol] eq "sync" } { if { [$_chipname.tpiu cget -port-width] != 4 } { error "Device only supports 4-bit sync traces" From bd89b91c697d9aea97abfcd3a3b8043a95216d6d Mon Sep 17 00:00:00 2001 From: Marc Schink Date: Wed, 1 May 2024 10:24:32 +0200 Subject: [PATCH 21/27] target/semihosting: Fix double free() Do not free the service in 'connection_closed_handler' because it is free'd by the server infrastructure. Checkpatch-ignore: COMMIT_LOG_LONG_LINE This error was detected with valgrind: ==272468== Invalid free() / delete / delete[] / realloc() ==272468== at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==272468== by 0x1F34C7: remove_service (server.c:374) ==272468== by 0x2ED3D5: semihosting_tcp_close_cnx (semihosting_common.c:1819) ==272468== by 0x2ED3D5: handle_common_semihosting_redirect_command (semihosting_common.c:1926) ==272468== by 0x1FC703: exec_command (command.c:520) ==272468== by 0x1FC703: jim_command_dispatch (command.c:931) ==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd) ==272468== by 0x1FFFFFFFFF: ??? ==272468== by 0x53ED09F: ??? ==272468== by 0x300000001: ??? ==272468== by 0x1FFEFFF7FF: ??? ==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd) ==272468== by 0x2: ??? ==272468== Address 0x5fff650 is 0 bytes inside a block of size 24 free'd ==272468== at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==272468== by 0x2ECA42: semihosting_service_connection_closed_handler (semihosting_common.c:1807) ==272468== by 0x1F2E39: remove_connection.isra.0 (server.c:164) ==272468== by 0x1F349E: remove_connections (server.c:350) ==272468== by 0x1F349E: remove_service (server.c:364) ==272468== by 0x2ED3D5: semihosting_tcp_close_cnx (semihosting_common.c:1819) ==272468== by 0x2ED3D5: handle_common_semihosting_redirect_command (semihosting_common.c:1926) ==272468== by 0x1FC703: exec_command (command.c:520) ==272468== by 0x1FC703: jim_command_dispatch (command.c:931) ==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd) ==272468== by 0x1FFFFFFFFF: ??? ==272468== by 0x53ED09F: ??? ==272468== by 0x300000001: ??? ==272468== by 0x1FFEFFF7FF: ??? ==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd) ==272468== Block was alloc'd at ==272468== at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==272468== by 0x2ED326: handle_common_semihosting_redirect_command (semihosting_common.c:1931) ==272468== by 0x1FC703: exec_command (command.c:520) ==272468== by 0x1FC703: jim_command_dispatch (command.c:931) ==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd) ==272468== by 0x1FFFFFFFFF: ??? ==272468== by 0x53ED09F: ??? ==272468== by 0x400000002: ??? ==272468== by 0x1FFEFFF7FF: ??? ==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd) ==272468== by 0x2: ??? ==272468== Change-Id: I3e5323f145a98d1ff9ea7d03f87ed96140f49a18 Signed-off-by: Marc Schink Reviewed-on: https://review.openocd.org/c/openocd/+/8257 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/target/semihosting_common.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/target/semihosting_common.c b/src/target/semihosting_common.c index f7acc6092d..1eb195712e 100644 --- a/src/target/semihosting_common.c +++ b/src/target/semihosting_common.c @@ -1802,10 +1802,8 @@ static int semihosting_service_input_handler(struct connection *connection) static int semihosting_service_connection_closed_handler(struct connection *connection) { struct semihosting_tcp_service *service = connection->service->priv; - if (service) { + if (service) free(service->name); - free(service); - } return ERROR_OK; } From ccc5c1642b32ab79d5ab315e4b308e53392a77c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernhard=20Rosenkr=C3=A4nzer?= Date: Thu, 9 May 2024 15:30:13 +0200 Subject: [PATCH 22/27] Fix build with clang even if it sets __GNUC__ to >= 4.4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clang doesn't support the gnu_printf attribute that OpenOCD uses if __GNUC__ and __GNUC_MINOR__ indicates gcc >= 4.4. Most clang builds set __GNUC__/__GNUC_MINOR__ to 4.2 for historical reasons, so they don't trigger this condition; however, some builds set it to something much higher to work around code using __GNUC__ to determine if a feature that does exist in clang (but not gcc 4.2) is available, causing OpenOCD to use attribute gnu_printf. The problem can be reproduced without a special clang build by adding -fgnuc-version=14.1 to CFLAGS. Change-Id: I3c0832d4201578b116c5214203f95b6153dad30e Signed-off-by: Bernhard Rosenkränzer Reviewed-on: https://review.openocd.org/c/openocd/+/8258 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/helper/command.h | 2 +- src/helper/log.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helper/command.h b/src/helper/command.h index dc45070420..fc26dda81a 100644 --- a/src/helper/command.h +++ b/src/helper/command.h @@ -21,7 +21,7 @@ /* To achieve C99 printf compatibility in MinGW, gnu_printf should be * used for __attribute__((format( ... ))), with GCC v4.4 or later */ -#if (defined(IS_MINGW) && (((__GNUC__ << 16) + __GNUC_MINOR__) >= 0x00040004)) +#if (defined(IS_MINGW) && (((__GNUC__ << 16) + __GNUC_MINOR__) >= 0x00040004)) && !defined(__clang__) #define PRINTF_ATTRIBUTE_FORMAT gnu_printf #else #define PRINTF_ATTRIBUTE_FORMAT printf diff --git a/src/helper/log.h b/src/helper/log.h index 818716a9df..d52c05f99d 100644 --- a/src/helper/log.h +++ b/src/helper/log.h @@ -19,7 +19,7 @@ /* To achieve C99 printf compatibility in MinGW, gnu_printf should be * used for __attribute__((format( ... ))), with GCC v4.4 or later */ -#if (defined(IS_MINGW) && (((__GNUC__ << 16) + __GNUC_MINOR__) >= 0x00040004)) +#if (defined(IS_MINGW) && (((__GNUC__ << 16) + __GNUC_MINOR__) >= 0x00040004)) && !defined(__clang__) #define PRINTF_ATTRIBUTE_FORMAT gnu_printf #else #define PRINTF_ATTRIBUTE_FORMAT printf From fbea7d5d38d0dcbdd71cb574da9bd12c78b568cf Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Tue, 9 Apr 2024 01:29:34 +0200 Subject: [PATCH 23/27] openocd: drop include of target_type.h Few files include target_type.h even if it is not needed. Drop the include. Other files access directly to target type's name instead of using the proper API target_type_name(). Use the API and drop the include. Change-Id: I86c0e0bbad51db93500c0efa27b7d6f1a67a02c2 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/8260 Tested-by: jenkins Reviewed-by: Tomas Vanek --- src/flash/nor/psoc6.c | 1 - src/rtos/FreeRTOS.c | 3 +-- src/rtos/ThreadX.c | 3 +-- src/rtos/chibios.c | 7 +++---- src/rtos/chromium-ec.c | 7 +++---- src/rtos/eCos.c | 7 +++---- src/rtos/embKernel.c | 5 ++--- src/rtos/hwthread.c | 1 - src/rtos/mqx.c | 7 +++---- src/rtos/nuttx.c | 1 - src/rtos/riot.c | 3 +-- src/rtos/rtkernel.c | 3 +-- src/rtos/uCOS-III.c | 5 ++--- src/rtos/zephyr.c | 1 - src/target/arm_tpiu_swo.c | 9 ++++----- src/target/target.c | 4 ++-- 16 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/flash/nor/psoc6.c b/src/flash/nor/psoc6.c index b7ba1027ed..859b3e9ec5 100644 --- a/src/flash/nor/psoc6.c +++ b/src/flash/nor/psoc6.c @@ -18,7 +18,6 @@ #include "target/target.h" #include "target/cortex_m.h" #include "target/breakpoints.h" -#include "target/target_type.h" #include "target/algorithm.h" /************************************************************************************************** diff --git a/src/rtos/FreeRTOS.c b/src/rtos/FreeRTOS.c index e8df030aad..20977e07e9 100644 --- a/src/rtos/FreeRTOS.c +++ b/src/rtos/FreeRTOS.c @@ -12,7 +12,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -533,7 +532,7 @@ static bool freertos_detect_rtos(struct target *target) static int freertos_create(struct target *target) { for (unsigned int i = 0; i < ARRAY_SIZE(freertos_params_list); i++) - if (strcmp(freertos_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(freertos_params_list[i].target_name, target_type_name(target)) == 0) { target->rtos->rtos_specific_params = (void *)&freertos_params_list[i]; return 0; } diff --git a/src/rtos/ThreadX.c b/src/rtos/ThreadX.c index 5bdd007d42..61c49264e8 100644 --- a/src/rtos/ThreadX.c +++ b/src/rtos/ThreadX.c @@ -12,7 +12,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -608,7 +607,7 @@ static int threadx_get_thread_detail(struct rtos *rtos, static int threadx_create(struct target *target) { for (unsigned int i = 0; i < ARRAY_SIZE(threadx_params_list); i++) - if (strcmp(threadx_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(threadx_params_list[i].target_name, target_type_name(target)) == 0) { target->rtos->rtos_specific_params = (void *)&threadx_params_list[i]; target->rtos->current_thread = 0; target->rtos->thread_details = NULL; diff --git a/src/rtos/chibios.c b/src/rtos/chibios.c index 20378274ec..c1e4e84192 100644 --- a/src/rtos/chibios.c +++ b/src/rtos/chibios.c @@ -15,7 +15,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "target/armv7m.h" #include "target/cortex_m.h" #include "rtos.h" @@ -470,7 +469,7 @@ static int chibios_get_thread_reg_list(struct rtos *rtos, int64_t thread_id, /* Update stacking if it can only be determined from runtime information */ if (!param->stacking_info && (chibios_update_stacking(rtos) != ERROR_OK)) { - LOG_ERROR("Failed to determine exact stacking for the target type %s", rtos->target->type->name); + LOG_ERROR("Failed to determine exact stacking for the target type %s", target_type_name(rtos->target)); return -1; } @@ -518,12 +517,12 @@ static bool chibios_detect_rtos(struct target *target) static int chibios_create(struct target *target) { for (unsigned int i = 0; i < ARRAY_SIZE(chibios_params_list); i++) - if (strcmp(chibios_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(chibios_params_list[i].target_name, target_type_name(target)) == 0) { target->rtos->rtos_specific_params = (void *)&chibios_params_list[i]; return 0; } LOG_WARNING("Could not find target \"%s\" in ChibiOS compatibility " - "list", target->type->name); + "list", target_type_name(target)); return -1; } diff --git a/src/rtos/chromium-ec.c b/src/rtos/chromium-ec.c index a95969e345..dbfe3b3c68 100644 --- a/src/rtos/chromium-ec.c +++ b/src/rtos/chromium-ec.c @@ -14,7 +14,6 @@ #include #include #include -#include #include "rtos_standard_stackings.h" @@ -120,7 +119,7 @@ static int chromium_ec_create(struct target *target) size_t t; for (t = 0; t < ARRAY_SIZE(chromium_ec_params_list); t++) - if (!strcmp(chromium_ec_params_list[t].target_name, target->type->name)) { + if (!strcmp(chromium_ec_params_list[t].target_name, target_type_name(target))) { params = malloc(sizeof(*params)); if (!params) { LOG_ERROR("Chromium-EC: out of memory"); @@ -133,11 +132,11 @@ static int chromium_ec_create(struct target *target) target->rtos->thread_details = NULL; target->rtos->thread_count = 0; - LOG_INFO("Chromium-EC: Using target: %s", target->type->name); + LOG_INFO("Chromium-EC: Using target: %s", target_type_name(target)); return ERROR_OK; } - LOG_ERROR("Chromium-EC: target not supported: %s", target->type->name); + LOG_ERROR("Chromium-EC: target not supported: %s", target_type_name(target)); return ERROR_FAIL; } diff --git a/src/rtos/eCos.c b/src/rtos/eCos.c index 10ed12809f..7048b006e0 100644 --- a/src/rtos/eCos.c +++ b/src/rtos/eCos.c @@ -10,7 +10,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "target/armv7m.h" #include "rtos.h" #include "helper/log.h" @@ -1137,7 +1136,7 @@ static int ecos_get_symbol_list_to_lookup(struct symbol_table_elem *symbol_list[ ARRAY_SIZE(ecos_symbol_list), sizeof(struct symbol_table_elem)); /* If the target reference was passed into this function we could limit - * the symbols we need to lookup to the target->type->name based + * the symbols we need to lookup to the target_type_name(target) based * range. For the moment we need to provide a single vector with all of * the symbols across all of the supported architectures. */ for (i = 0; i < ARRAY_SIZE(ecos_symbol_list); i++) { @@ -1189,8 +1188,8 @@ static int ecos_create(struct target *target) for (unsigned int i = 0; i < ARRAY_SIZE(ecos_params_list); i++) { const char * const *tnames = ecos_params_list[i].target_names; while (*tnames) { - if (strcmp(*tnames, target->type->name) == 0) { - /* LOG_DEBUG("eCos: matched target \"%s\"", target->type->name); */ + if (strcmp(*tnames, target_type_name(target)) == 0) { + /* LOG_DEBUG("eCos: matched target \"%s\"", target_type_name(target)); */ target->rtos->rtos_specific_params = (void *)&ecos_params_list[i]; ecos_params_list[i].flush_common = true; ecos_params_list[i].stacking_info = NULL; diff --git a/src/rtos/embKernel.c b/src/rtos/embKernel.c index a03b039e0c..7e6de7902a 100644 --- a/src/rtos/embKernel.c +++ b/src/rtos/embKernel.c @@ -12,7 +12,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -110,12 +109,12 @@ static int embkernel_create(struct target *target) { size_t i = 0; while ((i < ARRAY_SIZE(embkernel_params_list)) && - (strcmp(embkernel_params_list[i].target_name, target->type->name) != 0)) + (strcmp(embkernel_params_list[i].target_name, target_type_name(target)) != 0)) i++; if (i >= ARRAY_SIZE(embkernel_params_list)) { LOG_WARNING("Could not find target \"%s\" in embKernel compatibility " - "list", target->type->name); + "list", target_type_name(target)); return -1; } diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c index 895f11cfc9..937d01b874 100644 --- a/src/rtos/hwthread.c +++ b/src/rtos/hwthread.c @@ -7,7 +7,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "target/register.h" #include #include "rtos.h" diff --git a/src/rtos/mqx.c b/src/rtos/mqx.c index d9b694282a..b4a1821352 100644 --- a/src/rtos/mqx.c +++ b/src/rtos/mqx.c @@ -13,7 +13,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -249,13 +248,13 @@ static int mqx_create( { /* check target name against supported architectures */ for (unsigned int i = 0; i < ARRAY_SIZE(mqx_params_list); i++) { - if (strcmp(mqx_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(mqx_params_list[i].target_name, target_type_name(target)) == 0) { target->rtos->rtos_specific_params = (void *)&mqx_params_list[i]; - /* LOG_DEBUG("MQX RTOS - valid architecture: %s", target->type->name); */ + /* LOG_DEBUG("MQX RTOS - valid architecture: %s", target_type_name(target)); */ return 0; } } - LOG_ERROR("MQX RTOS - could not find target \"%s\" in MQX compatibility list", target->type->name); + LOG_ERROR("MQX RTOS - could not find target \"%s\" in MQX compatibility list", target_type_name(target)); return -1; } diff --git a/src/rtos/nuttx.c b/src/rtos/nuttx.c index 0616af0f4c..9100148895 100644 --- a/src/rtos/nuttx.c +++ b/src/rtos/nuttx.c @@ -12,7 +12,6 @@ #include #include "target/target.h" -#include "target/target_type.h" #include "target/armv7m.h" #include "target/cortex_m.h" #include "rtos.h" diff --git a/src/rtos/riot.c b/src/rtos/riot.c index be5452e9e7..b90b069148 100644 --- a/src/rtos/riot.c +++ b/src/rtos/riot.c @@ -12,7 +12,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -391,7 +390,7 @@ static int riot_create(struct target *target) /* lookup if target is supported by RIOT */ while ((i < RIOT_NUM_PARAMS) && - (strcmp(riot_params_list[i].target_name, target->type->name) != 0)) { + (strcmp(riot_params_list[i].target_name, target_type_name(target)) != 0)) { i++; } if (i >= RIOT_NUM_PARAMS) { diff --git a/src/rtos/rtkernel.c b/src/rtos/rtkernel.c index ba1de25172..aebbf3d94d 100644 --- a/src/rtos/rtkernel.c +++ b/src/rtos/rtkernel.c @@ -12,7 +12,6 @@ #include #include #include "target/target.h" -#include "target/target_type.h" #include "rtos.h" #include "helper/log.h" #include "helper/types.h" @@ -363,7 +362,7 @@ static bool rtkernel_detect_rtos(struct target *target) static int rtkernel_create(struct target *target) { for (size_t i = 0; i < ARRAY_SIZE(rtkernel_params_list); i++) { - if (strcmp(rtkernel_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(rtkernel_params_list[i].target_name, target_type_name(target)) == 0) { target->rtos->rtos_specific_params = (void *)&rtkernel_params_list[i]; return 0; } diff --git a/src/rtos/uCOS-III.c b/src/rtos/uCOS-III.c index 4d704a44fe..f19d06e3c0 100644 --- a/src/rtos/uCOS-III.c +++ b/src/rtos/uCOS-III.c @@ -14,7 +14,6 @@ #include #include #include -#include #include "rtos_ucos_iii_stackings.h" @@ -253,7 +252,7 @@ static int ucos_iii_create(struct target *target) struct ucos_iii_private *params; for (size_t i = 0; i < ARRAY_SIZE(ucos_iii_params_list); i++) - if (strcmp(ucos_iii_params_list[i].target_name, target->type->name) == 0) { + if (strcmp(ucos_iii_params_list[i].target_name, target_type_name(target)) == 0) { params = calloc(1, sizeof(*params)); if (!params) { LOG_ERROR("uCOS-III: out of memory"); @@ -268,7 +267,7 @@ static int ucos_iii_create(struct target *target) return ERROR_OK; } - LOG_ERROR("uCOS-III: target not supported: %s", target->type->name); + LOG_ERROR("uCOS-III: target not supported: %s", target_type_name(target)); return ERROR_FAIL; } diff --git a/src/rtos/zephyr.c b/src/rtos/zephyr.c index a4c60904b5..023db60c9e 100644 --- a/src/rtos/zephyr.c +++ b/src/rtos/zephyr.c @@ -20,7 +20,6 @@ #include "rtos.h" #include "rtos_standard_stackings.h" #include "target/target.h" -#include "target/target_type.h" #include "target/armv7m.h" #include "target/arc.h" diff --git a/src/target/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c index 340d3323d4..b5a4882011 100644 --- a/src/target/arm_tpiu_swo.c +++ b/src/target/arm_tpiu_swo.c @@ -40,7 +40,6 @@ /* START_DEPRECATED_TPIU */ #include -#include #define MSG "DEPRECATED \'tpiu config\' command: " /* END_DEPRECATED_TPIU */ @@ -645,8 +644,8 @@ COMMAND_HANDLER(handle_arm_tpiu_swo_enable) /* START_DEPRECATED_TPIU */ if (obj->recheck_ap_cur_target) { - if (strcmp(target->type->name, "cortex_m") && - strcmp(target->type->name, "hla_target")) { + if (strcmp(target_type_name(target), "cortex_m") && + strcmp(target_type_name(target), "hla_target")) { LOG_ERROR(MSG "Current target is not a Cortex-M nor a HLA"); return ERROR_FAIL; } @@ -1043,8 +1042,8 @@ COMMAND_HANDLER(handle_tpiu_deprecated_config_command) struct arm_tpiu_swo_object *obj = NULL; int retval; - if (strcmp(target->type->name, "cortex_m") && - strcmp(target->type->name, "hla_target")) { + if (strcmp(target_type_name(target), "cortex_m") && + strcmp(target_type_name(target), "hla_target")) { LOG_ERROR(MSG "Current target is not a Cortex-M nor a HLA"); return ERROR_FAIL; } diff --git a/src/target/target.c b/src/target/target.c index 5168305dee..efc168903b 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1498,7 +1498,7 @@ static int target_init_one(struct command_context *cmd_ctx, */ if (type->mmu) { if (!type->virt2phys) { - LOG_ERROR("type '%s' is missing virt2phys", type->name); + LOG_ERROR("type '%s' is missing virt2phys", target_name(target)); type->virt2phys = identity_virt2phys; } } else { @@ -1507,7 +1507,7 @@ static int target_init_one(struct command_context *cmd_ctx, * ensure that virt2phys() is always an identity mapping. */ if (type->write_phys_memory || type->read_phys_memory || type->virt2phys) - LOG_WARNING("type '%s' has bad MMU hooks", type->name); + LOG_WARNING("type '%s' has bad MMU hooks", target_name(target)); type->mmu = no_mmu; type->write_phys_memory = type->write_memory; From 1fba7d53bb92fca82336f6615324bc597ab0a6c4 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sun, 21 Apr 2024 22:58:21 +0200 Subject: [PATCH 24/27] configure.ac: show the dummy adapter in the configuration summary The dummy adapter now uses the same config logic as most adapters. Its name has changed from "dummy port driver" to "Dummy Adapter". Change-Id: Ic9ee617aab1f54215835d4d8db03f6637b797082 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/7340 Tested-by: jenkins Reviewed-by: Antonio Borneo --- configure.ac | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 7613848371..becc531b0d 100644 --- a/configure.ac +++ b/configure.ac @@ -157,6 +157,11 @@ m4_define([PCIE_ADAPTERS], m4_define([SERIAL_PORT_ADAPTERS], [[[buspirate], [Bus Pirate], [BUS_PIRATE]]]) +# The word 'Adapter' in "Dummy Adapter" below must begin with a capital letter +# because there is an M4 macro called 'adapter'. +m4_define([DUMMY_ADAPTER], + [[[dummy], [Dummy Adapter], [DUMMY]]]) + m4_define([OPTIONAL_LIBRARIES], [[[capstone], [Use Capstone disassembly framework], []]]) @@ -234,10 +239,6 @@ AS_IF([test "x$debug_malloc" = "xyes"], [ AC_DEFINE([_DEBUG_FREE_SPACE_],[1], [Include malloc free space in logging]) ]) -AC_ARG_ENABLE([dummy], - AS_HELP_STRING([--enable-dummy], [Enable building the dummy port driver]), - [build_dummy=$enableval], [build_dummy=no]) - AC_ARG_ENABLE([rshim], AS_HELP_STRING([--enable-rshim], [Enable building the rshim driver]), [build_rshim=$enableval], [build_rshim=no]) @@ -266,6 +267,8 @@ AC_ARG_ADAPTERS([ LIBJAYLINK_ADAPTERS ],[auto]) +AC_ARG_ADAPTERS([DUMMY_ADAPTER],[no]) + AC_ARG_ENABLE([parport], AS_HELP_STRING([--enable-parport], [Enable building the pc parallel port driver]), [build_parport=$enableval], [build_parport=no]) @@ -492,11 +495,8 @@ AS_IF([test "x$build_dmem" = "xyes"], [ AC_DEFINE([BUILD_DMEM], [0], [0 if you don't want to debug via Direct Mem.]) ]) -AS_IF([test "x$build_dummy" = "xyes"], [ +AS_IF([test "x$ADAPTER_VAR([dummy])" = "xyes"], [ build_bitbang=yes - AC_DEFINE([BUILD_DUMMY], [1], [1 if you want dummy driver.]) -], [ - AC_DEFINE([BUILD_DUMMY], [0], [0 if you don't want dummy driver.]) ]) AS_IF([test "x$build_ep93xx" = "xyes"], [ @@ -678,6 +678,11 @@ PKG_CHECK_MODULES([LIBGPIOD], [libgpiod < 2.0], [ PKG_CHECK_MODULES([LIBJAYLINK], [libjaylink >= 0.2], [use_libjaylink=yes], [use_libjaylink=no]) +# Arg $1: The adapter name, used to derive option and variable names for the adapter. +# Arg $2: Whether the adapter can be enabled, for example, because +# its prerequisites are installed in the system. +# Arg $3: What prerequisites are missing, to be shown in an error message +# if the adapter was requested but cannot be enabled. m4_define([PROCESS_ADAPTERS], [ m4_foreach([adapter], [$1], [ AS_IF([test $2], [ @@ -704,6 +709,7 @@ PROCESS_ADAPTERS([LIBFTDI_ADAPTERS], ["x$use_libftdi" = "xyes"], [libftdi]) PROCESS_ADAPTERS([LIBFTDI_USB1_ADAPTERS], ["x$use_libftdi" = "xyes" -a "x$use_libusb1" = "xyes"], [libftdi and libusb-1.x]) PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [libgpiod]) PROCESS_ADAPTERS([LIBJAYLINK_ADAPTERS], ["x$use_internal_libjaylink" = "xyes" -o "x$use_libjaylink" = "xyes"], [libjaylink-0.2]) +PROCESS_ADAPTERS([DUMMY_ADAPTER], [true], [unused]) AS_IF([test "x$enable_linuxgpiod" != "xno"], [ build_bitbang=yes @@ -743,7 +749,6 @@ AS_IF([test "x$enable_esp_usb_jtag" != "xno"], [ AM_CONDITIONAL([RELEASE], [test "x$build_release" = "xyes"]) AM_CONDITIONAL([PARPORT], [test "x$build_parport" = "xyes"]) -AM_CONDITIONAL([DUMMY], [test "x$build_dummy" = "xyes"]) AM_CONDITIONAL([GIVEIO], [test "x$parport_use_giveio" = "xyes"]) AM_CONDITIONAL([EP93XX], [test "x$build_ep93xx" = "xyes"]) AM_CONDITIONAL([AT91RM9200], [test "x$build_at91rm9200" = "xyes"]) @@ -852,6 +857,7 @@ m4_foreach([adapter], [USB1_ADAPTERS, LIBFTDI_USB1_ADAPTERS, LIBGPIOD_ADAPTERS, LIBJAYLINK_ADAPTERS, PCIE_ADAPTERS, SERIAL_PORT_ADAPTERS, + DUMMY_ADAPTER, OPTIONAL_LIBRARIES], [s=m4_format(["%-40s"], ADAPTER_DESC([adapter])) AS_CASE([$ADAPTER_VAR([adapter])], From cf48bf6d3d1a76ed418ad4e5ef62fb578741e79a Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sun, 12 May 2024 20:50:34 +0200 Subject: [PATCH 25/27] ChangeLog: fix warning "Wide character in print" from git2cl Change-Id: Iaefd989413753fb59642c3887807abd6ddac4b53 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8262 Reviewed-by: Antonio Borneo Reviewed-by: Tomas Vanek Tested-by: jenkins --- Makefile.am | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 1313d151b5..647b571cfa 100644 --- a/Makefile.am +++ b/Makefile.am @@ -114,9 +114,14 @@ TCL_PATH = tcl TCL_FILES = find $(srcdir)/$(TCL_PATH) -name '*.cfg' -o -name '*.tcl' -o -name '*.txt' | \ sed -e 's,^$(srcdir)/$(TCL_PATH),,' +# Without the PERL_UNICODE="IO" workaround below when running git2cl, you get several +# "Wide character" warnings and you also risk an invalid character encoding in +# the generated ChangeLog file. For more information, see this bug report: +# Warning "Wide character in print" +# https://savannah.nongnu.org/bugs/?65689 dist-hook: if test -d $(srcdir)/.git -a \( ! -e $(distdir)/ChangeLog -o -w $(distdir)/ChangeLog \) ; then \ - git --git-dir $(srcdir)/.git log | $(srcdir)/tools/git2cl/git2cl > $(distdir)/ChangeLog ; \ + git --git-dir $(srcdir)/.git log | PERL_UNICODE="IO" $(srcdir)/tools/git2cl/git2cl > $(distdir)/ChangeLog ; \ fi for i in $$($(TCL_FILES)); do \ j="$(distdir)/$(TCL_PATH)/$$i" && \ From 7c4ddfb5467b096156e7e12ec7e2ad114e88d648 Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sun, 12 May 2024 22:03:08 +0200 Subject: [PATCH 26/27] Documentation: Fix 2 warnings "Underfull \hbox (badness 10000)" The link to product GW16042 was broken, so it has been reduced to the manufacturer's website, which fixes the warning as a side effect. The link to Raisonance RLink was broken, and the new one is shorter, which fixes the warning as a side effect. Change-Id: I4df9acf2d994d51cd8f375bdac6c803270029506 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8264 Tested-by: jenkins Reviewed-by: Antonio Borneo Reviewed-by: Tomas Vanek --- doc/openocd.texi | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 55e6e76808..32697f86d0 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -399,8 +399,7 @@ to be available anymore as of April 2012. @* Link @url{http://www.distortec.com/jtag-lock-pick-tiny-2} FT232H-based @item @b{GW16042} -@* Link: @url{http://shop.gateworks.com/index.php?route=product/product&path=70_80&product_id=64} -FT2232H-based +@* Link: @url{https://www.gateworks.com/} FT2232H-based @end itemize @section USB-JTAG / Altera USB-Blaster compatibles @@ -442,7 +441,7 @@ SWD and not JTAG, thus not supported. @itemize @bullet @item @b{Raisonance RLink} -@* Link: @url{http://www.mcu-raisonance.com/~rlink-debugger-programmer__@/microcontrollers__tool~tool__T018:4cn9ziz4bnx6.html} +@* Link: @url{https://www.raisonance.com/rlink.html} @item @b{STM32 Primer} @* Link: @url{http://www.stm32circle.com/resources/stm32primer.php} @item @b{STM32 Primer2} From 437dde701c13e707e5fd912ef6403e09052e4d9b Mon Sep 17 00:00:00 2001 From: "R. Diez" Date: Sun, 12 May 2024 18:45:16 +0200 Subject: [PATCH 27/27] Documentation: fix warning "unbalanced square brackets" Change-Id: I17b716533f5c9371600f0d932bf9b81c95c349e7 Signed-off-by: R. Diez Reviewed-on: https://review.openocd.org/c/openocd/+/8261 Tested-by: jenkins Reviewed-by: Antonio Borneo Reviewed-by: Tomas Vanek --- doc/openocd.texi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 32697f86d0..5eef81e40f 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9371,7 +9371,7 @@ Loads an image stored in memory by @command{fast_load_image} to the current target. Must be preceded by fast_load_image. @end deffn -@deffn {Command} {fast_load_image} filename [address [@option{bin}|@option{ihex}|@option{elf}|@option{s19} [@option{min_addr} [@option{max_length}]]]]]] +@deffn {Command} {fast_load_image} filename [address [@option{bin}|@option{ihex}|@option{elf}|@option{s19} [@option{min_addr} [@option{max_length}]]]] Normally you should be using @command{load_image} or GDB load. However, for testing purposes or when I/O overhead is significant(OpenOCD running on an embedded host), storing the image in memory and uploading the image to the target