Skip to content

Conversation

seijikun
Copy link
Contributor

@seijikun seijikun commented Sep 3, 2025

I still had this lying around as experiment.
Polished it up a bit for the merge request.

It's best to review this commit-by-commit.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@seijikun seijikun changed the title Mr hii configrouting uefi: Add bindings for the HII_CONFIG_ROUTING protocol Sep 3, 2025
@seijikun seijikun force-pushed the mr-hii-configrouting branch 4 times, most recently from 6380ed3 to 2e556f1 Compare September 4, 2025 15:05
@seijikun seijikun force-pushed the mr-hii-configrouting branch from 2e556f1 to 0c54e18 Compare September 10, 2025 15:21
Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

I reviewed everything and it LGTM! Thanks for working on this :) Left a few nits. Please answer my comments and then we are good to go

@seijikun seijikun force-pushed the mr-hii-configrouting branch 2 times, most recently from 2977208 to ef2b00d Compare September 25, 2025 14:04
@phip1611
Copy link
Member

please rebase, then I think we should be good to go :)

@seijikun seijikun force-pushed the mr-hii-configrouting branch from ef2b00d to b8609c9 Compare October 17, 2025 19:04
@seijikun seijikun requested a review from phip1611 October 17, 2025 19:17
@phip1611
Copy link
Member

Will do a final review tomorrow

@phip1611 phip1611 assigned phip1611 and unassigned phip1611 Oct 17, 2025
@seijikun seijikun force-pushed the mr-hii-configrouting branch 2 times, most recently from 0c7b3cd to 33dc406 Compare October 17, 2025 19:57
@phip1611
Copy link
Member

Could you please move the additions to aligned_buffer.rs (9b5cb8c) to a dedicated PR?

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

LGTM, will merge once the pre-requisite in aligned_buffer.rs is moved into a dedicated PR.

@seijikun seijikun force-pushed the mr-hii-configrouting branch from 33dc406 to f0cb7d9 Compare October 18, 2025 19:27
Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

almost, almost there 🤏

Name,
/// UEFI ConfigurationString {PathHdr} element
Path,
/// UEFI ConfigurationString {DescHdr} element
Copy link
Member

Choose a reason for hiding this comment

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

out of curiosity: What does this {} syntax refer to?

Copy link
Contributor Author

@seijikun seijikun Oct 18, 2025

Choose a reason for hiding this comment

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

I somehow wanted to illustrate that "DescHdr" is the name of an entity, declared in the UEFI spec.
The real name is <DescHdr>, but the CI didn't let me write that :(

https://uefi.org/sites/default/files/resources/UEFI_Spec_Final_2.11.pdf
Page 1803

  • 35.2.1.1 Basic forms
  • 35.2.1.2 Types
  • 35.2.1.3 Routing elements

@seijikun seijikun force-pushed the mr-hii-configrouting branch from f0cb7d9 to ea5b01d Compare October 18, 2025 20:15
@phip1611 phip1611 added this pull request to the merge queue Oct 19, 2025
Merged via the queue into rust-osdev:main with commit 66b1f57 Oct 19, 2025
16 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.

2 participants