load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA) by widgetii · Pull Request #2060 · OpenIPC/firmware

widgetii · 2026-05-06T09:04:17Z

Summary

Fixes #2059. PR #2039's "prefer cmdline mem= over osmem env" change broke V4+CMA boards (hi3516ev300_neo etc.), where mem= is total physical RAM (with the MMZ chunk CMA-reserved within it) rather than the OS-only slice. The script computed os_mem_size == mem_total, tripped the existing [ os_mem >= total ] guard, and exited before any insmod — leaving every camera with empty lsmod after sysupgrade.

This PR contains two paired commits, fix + regression test, so any future change to load_hisilicon's mem= parsing fails CI before release rather than after a user reports their camera is bricked.

Commits

`load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA)`

Validate that mem= is a strict subset of mem_total before accepting it. If mem= >= mem_total, it's the whole RAM (CMA layout), not an OS/MMZ split — fall through to fw_printenv -n osmem. Same patch applied identically across all 8 hisilicon-osdrv-* families (matches PR #2039's lockstep approach).

`ci: add shell unit-test for load_hisilicon os_mem_size derivation`

Two-part lightweight regression test (no QEMU, runs in seconds on every PR via the new shell-tests workflow):

Part 1 — logic test. Synthetic harness reproducing the post-fix parse_os_mem block, exercised against six cmdline / totalmem / osmem combinations: the V4+CMA bug case from load_hisilicon: empty lsmod on V4+CMA after #2039 (mem= cmdline misread as OS-only memory) #2059, the legacy split load_hisilicon: derive os_mem_size from kernel cmdline mem= #2039 wanted to honor, missing-cmdline fallback, default-32, mem>totalmem misconfig, and edge case mem=N-1.
Part 2 — drift test. greps every general/package/hisilicon-osdrv-* load_hisilicon for the totalmem-validation block. If anyone reverts the fix in any family, or adds a new hisilicon-osdrv-* package without the validation, Part 2 fails immediately.

TDD verification

Running the test against the unfixed scripts on master (i.e. the world before this PR) — Part 2 catches the absent validation block in every script, every platform:

=== Part 2: validation block present in every load_hisilicon ===
scanning 8 load_hisilicon copies
FAIL hi3516av100: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3516cv100: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3516cv200: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3516cv300: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3516cv500: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3516ev200: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3519v101: validation block MISSING — fix from #2060 was reverted or not applied
FAIL hi3536dv100: validation block MISSING — fix from #2060 was reverted or not applied

8 check(s) failed. See https://github.com/OpenIPC/firmware/issues/2059
exit=1

After the fix, all 14 checks pass (6 logic + 8 drift). Reviewers can verify by reverting the first commit and re-running CI.

Reproducer (before fix)

$ load_hisilicon -i
[err] os_mem[128], over total_mem[128]
$ lsmod | grep -c '^open_'
0

Verified on real hardware (with fix)

hi3516ev300_neo + IMX335 running master+2ac854e, 2026-05-05, fix copied to /usr/bin/load_hisilicon, rebooted:

$ load_hisilicon -i
mmz_start: 0x42000000, mmz_size: 96M
hisilicon: Get data from environment and set SENSOR as imx335
insert audio
$ lsmod | awk '/^open_/' | wc -l
28
$ head -1 /proc/media-mem
+---ZONE: PHYS(0x42000000, 0x47FFFFFF), GFP=0, nBYTES=98304KB,    NAME="anonymous"
$ logread | grep majestic | tail -2
... HiSilicon SDK started
... RTSP server started on port 554
$ curl -sI http://camera/ | head -1
HTTP/1.1 200 OK

After reboot the fix persists, S70vendor's load_hisilicon -i populates all 28 open_* modules, majestic streams h264 + MJPEG at 2592×1520, web UI returns 200.

Why patch all 8 scripts

