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

Change the way stage 2 page table is managed #193

Merged
merged 12 commits into from
Oct 11, 2023

Conversation

zpzigi754
Copy link
Collaborator

@zpzigi754 zpzigi754 commented Oct 4, 2023

This PR is the implementation of the previous discussion. The stage 2 page table will be constructed in the location and timing where the Host has intended with the APIs.

Page table difference handling

The host can determine the type of the stage 2 table. For example, realm-linux's stage 2 page table walking starts from level 2 instead of level 0 while using 8 concatenated tables. This PR handles the table's difference with realm-linux feature and configurable VTCR (Virtualization Translation Control Register). A refactoring work would be needed to remove the feature in the future.

Result summary

Regarding tf-a-tests,
This PR will pass all rmi-related tests (We don't conduct rsi checks yet).

Regarding ACS tests, this will
pass 2 additional checks in rtt_read_entry
pass 13 additional checks in rtt_init_ripas
pass 16 additional checks in rtt_create
pass 4 additional checks in cmd_data_create
miss 1 additional checks in realm_activate
pass all checks in mm_rtt_level_start

@zpzigi754 zpzigi754 force-pushed the stage2tbl-mgmt branch 2 times, most recently from ab6ff8a to 8909550 Compare October 10, 2023 08:24
@zpzigi754 zpzigi754 marked this pull request as ready for review October 10, 2023 08:35
new_s2tte |= bits_in_reg(S2TTE::NS, 1)
| bits_in_reg(S2TTE::XN, 1)
| bits_in_reg(S2TTE::AF, 1)
| bits_in_reg(S2TTE::DESC_TYPE, desc_type::L3_PAGE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason of removing bits_in_reg(S2TTE::MEMATTR, pte::attribute::NORMAL_FWB) and bits_in_reg(S2TTE::SH, pte::shareable::INNER)?
Without this, doesn't it make the access non-cacheable and non-sharable?

Copy link
Collaborator

@bokdeuk-jeong bokdeuk-jeong Oct 11, 2023

Choose a reason for hiding this comment

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

I found that host_s2tte, which is one of prameters for RMI_RTT_MAP_UNPROTECTED, already includes the properties. So it will eventually inculde chachability(FWB), sharability(inner ), access permission(RW)
https://github.com/Samsung/islet-asset/blob/604bc2c405be75be6645ddf1dcfeeca8bbd86b9f/arch/arm64/kvm/rme.c#L646

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the investigation!

rmm/src/realm/mm/stage2_tte.rs Outdated Show resolved Hide resolved
rmm/src/realm/mm/stage2_translation.rs Show resolved Hide resolved
@zpzigi754 zpzigi754 force-pushed the stage2tbl-mgmt branch 2 times, most recently from 15b1b6e to e0738fa Compare October 11, 2023 02:52
This commit is the implementation of the previous discussion
(https://github.com//discussions/180). The stage 2 page table
will be constructed in the location where the Host has intended
with the APIs at the right timing.
Realm-linux's stage 2 page table walking starts from level 2 instead of
level 0. It also consists of 8 concatenated tables. This commit handles
the table's difference with `realm-linux` feature. A refactoring work
would be needed to remove this feature in the future.
Copy link
Collaborator

@bokdeuk-jeong bokdeuk-jeong left a comment

Choose a reason for hiding this comment

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

nice work!

@zpzigi754 zpzigi754 merged commit b9d401c into islet-project:main Oct 11, 2023
@zpzigi754 zpzigi754 deleted the stage2tbl-mgmt branch October 11, 2023 05:55
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