Skip to content

Conversation

@djurny
Copy link
Contributor

@djurny djurny commented May 13, 2025

Description

  1. Avoid oboe overlap trip during loading of images
    • Add switch to turn avoidance on/off
    • Increment base address before alignment to resolve the oboe
    • oboe observed in U-Boot v2021.04 and in particular with DT loading
  2. Add early exits for critical errors
    • Check all load actions
    • Add optional early exit with explanation in case a load fails
  3. Bootscript enhancements
    • Added FDT resize/trim to allow big DT and overlays and still use
      configured load addresses in case setexpr not available
    • Moved FDT resize and next load address alignment
    • Added filename information to load instructions
    • Moved all "global" variable determinations to the top of the
      bootscript

GitHub issue reference: 8178
Jira reference number AR-2680

Documentation summary for feature / change

If documentation entry is predicted, please provide key elements for further implementation into main documentation and set label to "Needs Documentation". You are welcome to open a PR to documentation or you can leave following information for technical writer:

  • overlap oboe avoidance

    • short description (copy / paste of PR title)
      [Bug]: U-Boot load address calculation bug and DT overlap oobe
    • summary (description relevant for end users)
      In U-Boot v2021.04 the bootm_find_images() function has an oboe when checking for overlap. To avoid this, a setting was added to simply add 1 to the load address during alignment.
    • example of usage (how to see this in function)
      The overlap avoidance is enabled per default, but it can be disabled by adding align_overlap_oboe_avoidance=off to armbianEnv.txt.
  • ease boot issue troubleshooting

    • short description (copy / paste of PR title)
      Stop processing the bootscript whenever a situation arises that cannot be recovered from.
    • summary (description relevant for end users)
      The bootscript will continue even when certain mandatory/required things fail to process. To speed up advancin to the next bootmeth[od], add early exits in these situations. Such as, if device tree cannot be loaded, if device tree cannot be restored after an overlay failed to apply, when kernel image fails to load, initial ramdisk fails to load.
    • example of usage (how to see this in function)
      The early exit is enabled per default, but can be disabled by adding exit_on_critical_errors=off to armbianEnv.txt.

How Has This Been Tested?

  • Test A
    Good weather tested updated boot.scr on 25.5.0-trunk.435.

    U-Boot 2019.04 (May 10 2025 - 20:29:15 -0700)
    
    SoC:   MV88F6828-A0 at 1600 MHz
    DRAM:  2 GiB (800 MHz, 32-bit, ECC enabled)
    MMC:   mv_sdh: 0
    Loading Environment from MMC... *** Warning - bad CRC, using default environment
    
    Model: Helios4
    Board: Helios4
    SCSI:  MVEBU SATA INIT
    Target spinup took 0 ms.
    Target spinup took 0 ms.
    AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
    flags: 64bit ncq led only pmp fbss pio slum part sxs 
    
    Net:   
    Warning: ethernet@70000 (eth1) using random MAC address - c2:92:1b:ee:5a:8e
    eth1: ethernet@70000
    Hit any key to stop autoboot:  0 
    switch to partitions #0, OK
    mmc0 is current device
    Scanning mmc 0:1...
    Found U-Boot script /boot/boot.scr
    6586 bytes read in 418 ms (14.6 KiB/s)
    ## Executing script at 00200000
    Boot script loaded from mmc
    Loading environment /boot/armbianEnv.txt from mmc to 0x00300000 ...
    177 bytes read in 403 ms (0 Bytes/s)
    Loading DT /boot/dtb/armada-388-helios4.dtb from mmc to 0x02040000 ...
    28834 bytes read in 843 ms (33.2 KiB/s)
    Trimming DT ...
    Loading kernel /boot/zImage from mmc to 2049000 ...
    8858728 bytes read in 2051 ms (4.1 MiB/s)
    Loading initial ramdisk /boot/uInitrd from mmc to 28bc000 ...
    11504566 bytes read in 2451 ms (4.5 MiB/s)
    Booting kernel from 2049000 ...
    ## Loading init Ramdisk from Legacy Image at 028bc000 ...
       Image Name:   uInitrd
       Image Type:   ARM Linux RAMDisk Image (gzip compressed)
       Data Size:    11504502 Bytes = 11 MiB
       Load Address: 00000000
       Entry Point:  00000000
       Verifying Checksum ... OK
    ## Flattened Device Tree blob at 02040000
       Booting using the fdt blob at 0x2040000
       Loading Ramdisk to 0f507000, end 0ffffb76 ... OK
       Loading Device Tree to 0f4fc000, end 0f506fff ... OK
    
    Starting kernel ...
    
  • Test B
    Changed DT filename to test DT load failure on 25.5.0-trunk.435.

    U-Boot 2019.04 (May 10 2025 - 20:29:15 -0700)
    
    SoC:   MV88F6828-A0 at 1600 MHz
    DRAM:  2 GiB (800 MHz, 32-bit, ECC enabled)
    MMC:   mv_sdh: 0
    Loading Environment from MMC... *** Warning - bad CRC, using default environment
    
    Model: Helios4
    Board: Helios4
    SCSI:  MVEBU SATA INIT
    Target spinup took 0 ms.
    Target spinup took 0 ms.
    AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
    flags: 64bit ncq led only pmp fbss pio slum part sxs 
    
    Net:   
    Warning: ethernet@70000 (eth1) using random MAC address - c2:92:1b:ee:5a:8e
    eth1: ethernet@70000
    Hit any key to stop autoboot:  0 
    switch to partitions #0, OK
    mmc0 is current device
    Scanning mmc 0:1...
    Found U-Boot script /boot/boot.scr
    6413 bytes read in 409 ms (14.6 KiB/s)
    ## Executing script at 00200000
    Boot script loaded from mmc
    Loading environment from mmc to 0x00300000 ...
    177 bytes read in 395 ms (0 Bytes/s)
    Loading DT from mmc to 0x02040000 ...
    !! CRITICAL - Could not load DT from mmc to 0x02040000
    SCRIPT FAILED: continuing...
    starting USB...
    

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
    Warnings have been added to boot-mvebu.cmd to ease boot issue investigation.
  • Any dependent changes have been merged and published in downstream modules