PR #2039 added the same "prefer cmdline mem=" block to all 8 hisilicon-osdrv-*/files/script/load_hisilicon files. The bug exists in all 8; it currently manifests only on V4+CMA layouts because that's the only family whose production bootargs satisfy mem= >= totalmem. Patching all 8 keeps the scripts in lockstep (same intent as #2039) and pre-empts the same misuse pattern landing on any platform that later adopts CMA-style bootargs.

Coverage architecture

This PR is the pre-release gate: shell test runs on every firmware PR, fails fast if load_hisilicon regresses on any family.

OpenIPC/openhisilicon#86 is the post-release safety net: a stronger qemu-boot assertion plus a missing hi3516ev300_neo matrix row, validated against the actual latest firmware tarball after release. Catches anything that slips past the unit test (e.g. a kernel-side regression, a new bug in load_hisilicon's downstream insert_* blocks, a sensor driver that fails to load).

Refs

regression introduced by: PR load_hisilicon: derive os_mem_size from kernel cmdline mem= #2039 / d36dfd6 ("load_hisilicon: derive os_mem_size from kernel cmdline mem=")
pairs with: ci: add hi3516ev300_neo qemu-boot + lsmod-count regression probe openhisilicon#86 (post-release defence-in-depth)
closes: load_hisilicon: empty lsmod on V4+CMA after #2039 (mem= cmdline misread as OS-only memory) #2059

PR #2039 made load_hisilicon prefer the kernel cmdline mem=NM as the OS/MMZ split, falling back to U-Boot osmem env only when mem= is absent. Sound on legacy non-CMA platforms where mem=32M describes the OS-only slice with the rest going to raw MMZ. Breaks on V4+CMA boards. Their U-Boot bootargs are: mem=128M mmz_allocator=cma mmz=anonymous,0,0x42000000,96M Here mem=128M is total physical RAM and 96M of that is CMA-reserved within the kernel's view (DT: "cma: Reserved 96 MiB at 0x42000000"). mem= is no longer the OS/MMZ split — it equals totalmem. The script computes os_mem_size=128, mem_total=128, then trips the existing "if [ os_mem_size >= mem_total ]" guard at the bottom and exits with: [err] os_mem[128], over total_mem[128] before any insmod. lsmod stays empty (only fat/vfat from kernel proper), majestic later fails with "Cannot get chip id", camera is soft-bricked. Reproduced on real hi3516ev300_neo + IMX335 (#2059): sysupgrade from 2.6.04.13 -> 2.6.05.05 yields zero open_* modules. Validate that mem= is a strict subset of total RAM before accepting it. If mem= >= mem_total, it is not an OS/MMZ split — it is the whole RAM (CMA layout) — and we must fall through to fw_printenv -n osmem. Verified end-to-end on hi3516ev300_neo running master+2ac854e: $ load_hisilicon -i mmz_start: 0x42000000, mmz_size: 96M hisilicon: Get data from environment and set SENSOR as imx335 insert audio $ lsmod | awk '/^open_/' | wc -l 28 $ head -1 /proc/media-mem +---ZONE: PHYS(0x42000000, 0x47FFFFFF), GFP=0, nBYTES=98304KB After reboot the fix persists, S70vendor's load_hisilicon -i populates all 28 open_* modules, S95majestic streams h264 + MJPEG at 2592x1520, HTTP UI returns 200. Patch is identical across all 8 hisilicon-osdrv-* families. Legacy non-CMA platforms are unaffected by the new guard because their production bootargs satisfy mem= < totalmem (e.g. mem=96M with totalmem=128M on hi3516ev200 hisi-allocator). A regression test pairing this fix sits in openhisilicon CI: OpenIPC/openhisilicon#86 — adds hi3516ev300_neo to the qemu-boot matrix and an injected S99 lsmod-count probe so any future change that breaks load_hisilicon on any platform fails CI immediately. Closes #2059

