Skip to content

Conversation

A0lson
Copy link
Contributor

@A0lson A0lson commented Mar 7, 2023

In the absence of the allocator_api feature being enabled and/or other situtations it is useful to be able to access ACPI table fields directly.

This also allows the low-level use of the tables, especially when higher-level processing is not yet implemented.

Thus, this revision makes them and the Mcfg 'entries()' method public.

acpi/src/madt.rs Outdated
global_system_interrupt_base: u32,
pub header: EntryHeader,
pub io_apic_id: u8,
pub _reserved: u8,
Copy link
Contributor

Choose a reason for hiding this comment

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

It might not be necessary to expose reserved fields unless users should be allowed to construct these structs themselves. If the latter is the case, then maybe a new function could be provided to get a default instance and then the user may assign to relevant fields. My suggestion sounds like it might overcomplicate things, though.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, I don't feel that reserved fields need to be exposed, and just clutter the documentation.

I don't foresee an instance where people will be constructing tables themselves from outside the library tbh, so that shouldn't be included in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I updated the MR to not make the _reserved fields public.

@A0lson A0lson force-pushed the access_without_allocator branch from ffb1b87 to eaa0a1a Compare March 31, 2023 14:35
Copy link
Contributor

@rcerc rcerc left a comment

Choose a reason for hiding this comment

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

GicdEntry::gic_version should also be exposed, right?

In the absence of the allocator_api feature being enabled and/or other situtations it is useful to be able to access ACPI table fields directly.

This also allows the low-level use of the tables, especially when higher-level processing is not yet implemented.

Thus, this revision makes them and the Mcfg 'entries()' method public.
@A0lson A0lson force-pushed the access_without_allocator branch from eaa0a1a to 0b5d2f6 Compare March 31, 2023 16:31
@A0lson
Copy link
Contributor Author

A0lson commented Mar 31, 2023

GicdEntry::gic_version should also be exposed, right?

Yes, I somehow missed that before. Now fixed.

Copy link
Member

@IsaacWoods IsaacWoods left a comment

Choose a reason for hiding this comment

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

Thanks, and apologies for the delay in getting this merged!

@IsaacWoods IsaacWoods merged commit c6f3327 into rust-osdev:main Apr 16, 2023
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.

3 participants