@djurny djurny requested a review from igorpecovnik as a code owner May 13, 2025 18:11
@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines 05 Milestone: Second quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... labels May 13, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 13, 2025

"""

Walkthrough

The changes enhance the boot-mvebu.cmd boot script by introducing improved error handling and more descriptive logging for all critical load operations. Two new environment variables, exit_on_critical_errors and align_overlap_oboe_avoidance, are added to control script exit behavior on critical errors and to refine address alignment logic, respectively. The script now conditionally checks the success of loading environment, device tree, overlays, fixup scripts, kernel, and ramdisk, providing detailed error messages and optionally exiting on failure. The device tree load and overlay application logic have been restructured for clarity and robustness.

Assessment against linked issues

Objective Addressed Explanation
Fix address alignment logic in align_addr_next to avoid overlap/oobe detection bug (AR-2680)
Use itest instead of test for hex value comparisons in alignment logic (AR-2680)
Add flag/logic to increment address before modulo for alignment (AR-2680)
Improve error handling and logging for critical boot script operations (AR-2680)

Possibly related PRs

  • Update boot-mvebu.cmd #8166: Introduced address alignment, dynamic DT resizing, and load sequencing improvements, which are further extended by the current PR with robust error handling and logging.

Suggested labels

ready to merge

Suggested reviewers

  • igorpecovnik
    """

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbf285d and 615287e.

📒 Files selected for processing (1)
  • config/bootscripts/boot-mvebu.cmd (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • config/bootscripts/boot-mvebu.cmd

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label May 13, 2025
@djurny
Copy link
Contributor Author

djurny commented May 13, 2025

Trying to work towards applying the same fixes to boot-sunxi.cmd as some people have reported similar overlap issues on Orangepi/Bananapi/allwinner images.

@igorpecovnik
Copy link
Member

igorpecovnik commented May 14, 2025

@djurny Wanna help covering this board officially? https://www.armbian.com/update-data/ Those are directions: https://docs.armbian.com/User-Guide_Board-Support-Rules/#standard-support

Also you can prepare release images if you want:
https://docs.armbian.com/Process_CI/#automation-for-developers-and-maintainers

djurny added 3 commits May 14, 2025 12:00
armbian#8178
- Add switch to turn avoidance on/off
- Increment base address before alignment to resolve the oboe
- oboe observed in U-Boot v2021.04 and in particular with DT loading
- Check all load actions
- Add optional early exit with explanation in case a load fails
- Added FDT resize/trim to allow big DT and overlays and still use
  configured load addresses in case `setexpr` not available
- Moved FDT resize and next load address alignment
- Added filename information to load instructions
- Moved all "global" variable determinations to the top of the
  bootscript
@github-actions github-actions bot removed the Ready to merge Reviewed, tested and ready for merge label May 14, 2025
@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label May 14, 2025
Copy link
Member

@igorpecovnik igorpecovnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested, works.

@igorpecovnik igorpecovnik merged commit d027115 into armbian:main May 14, 2025
1 check passed
@coderabbitai coderabbitai bot mentioned this pull request Aug 11, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

05 Milestone: Second quarter release Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review Ready to merge Reviewed, tested and ready for merge size/medium PR with more then 50 and less then 250 lines

Development

Successfully merging this pull request may close these issues.

2 participants