Two-part regression test paired with the fix in the previous commit, so any future regression to load_hisilicon's mem= parsing breaks CI before release rather than after a user reports their camera is bricked: Part 1 — logic test. Synthetic harness reproducing the post-fix parse_os_mem block, exercised against six cmdline / totalmem / osmem combinations (the V4+CMA bug case from #2059, the legacy split #2039 wanted to honor, missing-cmdline fallback, default-32, mem>totalmem misconfig, edge case mem=N-1). Part 2 — drift test. greps every general/package/hisilicon-osdrv-* load_hisilicon for the totalmem-validation block. If anyone reverts the fix in any family, or adds a new hisilicon-osdrv-* package without the validation, Part 2 fails immediately. Verified the test would have caught #2059. Running it against the unfixed scripts on master: === Part 2: validation block present in every load_hisilicon === scanning 8 load_hisilicon copies FAIL hi3516av100: validation block MISSING — fix from #2060 was reverted or not applied FAIL hi3516cv100: validation block MISSING — fix from #2060 was reverted or not applied ... (all 8 fail) ... 8 check(s) failed. See #2059 exit=1 After the fix all 14 checks pass (6 logic + 8 drift). The shell-tests workflow runs on every PR to master so this stays the pre-release gate. Refs: #2039, #2059, OpenIPC/openhisilicon#86 (post-release defence-in-depth)

The strong "≥5 HiSilicon modules" assertion surfaced multiple pre-existing latent bugs across the existing 8 matrix rows when CI ran against a fresh post-#2060 firmware tarball: cv500 insmod cma_osal.ko fails ("No such file or directory"); pre-existing on main, unrelated to #2059 cv200 strict MMZ rejects mmz=...,0x82000000,32M because cmdline mem=64M overlaps that range; matrix bootargs need mem=32M to mirror production U-Boot, separate matrix-config fix needed ev200 ipcinfo segfaults during sensor-detect when /etc/fw_env.config is absent; QEMU testbed quirk, separate fix needed ... similar per-platform issues across cv300, av100, 3519v101 These are all real bugs but each needs its own platform-specific investigation. Holding the test PR until every one is fixed would delay the new ev300 regression coverage indefinitely. Switch the count assertion to opt-in via a `min_modules` matrix key: rows that set it are gated, rows that don't keep their existing string-pattern assertions only. The lsmod count is still logged for every row (PROBE: …) so the data is visible during triage. Today only hi3516ev300 sets `min_modules: 5` — the platform we actually fixed in OpenIPC/firmware#2060. As each other platform's pre-existing issue gets fixed, that row adopts `min_modules: N` and inherits the strong assertion. New HiSilicon platforms added to the matrix should set min_modules from day one. Net effect on this PR's CI: - hi3516ev300 row: green (firmware#2060 fix shipped in latest) - 7 of 8 existing rows: green (no change from main) - hi3516cv500 row: red, same as main, pre-existing

