Skip to content

Commit eabc7aa

Browse files
Quentin PerretMarc Zyngier
authored andcommitted
KVM: arm64: Simplify np-guest hypercalls
When the handling of a guest stage-2 permission fault races with an MMU notifier, the faulting page might be gone from the guest's stage-2 by the point we attempt to call (p)kvm_pgtable_stage2_relax_perms(). In the normal KVM case, this leads to returning -EAGAIN which user_mem_abort() handles correctly by simply re-entering the guest. However, the pKVM hypercall implementation has additional logic to check the page state using __check_host_shared_guest() which gets confused with absence of a page mapped at the requested IPA and returns -ENOENT, hence breaking user_mem_abort() and hilarity ensues. Luckily, several of the hypercalls for managing the stage-2 page-table of NP guests have no effect on the pKVM ownership tracking (wrprotect, test_clear_young, mkyoung, and crucially relax_perms), so the extra state checking logic is in fact not strictly necessary. So, to fix the discrepancy between standard KVM and pKVM, let's just drop the superfluous __check_host_shared_guest() logic from those hypercalls and make the extra state checking a debug assertion dependent on CONFIG_NVHE_EL2_DEBUG as we already do for other transitions. Signed-off-by: Quentin Perret <[email protected]> Reviewed-by: Oliver Upton <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Marc Zyngier <[email protected]>
1 parent c53fbdb commit eabc7aa

File tree

1 file changed

+38
-31
lines changed

1 file changed

+38
-31
lines changed

arch/arm64/kvm/hyp/nvhe/mem_protect.c

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -998,63 +998,73 @@ int __pkvm_host_unshare_guest(u64 gfn, struct pkvm_hyp_vm *vm)
998998
return ret;
999999
}
10001000

1001-
int __pkvm_host_relax_perms_guest(u64 gfn, struct pkvm_hyp_vcpu *vcpu, enum kvm_pgtable_prot prot)
1001+
static void assert_host_shared_guest(struct pkvm_hyp_vm *vm, u64 ipa)
10021002
{
1003-
struct pkvm_hyp_vm *vm = pkvm_hyp_vcpu_to_hyp_vm(vcpu);
1004-
u64 ipa = hyp_pfn_to_phys(gfn);
10051003
u64 phys;
10061004
int ret;
10071005

1008-
if (prot & ~KVM_PGTABLE_PROT_RWX)
1009-
return -EINVAL;
1006+
if (!IS_ENABLED(CONFIG_NVHE_EL2_DEBUG))
1007+
return;
10101008

10111009
host_lock_component();
10121010
guest_lock_component(vm);
10131011

10141012
ret = __check_host_shared_guest(vm, &phys, ipa);
1015-
if (!ret)
1016-
ret = kvm_pgtable_stage2_relax_perms(&vm->pgt, ipa, prot, 0);
10171013

10181014
guest_unlock_component(vm);
10191015
host_unlock_component();
10201016

1021-
return ret;
1017+
WARN_ON(ret && ret != -ENOENT);
10221018
}
10231019

1024-
int __pkvm_host_wrprotect_guest(u64 gfn, struct pkvm_hyp_vm *vm)
1020+
int __pkvm_host_relax_perms_guest(u64 gfn, struct pkvm_hyp_vcpu *vcpu, enum kvm_pgtable_prot prot)
10251021
{
1022+
struct pkvm_hyp_vm *vm = pkvm_hyp_vcpu_to_hyp_vm(vcpu);
10261023
u64 ipa = hyp_pfn_to_phys(gfn);
1027-
u64 phys;
10281024
int ret;
10291025

1030-
host_lock_component();
1031-
guest_lock_component(vm);
1026+
if (pkvm_hyp_vm_is_protected(vm))
1027+
return -EPERM;
10321028

1033-
ret = __check_host_shared_guest(vm, &phys, ipa);
1034-
if (!ret)
1035-
ret = kvm_pgtable_stage2_wrprotect(&vm->pgt, ipa, PAGE_SIZE);
1029+
if (prot & ~KVM_PGTABLE_PROT_RWX)
1030+
return -EINVAL;
10361031

1032+
assert_host_shared_guest(vm, ipa);
1033+
guest_lock_component(vm);
1034+
ret = kvm_pgtable_stage2_relax_perms(&vm->pgt, ipa, prot, 0);
10371035
guest_unlock_component(vm);
1038-
host_unlock_component();
10391036

10401037
return ret;
10411038
}
10421039

1043-
int __pkvm_host_test_clear_young_guest(u64 gfn, bool mkold, struct pkvm_hyp_vm *vm)
1040+
int __pkvm_host_wrprotect_guest(u64 gfn, struct pkvm_hyp_vm *vm)
10441041
{
10451042
u64 ipa = hyp_pfn_to_phys(gfn);
1046-
u64 phys;
10471043
int ret;
10481044

1049-
host_lock_component();
1045+
if (pkvm_hyp_vm_is_protected(vm))
1046+
return -EPERM;
1047+
1048+
assert_host_shared_guest(vm, ipa);
10501049
guest_lock_component(vm);
1050+
ret = kvm_pgtable_stage2_wrprotect(&vm->pgt, ipa, PAGE_SIZE);
1051+
guest_unlock_component(vm);
10511052

1052-
ret = __check_host_shared_guest(vm, &phys, ipa);
1053-
if (!ret)
1054-
ret = kvm_pgtable_stage2_test_clear_young(&vm->pgt, ipa, PAGE_SIZE, mkold);
1053+
return ret;
1054+
}
10551055

1056+
int __pkvm_host_test_clear_young_guest(u64 gfn, bool mkold, struct pkvm_hyp_vm *vm)
1057+
{
1058+
u64 ipa = hyp_pfn_to_phys(gfn);
1059+
int ret;
1060+
1061+
if (pkvm_hyp_vm_is_protected(vm))
1062+
return -EPERM;
1063+
1064+
assert_host_shared_guest(vm, ipa);
1065+
guest_lock_component(vm);
1066+
ret = kvm_pgtable_stage2_test_clear_young(&vm->pgt, ipa, PAGE_SIZE, mkold);
10561067
guest_unlock_component(vm);
1057-
host_unlock_component();
10581068

10591069
return ret;
10601070
}
@@ -1063,18 +1073,15 @@ int __pkvm_host_mkyoung_guest(u64 gfn, struct pkvm_hyp_vcpu *vcpu)
10631073
{
10641074
struct pkvm_hyp_vm *vm = pkvm_hyp_vcpu_to_hyp_vm(vcpu);
10651075
u64 ipa = hyp_pfn_to_phys(gfn);
1066-
u64 phys;
10671076
int ret;
10681077

1069-
host_lock_component();
1070-
guest_lock_component(vm);
1071-
1072-
ret = __check_host_shared_guest(vm, &phys, ipa);
1073-
if (!ret)
1074-
kvm_pgtable_stage2_mkyoung(&vm->pgt, ipa, 0);
1078+
if (pkvm_hyp_vm_is_protected(vm))
1079+
return -EPERM;
10751080

1081+
assert_host_shared_guest(vm, ipa);
1082+
guest_lock_component(vm);
1083+
kvm_pgtable_stage2_mkyoung(&vm->pgt, ipa, 0);
10761084
guest_unlock_component(vm);
1077-
host_unlock_component();
10781085

10791086
return ret;
10801087
}

0 commit comments

Comments
 (0)