On 02/02/2025 07:12, Gavin Shan wrote: > On 12/13/24 1:55 AM, Steven Price wrote: >> Physical device assignment is not yet supported by the RMM, so it >> doesn't make much sense to allow device mappings within the realm. >> Prevent them when the guest is a realm. >> >> Signed-off-by: Steven Price <steven.price@xxxxxxx> >> --- >> Changes from v5: >> * Also prevent accesses in user_mem_abort() >> --- >> arch/arm64/kvm/mmu.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c >> index 9ede143ccef1..cef7c3dcbf99 100644 >> --- a/arch/arm64/kvm/mmu.c >> +++ b/arch/arm64/kvm/mmu.c >> @@ -1149,6 +1149,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, >> phys_addr_t guest_ipa, >> if (is_protected_kvm_enabled()) >> return -EPERM; >> + /* We don't support mapping special pages into a Realm */ >> + if (kvm_is_realm(kvm)) >> + return -EINVAL; >> + > > return -EPERM; > >> size += offset_in_page(guest_ipa); >> guest_ipa &= PAGE_MASK; >> @@ -1725,6 +1729,14 @@ static int user_mem_abort(struct kvm_vcpu >> *vcpu, phys_addr_t fault_ipa, >> if (exec_fault && device) >> return -ENOEXEC; >> + /* >> + * Don't allow device accesses to protected memory as we don't (yet) >> + * support protected devices. >> + */ >> + if (device && kvm_is_realm(kvm) && >> + kvm_gpa_from_fault(kvm, fault_ipa) == fault_ipa) >> + return -EINVAL; >> + > > s/kvm_is_realm/vcpu_is_rec > > I don't understand the check very well. What I understood is mem_abort() > is called > only when kvm_gpa_from_fault(kvm, fault_ipa) != fault_ipa, meaning only > the page > faults in the shared address space is handled by mem_abort(). So I guess > we perhaps > need something like below. private_memslot_fault() is only when the memslot is a private guest_memfd(). So there's also the situation of a 'normal' memslot which can still end up in user_mem_abort(). As things currently stand we can't really deal with this (the bottom part of the IPA is protected, and therefore must come from guest_memfd()). But in the future we are expecting to be able to support protected devices. So I think really the correct solution for now is to drop the "device" check and update the comment to reflect the fact that private_memslot_fault() should be handling all protected address until we get support for assigning devices. Thanks, Steve > if (vcpu_is_rec(vcpu) && device) > return -EPERM; > > kvm_handle_guest_abort > kvm_slot_can_be_private > private_memslot_fault // page fault in the private space is > handled here > io_mem_abort // MMIO emulation is handled here > user_mem_abort // page fault in the shared space is > handled here > >> /* >> * Potentially reduce shadow S2 permissions to match the guest's >> own >> * S2. For exec faults, we'd only reach this point if the guest > > Thanks, > Gavin >