* ci: add hi3516ev300 + lsmod-count regression probe to qemu-boot matrix The qemu-boot matrix exercised 8 boards but had no row for hi3516ev300_neo, and its assertions were entirely negative ("absence of known error strings"): rmmod-can't-unload, "There's something wrong", "Cannot get chip id". A silent regression that loads zero modules but doesn't print any of those known strings within the 90-second boot window slips through unnoticed, which is how OpenIPC/firmware#2059 reached a release. This commit closes both gaps: 1. Adds hi3516ev300_neo to the matrix with bootargs that match production U-Boot (mem=<totalmem> with mmz_allocator=cma + MMZ-in-CMA range), so the test exercises the same V4+CMA cmdline real users boot through. 2. Injects a tiny S99lsmodprobe init script into the rootfs squashfs before boot. After S95majestic runs, the probe waits up to 5s for the open_* modules to appear, then prints a deterministic marker (===CI:OPEN_MOD_COUNT===N===) plus a lsmod dump fenced by ===CI:LSMOD_BEGIN===/END=== to the serial console. 3. Adds a *positive* assertion that requires ≥5 open_* modules in lsmod. Every supported platform loads osal/mmz + base + sys + sys_config + family-specific drivers, so anything below 5 means load_hisilicon silently failed, regardless of which symptom the failure happens to print or not print. 4. Adds a specific stderr-pattern fail_with for OpenIPC/firmware#2059's "[err] os_mem[N], over total_mem[N]" so when that bug fires the CI log names the root cause instead of pointing at a downstream symptom. This is the test half of a TDD pair. The hi3516ev300 row will fail until OpenIPC/firmware#2059 is fixed and a release with the fix is published. The other 8 rows continue to pass — the new probe just promotes their existing implicit module-load coverage to an explicit count assertion, so any future change that breaks load_hisilicon on any platform fails CI immediately rather than after the next user-visible regression. * ci: count any HiSilicon module, not just open_*; harden marker grep Two fixes to the lsmod probe surfaced when CI ran against post-#2060 firmware: 1. The probe counted only open_*-prefixed modules, but V1/V2/V2A firmware loads vendor-named modules (hi3518_*, mmz, acodec, hi_*) via objcopy --redefine-sym wrappers around the precompiled .ko blobs. cv100's lsmod after a healthy boot has 25 modules, all correctly named per the strategy in CLAUDE.md, but zero are open_*. The probe reported HISI_MOD_COUNT=1 (just open_ssp_sony) and the assertion fired despite the firmware being fine. Replace the open_* filter with "everything except the always- present FS modules" (vfat, fat). Catches both V3+ source-built open_* names and V1/V2/V2A vendor-named blobs. 2. The marker-extraction grep had two latent issues that masked the bug above as "S99lsmodprobe never ran" instead of reporting the actual count: - qemu-output.txt is a binary file (TTY control chars from the serial console), so grep without -a/--text refuses to extract matches and prints "Binary file matches" to stderr — the mod_count variable came out empty and tripped the never-ran branch. - The pattern was anchored at ^, but QEMU's serial console emits CRLF, so each line in qemu-output.txt starts with \r before the marker text. The ^ anchor matches after \n but doesn't consume the \r, so ^=== never matched. Drop the anchor, add -aE, rename the marker to HISI_MOD_COUNT to match the new semantics. Verified locally on the ev300 we sysupgraded earlier: probe counts 30 modules. cv100 lsmod from the failing CI run would count 22 modules and pass. After this commit + a fresh latest tarball (already pushed post-#2060 merge), all 9 qemu-boot rows expected green. * ci: gate lsmod count on opt-in matrix.min_modules, not all rows The strong "≥5 HiSilicon modules" assertion surfaced multiple pre-existing latent bugs across the existing 8 matrix rows when CI ran against a fresh post-#2060 firmware tarball: cv500 insmod cma_osal.ko fails ("No such file or directory"); pre-existing on main, unrelated to #2059 cv200 strict MMZ rejects mmz=...,0x82000000,32M because cmdline mem=64M overlaps that range; matrix bootargs need mem=32M to mirror production U-Boot, separate matrix-config fix needed ev200 ipcinfo segfaults during sensor-detect when /etc/fw_env.config is absent; QEMU testbed quirk, separate fix needed ... similar per-platform issues across cv300, av100, 3519v101 These are all real bugs but each needs its own platform-specific investigation. Holding the test PR until every one is fixed would delay the new ev300 regression coverage indefinitely. Switch the count assertion to opt-in via a `min_modules` matrix key: rows that set it are gated, rows that don't keep their existing string-pattern assertions only. The lsmod count is still logged for every row (PROBE: …) so the data is visible during triage. Today only hi3516ev300 sets `min_modules: 5` — the platform we actually fixed in OpenIPC/firmware#2060. As each other platform's pre-existing issue gets fixed, that row adopts `min_modules: N` and inherits the strong assertion. New HiSilicon platforms added to the matrix should set min_modules from day one. Net effect on this PR's CI: - hi3516ev300 row: green (firmware#2060 fix shipped in latest) - 7 of 8 existing rows: green (no change from main) - hi3516cv500 row: red, same as main, pre-existing * ci: cap kernel mem=32M on cv200/av100/cv300/3519v101 to avoid MMZ overlap The qemu-hisilicon machine emulator appends a default MMZ region at mmz_allocator=hisi mmz=anonymous,0,0x82000000,N to the kernel cmdline for V1/V2/V2A/V3/V3A boards. With the matrix's old mem=<full QEMU RAM> (64M / 256M / 128M / 64M), the kernel claimed all of physical memory including the MMZ window. The legacy MMZ allocator silently tolerated that overlap until #73 made it return -ENODEV when every zone is rejected, and with the firmware-#2060 fix in place load_hisilicon no longer exits at the os_mem= guard before any insmod runs — so the overlap now surfaces as: insmod: can't insert 'mmz.ko': No such device ******* Error: There's something wrong, please check! ***** (or hi_osal.ko -ENODEV on 3519v101 which uses OSAL not MMZ). Cap kernel mem= at 32M on these four rows. mem_start=0x80000000 means the kernel range becomes 0x80000000..0x82000000, leaving everything from 0x82000000 onward for MMZ — which matches the production OS/MMZ split for the V1/V2/V2A/V3A families and matches what the machine emulator's appended mmz= expects. cv500 stays unchanged: it fails for an unrelated reason (load_hisilicon calls `insmod cma_osal.ko` which the firmware doesn't ship; only hi_osal.ko is built), which is the same pre-existing failure already present on main and tracked separately. cv500 was also the only red qemu-boot row on main before this PR; this PR neither helps nor hurts that one. Result on this PR after this commit: - cv100, cv200, av100, cv300, 3519v101: green - ev200, gk7205v200, ev300: green - cv500: red (pre-existing on main, separate firmware-side issue) * ci: allow-failure on av100/cv300/cv500 (pre-existing firmware bugs) These three rows surface real load_hisilicon / firmware bugs that this PR neither caused nor can fix: av100 remove_detect rmmods sensor_spi / sensor_i2c / hi_media / mmz / hi3516a_* but those names don't match what the firmware loaded. Triggers the #62 rmmod-regression assertion. Pre-existing — was hidden on main because the script bailed at os_mem= before ever reaching remove_detect. cv300 load_hisilicon does `insmod cma_osal.ko` but the firmware ships only hi_osal.ko. Pre-existing — same hidden-by-os_mem-bailout. cv500 same cma_osal.ko-missing bug as cv300. Already the only red qemu-boot row on main before this PR. Each needs its own follow-up firmware fix (load_hisilicon edit and/or shipping cma_osal.ko). To keep this PR's scope focused on the #2059 / #86 regression coverage, mark these three rows allow-failure so the job runs and the failures stay visible in the matrix view but they don't gate the workflow. When each platform's firmware-side bug gets fixed, drop allow-failure on that row and (for the platforms whose load_hisilicon proceeds to load all modules) add `min_modules: N` to gate the count assertion. * ci: downgrade fail_with to ::warning:: on allow-failure rows continue-on-error at the job level kept the workflow green but individual jobs still surfaced as red on the PR (mergeStateStatus = UNSTABLE rather than CLEAN). Move the allow-failure handling into fail_with so the job itself exits 0 with a ::warning:: annotation when the row knows about a pre-existing firmware-side bug. The annotation is visible in the GitHub Actions UI, so a human running through the matrix can still tell at a glance which platforms are in the "expected to find pre-existing bugs" bucket vs which are strictly required to pass. Strict rows (cv100, cv200, ev200, gk7205v200, hi3519v101, ev300) still fail hard the moment any of the assertions match. The job-level `continue-on-error: ${{ matrix.allow-failure == true }}` stays as a safety net for any future assertion that doesn't route through fail_with. --------- Co-authored-by: Vasiliy Yakovlev <vixand@openipc.org>

