target: clarify usage of `coreid` By definition in `target/target.h`, `coreid` is not a unique identifier of a target -- it can be the same for targets on different TAPs. Change-Id: Ifce78da55fffe28dd8b6b06ecae7d8c4e305c0a2 Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7997 Tested-by: jenkins Reviewed-by: Marek Vrbka <marek.vrbka@codasip.com> Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
arm_opcode: Add support for ARM MCRR/MRRC Add support for the ARM MCRR/MRRC instructions which require the use of two registers to transfer a 64-bit co-processor registers. We are going to use this in a subsequent patch in order to properly dump 64-bit page table descriptors that exist on ARMv7A with VMSA extensions. We make use of r0 and r1 to transfer 64-bit quantities to/from DCC. Change-Id: Ic4975026c1ae4f2853795575ac7701d541248736 Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> Signed-off-by: Michael Chalfant <michael.chalfant@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/5228 Tested-by: jenkins Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
target: fix messages and return values of failed op because not halted Lot of messages was logged as LOG_WARNING, but the operation failed immediately. Sometimes no error message was logged at all. Add missing messages, change warnings to errors. Sometimes ERROR_TARGET_INVALID was returned. Some command handlers returned ERROR_OK! Always return ERROR_TARGET_NOT_HALTED. While on it use LOG_TARGET_ERROR() whenever possible. Prefix command_print() message with 'Error:' to get closer to LOG_TARGET_ERROR() variant. Error message was not added to get() and set() methods of struct xxx_reg_type - the return value is properly checked and a message is logged by the caller in case of ERROR_TARGET_NOT_HALTED. Signed-off-by: Tomas Vanek <vanekt@fbl.cz> Change-Id: I2fe4187c6025f0038956ab387edbf3f461c69398 Reviewed-on: https://review.openocd.org/c/openocd/+/7819 Tested-by: jenkins Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
openocd: trivial replace of jim-nvp with new nvp For some trivial case only, replace calls to jim-nvp with calls to the new OpenOCD nvp. Change-Id: Ifd9aff32b67748af8ab808e6a6b6e64f5271b888 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7553 Tested-by: jenkins
target: cortex_a: fix clang error core.CallAndMessage Clang complains about the variable 'orig_dfsr' that can be used uninitialized both in cortex_a_read_cpu_memory() and in cortex_a_write_cpu_memory(). The issue is caused by an incorrect error path that used to jump through 'goto out'. The code after the label 'out' is specific to handle the case of an error during memory R/W; it is incorrect to jump there to handle an error during the initialization that precedes the memory R/W. Replace the 'goto out' with 'return retval'. Remove the label 'out' that is now unused. Change-Id: Ib4b140221d1c1b63419de109579bde8b63fc2e8c Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7393 Tested-by: jenkins
arm_adi_v5: fix SIGSEGV due to failing re-examine Commit 35a503b08d14 ("arm_adi_v5: add ap refcount and add get/put around ap use") modifies the examine functions of mem_ap, cortex_m, cortex_a and aarch64 by calling dap_put_ap() and then looking again for the mem-ap and calling dap_get_ap(). This causes an issue if the system is irresponsive and the examine fails and left the AP pointer to NULL. If the system was already examined the NULL pointer will cause a SIGSEGV. Commit b6dad912b85d ("target/cortex_m: prevent segmentation fault in cortex_m_poll()") proposes a fix for one specific case and only on cortex_m. Modify all the examine functions by skipping look-up for the AP if it was already set in a previous examine; the target's AP is not supposed to change during runtime. Remove the partial fix for cortex_m as it is not needed anymore. Change-Id: I806ec3b1b02fcc76e141c8dd3a65044febbf0a8c Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Fixes: 35a503b08d14 ("arm_adi_v5: add ap refcount and add get/put around ap use") Reviewed-on: https://review.openocd.org/c/openocd/+/7392 Tested-by: jenkins Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
openocd: fix SPDX tag format for files .c With the old checkpatch we cannot use the correct format for the SPDX tags in the file .c, in fact the C99 comments are not allowed and we had to use the block comment. With the new checkpatch, let's switch to the correct SPDX format. Change created automatically through the command: sed -i \ 's,^/\* *\(SPDX-License-Identifier: .*[^ ]\) *\*/$,// \1,' \ $(find src/ contrib/ -name \*.c) Change-Id: I6da16506baa7af718947562505dd49606d124171 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7153 Tested-by: jenkins
openocd: remove CamelCase symbols *xPSR* We have left the camelcase symbols *xPSR* for some time, to avoid any conflict with possibly pending patches in gerrit. With the approaching v0.12.0-rc1, it's time to revisit it. The patches in gerrit that conflict with this rename are all not merge-able due to conflicts or due to negative review. Drop these CamelCase symbols. Change-Id: Ifbac4c1df9cc55994e024971a2aaebeed2ea4ed3 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7155 Tested-by: jenkins Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
openocd: src/target: replace the GPL-2.0-or-later license tag Replace the FSF boilerplate with the SPDX tag. The SPDX tag on files *.c is incorrect, as it should use the C99 single line comment using '//'. But current checkpatch doesn't allow C99 comments, so keep using standard C comments, by now. Change-Id: I255ad17235ff1e01bf0aa4deed4d944e1d693ddb Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/7071 Tested-by: jenkins
arm_adi_v5: add ap refcount and add get/put around ap use While an ADIv5 DAP can only have 256 AP, ADIv6 can provide till 2**40 (1,099,511,627,776) AP per DAP. The actual trivial code implementation for ADIv5 (that uses an array of 256 ap in the struct adiv5_dap) cannot be extended as-is to handle ADIv6. The simple array of 256 AP can be reused as a dynamic storage for ADIv6 ap: - the ADIv5 AP number is replaced by the ADIv6 base address; - the index of the array (equal to ADIv5 AP number) has no link to any ADIv6 property; - the ADIv6 base_address has to be searched in the array of AP. The 256 elements in the AP array should be enough for any device available today. In future it can be easily increased, if needed. To efficiently use the 256 elements in the AP array, the code should associate one element of the array to an ADIv6 AP (through the AP base address), then cancel the association when the AP is not anymore needed. This is important to avoid saturating the AP array while exploring the device through 'dap apreg' commands. Add a reference counter in the struct adiv5_ap to track how many times the struct has been associated with the same base address. Introduce the function dap_get_ap() to associate and return the struct, and dap_put_ap() to release the struct. For the moment the code covers ADIv5 only, so the association is through the index. Use the two functions above and dap_find_get_ap() throughout the code. Check the return value of dap_get_ap(). It is always not NULL in the current ADIv5-only implementation, but can be NULL for ADIv6 when there are no more available AP in the array. Instrument dap_queue_ap_read() and dap_queue_ap_write() to log an error message if the AP has reference counter zero, meaning that the AP has not been 'get' yet. This helps identifying AP used without get/put, e.g. code missed by this patch, or merged later. Instrument dap_cleanup_all() to log an error message if an AP has reference counter not zero at openocd exit, meaning that the AP has not been 'put' yet. Change-Id: I98316eb42b9f3d9c9bbbb6c73b1091b53f629092 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6455 Reviewed-by: Daniel Goehring <dgoehrin@os.amperecomputing.com> Tested-by: jenkins
arm_adi_v5: let dap_lookup_cs_component() to get AP dbgbase Simplify the code in cortex_a and aarch64 by moving the call to dap_get_debugbase() inside dap_lookup_cs_component(). This has the further effects: - dap_get_debugbase() is not referenced outside arm_adi_v5.c and becomes static; - dap_lookup_cs_component() looses one parameter; - the coreid parameter 'idx' is passed as value; - the caller in aarch64 don't have and don't print the irrelevant value of AP register APID; - fixes the debug message in the caller in aarch64 to print the coreid value instead of always zero. Change-Id: Ic7f0f643fdf067c059c8f2455a02ff18a3fed054 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6823 Tested-by: jenkins
target: Rework 'set' variable of break-/watchpoints The 'set' variable name suggests a boolean data type which determines whether a breakpoint (or watchpoint) is active. However, it is also used to store the number of the breakpoint. This encoding leads to inconsistent value assignments: boolean and integer values are mixed. Also, associated hardware comparator numbers, which are usually numbered from 0, cannot be used directly. An additional offset is required to store the comparator numbers. In order to make the code more readable and the value assignment more consistent, change the variable name to 'is_set', its data type to 'bool' and introduce a dedicated variable for the break-/watchpoint number. In order to make the review easier, the data types of various related variables (e.g. number of breakpoints) are not changed. While at it, fix a few coding style issues. Change-Id: I2193f5639247cce6b80580d4c1c6afee916aeb82 Signed-off-by: Marc Schink <dev@zapb.de> Reviewed-on: https://review.openocd.org/c/openocd/+/6319 Tested-by: jenkins Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
target/smp: use a struct list_head to hold the smp targets Instead of reinventing a simply linked list, reuse the list helper for the list of targets in a smp cluster. Using the existing helper, that implements a double linked list, makes trivial going through the list in reverse order. Change-Id: Ib36ad2955f15cd2a601b0b9e36ca6d948b12d00f Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6783 Tested-by: jenkins
arm_coresight: add include file and use it Several magic numbers related to ARM CoreSight specification IHI0029E are spread around OpenOCD code. Define through macros the ARM CoreSight magic numbers and collect them in a single include file. Use the new macros wherever possible. Change-Id: I9b0c1c651ce4ffbaf08d31791ef16e95983ee4cb Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6446 Tested-by: jenkins Reviewed-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com> Reviewed-by: Daniel Goehring <dgoehrin@os.amperecomputing.com>
openocd: remove last NULL comparisons The NULL pointers preceded by cast where not detected by the scripting tools looking for NULL pointer comparison. Remove them and, while there, further simplify the code and apply the other coding style rules. Change-Id: Ia7406122e07ef56ef311579ab0ee7ddb22c8e4b5 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6539 Tested-by: jenkins Reviewed-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com>
cortex_a: use the ap number specified at target create Current implementation ignores the flag '-ap-num' provided to command 'target create' and searches for the first AP of APB type. If specified, use the ap number. Change-Id: If1ac12345220d14a4a60515efe46dc2a2eac079a Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: https://review.openocd.org/c/openocd/+/6413 Tested-by: jenkins
openocd: remove NULL comparisons with checkpatch [1/2] Patch generated automatically through the new checkpatch with flags "--types COMPARISON_TO_NULL --fix-inplace". This only fixes the comparisons if (symbol == NULL) if (symbol != NULL) The case of NULL on the left side of the comparison is not tested. Some automatic fix is incorrect and has been massaged by hands: - if (*psig == NULL) + if (*!psig) changed as + if (!*psig) Change-Id: If4a1e2b4e547e223532e8e3d9da89bf9cb382ce6 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: http://openocd.zylin.com/6351 Tested-by: jenkins
openocd: fix simple cases of NULL comparison There are more than 1000 NULL comparisons to be aligned to the coding style. For recurrent NULL comparison it's preferable using trivial scripts in order to minimize the review effort. Patch generated automatically with the command: sed -i PATTERN $(find src/ -type f) where PATTERN is in the list: 's/(\([a-z][a-z0-9_]*\) == NULL)/(!\1)/g' 's/(\([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\) == NULL)/(!\1)/g' 's/(\([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\) == NULL)/(!\1)/g' 's/(\([a-z][a-z0-9_]*\) != NULL)/(\1)/g' 's/(\([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\) != NULL)/(\1)/g' 's/(\([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\) != NULL)/(\1)/g' 's/(NULL == \([a-z][a-z0-9_]*\))/(!\1)/g' 's/(NULL == \([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\))/(!\1)/g' 's/(NULL == \([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\))/(!\1)/g' 's/(NULL != \([a-z][a-z0-9_]*\))/(\1)/g' 's/(NULL != \([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\))/(\1)/g' 's/(NULL != \([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\))/(\1)/g' Change-Id: Ida103e325d6d0600fb69c0b7a1557ee969db4417 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: http://openocd.zylin.com/6350 Tested-by: jenkins
openocd: fix simple cases of Yoda condition There are ~900 Yoda conditions to be aligned to the coding style. For recurrent Yoda conditions it's preferable using a trivial script in order to minimize the review effort. E.g. comparison of uppercase macro/enum with lowercase variable: - ...(ERROR_OK == retval)... + ...(retval == ERROR_OK)... Patch generated automatically with the command: sed -i \ 's/(\([A-Z][A-Z0-9_]*\) \([=!]=\) \([a-z][a-z0-9_]*\))/(\3 \2 \1)/g' \ $(find src/ -type f) While there, remove the braces {} around a single statement block to prevent warning from checkpatch. Change-Id: If585b0a4b4578879c87b2dd74d9e0025e275ec6b Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: http://openocd.zylin.com/6354 Tested-by: jenkins Reviewed-by: Xiang W <wxjstz@126.com>
target/arm: rename CamelCase symbols No major cross dependencies, mostly changes internal to each file/function. Change-Id: I3f0879f0f33c6badc36a0dc60229323978a7e280 Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Reviewed-on: http://openocd.zylin.com/6338 Tested-by: jenkins Reviewed-by: Oleksij Rempel <linux@rempel-privat.de> Reviewed-by: Xiang W <wxjstz@126.com>