Repository navigation
feat(freertos-x5h): move the resource table to the demo carveout - #59
Merged
Merged
Conversation
This was referenced Sep 23, 2026
youtalk
force-pushed
the
si-upstream-rpmsg-si
branch
from
October 1, 2026 17:19
da843e7 to
6ccd379
Compare
youtalk
force-pushed
the
si-upstream-rpmsg-si
branch
from
October 1, 2026 20:50
6ccd379 to
77b4dfa
Compare
youtalk
force-pushed
the
si-upstream-resource-table
branch
from
October 1, 2026 20:50
39befd8 to
54dd34f
Compare
oguzkaganozt
force-pushed
the
si-upstream-resource-table
branch
from
October 8, 2026 12:11
54dd34f to
aa2a5e0
Compare
Member
Author
|
@oguzkaganozt Could you also review #57 and #58? |
…ault latch A second named endpoint beside rpmsg-eth. The CR52 sends one "hb seq=N uptime_ms=N fault=F" line per second and latches fault=1 / fault=0 from Linux. build.sh now runs the ELF contract checker for the second service name too, so both strings have to survive the link. si_channel.c/.h is deliberately pure C (no FreeRTOS, no OpenAMP) so the message parsing, formatting and fault latch run as a host unit test under BUILD_TEST=1. The fault latch has a single writer, si_channel_rx() in poll-task context, and several readers; that invariant is what makes the plain reads safe on this single-core target, and the source says so. A _Static_assert ties the heartbeat line buffer to the worst-case formatted length, so growing the format string fails the build instead of silently quieting the heartbeat on the board. The fault input is one more input to the SI supervisor. On the X5H build the control tick reads si_channel_fault() and latches SI_STOP on its rising edge. fault=0 does not release the stop: in the demo, fault=1 is an operator injection, and only the operator re-enable may resume driving. Re-enable also waits until the input is back at 0. freertos_x5h/CMakeLists.txt puts this directory on actuation_x5h's include path unconditionally rather than only under X5H_DIAG_TASK_TABLE. The heartbeat task runs at the same priority as tcpip_thread and shares rdev->lock with it through rpmsg_trysend(). That is safe for three reasons the source records: the non-blocking path never sleeps while holding the lock, nothing in this tree uses the blocking path, and the mutex carries FreeRTOS priority inheritance. The source also records why early heartbeats never reach Linux (dest_addr stays RPMSG_ADDR_ANY until Linux binds), and why the bring-up failure path leaves the endpoint alive instead of calling rpmsg_destroy_ept(), whose blocking NS DESTROY wait could stall bring-up for up to 15 seconds. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
si_heartbeat_task waited with a relative vTaskDelay(), so the work it does after waking -- formatting the line and pushing it down the vring, about 2.2 ms -- was added to every period instead of being absorbed by it. The heartbeat therefore ran permanently slow rather than jittering around 1 Hz. Gate D3 on board 2, 2026-09-18, measured 1002.24 ms per heartbeat by the firmware's own tick count over 119 consecutive heartbeats. Across the gate's 600 s window that is 1.3 heartbeats lost to drift plus one more at the window edge, so 597 arrived where the gate wants at least 598, and it failed with reason=too_few. Sequence numbers were consecutive throughout and gaps was 0: the link lost nothing, the period was simply wrong. The error is invisible at short windows, which is why the gate runs for ten minutes. vTaskDelayUntil() schedules on last_wake + period, so the send cost no longer accumulates. If a cycle overruns its period the call returns immediately and catches up. This also matters beyond D3. The firmware trips its stop 0.5 s after the last heartbeat, so a slow heartbeat eats into the margin that staleness check depends on. Drop the comment block that defended the old relative delay against a "No vTaskDelay() here" rule this file no longer states. There is no vTaskDelay() left on this path to defend. The 5 s diagnostic beacon in x5h_diag.c has the same relative-delay shape and measured 5011 ms. It is left alone: it is diagnostic output only, nothing is timed against it, and the uptime_ms it prints comes from xTaskGetTickCount() so the value it reports stays truthful. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
The control tick sampled only the current rpmsg-si fault level. If "fault=1" and then "fault=0" arrived between two ticks, the tick saw 0 and never latched SI_STOP, so the vehicle kept driving. si_channel.c now counts fault 0->1 edges, and the controller latches when the count changes. A counter keeps si_channel_rx() the only writer, so the existing lock-free single-writer rule still holds. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
oguzkaganozt
force-pushed
the
si-upstream-resource-table
branch
from
October 11, 2026 12:52
aa2a5e0 to
9261dbb
Compare
Resolve the controller state conflict by retaining main's fresh-odometry, monotonic StopRamp alongside the rpmsg-si fault-event counter. Preserve the supervisor fault pulse and explicit re-enable behavior. Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>
Place .resource_table at 0x5da00000, the base of cr52_ram1@5da00000 that the demo boot role reserves outside every NPU region, through a new linker script under vendor_patched. Both ELF checkers now assert the new address, and README.md's "must not be modified" rule for check-elf-contract.sh is reworded to allow the one legitimate exception this change takes: the contract itself moving. Add the MPU region the carveout needs by shadowing system_rcar_gen5.c. The region is 4 MiB, not the 2 MiB cr52_ram1 alone: the vrings and the rpmsg buffer pool are separate carveouts that sit above it, and every access to one faulted without them mapped. Gate D1b on board 2 took a data abort in rpmsg_init_vdev at DFAR=0x40f8e000 before the region was widened. The resource table keeps FW_RSC_ADDR_ANY, because rproc_alloc_vring matches carveouts by name and not by the address the firmware publishes, and writes the address Linux chose back into the table. The new linker script carries this repository's copyright line and SPDX identifier, matching every other file in vendor_patched/. The vendor script it replaces, common/linker/lscript_rsc_table_vram2.ld, carries no header at all, so there is no upstream notice to preserve. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
… frames rpmsg_transport_init() now reads the negotiated tx buffer size and logs if it is smaller than RPMSG_ETH_MAX_FRAME, naming the 2048-byte kernel patch an operator needs to check for. The existing _Static_assert only proved the frame fits the compile-time RPMSG_BUFFER_SIZE this port was written against. What governs the wire is the size negotiated with the vring Linux publishes, which is 512 B on a kernel without that patch. rpmsg_virtio_send_offchannel_raw() truncates an oversized send to the negotiated size and still returns success, so a stock kernel would put malformed, truncated Ethernet frames on the Linux side with a tx_ok counter that keeps climbing and nothing on the console to show it. This checks and does not abort. A truncating link is still worth bringing up for diagnosis. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
check-dds-config.sh asserts RPMSG_BUFFER_SIZE=2048 against freertos_x5h/CMakeLists.txt, but that assertion sat inside the guard on rpmsg_netif_core.h existing. An unrelated header going missing would have stopped enforcing this branch's central wire constant silently instead of failing loudly. Give the assertion its own guard on the file it actually reads. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Deletion-only cleanup. Both firmware ELFs are byte-identical before and after. CI: the "Re-verify" steps re-ran check-dds-config.sh, check-elf-contract.sh and check-image-budget.sh with the exact commands and arguments build.sh already runs inside the build step of the same job. build.sh uses `set -e`, so a failing checker already fails that step. The two "Assert" steps in the arm64 edge-ECU job repeated assertions build-edge-ecu-peer-arm64.sh makes internally. Comments: - freertos_main.cpp restated the frozen memory layout, and its copy still named the old resource-table address 0x96650000. README.md and check-elf-contract.sh carry the real one. - README.md stated three times that build.sh runs both checkers against both ELFs. One copy per checker section is enough. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
oguzkaganozt
force-pushed
the
si-upstream-resource-table
branch
from
October 11, 2026 13:37
9261dbb to
666c072
Compare
25 of 39 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
This moves
.resource_tablefrom 0x96650000 to 0x5da00000, the base of thecr52_ram1@5da00000carveout that the demo boot role reserves outside every NPU region. A new linker script undervendor_patched/places it there, and both ELF checkers now assert the new address. This also adds the MPU region the carveout needs, by shadowingsystem_rcar_gen5.cthe same wayinterrupts.cis already shadowed.Why the region is 4 MiB and not 2 MiB
cr52_ram1alone is 2 MiB, and it holds only the resource table. The vrings and the rpmsg buffer pool are three separate carveouts that sit above it, and every access to one faulted while they were unmapped. Gate D1b on board 2 took a data abort inrpmsg_init_vdevatDFAR=0x40f8e000before the region was widened. One 4 MiB region now covers all four carveouts.The resource table keeps
FW_RSC_ADDR_ANY.rproc_alloc_vringmatches carveouts by name and not by the address the firmware publishes, and it writes the address Linux chose back into the table. The device tree node names and this region have to stay in step. A carveout outside this window is a data abort, not a diagnostic.The other three commits
rpmsg_transport_init()now reads the negotiated rpmsg tx buffer size and logs when it is smaller thanRPMSG_ETH_MAX_FRAME. The existing_Static_assertonly proved that the frame fits the compile-timeRPMSG_BUFFER_SIZEthis port was written against. What governs the wire is the size negotiated with the vring Linux publishes, which is 512 bytes on a kernel without the 2048 byte patch.rpmsg_virtio_send_offchannel_raw()truncates an oversized send and still returns success, so a stock kernel would put malformed frames on the Linux side with atx_okcounter that keeps climbing. This checks and does not abort, because a truncating link is still worth bringing up for diagnosis.check-dds-config.shassertsRPMSG_BUFFER_SIZE=2048againstfreertos_x5h/CMakeLists.txt, but that assertion sat inside the guard on an unrelated header existing. It now has its own guard on the file it actually reads.The last commit is deletion only. The "Re-verify" workflow steps re-ran three checkers with the exact commands
build.shalready runs inside the build step of the same job, andbuild.shusesset -e, so a failing checker already fails that step. The two "Assert" steps in the arm64 edge-ECU job repeated assertions thatbuild-edge-ecu-peer-arm64.shmakes internally. Two stale comments go with them, including one infreertos_main.cppthat still named the old resource-table address.How this was verified
./build.sh --platform freertos-x5hsucceeds. Both ELFs passcheck-elf-contract.shfor both service names, and both passcheck-image-budget.shat 84.6 percent and 66.2 percent of the 10 MiB slot window.readelf -SWconfirms.resource_tableat5da00000, size0x100../build.sh --platform freertos-posix --unit-testsucceeds. All seven host test binaries exit 0.check-dds-config.shpasses.shellcheck -S erroris clean oncheck-elf-contract.sh,check-image-budget.sh,check-dds-config.sh, andbuild.sh.The QEMU gate cannot see this class of change, and the board evidence quoted above comes from earlier sessions. No board session was run for this pull request. The board smoke test is still the final arbiter for the MPU change.
What this depends on
This is based on #58 (
si-upstream-rpmsg-si), then #57 (si-upstream-stop-profile), then #71. All three have to land first.Refs: autowarefoundation/openadkit#147