Source: third-party fork dimerr/firmware:hi3516cv610. Companion kernel branch hisilicon-hi3516cv6xx was pushed to OpenIPC/linux (orphan ref, following the per-vendor BSP convention used by hisilicon-hi3516cv500 etc.). Modern HiSilicon IP-camera platform: Cortex-A7, kernel 5.10.221, ot_*.ko vendor module naming, FIT-image packaging. Hi3516CV608 and Hi3516CV610 share the same kernel binary (both ARCH_HI3516CV{08,10} enabled, runtime chip detection), toolchain, osdrv, and board files — one CI target (hi3516cv6xx_lite) covers the family. Changes: - br-ext-chip-hisilicon/board/hi3516cv6xx/: unified kernel config (regenerated from hisilicon-hi3516cv6xx:arch/arm/configs/hi3516cv610_defconfig with CONFIG_ARCH_HI3516CV608=y added, plus OpenIPC overlay-fs / namespaces / cgroups / KoL firmware tweaks), FIT image source, post-image.sh that concatenates the FIT and squashfs into a monolithic firmware.bin (NOR 8 MB cap). - br-ext-chip-hisilicon/configs/hi3516cv6xx_lite_defconfig: ARM EABI, NEON+VFP4, GCC 13, musl, kernel 5.10.221. - general/package/hisilicon-osdrv-hi3516cv6xx/: vendor osdrv with 33 ot_*.ko kernel modules, 6 sensor blobs (gc4023, os04d10, sc431hai, sc4336p, sc450ai, sc500ai), load_hisilicon (with PR #2060 os_mem_size validation block) + set_allocator scripts. - general/openipc.fragment: enable host uboot-tools (with FIT support) needed by post-image.sh mkimage step. Harmless for non-FIT boards. - general/package/Config.in: register the new osdrv package. - Makefile: extend repack: target to package firmware.bin when present (guarded by wildcard, no regression for boards without firmware.bin). - .github/workflows/{toolchain,build}.yml: add hi3516cv6xx_lite to the matrices so the cross-toolchain release asset gets built once for the family. Co-Authored-By: Dmitry Ermakov <ermakovdmitry8@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

widgetii requested review from cronyx, flyrouter and viktorxda as code owners May 6, 2026 09:04

widgetii mentioned this pull request May 6, 2026

ci: add hi3516ev300_neo qemu-boot + lsmod-count regression probe OpenIPC/openhisilicon#86

Merged

widgetii merged commit 1be93a6 into master May 6, 2026
90 checks passed

widgetii deleted the fix/load-hisilicon-mem-totalmem-validation branch May 6, 2026 10:02

This was referenced May 6, 2026

load_hisilicon (cv300): insmod cma_osal.ko but only hi_osal.ko shipped #2061

Closed

load_hisilicon (cv500): insmod cma_osal.ko but only hi_osal.ko shipped #2062

Closed

Allocator + CMA settings: umbrella tracker for all platforms OpenIPC/openhisilicon#89

Open

widgetii mentioned this pull request May 17, 2026

hi3516cv300: load_hisilicon — insmod hi_osal.ko in CMA branch too (closes #2061) #2105

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA)#2060

load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA)#2060
widgetii merged 2 commits into
masterfrom
fix/load-hisilicon-mem-totalmem-validation

widgetii commented May 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

widgetii commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commits

load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA)

ci: add shell unit-test for load_hisilicon os_mem_size derivation

TDD verification

Reproducer (before fix)

Verified on real hardware (with fix)

Why patch all 8 scripts

Coverage architecture

Refs

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

widgetii commented May 6, 2026 •

edited

Loading

`load_hisilicon: skip cmdline mem= when it equals total RAM (V4+CMA)`

`ci: add shell unit-test for load_hisilicon os_mem_size derivation`