debug: openocd: Fix building for SMP platforms #18125
Conversation
subsys/debug/openocd.c
Outdated
| @@ -36,7 +36,9 @@ __attribute__((used, section(".openocd_dbg"))) | |||
| size_t _kernel_openocd_offsets[] = { | |||
| /* Version 0 starts */ | |||
| [OPENOCD_OFFSET_VERSION] = 1, | |||
| #ifndef CONFIG_SMP | |||
There was a problem hiding this comment.
is this going to cause issues w/openocd and the assumed layout of this data? Wondering if setting the value to 0 is better?
There was a problem hiding this comment.
0 is probably better, though might break things as well (don't know if openocd/gdb zephyr awareness relies on this).
Openocd Zephyr-SMP support might require more. Maybe instead of working around this, do not allow/use combination of SMP and openocd zephyr awareness for now?
There was a problem hiding this comment.
OPENOCD_UNIMPLEMENTED would be better than zero, but I also like @abrodkin's idea to handle this in KConfig.
|
All checks are passing now. Review history of this comment for details about previous failed status. |
|
@galak @ruuddw so maybe something like this until we really make sure OpenOCD works with Zephyr-aware on SMP systems? |
vonhust
force-pushed
the
topic-synchronization-openocd-smp-build
branch
from
700782c to
3d21cb1
Compare
|
Moving to next release since the review was not finalized for v.2.0.0 |
|
Any updates on this? |
|
@EvgeniiDidin could you please take a look at this one since you anyways deal with OpenOCD/GDB thingy these days? |
|
@abrodkin, @EvgeniiDidin any update on this? |
|
Hi, I tested SMP Zephyr smp/pi example with these changes on HSDK(4 core) board using OpenOCD/ gdb. It is possible to load and start the application on all cores, also I was able to get callee registers from stack. But after stopping execution "continue" command failed with next message: I guess it is related to the lack of current_thread{id} values in OpenOCD, which come from "current" struct. If incomplete smp aware support is permissible for Zephyr/OpenOCD, this commit is fine. Otherwise we should disable simultaneous usage of CONFIG_SMP and CONFIG_OPENOCD options. |
|
@abrodkin, @EvgeniiDidin any update on this? |
|
@galak I think the most recent comment of Eugeniy (#18125 (comment)) still valid. We either declare SMP mode doesn't support additional OpenOCD-related visibility options or ignore pointer to the "current thread" implemented in this commit. I now lean towards making |
|
Let's prevent openocd and SMP / multiple CPUs in kconfig. |
In case of SMP (i.e. multiple execution units processing the same list of tasks) we cannot use the same data structures for getting data about active tasks as with just one processor (UP). So until explicit support of SMP is added make sure we don't allow to select both OPENOCD & SMP simultaneously. Moreover starting from commit a203d21 ("kernel: remove legacy fields in _kernel") this will lead to build-time error if MP_NUM_CPUS > 1. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
abrodkin
force-pushed
the
topic-synchronization-openocd-smp-build
branch
from
3d21cb1 to
69af584
Compare
There was a problem hiding this comment.
LGTM but I need @andrewboie, @MaureenHelm, @galak or @nashif to approve
struct k_thread *current is only defined for !SMP platforms
leading to the following build-time failure:
| openocd.c:39:35: error: 'struct z_kernel' has no member named 'current'
| [OPENOCD_OFFSET_K_CURR_THREAD] = offsetof(struct z_kernel, current),
^~~~~~~~
Disable calculation of meaningless offset then.
Fixes #18124.