Skip to content

Conversation

jackfiled
Copy link
Contributor

📝 Click for commit rules checklist / 点击查看提交规范检查表
  • Commit Rules (EN)
  • Commit Rules (ZH)

The main questions in allocating the patched fdt on the heap are:

  • How to get the actual length of patched fdt?
  • Is the heap supported to allocate such long buffer?

In this pr,

  • The length of patched fdt is acquired by writing twice strategy.
  • The heap is adjusted to allocate buffer with max length 65536 bytes.

The serde_device_tree crate may provide a function to acquire the actual length of patched fdt.

Signed-off-by: jackfiled <xcrenchangjun@outlook.com>
@luojia65 luojia65 requested review from Copilot, guttatus and woshiluo and removed request for Copilot and woshiluo September 19, 2025 15:36
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the device tree patching mechanism in the prototyper to use heap allocation instead of static buffers. The main purpose is to dynamically allocate the patched FDT (Flattened Device Tree) on the heap with the exact required size.

  • Removes static buffer allocation for patched FDT and uses heap allocation
  • Implements a two-pass strategy to determine exact buffer size needed
  • Increases heap size and buddy allocator max order to support larger allocations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
prototyper/prototyper/src/sbi/heap.rs Increases buddy allocator max order and adds detailed heap allocation error reporting
prototyper/prototyper/src/firmware/mod.rs Replaces static FDT buffer with dynamic heap allocation using two-pass sizing
prototyper/prototyper/config/default.toml Increases heap size from 32KB to 128KB to support larger allocations
prototyper/prototyper/build.rs Removes patched_fdt section from linker script since it's no longer needed

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +134 to +143
let mut temporary_buffer = vec![0u8; original_length + 2048];
serde_device_tree::ser::to_dtb(&tree, &list, &mut temporary_buffer).unwrap();
let Ok(patched_dtb_ptr) = DtbPtr::from_raw(temporary_buffer.as_mut_ptr()) else {
panic!("Failed to parse the patched dtb.")
};
let patched_length = patched_dtb_ptr.align();

// Secondly, allocate the exactly buffer to store the fdt.
let mut patched_dtb_buffer = vec![0u8; patched_length];
serde_device_tree::ser::to_dtb(&tree, &list, &mut patched_dtb_buffer).unwrap();
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The two-pass approach performs device tree serialization twice, which is inefficient. Consider using a single buffer with appropriate size calculation or checking if the serialization library provides a way to calculate required size without full serialization.

Suggested change
let mut temporary_buffer = vec![0u8; original_length + 2048];
serde_device_tree::ser::to_dtb(&tree, &list, &mut temporary_buffer).unwrap();
let Ok(patched_dtb_ptr) = DtbPtr::from_raw(temporary_buffer.as_mut_ptr()) else {
panic!("Failed to parse the patched dtb.")
};
let patched_length = patched_dtb_ptr.align();
// Secondly, allocate the exactly buffer to store the fdt.
let mut patched_dtb_buffer = vec![0u8; patched_length];
serde_device_tree::ser::to_dtb(&tree, &list, &mut patched_dtb_buffer).unwrap();
// Allocate a buffer large enough to hold the patched device tree.
let mut patched_dtb_buffer = vec![0u8; original_length + 2048];
let written = serde_device_tree::ser::to_dtb(&tree, &list, &mut patched_dtb_buffer).unwrap();
// If the function returns the number of bytes written, use it as the length.
let patched_length = written;

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function to calculate required size for serialization library is unimplemented, so currently the two-pass approach is needed.

…nd fix typo.

Signed-off-by: jackfiled <xcrenchangjun@outlook.com>
Signed-off-by: jackfiled <xcrenchangjun@outlook.com>
Signed-off-by: jackfiled <xcrenchangjun@outlook.com>
Signed-off-by: jackfiled <xcrenchangjun@outlook.com>
@luojia65 luojia65 merged commit d93209b into rustsbi:main Sep 29, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants