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

Support TTBR1 as well as TTBR0 #19

Merged
merged 15 commits into from
Aug 15, 2022
Merged

Support TTBR1 as well as TTBR0 #19

merged 15 commits into from
Aug 15, 2022

Conversation

qwandor
Copy link
Collaborator

@qwandor qwandor commented Aug 11, 2022

Fixes #14.

@qwandor qwandor marked this pull request as ready for review August 12, 2022 14:46
src/paging.rs Outdated
@@ -25,6 +25,17 @@ pub const PAGE_SIZE: usize = 1 << PAGE_SHIFT;
/// page size.
pub const BITS_PER_LEVEL: usize = PAGE_SHIFT - 3;

/// Which TTBR register to use for a page table.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's easiest to use the register names, but you could alternatively be slightly more abstract and refer to the lower or upper VA range; that's the ARM ARM terminology.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

src/linearmap.rs Outdated

// The first page in the region covered by TTBR1.
assert_eq!(
translation.virtual_to_physical(VirtualAddress(0xffffff8000000000)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add underscores to make these more readable - 0xffff_ff80_0000_0000? Or compute it some other way, so it's obvious? -GIB_512, perhaps.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

src/linearmap.rs Outdated
@@ -318,10 +373,29 @@ mod tests {
)
}

#[test]
fn virtual_address_range_ttbr1() {
const GIB_512: isize = 512 * 1024 * 1024 * 1024;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract to top level rather than repeating?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@ardbiesheuvel ardbiesheuvel left a comment

Choose a reason for hiding this comment

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

The linear map can by definition only be used with TTBR0 so this patch seems wrong or unnecessary at least?

@qwandor
Copy link
Collaborator Author

qwandor commented Aug 15, 2022

Uh, why can't the linear map be used with TTBR1? It seems reasonable to use TTBR1 to map some or all of physical address space to the upper virtual address space at a fixed offset.

@ardbiesheuvel
Copy link
Collaborator

Uh, why can't the linear map be used with TTBR1? It seems reasonable to use TTBR1 to map some or all of physical address space to the upper virtual address space at a fixed offset.

Apologies, you're quite right - I got confused and read 'identity map' instead of 'linear map'. Ignore me ...

src/paging.rs Outdated
/// will be used with `TTBR0`.
Lower,
/// The page table covers the top of the virtual address space (ending at address
/// 0xffffffffffffffff), so will be used with `TTBR1`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: 0xffff_ffff_ffff_ffff

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@qwandor qwandor merged commit 2ed36a5 into main Aug 15, 2022
@qwandor qwandor deleted the ttbr1 branch August 15, 2022 13:38
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.

Support setting TTBR1_EL1 too
3 participants