Patch for the setup of realm in ACS #197
Conversation
nook1208
force-pushed
the
acs_realm_setup_failed_resolve1
branch
from
009d487
to
01cf1ac
Compare
nook1208
force-pushed
the
acs_realm_setup_failed_resolve1
branch
2 times, most recently
from
ec595f9
to
a51150b
Compare
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Remove Rec::rd field based by RMM spec in A2.3.2 REC attributes Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
nook1208
force-pushed
the
acs_realm_setup_failed_resolve1
branch
from
a51150b
to
209c479
Compare
| let flags = bits_in_reg(S2TTE::INVALID_RIPAS, invalid_ripas::RAM); | ||
| let new_s2tte = pa as u64 | flags; | ||
| let new_s2tte = s2tte.get() | flags; |
There was a problem hiding this comment.
Uoops, it's my fault. Thank you for finding and fixing it!
nook1208
force-pushed
the
acs_realm_setup_failed_resolve1
branch
from
d9d85c2
to
895a0e2
Compare
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
The previous TIMEOUT is not enough Signed-off-by: Sunwook Eom <speed.eom@samsung.com>
nook1208
force-pushed
the
acs_realm_setup_failed_resolve1
branch
from
895a0e2
to
7840a8c
Compare
nook1208
requested review from
bitboom,
zpzigi754,
bokdeuk-jeong,
jinbpark and
hihi-wang
| let vcpuid = rec.id(); | ||
| let config_ipa = rmi.get_reg(realmid, vcpuid, 1)?; | ||
| rmi.realm_config(realmid, config_ipa)?; | ||
| rmi.realm_config(realmid, config_ipa, rec.ipa_bits()?)?; |
There was a problem hiding this comment.
Rec.ipa_bits() should inquire Rd to get this. This makes another wrapper.
I think Rd should directly expose the ipa_bits() method because Rd holds the information.
Why don't we get it as following?
let rd = rec.rd()
rm.realm_config(rd.id(), config_ipa, rd.ipa_bits());
There was a problem hiding this comment.
I tried to make a rec.rd() which gets the granule of the rd and return it.
But in rust, it's impossible to return a reference to a variable created in a function.
So maybe I can write like this :
let rd = get_granule_if!(rec.owner(), GranuleState::RD)?;
rm.realm_config(rd.id(), config_ipa, rd.ipa_bits());l'll make a new PR to apply your comment. Thank you :)
rmm/src/rsi/mod.rs
Outdated
| .lock() | ||
| .page_table | ||
| .lock() | ||
| .ipa_to_pa(crate::realm::mm::address::GuestPhysAddr::from(ipa), 3) |
There was a problem hiding this comment.
[minor]
How about using const value instead of 3 like the below?
.ipa_to_pa(crate::realm::mm::address::GuestPhysAddr::from(ipa), RTT_PAGE_LEVEL)
There was a problem hiding this comment.
Thank you for the review! I'll make a new PR to apply your comments after pushing this PR :)
| unsafe { | ||
| // TODO: copy rec::entry gprs to host_call gprs | ||
| let ipa: u64 = 0x800088e00000; | ||
| if run.entry_gpr0() == ipa { | ||
| // TODO: Get ipa from rec->regs[1] and map to pa | ||
| let pa: usize = 0x88b0_6000; | ||
| let host_call = rsi::hostcall::HostCall::parse_mut(pa); | ||
| host_call.set_gpr0(ipa); | ||
| } | ||
| if rec.host_call_pending() { | ||
| do_host_call(&arg, ret, rmm, rec, &mut run)?; |
There was a problem hiding this comment.
Good to remove the hardcoded values!
| .lock() | ||
| .page_table | ||
| .lock() | ||
| .ipa_to_pa(crate::realm::mm::address::GuestPhysAddr::from(ipa), 3) |
There was a problem hiding this comment.
[minor]
How about using the const value instead of 3 (the same comment as before)?
.ipa_to_pa(crate::realm::mm::address::GuestPhysAddr::from(ipa), RTT_PAGE_LEVEL)
This PR is related with #188 issue.
With this PR, we can see the realm is boot successfully in ACS test.