Skip to content

loader: Make arm64_image_header public #205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlexOrozco1256
Copy link

Summary of the PR

This header is equivalent to the boot_params header found at the top of x86 kernels. We want to make this struct public to use it in a fw_cfg implementation that reads the kernel header for both arm and x86

Copy link
Contributor

@likebreath likebreath left a comment

Choose a reason for hiding this comment

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

The CI is complaining about missing docs for public data structure:

error: missing documentation for a struct
--
  | --> src/loader/pe/mod.rs:78:1
  | \|
  | 78 \| pub struct arm64_image_header {
  | \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | \|
  | note: the lint level is defined here
  | --> src/lib.rs:10:9
  | \|
  | 10 \| #![deny(missing_docs)]
  | \|         ^^^^^^^^^^^^

@AlexOrozco1256
Copy link
Author

The CI is complaining about missing docs for public data structure:

error: missing documentation for a struct
--
  | --> src/loader/pe/mod.rs:78:1
  | \|
  | 78 \| pub struct arm64_image_header {
  | \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | \|
  | note: the lint level is defined here
  | --> src/lib.rs:10:9
  | \|
  | 10 \| #![deny(missing_docs)]
  | \|         ^^^^^^^^^^^^

fixed! thanks for the review!

Copy link
Member

@roypat roypat left a comment

Choose a reason for hiding this comment

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

Will the fields also need to be public, or is the struct + its ByteValued impl enough for what you want to do? And if we're at it, let's also update the riscv struct :)

This header is equivalent to the boot_params header found at the top of
x86 kernels. We want to make this struct public to use it in a fw_cfg
implementation that reads the kernel header for both arm and x86

Signed-off-by: Alex Orozco <alexorozco@google.com>
@AlexOrozco1256
Copy link
Author

AlexOrozco1256 commented Jul 8, 2025

Will the fields also need to be public, or is the struct + its ByteValued impl enough for what you want to do? And if we're at it, let's also update the riscv struct :)

I need to be able to read text_offset to know where the kernel starts. I made all the fields public to mirror the x86 kernel headers. I also updated the riscv struct, thanks for the review!

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