Skip to content
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

acpi: fix length of "extended_area_bytes" in RSDP search #164

Merged
merged 1 commit into from
May 6, 2023

Conversation

A0lson
Copy link
Contributor

@A0lson A0lson commented Mar 7, 2023

Since the memory mapping is already including "RSDP_V2_EXT_LENGTH" bytes to allow for an ACPI 1.0 table at the end of the region, it seems that the slice should just use the actual mapping length instead of going beyond it...

rsdp/src/lib.rs Outdated
)
};
let extended_area_bytes =
unsafe { slice::from_raw_parts(mapping.virtual_start().as_ptr(), mapping.mapped_length()) };
Copy link
Contributor

@rcerc rcerc Mar 21, 2023

Choose a reason for hiding this comment

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

This was my mistake, thank you for catching it! However, I think region_length must still be used; mapped_length returns the entire length after the start of mapping that the user mapped (which could be well over the size of the search area). Edit: If as you explained in another PR, there is no guarantee that what is returned by region_length is the same as the size passed to AcpiHandler::map_physical_region, then area.end - area.start + RSDP_V2_EXT_LENGTH should be passed to from_raw_parts instead of mapping.region_length().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (now I understand the difference between the two lengths!)

Since the memory mapping is already including "RSDP_V2_EXT_LENGTH"
bytes to allow for an ACPI 1.0 table at the end of the region,
it the slice should just use the actual region length
instead of going beyond it...
@A0lson A0lson force-pushed the rsdp_extended_area_bytes branch from 932c202 to 7883a67 Compare March 31, 2023 16:12
@A0lson A0lson changed the title acpi: attempt correct length of "extended_area_bytes" acpi: fix length of "extended_area_bytes" in RSDP search Mar 31, 2023
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.

Thanks!

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.

Apologies for the delay! Thanks very much!

@IsaacWoods IsaacWoods merged commit 4e08a21 into rust-osdev:main May 6, 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