Skip to content

Conversation

AlexOrozco1256
Copy link
Contributor

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
Contributor 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
Contributor 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!

@likebreath
Copy link
Contributor

@roypat @rbradford Can any of you merge the PR? We will also likely need a new release from that Cloud Hypervisor can properly consume such change. The last release was a while ago too. Please let me know if you need help here. Thank you.

@roypat roypat merged commit d5f39c0 into rust-vmm:main Jul 11, 2025
2 checks passed
@roypat
Copy link
Member

roypat commented Jul 11, 2025

@roypat @rbradford Can any of you merge the PR? We will also likely need a new release from that Cloud Hypervisor can properly consume such change. The last release was a while ago too. Please let me know if you need help here. Thank you.

can you do a release prep PR that updates the changelog and bumps the Cargo.toml version?

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