KVM: arm64: Add support for protected guest memory with pKVM
Subsystems (4)
Will Deacon
maintainercover KVM: arm64: Add support for protected guest memory with pKVM
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for guest_memfd() but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while the guest_memfd() work continues to evolve.
Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer Kconfig option to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.
More information about what is and what isn't implemented is described in the pkvm.rst documentation added by this series. The intention is to update this file as we introduce additional protection features and ultimately to remove the taint.
The series is loosely structured as follows:
Patch 1: A dependent pgtable fix that I sent out previously
Patches 2-8: Cleanups/fixes to the existing code to prepare for pVMs
Patches 9-14: Support for memory donation and reclaim Patches 15-22: Handling of bad host accesses to protected memory Patches 23-25: Support for SHARE and UNSHARE guest hypercalls Patches 26-27: UAPI and developer documentation
Patches 28-30: Selftest additions for new page ownership transitions
Patches are based on v6.19-rc4 and are also available at [3].
All feedback welcome.
Cheers,
Will
[1] https://lore.kernel.org/all/[email protected]/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/log/?h=pkvm
[3] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=kvm/protected-memory
--->8
Fuad Tabba (1):
KVM: arm64: Expose self-hosted debug regs as RAZ/WI for protected
guests
Quentin Perret (2):
KVM: arm64: Refactor enter_exception64()
KVM: arm64: Inject SIGSEGV on illegal accesses
Will Deacon (27):
KVM: arm64: Invert KVM_PGTABLE_WALK_HANDLE_FAULT to fix pKVM walkers
KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
KVM: arm64: Rename __pkvm_pgtable_stage2_unmap()
KVM: arm64: Don't advertise unsupported features for protected guests
KVM: arm64: Remove pointless is_protected_kvm_enabled() checks from
hyp
KVM: arm64: Ignore MMU notifier callbacks for protected VMs
KVM: arm64: Prevent unsupported memslot operations on protected VMs
KVM: arm64: Split teardown hypercall into two phases
KVM: arm64: Introduce __pkvm_host_donate_guest()
KVM: arm64: Hook up donation hypercall to pkvm_pgtable_stage2_map()
KVM: arm64: Handle aborts from protected VMs
KVM: arm64: Introduce __pkvm_reclaim_dying_guest_page()
KVM: arm64: Hook up reclaim hypercall to pkvm_pgtable_stage2_destroy()
KVM: arm64: Generalise kvm_pgtable_stage2_set_owner()
KVM: arm64: Introduce host_stage2_set_owner_metadata_locked()
KVM: arm64: Annotate guest donations with handle and gfn in host
stage-2
KVM: arm64: Introduce hypercall to force reclaim of a protected page
KVM: arm64: Reclaim faulting page from pKVM in spurious fault handler
KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
KVM: arm64: Add hvc handler at EL2 for hypercalls from protected VMs
KVM: arm64: Implement the MEM_SHARE hypercall for protected VMs
KVM: arm64: Implement the MEM_UNSHARE hypercall for protected VMs
KVM: arm64: Allow userspace to create protected VMs when pKVM is
enabled
KVM: arm64: Add some initial documentation for pKVM
KVM: arm64: Extend pKVM page ownership selftests to cover guest
donation
KVM: arm64: Register 'selftest_vm' in the VM table
KVM: arm64: Extend pKVM page ownership selftests to cover forced
reclaim
.../admin-guide/kernel-parameters.txt | 4 +- Documentation/virt/kvm/arm/index.rst | 1 + Documentation/virt/kvm/arm/pkvm.rst | 101 ++++ arch/arm64/include/asm/kvm_asm.h | 7 +- arch/arm64/include/asm/kvm_emulate.h | 5 + arch/arm64/include/asm/kvm_host.h | 7 + arch/arm64/include/asm/kvm_pgtable.h | 27 +- arch/arm64/include/asm/kvm_pkvm.h | 4 +- arch/arm64/include/asm/virt.h | 6 + arch/arm64/kvm/Kconfig | 10 + arch/arm64/kvm/arm.c | 8 +- arch/arm64/kvm/hyp/exception.c | 100 ++-- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 9 + arch/arm64/kvm/hyp/include/nvhe/memory.h | 6 + arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 7 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 95 ++-- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 500 ++++++++++++++++-- arch/arm64/kvm/hyp/nvhe/pkvm.c | 223 +++++++- arch/arm64/kvm/hyp/nvhe/switch.c | 1 + arch/arm64/kvm/hyp/nvhe/sys_regs.c | 8 + arch/arm64/kvm/hyp/pgtable.c | 22 +- arch/arm64/kvm/mmu.c | 122 ++++- arch/arm64/kvm/pkvm.c | 149 +++++- arch/arm64/mm/fault.c | 31 +- include/uapi/linux/kvm.h | 5 + 25 files changed, 1249 insertions(+), 209 deletions(-) create mode 100644 Documentation/virt/kvm/arm/pkvm.rst
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 1/30 KVM: arm64: Invert KVM_PGTABLE_WALK_HANDLE_FAULT to fix pKVM walkers
Commit ddcadb297ce5 ("KVM: arm64: Ignore EAGAIN for walks outside of a fault") introduced a new walker flag ('KVM_PGTABLE_WALK_HANDLE_FAULT') to KVM's page-table code. When set, the walk logic maintains its previous behaviour of terminating a walk as soon as the visitor callback returns an error. However, when the flag is clear, the walk will continue if the visitor returns -EAGAIN and the error is then suppressed and returned as zero to the caller.
Clearing the flag is beneficial when write-protecting a range of IPAs with kvm_pgtable_stage2_wrprotect() but is not useful in any other cases, either because we are operating on a single page (e.g. kvm_pgtable_stage2_mkyoung() or kvm_phys_addr_ioremap()) or because the early termination is desirable (e.g. when mapping pages from a fault in user_mem_abort()).
Subsequently, commit e912efed485a ("KVM: arm64: Introduce the EL1 pKVM MMU") hooked up pKVM's hypercall interface to the MMU code at EL1 but failed to propagate any of the walker flags. As a result, page-table walks at EL2 fail to set KVM_PGTABLE_WALK_HANDLE_FAULT even when the early termination semantics are desirable on the fault handling path.
Rather than complicate the pKVM hypercall interface, invert the flag so that the whole thing can be simplified and only pass the new flag ('KVM_PGTABLE_WALK_IGNORE_EAGAIN') from the wrprotect code.
--- arch/arm64/include/asm/kvm_pgtable.h | 6 +++--- arch/arm64/kvm/hyp/pgtable.c | 5 +++-- arch/arm64/kvm/mmu.c | 8 +++----- 3 files changed, 9 insertions(+), 10 deletions(-)
arch/arm64/include/asm/kvm_pgtable.h +3 -3
@@ -293,8 +293,8 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,* children. * @KVM_PGTABLE_WALK_SHARED: Indicates the page-tables may be shared * with other software walkers. -* @KVM_PGTABLE_WALK_HANDLE_FAULT: Indicates the page-table walk was -* invoked from a fault handler. +* @KVM_PGTABLE_WALK_IGNORE_EAGAIN: Don't terminate the walk early if +* the walker returns -EAGAIN. * @KVM_PGTABLE_WALK_SKIP_BBM_TLBI: Visit and update table entries * without Break-before-make's * TLB invalidation.
@@ -307,7 +307,7 @@ enum kvm_pgtable_walk_flags {KVM_PGTABLE_WALK_TABLE_PRE =BIT (1 ),KVM_PGTABLE_WALK_TABLE_POST =BIT (2 ),KVM_PGTABLE_WALK_SHARED =BIT (3 ), -KVM_PGTABLE_WALK_HANDLE_FAULT =BIT (4 ), +KVM_PGTABLE_WALK_IGNORE_EAGAIN =BIT (4 ),KVM_PGTABLE_WALK_SKIP_BBM_TLBI =BIT (5 ), KVM_PGTABLE_WALK_SKIP_CMO = BIT(6), };
arch/arm64/kvm/hyp/pgtable.c +3 -2
@@ -144,7 +144,7 @@ static bool kvm_pgtable_walk_continue(const struct kvm_pgtable_walker *walker,* page table walk. */ if (r == -EAGAIN ) -return !(walker->flags &KVM_PGTABLE_WALK_HANDLE_FAULT ); +return walker->flags &KVM_PGTABLE_WALK_IGNORE_EAGAIN ;return !r; }
@@ -1262,7 +1262,8 @@ int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size) {return stage2_update_leaf_attrs (pgt, addr, size,0 ,KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W , -NULL ,NULL ,0 ); +NULL ,NULL , +KVM_PGTABLE_WALK_IGNORE_EAGAIN ); } void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
arch/arm64/kvm/mmu.c +3 -5
@@ -1563,14 +1563,12 @@ static void adjust_nested_exec_perms(struct kvm *kvm, *prot &= ~KVM_PGTABLE_PROT_PX ; } -#define KVM_PGTABLE_WALK_MEMABORT_FLAGS (KVM_PGTABLE_WALK_HANDLE_FAULT | KVM_PGTABLE_WALK_SHARED) -static int gmem_abort (struct kvm_vcpu *vcpu,phys_addr_t fault_ipa,struct kvm_s2_trans *nested,struct kvm_memory_slot *memslot,bool is_perm) {bool write_fault, exec_fault, writable; -enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_MEMABORT_FLAGS ; +enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_SHARED ;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_R ;struct kvm_pgtable *pgt = vcpu->arch.hw_mmu->pgt;unsigned long mmu_seq;
@@ -1665,7 +1663,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,struct kvm_pgtable *pgt;struct page *page;vm_flags_t vm_flags; -enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_MEMABORT_FLAGS ; +enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_SHARED ;if (fault_is_perm) fault_granule =kvm_vcpu_trap_get_perm_fault_granule (vcpu);
@@ -1933,7 +1931,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,/* Resolve the access fault by making the page young again. */ static void handle_access_fault (struct kvm_vcpu *vcpu,phys_addr_t fault_ipa) { -enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_HANDLE_FAULT |KVM_PGTABLE_WALK_SHARED ; +enum kvm_pgtable_walk_flags flags =KVM_PGTABLE_WALK_SHARED ;struct kvm_s2_mmu *mmu;trace_kvm_access_fault (fault_ipa);
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 01/30] KVM: arm64: Invert KVM_PGTABLE_WALK_HANDLE_FAULT to fix pKVM walkers
Quoted 21 lines · Will Deacon
Commit
ddcadb297ce5("KVM: arm64: Ignore EAGAIN for walks outside of a fault") introduced a new walker flag ('KVM_PGTABLE_WALK_HANDLE_FAULT') to KVM's page-table code. When set, the walk logic maintains its previous behaviour of terminating a walk as soon as the visitor callback returns an error. However, when the flag is clear, the walk will continue if the visitor returns-EAGAINand the error is then suppressed and returned as zero to the caller.Clearing the flag is beneficial when write-protecting a range of IPAs with
kvm_pgtable_stage2_wrprotect()but is not useful in any other cases, either because we are operating on a single page (e.g.kvm_pgtable_stage2_mkyoung()orkvm_phys_addr_ioremap()) or because the early termination is desirable (e.g. when mapping pages from a fault inuser_mem_abort()).Subsequently, commit
e912efed485a("KVM: arm64: Introduce the EL1 pKVM MMU") hooked up pKVM's hypercall interface to the MMU code at EL1 but failed to propagate any of the walker flags. As a result, page-table walks at EL2 fail to setKVM_PGTABLE_WALK_HANDLE_FAULTeven when the early termination semantics are desirable on the fault handling path.
Quoted 3 lines
Rather than complicate the pKVM hypercall interface, invert the flag so that the whole thing can be simplified and only pass the new flag ('
KVM_PGTABLE_WALK_IGNORE_EAGAIN') from the wrprotect code.
Re: (subset) [PATCH 01/30] KVM: arm64: Invert KVM_PGTABLE_WALK_HANDLE_FAULT to fix pKVM walkers
In reply to Will DeaconQuoted 9 lines · Will Deacon
Commit
ddcadb297ce5("KVM: arm64: Ignore EAGAIN for walks outside of a fault") introduced a new walker flag ('KVM_PGTABLE_WALK_HANDLE_FAULT') to KVM's page-table code. When set, the walk logic maintains its previous behaviour of terminating a walk as soon as the visitor callback returns an error. However, when the flag is clear, the walk will continue if the visitor returns-EAGAINand the error is then suppressed and returned as zero to the caller.[...]
Applied to fixes, thanks!
[01/30] KVM: arm64: Invert KVM_PGTABLE_WALK_HANDLE_FAULT to fix pKVM walkers
https://git.kernel.org/kvmarm/kvmarm/c/19cffd16ed64
--
Best,
Oliver
Will Deacon
maintainerPATCH 2/30 KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
In reply to Will DeaconThe MMU notifiers are registered by kvm_init_mmu_notifier() only after kvm_arch_init_vm() has returned successfully. Since the latter function initialises the 'kvm->arch.mmu.pgt' pointer (and allocates the VM handle when pKVM is enabled), the initialisation checks in the MMU notifiers are not required.
Remove the redundant checks from the MMU notifier callbacks.
--- arch/arm64/kvm/mmu.c | 9 --------- arch/arm64/kvm/pkvm.c | 9 ++++++--- 2 files changed, 6 insertions(+), 12 deletions(-)
arch/arm64/kvm/mmu.c +0 -9
@@ -2208,9 +2208,6 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)bool kvm_unmap_gfn_range (struct kvm *kvm,struct kvm_gfn_range *range) { -if (!kvm->arch.mmu.pgt) -return false; -__unmap_stage2_range (&kvm->arch.mmu, range->start <<PAGE_SHIFT , (range->end - range->start) <<PAGE_SHIFT , range->may_block);
@@ -2223,9 +2220,6 @@ bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) {u64 size = (range->end - range->start) <<PAGE_SHIFT ; -if (!kvm->arch.mmu.pgt) -return false; -return KVM_PGT_FN (kvm_pgtable_stage2_test_clear_young)(kvm->arch.mmu.pgt, range->start <<PAGE_SHIFT , size, true);
@@ -2239,9 +2233,6 @@ bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) {u64 size = (range->end - range->start) <<PAGE_SHIFT ; -if (!kvm->arch.mmu.pgt) -return false; -return KVM_PGT_FN (kvm_pgtable_stage2_test_clear_young)(kvm->arch.mmu.pgt, range->start <<PAGE_SHIFT , size, false);
arch/arm64/kvm/pkvm.c +6 -3
@@ -329,9 +329,6 @@ static int __pkvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 start, u64 estruct pkvm_mapping *mapping;int ret; -if (!handle) -return 0 ; -for_each_mapping_in_range_safe (pgt, start, end, mapping) { ret =kvm_call_hyp_nvhe (__pkvm_host_unshare_guest, handle, mapping->gfn, mapping->nr_pages);
@@ -347,6 +344,12 @@ static int __pkvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 start, u64 evoid pkvm_pgtable_stage2_destroy_range (struct kvm_pgtable *pgt,u64 addr,u64 size) { +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); +pkvm_handle_t handle = kvm->arch.pkvm.handle; + +if (!handle) +return ; +__pkvm_pgtable_stage2_unmap (pgt, addr, addr + size); }
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 02/30] KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
Hey Will,
Quoted 5 lines · Will Deacon
The MMU notifiers are registered by
kvm_init_mmu_notifier()only afterkvm_arch_init_vm()has returned successfully. Since the latter function initialises the 'kvm->arch.mmu.pgt' pointer (and allocates the VM handle when pKVM is enabled), the initialisation checks in the MMU notifiers are not required.
It took me a while to remember, but I think these checks are needed for the free path rather than init. In particular, the doc for mmu_notifier_ops::release() (from which we free the pgt) says that it "can run concurrently with other mmu notifier" (see mmu_notifier.h), which is fun.
Had you considered that path? If so, probably worth expanding in the commit description why this is safe?
Cheers,
Quentin
Will Deacon
maintainerRe: [PATCH 02/30] KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
Quoted 7 lines · Quentin Perret
Quoted 5 lines · Will Deacon
The MMU notifiers are registered by
kvm_init_mmu_notifier()only afterkvm_arch_init_vm()has returned successfully. Since the latter function initialises the 'kvm->arch.mmu.pgt' pointer (and allocates the VM handle when pKVM is enabled), the initialisation checks in the MMU notifiers are not required.
Quoted 8 lines
It took me a while to remember, but I think these checks are needed for the free path rather than init. In particular, the doc for
mmu_notifier_ops::release()(from which we free the pgt) says that it "can run concurrently with other mmu notifier" (seemmu_notifier.h), which is fun.Had you considered that path? If so, probably worth expanding in the commit description why this is safe?
Urgh....
Let me get back to you on that :)
Will
Will Deacon
maintainerRe: [PATCH 02/30] KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
WARNING: long and boring reply, but I might find this helpful in future so I'm sending it anyway. The short answer is, you're right and this patch is busted :)
Quoted 14 lines · Will Deacon
Quoted 13 lines · Quentin Perret
Quoted 5 lines · Will Deacon
The MMU notifiers are registered by
kvm_init_mmu_notifier()only afterkvm_arch_init_vm()has returned successfully. Since the latter function initialises the 'kvm->arch.mmu.pgt' pointer (and allocates the VM handle when pKVM is enabled), the initialisation checks in the MMU notifiers are not required.It took me a while to remember, but I think these checks are needed for the free path rather than init. In particular, the doc for
mmu_notifier_ops::release()(from which we free the pgt) says that it "can run concurrently with other mmu notifier" (seemmu_notifier.h), which is fun.
Quoted 7 lines
Quoted 2 lines
Had you considered that path? If so, probably worth expanding in the commit description why this is safe?
Urgh....
Let me get back to you on that :)
Ok, so the 'pgt' is set to NULL by kvm_free_stage2_pgd() with 'kvm->mmu_lock' held for write. There are three notifier callbacks that I'm changing not to check for a NULL pgt:
1. kvm_unmap_gfn_range()
=> Runs with the MMU lock held for write. See how kvm_mmu_notifier_invalidate_range_start() calls kvm_handle_hva_range() with '.lockless' unset (i.e. false) in 'hva_range'
2. kvm_age_gfn()
=> Takes the lock unless KVM_MMU_LOCKLESS_AGING, which isn't selected for arm64
3. kvm_test_age_gfn()
=> Same as above.
so I think that means we're serialised with teardown.
Looking at it a different way, suppose that the notifiers *could* run concurrently with kvm_free_stage2_pgd(). In that case, the check of the pgt wouldn't buy us anything because the pgt could be cleared (and the page-table freed) right after we checked that it wasn't NULL but before we tried touching the page-table.
The next question is whether we can simply run _after_ the page-table teardown?
For the usual teardown case via exit_mmap(), it looks pretty hard to trigger since the release notifier is only called once 'mm_users' hits zero and the 'mm' has been NULLified.
The more obvious case is when the release notifier is run from mmu_notifier_unregister() by kvm_destroy_vm(). In this case, there's a synchronize_srcu() but only _after_ calling ->release() and so there is a small window where another notifier could run into the NULL pgt.
I hacked in an mdelay(5000) between calling ->release and removing the MMU notifier subscription and then bodged kvmtool to use MAP_POPULATE for guest memory and to close the VM fd after creating the memslots. Sure enough, that was enough to trigger the crash (see below).
So I'll rework this patch to preserve the NULL checks on the pgt but I'll keep the part moving the handle check out of __pkvm_pgtable_stage2_unmap(), as the handle is cleared by kvm_arch_destroy_vm() after the notifiers really have gone away.
Cheers,
Will
--->8
[ 1607.241011] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 1607.241145] Mem abort info: [ 1607.241395] ESR = 0x0000000096000004 [ 1607.241441] EC = 0x25: DABT (current EL), IL = 32 bits [ 1607.241597] SET = 0, FnV = 0 [ 1607.241618] EA = 0, S1PTW = 0 [ 1607.241643] FSC = 0x04: level 0 translation fault [ 1607.241686] Data abort info: [ 1607.241711] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 [ 1607.241733] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 1607.241759] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 1607.241883] user pgtable: 4k pages, 52-bit VAs, pgdp=00000000400ad400 [ 1607.241915] [0000000000000000] pgd=080000004550e403, p4d=0000000000000000 [ 1607.242828] Internal error: Oops: 0000000096000004 [#1] SMP [ 1607.246029] Modules linked in: [ 1607.246394] CPU: 0 UID: 0 PID: 51 Comm: kswapd0 Not tainted 6.19.0-rc4-00030-g93ba3d7c3693-dirty #3 PREEMPT [ 1607.246823] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 [ 1607.247213] pstate: 81402009 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) [ 1607.247584] pc : kvm_pgtable_walk+0x54/0x150 [ 1607.247988] lr : kvm_pgtable_stage2_test_clear_young+0x48/0x80 [ 1607.248434] sp : ffff800080463510 [ 1607.248717] x29: ffff800080463540 x28: fff0000004512488 x27: 0000000000000001 [ 1607.249129] x26: fff0000001241d50 x25: 0000ffff869fffff x24: 0000ffff86800000 [ 1607.249455] x23: 0000000082400000 x22: 0000000082200000 x21: 0000000000000001 [ 1607.249796] x20: 0000000000000000 x19: ffff800080463588 x18: ffff9bea62668cc0 [ 1607.250154] x17: 001ada185af13066 x16: 0000000000000028 x15: ffffc1ffc0000000 [ 1607.250500] x14: 0010000000000000 x13: 0000ffff86800000 x12: 0000000000080000 [ 1607.250827] x11: 0000000000080000 x10: 0000ffff84600000 x9 : 0000000000000001 [ 1607.251186] x8 : 0000000082400fff x7 : 0c8a13c0ffff8660 x6 : 0c8a43e0ffff867f [ 1607.251731] x5 : fff00000070fa1c0 x4 : 0000000000000000 x3 : ffff800080463588 [ 1607.252312] x2 : 0000000000200000 x1 : 0000000082200000 x0 : 0000000000000000 [ 1607.252719] Call trace: [ 1607.252909] kvm_pgtable_walk+0x54/0x150 (P) [ 1607.253133] kvm_pgtable_stage2_test_clear_young+0x48/0x80 [ 1607.253391] kvm_age_gfn+0x44/0x88 [ 1607.253580] kvm_mmu_notifier_clear_flush_young+0xc4/0x240 [ 1607.253842] __mmu_notifier_clear_flush_young+0xa0/0xb0 [ 1607.254096] folio_referenced_one+0x260/0x3b8 [ 1607.254312] rmap_walk_anon+0x148/0x1d4 [ 1607.254501] folio_referenced+0x118/0x200 [ 1607.254691] shrink_folio_list+0x3c8/0x1100 [ 1607.254896] shrink_lruvec+0x720/0xbec [ 1607.255122] shrink_node+0x4cc/0x8e4 [ 1607.255311] balance_pgdat+0x488/0x77c [ 1607.255521] kswapd+0x260/0x2e0 [ 1607.255691] kthread+0x1bc/0x200 [ 1607.255898] ret_from_fork+0x10/0x20 [ 1607.256326] Code: 9274cd17 a901dff6 36180049 9402bc60 (b9400288) [ 1607.257126] ---[ end trace 0000000000000000 ]---
Will Deacon
maintainerPATCH 3/30 KVM: arm64: Rename __pkvm_pgtable_stage2_unmap()
In reply to Will DeaconIn preparation for adding support for protected VMs, where pages are donated rather than shared, rename __pkvm_pgtable_stage2_unmap() to __pkvm_pgtable_stage2_unshare() to make it clearer about what is going on.
--- arch/arm64/kvm/pkvm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/kvm/pkvm.c +4 -4
@@ -322,7 +322,7 @@ int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,return 0 ; } -static int __pkvm_pgtable_stage2_unmap (struct kvm_pgtable *pgt,u64 start,u64 end) +static int __pkvm_pgtable_stage2_unshare (struct kvm_pgtable *pgt,u64 start,u64 end) {struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu);pkvm_handle_t handle = kvm->arch.pkvm.handle;
@@ -350,7 +350,7 @@ void pkvm_pgtable_stage2_destroy_range(struct kvm_pgtable *pgt,if (!handle)return ; -__pkvm_pgtable_stage2_unmap (pgt, addr, addr + size); +__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }void pkvm_pgtable_stage2_destroy_pgd (struct kvm_pgtable *pgt)
@@ -386,7 +386,7 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EAGAIN ;/* Remove _any_ pkvm_mapping overlapping with the range, bigger or smaller. */ - ret =__pkvm_pgtable_stage2_unmap (pgt, addr, addr + size); + ret =__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size);if (ret)return ret; mapping =NULL ;
@@ -409,7 +409,7 @@ int pkvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size) {lockdep_assert_held_write (&kvm_s2_mmu_to_kvm (pgt->mmu)->mmu_lock); -return __pkvm_pgtable_stage2_unmap (pgt, addr, addr + size); +return __pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }int pkvm_pgtable_stage2_wrprotect (struct kvm_pgtable *pgt,u64 addr,u64 size)
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 4/30 KVM: arm64: Don't advertise unsupported features for protected guests
In reply to Will DeaconBoth SVE and PMUv3 are treated as "restricted" features for protected guests and attempts to access their corresponding architectural state from a protected guest result in an undefined exception being injected by the hypervisor.
Since these exceptions are unexpected and typically fatal for the guest, don't advertise these features for protected guests.
--- arch/arm64/include/asm/kvm_pkvm.h | 2 -- 1 file changed, 2 deletions(-)
arch/arm64/include/asm/kvm_pkvm.h +0 -2
@@ -37,8 +37,6 @@ static inline bool kvm_pvm_ext_allowed(long ext)case KVM_CAP_MAX_VCPU_ID :case KVM_CAP_MSI_DEVID :case KVM_CAP_ARM_VM_IPA_SIZE : -case KVM_CAP_ARM_PMU_V3 : -case KVM_CAP_ARM_SVE :case KVM_CAP_ARM_PTRAUTH_ADDRESS :case KVM_CAP_ARM_PTRAUTH_GENERIC :return true;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 5/30 KVM: arm64: Expose self-hosted debug regs as RAZ/WI for protected guests
In reply to Will DeaconFrom: Fuad Tabba <[email protected]>
Debug and trace are not currently supported for protected guests, so trap accesses to the related registers and emulate them as RAZ/WI for now. Although this isn't strictly compatible with the architecture, it's sufficient for Linux guests and means that debug support can be added later on.
--- arch/arm64/kvm/hyp/nvhe/sys_regs.c | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +8 -0
@@ -372,6 +372,14 @@ static const struct sys_reg_desc pvm_sys_reg_descs[] = {/* Cache maintenance by set/way operations are restricted. */ /* Debug and Trace Registers are restricted. */ +RAZ_WI (SYS_DBGBVRn_EL1 (0 )), +RAZ_WI (SYS_DBGBCRn_EL1 (0 )), +RAZ_WI (SYS_DBGWVRn_EL1 (0 )), +RAZ_WI (SYS_DBGWCRn_EL1 (0 )), +RAZ_WI (SYS_MDSCR_EL1 ), +RAZ_WI (SYS_OSLAR_EL1 ), +RAZ_WI (SYS_OSLSR_EL1 ), +RAZ_WI (SYS_OSDLR_EL1 ),/* Group 1 ID registers */ HOST_HANDLED(SYS_REVIDR_EL1),
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 6/30 KVM: arm64: Remove pointless is_protected_kvm_enabled() checks from hyp
In reply to Will DeaconWhen pKVM is not enabled, the host shouldn't issue pKVM-specific hypercalls and so there's no point checking for this in the EL2 hypercall handling code.
Remove the redundant is_protected_kvm_enabled() checks.
--- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 33 ++---------------------------- 1 file changed, 2 insertions(+), 31 deletions(-)
arch/arm64/kvm/hyp/nvhe/hyp-main.c +2 -31
@@ -169,9 +169,6 @@ static void handle___pkvm_vcpu_load(struct kvm_cpu_context *host_ctxt)DECLARE_REG (u64, hcr_el2, host_ctxt,3 );struct pkvm_hyp_vcpu *hyp_vcpu; -if (!is_protected_kvm_enabled ()) -return ; - hyp_vcpu =pkvm_load_hyp_vcpu (handle, vcpu_idx);if (!hyp_vcpu)return ;
@@ -185,12 +182,8 @@ static void handle___pkvm_vcpu_load(struct kvm_cpu_context *host_ctxt)static void handle___pkvm_vcpu_put (struct kvm_cpu_context *host_ctxt) { -struct pkvm_hyp_vcpu *hyp_vcpu; +struct pkvm_hyp_vcpu *hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); -if (!is_protected_kvm_enabled ()) -return ; - - hyp_vcpu =pkvm_get_loaded_hyp_vcpu ();if (hyp_vcpu)pkvm_put_hyp_vcpu (hyp_vcpu); }
@@ -254,9 +247,6 @@ static void handle___pkvm_host_share_guest(struct kvm_cpu_context *host_ctxt)struct pkvm_hyp_vcpu *hyp_vcpu;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vcpu =pkvm_get_loaded_hyp_vcpu ();if (!hyp_vcpu ||pkvm_hyp_vcpu_is_protected (hyp_vcpu)) goto out;
@@ -278,9 +268,6 @@ static void handle___pkvm_host_unshare_guest(struct kvm_cpu_context *host_ctxt)struct pkvm_hyp_vm *hyp_vm;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vm =get_np_pkvm_hyp_vm (handle);if (!hyp_vm) goto out;
@@ -298,9 +285,6 @@ static void handle___pkvm_host_relax_perms_guest(struct kvm_cpu_context *host_ctstruct pkvm_hyp_vcpu *hyp_vcpu;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vcpu =pkvm_get_loaded_hyp_vcpu ();if (!hyp_vcpu ||pkvm_hyp_vcpu_is_protected (hyp_vcpu)) goto out;
@@ -318,9 +302,6 @@ static void handle___pkvm_host_wrprotect_guest(struct kvm_cpu_context *host_ctxtstruct pkvm_hyp_vm *hyp_vm;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vm =get_np_pkvm_hyp_vm (handle);if (!hyp_vm) goto out;
@@ -340,9 +321,6 @@ static void handle___pkvm_host_test_clear_young_guest(struct kvm_cpu_context *hostruct pkvm_hyp_vm *hyp_vm;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vm =get_np_pkvm_hyp_vm (handle);if (!hyp_vm) goto out;
@@ -359,9 +337,6 @@ static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt)struct pkvm_hyp_vcpu *hyp_vcpu;int ret = -EINVAL ; -if (!is_protected_kvm_enabled ()) - goto out; - hyp_vcpu =pkvm_get_loaded_hyp_vcpu ();if (!hyp_vcpu ||pkvm_hyp_vcpu_is_protected (hyp_vcpu)) goto out;
@@ -421,12 +396,8 @@ static void handle___kvm_tlb_flush_vmid(struct kvm_cpu_context *host_ctxt)static void handle___pkvm_tlb_flush_vmid (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 ); -struct pkvm_hyp_vm *hyp_vm; +struct pkvm_hyp_vm *hyp_vm =get_np_pkvm_hyp_vm (handle); -if (!is_protected_kvm_enabled ()) -return ; - - hyp_vm =get_np_pkvm_hyp_vm (handle);if (!hyp_vm)return ;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 06/30] KVM: arm64: Remove pointless is_protected_kvm_enabled() checks from hyp
Quoted 5 lines · Will Deacon
When pKVM is not enabled, the host shouldn't issue pKVM-specific hypercalls and so there's no point checking for this in the EL2 hypercall handling code.
Remove the redundant
is_protected_kvm_enabled()checks.
That made me wonder if we should further divide the HVC space to have a 'pKVM only' range in addition to the privileged/unprivileged split, so we could WARN in the core HVC handler in the pretty unlikely event that we take a pKVM-only call in {n,h}VHE.
But yes there is no point in littering the code with is_protected_kvm_enabled() checks all over, so:
Thanks,
Quentin
Will Deacon
maintainerRe: [PATCH 06/30] KVM: arm64: Remove pointless is_protected_kvm_enabled() checks from hyp
Quoted 11 lines · Quentin Perret
Quoted 5 lines · Will Deacon
When pKVM is not enabled, the host shouldn't issue pKVM-specific hypercalls and so there's no point checking for this in the EL2 hypercall handling code.
Remove the redundant
is_protected_kvm_enabled()checks.That made me wonder if we should further divide the HVC space to have a 'pKVM only' range in addition to the privileged/unprivileged split, so we could WARN in the core HVC handler in the pretty unlikely event that we take a pKVM-only call in {n,h}VHE.
Good idea. I can certainly look at that as a follow-up to this series but I'd prefer not to grow it beyond its current scope at the moment as it's already bigger than I would really like.
Quoted 4 lines
But yes there is no point in littering the code with
is_protected_kvm_enabled()checks all over, so:Reviewed-by Quentin Perret <[email protected]>
Thanks,
Will
Will Deacon
maintainerPATCH 7/30 KVM: arm64: Ignore MMU notifier callbacks for protected VMs
In reply to Will DeaconIn preparation for supporting the donation of pinned pages to protected VMs, return early from the MMU notifiers when called for a protected VM, as the necessary hypercalls are exposed only for non-protected guests.
--- arch/arm64/kvm/mmu.c | 12 ++++++++++++ arch/arm64/kvm/pkvm.c | 19 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-)
arch/arm64/kvm/mmu.c +12 -0
@@ -340,6 +340,9 @@ static void __unmap_stage2_range(struct kvm_s2_mmu *mmu, phys_addr_t start, u64void kvm_stage2_unmap_range (struct kvm_s2_mmu *mmu,phys_addr_t start,u64 size,bool may_block) { +if (kvm_vm_is_protected (kvm_s2_mmu_to_kvm (mmu))) +return ; +__unmap_stage2_range (mmu, start, size, may_block); }
@@ -2208,6 +2211,9 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)bool kvm_unmap_gfn_range (struct kvm *kvm,struct kvm_gfn_range *range) { +if (kvm_vm_is_protected (kvm)) +return false; +__unmap_stage2_range (&kvm->arch.mmu, range->start <<PAGE_SHIFT , (range->end - range->start) <<PAGE_SHIFT , range->may_block);
@@ -2220,6 +2226,9 @@ bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) {u64 size = (range->end - range->start) <<PAGE_SHIFT ; +if (kvm_vm_is_protected (kvm)) +return false; +return KVM_PGT_FN (kvm_pgtable_stage2_test_clear_young)(kvm->arch.mmu.pgt, range->start <<PAGE_SHIFT , size, true);
@@ -2233,6 +2242,9 @@ bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) {u64 size = (range->end - range->start) <<PAGE_SHIFT ; +if (kvm_vm_is_protected (kvm)) +return false; +return KVM_PGT_FN (kvm_pgtable_stage2_test_clear_young)(kvm->arch.mmu.pgt, range->start <<PAGE_SHIFT , size, false);
arch/arm64/kvm/pkvm.c +18 -1
@@ -407,7 +407,12 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,int pkvm_pgtable_stage2_unmap (struct kvm_pgtable *pgt,u64 addr,u64 size) { -lockdep_assert_held_write (&kvm_s2_mmu_to_kvm (pgt->mmu)->mmu_lock); +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); + +if (WARN_ON (kvm_vm_is_protected (kvm))) +return -EPERM ; + +lockdep_assert_held_write (&kvm->mmu_lock);return __pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }
@@ -419,6 +424,9 @@ int pkvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)struct pkvm_mapping *mapping;int ret =0 ; +if (WARN_ON (kvm_vm_is_protected (kvm))) +return -EPERM ; +lockdep_assert_held (&kvm->mmu_lock);for_each_mapping_in_range_safe (pgt, addr, addr + size, mapping) { ret = kvm_call_hyp_nvhe(__pkvm_host_wrprotect_guest, handle, mapping->gfn,
@@ -450,6 +458,9 @@ bool pkvm_pgtable_stage2_test_clear_young(struct kvm_pgtable *pgt, u64 addr, u64struct pkvm_mapping *mapping;bool young = false; +if (WARN_ON (kvm_vm_is_protected (kvm))) +return -EPERM ; +lockdep_assert_held (&kvm->mmu_lock);for_each_mapping_in_range_safe (pgt, addr, addr + size, mapping) young |= kvm_call_hyp_nvhe(__pkvm_host_test_clear_young_guest, handle, mapping->gfn,
@@ -461,12 +472,18 @@ bool pkvm_pgtable_stage2_test_clear_young(struct kvm_pgtable *pgt, u64 addr, u64int pkvm_pgtable_stage2_relax_perms (struct kvm_pgtable *pgt,u64 addr,enum kvm_pgtable_prot prot,enum kvm_pgtable_walk_flags flags) { +if (WARN_ON (kvm_vm_is_protected (kvm_s2_mmu_to_kvm (pgt->mmu)))) +return -EPERM ; +return kvm_call_hyp_nvhe (__pkvm_host_relax_perms_guest, addr >>PAGE_SHIFT , prot); }void pkvm_pgtable_stage2_mkyoung (struct kvm_pgtable *pgt,u64 addr,enum kvm_pgtable_walk_flags flags) { +if (WARN_ON (kvm_vm_is_protected (kvm_s2_mmu_to_kvm (pgt->mmu)))) +return ; +WARN_ON (kvm_call_hyp_nvhe (__pkvm_host_mkyoung_guest, addr >>PAGE_SHIFT )); }
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 8/30 KVM: arm64: Prevent unsupported memslot operations on protected VMs
In reply to Will DeaconProtected VMs do not support deleting or moving memslots after first run nor do they support read-only or dirty logging.
Return -EPERM to userspace if such an operation is attempted.
--- arch/arm64/kvm/mmu.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/arm64/kvm/mmu.c +13 -0
@@ -2419,6 +2419,19 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,hva_t hva, reg_end;int ret =0 ; +if (kvm_vm_is_protected (kvm)) { +/* Cannot modify memslots once a pVM has run. */ +if (pkvm_hyp_vm_is_created (kvm) && + (change ==KVM_MR_DELETE || change ==KVM_MR_MOVE )) { +return -EPERM ; + } + +if (new && + new->flags & (KVM_MEM_LOG_DIRTY_PAGES |KVM_MEM_READONLY )) { +return -EPERM ; + } + } +if (change !=KVM_MR_CREATE && change !=KVM_MR_MOVE && change !=KVM_MR_FLAGS_ONLY )return 0 ;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 9/30 KVM: arm64: Split teardown hypercall into two phases
In reply to Will DeaconIn preparation for reclaiming protected guest VM pages from the host during teardown, split the current 'pkvm_teardown_vm' hypercall into separate 'start' and 'finalise' calls.
The 'pkvm_start_teardown_vm' hypercall puts the VM into a new 'is_dying' state, which is a point of no return past which no vCPU of the pVM is allowed to run any more. Once in this new state, 'pkvm_finalize_teardown_vm' can be used to reclaim meta-data and page-table pages from the VM. A subsequent patch will add support for reclaiming the individual guest memory pages.
--- arch/arm64/include/asm/kvm_asm.h | 3 ++- arch/arm64/include/asm/kvm_host.h | 7 +++++ arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 4 ++- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 14 +++++++--- arch/arm64/kvm/hyp/nvhe/pkvm.c | 36 ++++++++++++++++++++++---- arch/arm64/kvm/pkvm.c | 7 ++++- 6 files changed, 60 insertions(+), 11 deletions(-)
arch/arm64/include/asm/kvm_asm.h +2 -1
@@ -85,7 +85,8 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu, - __KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm, + __KVM_HOST_SMCCC_FUNC___pkvm_start_teardown_vm, + __KVM_HOST_SMCCC_FUNC___pkvm_finalize_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load, __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put, __KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid,
arch/arm64/include/asm/kvm_host.h +7 -0
@@ -255,6 +255,13 @@ struct kvm_protected_vm {struct kvm_hyp_memcache stage2_teardown_mc;bool is_protected;bool is_created; + +/* +* True when the guest is being torn down. When in this state, the +* guest's vCPUs can't be loaded anymore, but its pages can be +* reclaimed by the host. +*/ +bool is_dying; };struct kvm_mpidr_data {
arch/arm64/kvm/hyp/include/nvhe/pkvm.h +3 -1
@@ -73,7 +73,9 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva, unsigned long pgd_hva);int __pkvm_init_vcpu (pkvm_handle_t handle,struct kvm_vcpu *host_vcpu,unsigned long vcpu_hva); -int __pkvm_teardown_vm (pkvm_handle_t handle); + +int __pkvm_start_teardown_vm (pkvm_handle_t handle); +int __pkvm_finalize_teardown_vm (pkvm_handle_t handle);struct pkvm_hyp_vcpu *pkvm_load_hyp_vcpu (pkvm_handle_t handle,unsigned int vcpu_idx);
arch/arm64/kvm/hyp/nvhe/hyp-main.c +11 -3
@@ -550,11 +550,18 @@ static void handle___pkvm_init_vcpu(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_init_vcpu (handle, host_vcpu, vcpu_hva); } -static void handle___pkvm_teardown_vm (struct kvm_cpu_context *host_ctxt) +static void handle___pkvm_start_teardown_vm (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 ); -cpu_reg (host_ctxt,1 ) =__pkvm_teardown_vm (handle); +cpu_reg (host_ctxt,1 ) =__pkvm_start_teardown_vm (handle); +} + +static void handle___pkvm_finalize_teardown_vm (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 ); + +cpu_reg (host_ctxt,1 ) =__pkvm_finalize_teardown_vm (handle); }typedef void (*hcall_t )(struct kvm_cpu_context *);
@@ -594,7 +601,8 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_unreserve_vm),HANDLE_FUNC (__pkvm_init_vm),HANDLE_FUNC (__pkvm_init_vcpu), -HANDLE_FUNC (__pkvm_teardown_vm), +HANDLE_FUNC (__pkvm_start_teardown_vm), +HANDLE_FUNC (__pkvm_finalize_teardown_vm),HANDLE_FUNC (__pkvm_vcpu_load),HANDLE_FUNC (__pkvm_vcpu_put), HANDLE_FUNC(__pkvm_tlb_flush_vmid),
arch/arm64/kvm/hyp/nvhe/pkvm.c +31 -5
@@ -256,7 +256,10 @@ struct pkvm_hyp_vcpu *pkvm_load_hyp_vcpu(pkvm_handle_t handle,hyp_spin_lock (&vm_table_lock); hyp_vm =get_vm_by_handle (handle); -if (!hyp_vm || hyp_vm->kvm.created_vcpus <= vcpu_idx) +if (!hyp_vm || hyp_vm->kvm.arch.pkvm.is_dying) + goto unlock; + +if (hyp_vm->kvm.created_vcpus <= vcpu_idx) goto unlock; hyp_vcpu = hyp_vm->vcpus[vcpu_idx];
@@ -829,7 +832,32 @@ teardown_donated_memory(struct kvm_hyp_memcache *mc, void *addr, size_t size)unmap_donated_memory_noclear (addr, size); } -int __pkvm_teardown_vm (pkvm_handle_t handle) +int __pkvm_start_teardown_vm (pkvm_handle_t handle) +{ +struct pkvm_hyp_vm *hyp_vm; +int ret =0 ; + +hyp_spin_lock (&vm_table_lock); + hyp_vm =get_vm_by_handle (handle); +if (!hyp_vm) { + ret = -ENOENT ; + goto unlock; + }else if (WARN_ON (hyp_page_count (hyp_vm))) { + ret = -EBUSY ; + goto unlock; + }else if (hyp_vm->kvm.arch.pkvm.is_dying) { + ret = -EINVAL ; + goto unlock; + } + + hyp_vm->kvm.arch.pkvm.is_dying = true; +unlock: +hyp_spin_unlock (&vm_table_lock); + +return ret; +} + +int __pkvm_finalize_teardown_vm (pkvm_handle_t handle) {struct kvm_hyp_memcache *mc, *stage2_mc;struct pkvm_hyp_vm *hyp_vm;
@@ -843,9 +871,7 @@ int __pkvm_teardown_vm(pkvm_handle_t handle)if (!hyp_vm) { err = -ENOENT ; goto err_unlock; - } - -if (WARN_ON (hyp_page_count (hyp_vm))) { + }else if (!hyp_vm->kvm.arch.pkvm.is_dying) { err = -EBUSY ; goto err_unlock; }
arch/arm64/kvm/pkvm.c +6 -1
@@ -88,7 +88,7 @@ void __init kvm_hyp_reserve(void)static void __pkvm_destroy_hyp_vm (struct kvm *kvm) {if (pkvm_hyp_vm_is_created (kvm)) { -WARN_ON (kvm_call_hyp_nvhe (__pkvm_teardown_vm, +WARN_ON (kvm_call_hyp_nvhe (__pkvm_finalize_teardown_vm, kvm->arch.pkvm.handle)); }else if (kvm->arch.pkvm.handle) {/*
@@ -350,6 +350,11 @@ void pkvm_pgtable_stage2_destroy_range(struct kvm_pgtable *pgt,if (!handle)return ; +if (pkvm_hyp_vm_is_created (kvm) && !kvm->arch.pkvm.is_dying) { +WARN_ON (kvm_call_hyp_nvhe (__pkvm_start_teardown_vm, handle)); + kvm->arch.pkvm.is_dying = true; + } +__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 10/30 KVM: arm64: Introduce __pkvm_host_donate_guest()
In reply to Will DeaconIn preparation for supporting protected VMs, whose memory pages are isolated from the host, introduce a new pKVM hypercall to allow the donation of pages to a guest.
--- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/include/asm/kvm_pgtable.h | 2 +- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 ++ arch/arm64/kvm/hyp/nvhe/hyp-main.c | 21 +++++++++++++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 30 +++++++++++++++++++ 5 files changed, 55 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kvm_asm.h +1 -0
@@ -64,6 +64,7 @@ enum __kvm_host_smccc_func {/* Hypercalls available after pKVM finalisation */ __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp, __KVM_HOST_SMCCC_FUNC___pkvm_host_unshare_hyp, + __KVM_HOST_SMCCC_FUNC___pkvm_host_donate_guest, __KVM_HOST_SMCCC_FUNC___pkvm_host_share_guest, __KVM_HOST_SMCCC_FUNC___pkvm_host_unshare_guest, __KVM_HOST_SMCCC_FUNC___pkvm_host_relax_perms_guest,
arch/arm64/include/asm/kvm_pgtable.h +1 -1
@@ -98,7 +98,7 @@ typedef u64 kvm_pte_t; KVM_PTE_LEAF_ATTR_HI_S2_XN)#define KVM_INVALID_PTE_OWNER_MASK GENMASK(9, 2) -#define KVM_MAX_OWNER_ID 1 +#define KVM_MAX_OWNER_ID 3/* * Used to indicate a pte for which a 'break-before-make' sequence is in
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +2 -0
@@ -28,6 +28,7 @@ enum pkvm_component_id {PKVM_ID_HOST ,PKVM_ID_HYP ,PKVM_ID_FFA , + PKVM_ID_GUEST, };extern unsigned long hyp_nr_cpus;
@@ -39,6 +40,7 @@ int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages);int __pkvm_hyp_donate_host (u64 pfn,u64 nr_pages);int __pkvm_host_share_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_unshare_ffa (u64 pfn,u64 nr_pages); +int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu);int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot);int __pkvm_host_unshare_guest (u64 gfn,u64 nr_pages,struct pkvm_hyp_vm *hyp_vm);
arch/arm64/kvm/hyp/nvhe/hyp-main.c +21 -0
@@ -238,6 +238,26 @@ static int pkvm_refill_memcache(struct pkvm_hyp_vcpu *hyp_vcpu) &host_vcpu->arch.pkvm_memcache); } +static void handle___pkvm_host_donate_guest (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (u64, pfn, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); +struct pkvm_hyp_vcpu *hyp_vcpu; +int ret = -EINVAL ; + + hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); +if (!hyp_vcpu) + goto out; + + ret =pkvm_refill_memcache (hyp_vcpu); +if (ret) + goto out; + + ret =__pkvm_host_donate_guest (pfn, gfn, hyp_vcpu); +out: +cpu_reg (host_ctxt,1 ) = ret; +} +static void handle___pkvm_host_share_guest (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (u64, pfn, host_ctxt,1 );
@@ -580,6 +600,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__pkvm_host_unshare_hyp), +HANDLE_FUNC (__pkvm_host_donate_guest),HANDLE_FUNC (__pkvm_host_share_guest),HANDLE_FUNC (__pkvm_host_unshare_guest), HANDLE_FUNC(__pkvm_host_relax_perms_guest),
arch/arm64/kvm/hyp/nvhe/mem_protect.c +30 -0
@@ -958,6 +958,36 @@ static int __guest_check_transition_size(u64 phys, u64 ipa, u64 nr_pages, u64 *sreturn 0 ; } +int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu); +u64 phys =hyp_pfn_to_phys (pfn); +u64 ipa =hyp_pfn_to_phys (gfn); +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_OWNED ); +if (ret) + goto unlock; + + ret =__guest_check_page_state_range (vm, ipa,PAGE_SIZE ,PKVM_NOPAGE ); +if (ret) + goto unlock; + +WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); +WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys, +pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), + &vcpu->vcpu.arch.pkvm_memcache,0 )); + +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot) {
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 10/30] KVM: arm64: Introduce __pkvm_host_donate_guest()
arch/arm64/kvm/hyp/nvhe/hyp-main.c static int pkvm_refill_memcache(struct pkvm_hyp_vcpu *hyp_vcpu) · line 249
Quoted 9 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/hyp-main.c +9
-0+static void handle___pkvm_host_donate_guest (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (u64, pfn, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); +struct pkvm_hyp_vcpu *hyp_vcpu; +int ret = -EINVAL ; + + hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); +if (!hyp_vcpu)
I guess we should check this is a protected VM here, else a malicious host could donate pages to an np-guest. I didn't try to think through the implications, perhaps it's fine, but it feels unecessary so I'd say let's be restrictive here.
Cheers,
Quentin
Quoted 66 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +12
-0+ goto out; + + ret =pkvm_refill_memcache (hyp_vcpu); +if (ret) + goto out; + + ret =__pkvm_host_donate_guest (pfn, gfn, hyp_vcpu); +out: +cpu_reg (host_ctxt,1 ) = ret; +} +static void handle___pkvm_host_share_guest (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (u64, pfn, host_ctxt,1 );@@ -580,6 +600,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__pkvm_host_unshare_hyp), +HANDLE_FUNC (__pkvm_host_donate_guest),HANDLE_FUNC (__pkvm_host_share_guest),HANDLE_FUNC (__pkvm_host_unshare_guest), HANDLE_FUNC(__pkvm_host_relax_perms_guest),arch/arm64/kvm/hyp/nvhe/mem_protect.c +30
-0@@ -958,6 +958,36 @@ static int __guest_check_transition_size(u64 phys, u64 ipa, u64 nr_pages, u64 *sreturn 0 ; } +int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu); +u64 phys =hyp_pfn_to_phys (pfn); +u64 ipa =hyp_pfn_to_phys (gfn); +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_OWNED ); +if (ret) + goto unlock; + + ret =__guest_check_page_state_range (vm, ipa,PAGE_SIZE ,PKVM_NOPAGE ); +if (ret) + goto unlock; + +WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); +WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys, +pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), + &vcpu->vcpu.arch.pkvm_memcache,0 )); + +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot) {Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 10/30] KVM: arm64: Introduce __pkvm_host_donate_guest()
Quoted 15 lines · Quentin Perret
arch/arm64/kvm/hyp/nvhe/hyp-main.c static int pkvm_refill_memcache(struct pkvm_hyp_vcpu *hyp_vcpu) · line 249
Quoted 9 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/hyp-main.c +9
-0+static void handle___pkvm_host_donate_guest (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (u64, pfn, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); +struct pkvm_hyp_vcpu *hyp_vcpu; +int ret = -EINVAL ; + + hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); +if (!hyp_vcpu)I guess we should check this is a protected VM here, else a malicious host could donate pages to an np-guest. I didn't try to think through the implications, perhaps it's fine, but it feels unecessary so I'd say let's be restrictive here.
It's definitely harmless to add the check, but it's also hard to justify in my opinion. Can you think of anything that can actually go wrong it a malicious host donates memory to a non-protected guest? Generally, I think the fewer places where we have to distinguish between a protected and a non-protected guest in the hypervisor, the better.
Will
Re: [PATCH 10/30] KVM: arm64: Introduce __pkvm_host_donate_guest()
Quoted 17 lines · Will Deacon
Quoted 15 lines · Quentin Perret
arch/arm64/kvm/hyp/nvhe/hyp-main.c static int pkvm_refill_memcache(struct pkvm_hyp_vcpu *hyp_vcpu) · line 249
Quoted 9 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/hyp-main.c +9
-0+static void handle___pkvm_host_donate_guest (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (u64, pfn, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); +struct pkvm_hyp_vcpu *hyp_vcpu; +int ret = -EINVAL ; + + hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); +if (!hyp_vcpu)I guess we should check this is a protected VM here, else a malicious host could donate pages to an np-guest. I didn't try to think through the implications, perhaps it's fine, but it feels unecessary so I'd say let's be restrictive here.
Quoted 5 lines
It's definitely harmless to add the check, but it's also hard to justify in my opinion. Can you think of anything that can actually go wrong it a malicious host donates memory to a non-protected guest? Generally, I think the fewer places where we have to distinguish between a protected and a non-protected guest in the hypervisor, the better.
I agree with the principle. Now, we already _must_ check that host-to-guest shares are not allowed for pVMs, so this would simply make things symmetrical I think, and this is really a simple check.
Right now a page donated to an np-guest is probably just leaked and doesn't harm security with the current code, but I'd personally rather not have to reason about this known-to-be-broken state as we add more transitions in the future -- I write more than enough bugs just thinking about the known-good ones :-).
Will Deacon
maintainerPATCH 11/30 KVM: arm64: Hook up donation hypercall to pkvm_pgtable_stage2_map()
In reply to Will DeaconMapping pages into a protected guest requires the donation of memory from the host.
Extend pkvm_pgtable_stage2_map() to issue a donate hypercall when the target VM is protected. Since the hypercall only handles a single page, the splitting logic used for the share path is not required.
--- arch/arm64/kvm/pkvm.c | 58 ++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 17 deletions(-)
arch/arm64/kvm/pkvm.c +41 -17
@@ -373,31 +373,55 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,struct kvm_hyp_memcache *cache = mc;u64 gfn = addr >>PAGE_SHIFT ;u64 pfn = phys >>PAGE_SHIFT ; +u64 end = addr + size;int ret; -if (size !=PAGE_SIZE && size !=PMD_SIZE ) -return -EINVAL ; -lockdep_assert_held_write (&kvm->mmu_lock); + mapping =pkvm_mapping_iter_first (&pgt->pkvm_mappings, addr, end -1 ); -/* -* Calling stage2_map() on top of existing mappings is either happening because of a race -* with another vCPU, or because we're changing between page and block mappings. As per -* user_mem_abort(), same-size permission faults are handled in the relax_perms() path. -*/ - mapping =pkvm_mapping_iter_first (&pgt->pkvm_mappings, addr, addr + size -1 ); -if (mapping) { -if (size == (mapping->nr_pages *PAGE_SIZE )) +if (kvm_vm_is_protected (kvm)) { +/* Protected VMs are mapped using RWX page-granular mappings */ +if (WARN_ON_ONCE (size !=PAGE_SIZE )) +return -EINVAL ; + +if (WARN_ON_ONCE (prot !=KVM_PGTABLE_PROT_RWX )) +return -EINVAL ; + +/* +* We raced with another vCPU. +*/ +if (mapping)return -EAGAIN ; -/* Remove _any_ pkvm_mapping overlapping with the range, bigger or smaller. */ - ret =__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); -if (ret) -return ret; - mapping =NULL ; + ret =kvm_call_hyp_nvhe (__pkvm_host_donate_guest, pfn, gfn); + }else { +if (WARN_ON_ONCE (size !=PAGE_SIZE && size !=PMD_SIZE )) +return -EINVAL ; + +/* +* We either raced with another vCPU or we're changing between +* page and block mappings. As per user_mem_abort(), same-size +* permission faults are handled in the relax_perms() path. +*/ +if (mapping) { +if (size == (mapping->nr_pages *PAGE_SIZE )) +return -EAGAIN ; + +/* +* Remove _any_ pkvm_mapping overlapping with the range, +* bigger or smaller. +*/ + ret =__pkvm_pgtable_stage2_unshare (pgt, addr, end); +if (ret) +return ret; + + mapping =NULL ; + } + + ret =kvm_call_hyp_nvhe (__pkvm_host_share_guest, pfn, gfn, + size /PAGE_SIZE , prot); } - ret =kvm_call_hyp_nvhe (__pkvm_host_share_guest, pfn, gfn, size /PAGE_SIZE , prot);if (WARN_ON (ret))return ret;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 12/30 KVM: arm64: Handle aborts from protected VMs
In reply to Will DeaconIntroduce a new abort handler for resolving stage-2 page faults from protected VMs by pinning and donating anonymous memory. This is considerably simpler than the infamous user_mem_abort() as we only have to deal with translation faults at the pte level.
--- arch/arm64/kvm/mmu.c | 89 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 8 deletions(-)
arch/arm64/kvm/mmu.c +81 -8
@@ -1641,6 +1641,74 @@ static int gmem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,return ret != -EAGAIN ? ret :0 ; } +static int pkvm_mem_abort (struct kvm_vcpu *vcpu,phys_addr_t fault_ipa, +struct kvm_memory_slot *memslot,unsigned long hva) +{ +unsigned int flags =FOLL_HWPOISON |FOLL_LONGTERM |FOLL_WRITE ; +struct kvm_pgtable *pgt = vcpu->arch.hw_mmu->pgt; +struct mm_struct *mm = current->mm; +struct kvm *kvm = vcpu->kvm; +void *hyp_memcache; +struct page *page; +int ret; + + ret =prepare_mmu_memcache (vcpu, true, &hyp_memcache); +if (ret) +return -ENOMEM ; + + ret =account_locked_vm (mm,1 , true); +if (ret) +return ret; + +mmap_read_lock (mm); + ret =pin_user_pages (hva,1 , flags, &page); +mmap_read_unlock (mm); + +if (ret == -EHWPOISON ) { +kvm_send_hwpoison_signal (hva,PAGE_SHIFT ); + ret =0 ; + goto dec_account; + }else if (ret !=1 ) { + ret = -EFAULT ; + goto dec_account; + }else if (!folio_test_swapbacked (page_folio (page))) { +/* +* We really can't deal with page-cache pages returned by GUP +* because (a) we may trigger writeback of a page for which we +* no longer have access and (b) page_mkclean() won't find the +* stage-2 mapping in the rmap so we can get out-of-whack with +* the filesystem when marking the page dirty during unpinning +* (see cc5095747edf ("ext4: don't BUG if someone dirty pages +* without asking ext4 first")). +* +* Ideally we'd just restrict ourselves to anonymous pages, but +* we also want to allow memfd (i.e. shmem) pages, so check for +* pages backed by swap in the knowledge that the GUP pin will +* prevent try_to_unmap() from succeeding. +*/ + ret = -EIO ; + goto unpin; + } + +write_lock (&kvm->mmu_lock); + ret =pkvm_pgtable_stage2_map (pgt, fault_ipa,PAGE_SIZE , +page_to_phys (page),KVM_PGTABLE_PROT_RWX , + hyp_memcache,0 ); +write_unlock (&kvm->mmu_lock); +if (ret) { +if (ret == -EAGAIN ) + ret =0 ; + goto unpin; + } + +return 0 ; +unpin: +unpin_user_pages (&page,1 ); +dec_account: +account_locked_vm (mm,1 , false); +return ret; +} + static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, struct kvm_s2_trans *nested, struct kvm_memory_slot *memslot, unsigned long hva,
@@ -2190,15 +2258,20 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu) goto out_unlock; } -VM_WARN_ON_ONCE (kvm_vcpu_trap_is_permission_fault (vcpu) && - !write_fault && !kvm_vcpu_trap_is_exec_fault (vcpu)); +if (kvm_vm_is_protected (vcpu->kvm)) { + ret =pkvm_mem_abort (vcpu, fault_ipa, memslot, hva); + }else { +VM_WARN_ON_ONCE (kvm_vcpu_trap_is_permission_fault (vcpu) && + !write_fault && + !kvm_vcpu_trap_is_exec_fault (vcpu)); -if (kvm_slot_has_gmem (memslot)) - ret =gmem_abort (vcpu, fault_ipa, nested, memslot, -esr_fsc_is_permission_fault (esr)); -else - ret =user_mem_abort (vcpu, fault_ipa, nested, memslot, hva, -esr_fsc_is_permission_fault (esr)); +if (kvm_slot_has_gmem (memslot)) + ret =gmem_abort (vcpu, fault_ipa, nested, memslot, +esr_fsc_is_permission_fault (esr)); +else + ret =user_mem_abort (vcpu, fault_ipa, nested, memslot, hva, +esr_fsc_is_permission_fault (esr)); + }if (ret ==0 ) ret =1 ; out:
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 13/30 KVM: arm64: Introduce __pkvm_reclaim_dying_guest_page()
In reply to Will DeaconTo enable reclaim of pages from a protected VM during teardown, introduce a new hypercall to reclaim a single page from a protected guest that is in the dying state.
Since the EL2 code is non-preemptible, the new hypercall deliberately acts on a single page at a time so as to allow EL1 to reschedule frequently during the teardown operation.
--- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 1 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 9 +++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 79 +++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/pkvm.c | 14 ++++ 6 files changed, 105 insertions(+)
arch/arm64/include/asm/kvm_asm.h +1 -0
@@ -86,6 +86,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu, + __KVM_HOST_SMCCC_FUNC___pkvm_reclaim_dying_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_start_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_finalize_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1 -0
@@ -41,6 +41,7 @@ int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages);int __pkvm_host_share_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_unshare_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu); +int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm);int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot);int __pkvm_host_unshare_guest (u64 gfn,u64 nr_pages,struct pkvm_hyp_vm *hyp_vm);
arch/arm64/kvm/hyp/include/nvhe/pkvm.h +1 -0
@@ -74,6 +74,7 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,int __pkvm_init_vcpu (pkvm_handle_t handle,struct kvm_vcpu *host_vcpu,unsigned long vcpu_hva); +int __pkvm_reclaim_dying_guest_page (pkvm_handle_t handle,u64 gfn);int __pkvm_start_teardown_vm (pkvm_handle_t handle);int __pkvm_finalize_teardown_vm (pkvm_handle_t handle);
arch/arm64/kvm/hyp/nvhe/hyp-main.c +9 -0
@@ -570,6 +570,14 @@ static void handle___pkvm_init_vcpu(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_init_vcpu (handle, host_vcpu, vcpu_hva); } +static void handle___pkvm_reclaim_dying_guest_page (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); + +cpu_reg (host_ctxt,1 ) =__pkvm_reclaim_dying_guest_page (handle, gfn); +} +static void handle___pkvm_start_teardown_vm (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 );
@@ -622,6 +630,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_unreserve_vm),HANDLE_FUNC (__pkvm_init_vm),HANDLE_FUNC (__pkvm_init_vcpu), +HANDLE_FUNC (__pkvm_reclaim_dying_guest_page),HANDLE_FUNC (__pkvm_start_teardown_vm),HANDLE_FUNC (__pkvm_finalize_teardown_vm), HANDLE_FUNC(__pkvm_vcpu_load),
arch/arm64/kvm/hyp/nvhe/mem_protect.c +79 -0
91 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/pkvm.c +14 -0
@@ -832,6 +832,20 @@ teardown_donated_memory(struct kvm_hyp_memcache *mc, void *addr, size_t size)unmap_donated_memory_noclear (addr, size); } +int __pkvm_reclaim_dying_guest_page (pkvm_handle_t handle,u64 gfn) +{ +struct pkvm_hyp_vm *hyp_vm; +int ret = -EINVAL ; + +hyp_spin_lock (&vm_table_lock); + hyp_vm =get_vm_by_handle (handle); +if (hyp_vm && hyp_vm->kvm.arch.pkvm.is_dying) + ret =__pkvm_host_reclaim_page_guest (gfn, hyp_vm); +hyp_spin_unlock (&vm_table_lock); + +return ret; +} +int __pkvm_start_teardown_vm (pkvm_handle_t handle) {struct pkvm_hyp_vm *hyp_vm;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 13/30] KVM: arm64: Introduce __pkvm_reclaim_dying_guest_page()
Quoted 11 lines · Will Deacon
To enable reclaim of pages from a protected VM during teardown, introduce a new hypercall to reclaim a single page from a protected guest that is in the dying state.
Since the EL2 code is non-preemptible, the new hypercall deliberately acts on a single page at a time so as to allow EL1 to reschedule frequently during the teardown operation.
Co-developed-by Quentin Perret <[email protected]>Signed-off-by Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>
Quoted 197 lines
--- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 1 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 9 +++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 79 +++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/pkvm.c | 14 ++++ 6 files changed, 105 insertions(+)arch/arm64/include/asm/kvm_asm.h +1
-0@@ -86,6 +86,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu, + __KVM_HOST_SMCCC_FUNC___pkvm_reclaim_dying_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_start_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_finalize_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1
-0@@ -41,6 +41,7 @@ int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages);int __pkvm_host_share_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_unshare_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu); +int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm);int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot);int __pkvm_host_unshare_guest (u64 gfn,u64 nr_pages,struct pkvm_hyp_vm *hyp_vm);arch/arm64/kvm/hyp/include/nvhe/pkvm.h +1
-0@@ -74,6 +74,7 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,int __pkvm_init_vcpu (pkvm_handle_t handle,struct kvm_vcpu *host_vcpu,unsigned long vcpu_hva); +int __pkvm_reclaim_dying_guest_page (pkvm_handle_t handle,u64 gfn);int __pkvm_start_teardown_vm (pkvm_handle_t handle);int __pkvm_finalize_teardown_vm (pkvm_handle_t handle);arch/arm64/kvm/hyp/nvhe/hyp-main.c +9
-0@@ -570,6 +570,14 @@ static void handle___pkvm_init_vcpu(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_init_vcpu (handle, host_vcpu, vcpu_hva); } +static void handle___pkvm_reclaim_dying_guest_page (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 ); +DECLARE_REG (u64, gfn, host_ctxt,2 ); + +cpu_reg (host_ctxt,1 ) =__pkvm_reclaim_dying_guest_page (handle, gfn); +} +static void handle___pkvm_start_teardown_vm (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 );@@ -622,6 +630,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_unreserve_vm),HANDLE_FUNC (__pkvm_init_vm),HANDLE_FUNC (__pkvm_init_vcpu), +HANDLE_FUNC (__pkvm_reclaim_dying_guest_page),HANDLE_FUNC (__pkvm_start_teardown_vm),HANDLE_FUNC (__pkvm_finalize_teardown_vm), HANDLE_FUNC(__pkvm_vcpu_load),arch/arm64/kvm/hyp/nvhe/mem_protect.c +79
-0@@ -725,6 +725,32 @@ static int __guest_check_page_state_range(struct pkvm_hyp_vm *vm, u64 addr,return check_page_state_range (&vm->pgt, addr, size, &d); } +static int get_valid_guest_pte (struct pkvm_hyp_vm *vm,u64 ipa,kvm_pte_t *ptep,u64 *physp) +{ +kvm_pte_t pte; +u64 phys; +s8 level; +int ret; + + ret =kvm_pgtable_get_leaf (&vm->pgt, ipa, &pte, &level); +if (ret) +return ret; +if (!kvm_pte_valid (pte)) +return -ENOENT ; +if (level !=KVM_PGTABLE_LAST_LEVEL ) +return -E2BIG ; + + phys =kvm_pte_to_phys (pte); + ret =check_range_allowed_memory (phys, phys +PAGE_SIZE ); +if (WARN_ON (ret)) +return ret; + + *ptep = pte; + *physp = phys; + +return 0 ; +} +int __pkvm_host_share_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);@@ -958,6 +984,59 @@ static int __guest_check_transition_size(u64 phys, u64 ipa, u64 nr_pages, u64 *sreturn 0 ; } +static void hyp_poison_page (phys_addr_t phys) +{ +void *addr =hyp_fixmap_map (phys); + +memset (addr,0 ,PAGE_SIZE ); +/* +* Prefer kvm_flush_dcache_to_poc() over __clean_dcache_guest_page() +* here as the latter may elide the CMO under the assumption that FWB +* will be enabled on CPUs that support it. This is incorrect for the +* host stage-2 and would otherwise lead to a malicious host potentially +* being able to read the contents of newly reclaimed guest pages. +*/ +kvm_flush_dcache_to_poc (addr,PAGE_SIZE ); +hyp_fixmap_unmap (); +} + +int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm) +{ +u64 ipa =hyp_pfn_to_phys (gfn); +kvm_pte_t pte; +u64 phys; +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =get_valid_guest_pte (vm, ipa, &pte, &phys); +if (ret) + goto unlock; + +switch (guest_get_page_state (pte, ipa)) { +case PKVM_PAGE_OWNED : +WARN_ON (__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_NOPAGE )); +hyp_poison_page (phys); +break ; +case PKVM_PAGE_SHARED_OWNED : +WARN_ON (__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_SHARED_BORROWED )); +break ; +default : + ret = -EPERM ; + goto unlock; + } + +WARN_ON (kvm_pgtable_stage2_unmap (&vm->pgt, ipa,PAGE_SIZE )); +WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_HOST )); + +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu) {struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu);arch/arm64/kvm/hyp/nvhe/pkvm.c +14
-0@@ -832,6 +832,20 @@ teardown_donated_memory(struct kvm_hyp_memcache *mc, void *addr, size_t size)unmap_donated_memory_noclear (addr, size); } +int __pkvm_reclaim_dying_guest_page (pkvm_handle_t handle,u64 gfn) +{ +struct pkvm_hyp_vm *hyp_vm; +int ret = -EINVAL ; + +hyp_spin_lock (&vm_table_lock); + hyp_vm =get_vm_by_handle (handle); +if (hyp_vm && hyp_vm->kvm.arch.pkvm.is_dying) + ret =__pkvm_host_reclaim_page_guest (gfn, hyp_vm); +hyp_spin_unlock (&vm_table_lock); + +return ret; +} +int __pkvm_start_teardown_vm (pkvm_handle_t handle) {struct pkvm_hyp_vm *hyp_vm;Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 14/30 KVM: arm64: Hook up reclaim hypercall to pkvm_pgtable_stage2_destroy()
In reply to Will DeaconDuring teardown of a protected guest, its memory pages must be reclaimed from the hypervisor by issuing the '__pkvm_reclaim_dying_guest_page' hypercall.
Add a new helper, __pkvm_pgtable_stage2_reclaim(), which is called during the VM teardown operation to reclaim pages from the hypervisor and drop the GUP pin on the host.
--- arch/arm64/kvm/pkvm.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
arch/arm64/kvm/pkvm.c +30 -1
@@ -322,6 +322,32 @@ int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,return 0 ; } +static int __pkvm_pgtable_stage2_reclaim (struct kvm_pgtable *pgt,u64 start,u64 end) +{ +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); +pkvm_handle_t handle = kvm->arch.pkvm.handle; +struct pkvm_mapping *mapping; +int ret; + +for_each_mapping_in_range_safe (pgt, start, end, mapping) { +struct page *page; + + ret =kvm_call_hyp_nvhe (__pkvm_reclaim_dying_guest_page, + handle, mapping->gfn); +if (WARN_ON (ret)) +return ret; + + page =pfn_to_page (mapping->pfn); +WARN_ON_ONCE (mapping->nr_pages !=1 ); +unpin_user_pages_dirty_lock (&page,1 , true); +account_locked_vm (current->mm,1 , false); +pkvm_mapping_remove (mapping, &pgt->pkvm_mappings); +kfree (mapping); + } + +return 0 ; +} +static int __pkvm_pgtable_stage2_unshare (struct kvm_pgtable *pgt,u64 start,u64 end) {struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu);
@@ -355,7 +381,10 @@ void pkvm_pgtable_stage2_destroy_range(struct kvm_pgtable *pgt, kvm->arch.pkvm.is_dying = true; } -__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); +if (kvm_vm_is_protected (kvm)) +__pkvm_pgtable_stage2_reclaim (pgt, addr, addr + size); +else +__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }void pkvm_pgtable_stage2_destroy_pgd (struct kvm_pgtable *pgt)
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 14/30] KVM: arm64: Hook up reclaim hypercall to pkvm_pgtable_stage2_destroy()
Quoted 32 lines · Will Deacon
During teardown of a protected guest, its memory pages must be reclaimed from the hypervisor by issuing the '
__pkvm_reclaim_dying_guest_page' hypercall.Add a new helper,
__pkvm_pgtable_stage2_reclaim(), which is called during the VM teardown operation to reclaim pages from the hypervisor and drop the GUP pin on the host.Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/pkvm.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)arch/arm64/kvm/pkvm.c +11
-0@@ -322,6 +322,32 @@ int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,return 0 ; } +static int __pkvm_pgtable_stage2_reclaim (struct kvm_pgtable *pgt,u64 start,u64 end) +{ +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); +pkvm_handle_t handle = kvm->arch.pkvm.handle; +struct pkvm_mapping *mapping; +int ret; + +for_each_mapping_in_range_safe (pgt, start, end, mapping) { +struct page *page; + + ret = kvm_call_hyp_nvhe(__pkvm_reclaim_dying_guest_page,
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu, · line 345
Quoted 10 lines
arch/arm64/kvm/pkvm.c +10
-0+ handle, mapping->gfn); +if (WARN_ON (ret)) +return ret; + + page =pfn_to_page (mapping->pfn); +WARN_ON_ONCE (mapping->nr_pages !=1 ); +unpin_user_pages_dirty_lock (&page,1 , true); +account_locked_vm (current->mm,1 , false); +pkvm_mapping_remove (mapping, &pgt->pkvm_mappings); +kfree (mapping);
Nit: this might take a while, worth adding a cond_resched() in here?
Quoted 23 lines
arch/arm64/kvm/pkvm.c +9
-1+ } + +return 0 ; +} +static int __pkvm_pgtable_stage2_unshare (struct kvm_pgtable *pgt,u64 start,u64 end) {struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu);@@ -355,7 +381,10 @@ void pkvm_pgtable_stage2_destroy_range(struct kvm_pgtable *pgt, kvm->arch.pkvm.is_dying = true; } -__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); +if (kvm_vm_is_protected (kvm)) +__pkvm_pgtable_stage2_reclaim (pgt, addr, addr + size); +else +__pkvm_pgtable_stage2_unshare (pgt, addr, addr + size); }void pkvm_pgtable_stage2_destroy_pgd (struct kvm_pgtable *pgt)Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 14/30] KVM: arm64: Hook up reclaim hypercall to pkvm_pgtable_stage2_destroy()
Quoted 37 lines · Quentin Perret
Quoted 36 lines · Will Deacon
During teardown of a protected guest, its memory pages must be reclaimed from the hypervisor by issuing the '
__pkvm_reclaim_dying_guest_page' hypercall.Add a new helper,
__pkvm_pgtable_stage2_reclaim(), which is called during the VM teardown operation to reclaim pages from the hypervisor and drop the GUP pin on the host.Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/pkvm.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)arch/arm64/kvm/pkvm.c +15
-0@@ -322,6 +322,32 @@ int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,return 0 ; } +static int __pkvm_pgtable_stage2_reclaim (struct kvm_pgtable *pgt,u64 start,u64 end) +{ +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); +pkvm_handle_t handle = kvm->arch.pkvm.handle; +struct pkvm_mapping *mapping; +int ret; + +for_each_mapping_in_range_safe (pgt, start, end, mapping) { +struct page *page; + + ret =kvm_call_hyp_nvhe (__pkvm_reclaim_dying_guest_page, + handle, mapping->gfn); +if (WARN_ON (ret)) +return ret; +
Quoted 8 lines
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu, · line 345
Quoted 6 lines
arch/arm64/kvm/pkvm.c +6
-0+ page =pfn_to_page (mapping->pfn); +WARN_ON_ONCE (mapping->nr_pages !=1 ); +unpin_user_pages_dirty_lock (&page,1 , true); +account_locked_vm (current->mm,1 , false); +pkvm_mapping_remove (mapping, &pgt->pkvm_mappings); +kfree (mapping);Nit: this might take a while, worth adding a
cond_resched()in here?
Commit 4ddfab5436b6 ("KVM: arm64: Reschedule as needed when destroying the stage-2 page-tables") breaks the destruction operation up into chunks so we now reschedule from the caller in stage2_destroy_range().
Do you think we need an additional reschedule?
Will
Re: [PATCH 14/30] KVM: arm64: Hook up reclaim hypercall to pkvm_pgtable_stage2_destroy()
Quoted 47 lines · Will Deacon
Quoted 37 lines · Quentin Perret
Quoted 36 lines · Will Deacon
During teardown of a protected guest, its memory pages must be reclaimed from the hypervisor by issuing the '
__pkvm_reclaim_dying_guest_page' hypercall.Add a new helper,
__pkvm_pgtable_stage2_reclaim(), which is called during the VM teardown operation to reclaim pages from the hypervisor and drop the GUP pin on the host.Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/pkvm.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)arch/arm64/kvm/pkvm.c +15
-0@@ -322,6 +322,32 @@ int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,return 0 ; } +static int __pkvm_pgtable_stage2_reclaim (struct kvm_pgtable *pgt,u64 start,u64 end) +{ +struct kvm *kvm =kvm_s2_mmu_to_kvm (pgt->mmu); +pkvm_handle_t handle = kvm->arch.pkvm.handle; +struct pkvm_mapping *mapping; +int ret; + +for_each_mapping_in_range_safe (pgt, start, end, mapping) { +struct page *page; + + ret =kvm_call_hyp_nvhe (__pkvm_reclaim_dying_guest_page, + handle, mapping->gfn); +if (WARN_ON (ret)) +return ret; +Quoted 8 lines
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu, · line 345
Quoted 6 lines
arch/arm64/kvm/pkvm.c +6
-0+ page =pfn_to_page (mapping->pfn); +WARN_ON_ONCE (mapping->nr_pages !=1 ); +unpin_user_pages_dirty_lock (&page,1 , true); +account_locked_vm (current->mm,1 , false); +pkvm_mapping_remove (mapping, &pgt->pkvm_mappings); +kfree (mapping);Nit: this might take a while, worth adding a
cond_resched()in here?
Quoted 5 lines
Commit
4ddfab5436b6("KVM: arm64: Reschedule as needed when destroying the stage-2 page-tables") breaks the destruction operation up into chunks so we now reschedule from the caller instage2_destroy_range().Do you think we need an additional reschedule?
I would intuitively think so, but I've also just realized we already don't have that resched for np guests, so this patch is absolutely fine as-is.
It's just that knocking down a whole 1G (assuming 4K pages) will be significantly faster for normal KVM than for pKVM. It's one call in pgtable.c with possibly a single deferred TLBI vs 1 hcall and 1 TLBI per 4K page. But I guess this is really performance tuning, so this can always come later. Feel free to ignore :)
Will Deacon
maintainerPATCH 15/30 KVM: arm64: Refactor enter_exception64()
In reply to Will DeaconFrom: Quentin Perret <[email protected]>
In order to simplify the injection of exceptions in the host in pkvm context, refactor enter_exception64() to separate the code calculating the exception offset from VBAR_EL1 and the cpsr.
No functional change intended.
--- arch/arm64/include/asm/kvm_emulate.h | 5 ++ arch/arm64/kvm/hyp/exception.c | 100 ++++++++++++++++----------- 2 files changed, 63 insertions(+), 42 deletions(-)
arch/arm64/include/asm/kvm_emulate.h +5 -0
@@ -71,6 +71,11 @@ static inline int kvm_inject_serror(struct kvm_vcpu *vcpu)return kvm_inject_serror_esr (vcpu, ESR_ELx_ISV); } +unsigned long get_except64_offset (unsigned long psr,unsigned long target_mode, +enum exception_type type); +unsigned long get_except64_cpsr (unsigned long old,bool has_mte, +unsigned long sctlr,unsigned long mode); +void kvm_vcpu_wfi (struct kvm_vcpu *vcpu);void kvm_emulate_nested_eret (struct kvm_vcpu *vcpu);
arch/arm64/kvm/hyp/exception.c +58 -42
126 lines not on this page · raw message
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 16/30 KVM: arm64: Inject SIGSEGV on illegal accesses
In reply to Will DeaconFrom: Quentin Perret <[email protected]>
The pKVM hypervisor will currently panic if the host tries to access memory that it doesn't own (e.g. protected guest memory). Sadly, as guest memory can still be mapped into the VMM's address space, userspace can trivially crash the kernel/hypervisor by poking into guest memory.
To prevent this, inject the abort back in the host with S1PTW set in the ESR, hence allowing the host to differentiate this abort from normal userspace faults and inject a SIGSEGV cleanly.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 50 ++++++++++++++++++++++++++- arch/arm64/mm/fault.c | 22 ++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +49 -1
@@ -603,6 +603,50 @@ static int host_stage2_idmap(u64 addr)return ret; } +static void host_inject_abort (struct kvm_cpu_context *host_ctxt) +{ +u64 spsr =read_sysreg_el2 (SYS_SPSR ); +u64 esr =read_sysreg_el2 (SYS_ESR ); +u64 ventry, ec; + +/* Repaint the ESR to report a same-level fault if taken from EL1 */ +if ((spsr &PSR_MODE_MASK ) != PSR_MODE_EL0t) { + ec =ESR_ELx_EC (esr); +if (ec == ESR_ELx_EC_DABT_LOW) + ec = ESR_ELx_EC_DABT_CUR; +else if (ec == ESR_ELx_EC_IABT_LOW) + ec = ESR_ELx_EC_IABT_CUR; +else +WARN_ON (1 ); + esr &= ~ESR_ELx_EC_MASK; + esr |= ec << ESR_ELx_EC_SHIFT; + } + +/* +* Since S1PTW should only ever be set for stage-2 faults, we're pretty +* much guaranteed that it won't be set in ESR_EL1 by the hardware. So, +* let's use that bit to allow the host abort handler to differentiate +* this abort from normal userspace faults. +* +* Note: although S1PTW is RES0 at EL1, it is guaranteed by the +* architecture to be backed by flops, so it should be safe to use. +*/ + esr |= ESR_ELx_S1PTW; + +write_sysreg_el1 (esr,SYS_ESR ); +write_sysreg_el1 (spsr,SYS_SPSR ); +write_sysreg_el1 (read_sysreg_el2 (SYS_ELR ),SYS_ELR ); +write_sysreg_el1 (read_sysreg_el2 (SYS_FAR ),SYS_FAR ); + + ventry =read_sysreg_el1 (SYS_VBAR ); + ventry +=get_except64_offset (spsr, PSR_MODE_EL1h, except_type_sync); +write_sysreg_el2 (ventry,SYS_ELR ); + + spsr =get_except64_cpsr (spsr,system_supports_mte (), +read_sysreg_el1 (SYS_SCTLR ), PSR_MODE_EL1h); +write_sysreg_el2 (spsr,SYS_SPSR ); +} +void handle_host_mem_abort (struct kvm_cpu_context *host_ctxt) {struct kvm_vcpu_fault_info fault;
@@ -627,7 +671,11 @@ void handle_host_mem_abort(struct kvm_cpu_context *host_ctxt) addr =FIELD_GET (HPFAR_EL2_FIPA , fault.hpfar_el2) <<12 ; ret =host_stage2_idmap (addr); -BUG_ON (ret && ret != -EAGAIN ); + +if (ret == -EPERM ) +host_inject_abort (host_ctxt); +else +BUG_ON (ret && ret != -EAGAIN ); }struct check_walk_data {
arch/arm64/mm/fault.c +22 -0
@@ -43,6 +43,7 @@#include <asm/system_misc.h> #include <asm/tlbflush.h> #include <asm/traps.h> +#include <asm/virt.h> struct fault_info { int (*fn)(unsigned long far, unsigned long esr,
@@ -269,6 +270,15 @@ static inline bool is_el1_permission_fault(unsigned long addr, unsigned long esrreturn false; } +static bool is_pkvm_stage2_abort (unsigned int esr) +{ +/* +* S1PTW should only ever be set in ESR_EL1 if the pkvm hypervisor +* injected a stage-2 abort -- see host_inject_abort(). +*/ +return is_pkvm_initialized () && (esr & ESR_ELx_S1PTW); +} +static bool __kprobes is_spurious_el1_translation_fault (unsigned long addr,unsigned long esr,struct pt_regs *regs)
@@ -279,6 +289,9 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,if (!is_el1_data_abort (esr) || !esr_fsc_is_translation_fault (esr))return false; +if (is_pkvm_stage2_abort (esr)) +return false; +local_irq_save (flags); asmvolatile ("at s1e1r, %0" ::"r" (addr));isb ();
@@ -395,6 +408,8 @@ static void __do_kernel_fault(unsigned long addr, unsigned long esr, msg ="read from unreadable memory" ; }else if (addr <PAGE_SIZE ) { msg ="NULL pointer dereference" ; + }else if (is_pkvm_stage2_abort (esr)) { + msg ="access to hypervisor-protected memory" ; }else {if (esr_fsc_is_translation_fault (esr) &&kfence_handle_page_fault (addr, esr & ESR_ELx_WNR, regs))
@@ -621,6 +636,13 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr, addr, esr, regs); } +if (is_pkvm_stage2_abort (esr)) { +if (!user_mode (regs)) + goto no_context; +arm64_force_sig_fault (SIGSEGV ,SEGV_ACCERR , far,"stage-2 fault" ); +return 0 ; + } +perf_sw_event (PERF_COUNT_SW_PAGE_FAULTS ,1 , regs, addr);if (!(mm_flags &FAULT_FLAG_USER ))
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 17/30 KVM: arm64: Generalise kvm_pgtable_stage2_set_owner()
In reply to Will Deaconkvm_pgtable_stage2_set_owner() can be generalised into a way to store up to 62 bits in the page tables, as long as we don't set the 'valid' or KVM_INVALID_PTE_LOCKED bits.
Let's do that and reduce the owner field to 4 bits to free up more annotation space for future work.
--- arch/arm64/include/asm/kvm_pgtable.h | 21 +++++++++++++-------- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 14 ++++++++++++-- arch/arm64/kvm/hyp/pgtable.c | 17 ++++++----------- 3 files changed, 31 insertions(+), 21 deletions(-)
arch/arm64/include/asm/kvm_pgtable.h +13 -8
@@ -97,14 +97,17 @@ typedef u64 kvm_pte_t; KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \ KVM_PTE_LEAF_ATTR_HI_S2_XN) -#define KVM_INVALID_PTE_OWNER_MASK GENMASK(9, 2) -#define KVM_MAX_OWNER_ID 3 +/* pKVM invalid pte encodings */ +#define KVM_INVALID_PTE_OWNER_MASK GENMASK(4, 1) +#define KVM_INVALID_PTE_EXTRA_MASK GENMASK(62, 5) +#define KVM_INVALID_PTE_ANNOT_MASK (KVM_INVALID_PTE_OWNER_MASK | \ + KVM_INVALID_PTE_EXTRA_MASK)/* * Used to indicate a pte for which a 'break-before-make' sequence is in * progress. */ -#define KVM_INVALID_PTE_LOCKED BIT(10) +#define KVM_INVALID_PTE_LOCKED BIT(63)static inline bool kvm_pte_valid (kvm_pte_t pte) {
@@ -657,14 +660,16 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, void *mc, enum kvm_pgtable_walk_flags flags);/** -* kvm_pgtable_stage2_set_owner() - Unmap and annotate pages in the IPA space to -* track ownership. +* kvm_pgtable_stage2_annotate() - Unmap and annotate pages in the IPA space +* to track ownership (and more). * @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*(). * @addr: Base intermediate physical address to annotate. * @size: Size of the annotated range. * @mc: Cache of pre-allocated and zeroed memory from which to allocate * page-table pages. -* @owner_id: Unique identifier for the owner of the page. +* @annotation: A 62-bit value that will be stored in the page tables. +* @annotation[0] and @annotation[63] must be 0. +* @annotation[62:1] is stored in the page tables. * * By default, all page-tables are owned by identifier 0. This function can be * used to mark portions of the IPA space as owned by other entities. When a
@@ -673,8 +678,8 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,* * Return: 0 on success, negative error code on failure. */ -int kvm_pgtable_stage2_set_owner (struct kvm_pgtable *pgt,u64 addr,u64 size, -void *mc,u8 owner_id); +int kvm_pgtable_stage2_annotate (struct kvm_pgtable *pgt,u64 addr,u64 size, +void *mc,kvm_pte_t annotation);/** * kvm_pgtable_stage2_unmap() - Remove a mapping from a guest stage-2 page-table.
arch/arm64/kvm/hyp/nvhe/mem_protect.c +12 -2
@@ -539,15 +539,25 @@ static void __host_update_page_state(phys_addr_t addr, u64 size, enum pkvm_page_set_host_state (page, state); } +static kvm_pte_t kvm_init_invalid_leaf_owner (u8 owner_id) +{ +return FIELD_PREP (KVM_INVALID_PTE_OWNER_MASK , owner_id); +} +int host_stage2_set_owner_locked (phys_addr_t addr,u64 size,u8 owner_id) { +kvm_pte_t annotation;int ret; +if (!FIELD_FIT (KVM_INVALID_PTE_OWNER_MASK , owner_id)) +return -EINVAL ; +if (!range_is_memory (addr, addr + size))return -EPERM ; - ret =host_stage2_try (kvm_pgtable_stage2_set_owner, &host_mmu.pgt, - addr, size, &host_s2_pool, owner_id); + annotation =kvm_init_invalid_leaf_owner (owner_id); + ret =host_stage2_try (kvm_pgtable_stage2_annotate, &host_mmu.pgt, + addr, size, &host_s2_pool, annotation);if (ret)return ret;
arch/arm64/kvm/hyp/pgtable.c +6 -11
@@ -114,11 +114,6 @@ static kvm_pte_t kvm_init_valid_leaf_pte(u64 pa, kvm_pte_t attr, s8 level)return pte; } -static kvm_pte_t kvm_init_invalid_leaf_owner (u8 owner_id) -{ -return FIELD_PREP (KVM_INVALID_PTE_OWNER_MASK , owner_id); -} -static int kvm_pgtable_visitor_cb (struct kvm_pgtable_walk_data *data,const struct kvm_pgtable_visit_ctx *ctx,enum kvm_pgtable_walk_flags visit)
@@ -563,7 +558,7 @@ void kvm_pgtable_hyp_destroy(struct kvm_pgtable *pgt)struct stage2_map_data {const u64 phys;kvm_pte_t attr; -u8 owner_id; +kvm_pte_t pte_annot;kvm_pte_t *anchor;kvm_pte_t *childp;
@@ -946,7 +941,7 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,if (!data->annotation) new =kvm_init_valid_leaf_pte (phys, data->attr, ctx->level);else - new =kvm_init_invalid_leaf_owner (data->owner_id); + new = data->pte_annot;/* * Skip updating the PTE if we are trying to recreate the exact
@@ -1100,16 +1095,16 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return ret; } -int kvm_pgtable_stage2_set_owner (struct kvm_pgtable *pgt,u64 addr,u64 size, -void *mc,u8 owner_id) +int kvm_pgtable_stage2_annotate (struct kvm_pgtable *pgt,u64 addr,u64 size, +void *mc,kvm_pte_t pte_annot) {int ret;struct stage2_map_data map_data = { .mmu = pgt->mmu, .memcache = mc, - .owner_id = owner_id, .force_pte = true, .annotation = true, + .pte_annot = pte_annot, };struct kvm_pgtable_walker walker = { .cb = stage2_map_walker,
@@ -1118,7 +1113,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size, .arg = &map_data, }; -if (owner_id >KVM_MAX_OWNER_ID ) +if (pte_annot & ~KVM_INVALID_PTE_ANNOT_MASK )return -EINVAL ; ret =kvm_pgtable_walk (pgt, addr, size, &walker);
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 17/30] KVM: arm64: Generalise kvm_pgtable_stage2_set_owner()
Quoted 18 lines · Will Deacon
/**
- *kvm_pgtable_stage2_set_owner()- Unmap and annotate pages in the IPA space to
- * track ownership.
+ *kvm_pgtable_stage2_annotate()- Unmap and annotate pages in the IPA space
+ * to track ownership (and more).
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Base intermediate physical address to annotate.
* @size: Size of the annotated range.
* @mc: Cache of pre-allocated and zeroed memory from which to allocate
* page-table pages.
- * @owner_id: Unique identifier for the owner of the page.
+ * @annotation: A 62-bit value that will be stored in the page tables.
+ * @annotation[0] and @annotation[63] must be 0.
+ * @annotation[62:1] is stored in the page tables.
*
* By default, all page-tables are owned by identifier 0. This function can be
* used to mark portions of the IPA space as owned by other entities. When a
arch/arm64/include/asm/kvm_pgtable.h int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 682
Quoted 7 lines
arch/arm64/include/asm/kvm_pgtable.h +2
-2@@ -673,8 +678,8 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,* * Return: 0 on success, negative error code on failure. */ -int kvm_pgtable_stage2_set_owner (struct kvm_pgtable *pgt,u64 addr,u64 size, -void *mc,u8 owner_id); +int kvm_pgtable_stage2_annotate (struct kvm_pgtable *pgt,u64 addr,u64 size, +void *mc,kvm_pte_t annotation);
While we're on this topic, perhaps we could go one step further and 'type' the annotation itself? For instance have a 'type' and 'meta' parameter directly at the kvm_pgatble_stage2_annotate() level instead of leaving that up to the callers. This would allow to have one place to allocate annotation 'types' (donated pages, locked PTE, MMIO guard, ...) and one way to serialize/deserialize them. That 'type' would be stored in top 2 or 3 bits of the PTE for instance, and decoding of the 'meta' field would be dependant on the type value. Thoughts?
Thanks,
Quentin
Will Deacon
maintainerRe: [PATCH 17/30] KVM: arm64: Generalise kvm_pgtable_stage2_set_owner()
Quoted 27 lines · Quentin Perret
Quoted 18 lines · Will Deacon
/**
- *kvm_pgtable_stage2_set_owner()- Unmap and annotate pages in the IPA space to
- * track ownership.
+ *kvm_pgtable_stage2_annotate()- Unmap and annotate pages in the IPA space
+ * to track ownership (and more).
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Base intermediate physical address to annotate.
* @size: Size of the annotated range.
* @mc: Cache of pre-allocated and zeroed memory from which to allocate
* page-table pages.
- * @owner_id: Unique identifier for the owner of the page.
+ * @annotation: A 62-bit value that will be stored in the page tables.
+ * @annotation[0] and @annotation[63] must be 0.
+ * @annotation[62:1] is stored in the page tables.
*
* By default, all page-tables are owned by identifier 0. This function can be
* used to mark portions of the IPA space as owned by other entities. When aarch/arm64/include/asm/kvm_pgtable.h int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 682
Quoted 7 lines
arch/arm64/include/asm/kvm_pgtable.h +2
-2@@ -673,8 +678,8 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,* * Return: 0 on success, negative error code on failure. */ -int kvm_pgtable_stage2_set_owner (struct kvm_pgtable *pgt,u64 addr,u64 size, -void *mc,u8 owner_id); +int kvm_pgtable_stage2_annotate (struct kvm_pgtable *pgt,u64 addr,u64 size, +void *mc,kvm_pte_t annotation);
Quoted 8 lines
While we're on this topic, perhaps we could go one step further and 'type' the annotation itself? For instance have a 'type' and 'meta' parameter directly at the
kvm_pgatble_stage2_annotate()level instead of leaving that up to the callers. This would allow to have one place to allocate annotation 'types' (donated pages, locked PTE, MMIO guard, ...) and one way to serialize/deserialize them. That 'type' would be stored in top 2 or 3 bits of the PTE for instance, and decoding of the 'meta' field would be dependant on the type value. Thoughts?
I don't think a global 'type' space is particularly beneficial, as most annotations (with the exception of PTE_LOCKED) are specific to the owner and putting them into a single number space will just waste bits.
But I do like the idea of encoding an annotation type in the pte and defining those per-owner. I think it would also make some of the code more robust; for example, I noticed that __pkvm_guest_unshare_host() isn't putting back the right annotation with my series when I started looking at implementing your idea.
I'll come back with a diff. It won't be quite what you're suggesting, but let's see what you think.
Will
Will Deacon
maintainerRe: [PATCH 17/30] KVM: arm64: Generalise kvm_pgtable_stage2_set_owner()
Quoted 41 lines · Will Deacon
Quoted 27 lines · Quentin Perret
Quoted 18 lines · Will Deacon
/**
- *kvm_pgtable_stage2_set_owner()- Unmap and annotate pages in the IPA space to
- * track ownership.
+ *kvm_pgtable_stage2_annotate()- Unmap and annotate pages in the IPA space
+ * to track ownership (and more).
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Base intermediate physical address to annotate.
* @size: Size of the annotated range.
* @mc: Cache of pre-allocated and zeroed memory from which to allocate
* page-table pages.
- * @owner_id: Unique identifier for the owner of the page.
+ * @annotation: A 62-bit value that will be stored in the page tables.
+ * @annotation[0] and @annotation[63] must be 0.
+ * @annotation[62:1] is stored in the page tables.
*
* By default, all page-tables are owned by identifier 0. This function can be
* used to mark portions of the IPA space as owned by other entities. When aarch/arm64/include/asm/kvm_pgtable.h int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 682
Quoted 7 lines
arch/arm64/include/asm/kvm_pgtable.h +2
-2@@ -673,8 +678,8 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,* * Return: 0 on success, negative error code on failure. */ -int kvm_pgtable_stage2_set_owner (struct kvm_pgtable *pgt,u64 addr,u64 size, -void *mc,u8 owner_id); +int kvm_pgtable_stage2_annotate (struct kvm_pgtable *pgt,u64 addr,u64 size, +void *mc,kvm_pte_t annotation);Quoted 8 lines
While we're on this topic, perhaps we could go one step further and 'type' the annotation itself? For instance have a 'type' and 'meta' parameter directly at the
kvm_pgatble_stage2_annotate()level instead of leaving that up to the callers. This would allow to have one place to allocate annotation 'types' (donated pages, locked PTE, MMIO guard, ...) and one way to serialize/deserialize them. That 'type' would be stored in top 2 or 3 bits of the PTE for instance, and decoding of the 'meta' field would be dependant on the type value. Thoughts?I don't think a global 'type' space is particularly beneficial, as most annotations (with the exception of
PTE_LOCKED) are specific to the owner and putting them into a single number space will just waste bits.
Quoted 8 lines
But I do like the idea of encoding an annotation type in the pte and defining those per-owner. I think it would also make some of the code more robust; for example, I noticed that
__pkvm_guest_unshare_host()isn't putting back the right annotation with my series when I started looking at implementing your idea.I'll come back with a diff. It won't be quite what you're suggesting, but let's see what you think.
Ok, so this took a fair bit longer than I initially thought it would.
In the end, I've ended up with something closer to what you seem to have in mind: kvm_pgtable_stage2_annotate() now takes both a 'type' and an annotation but it's not without complexity:
* We're extremely short on bits when endoding the guest gfn + handle. The gfn can be 40 bits with LPA2 and 4k pages, so with the 16-bit handle and a 4-bit type, that leaves only 3 bits for the owner.
* Non-zero invalid ptes are 'counted', so I've had to adjust the reclaim path to map pages back into the host immediately rather than go via a typed annotation.
Despite that, I think it's a slight improvement for now. We may end up revisiting it if we want to squeeze more bits out of the host annotations, but I'll include it in v2 for you to have a look at.
Will
Will Deacon
maintainerPATCH 18/30 KVM: arm64: Introduce host_stage2_set_owner_metadata_locked()
In reply to Will DeaconRework host_stage2_set_owner_locked() to add a new helper function, host_stage2_set_owner_metadata_locked(), which will allow us to store additional metadata alongside the owner ID for invalid host stage-2 entries.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +12 -7
@@ -539,12 +539,8 @@ static void __host_update_page_state(phys_addr_t addr, u64 size, enum pkvm_page_set_host_state (page, state); } -static kvm_pte_t kvm_init_invalid_leaf_owner (u8 owner_id) -{ -return FIELD_PREP (KVM_INVALID_PTE_OWNER_MASK , owner_id); -} - -int host_stage2_set_owner_locked (phys_addr_t addr,u64 size,u8 owner_id) +static int host_stage2_set_owner_metadata_locked (phys_addr_t addr,u64 size, +u8 owner_id,u64 meta) {kvm_pte_t annotation;int ret;
@@ -552,10 +548,14 @@ int host_stage2_set_owner_locked(phys_addr_t addr, u64 size, u8 owner_id)if (!FIELD_FIT (KVM_INVALID_PTE_OWNER_MASK , owner_id))return -EINVAL ; +if (!FIELD_FIT (KVM_INVALID_PTE_EXTRA_MASK , meta)) +return -EINVAL ; +if (!range_is_memory (addr, addr + size))return -EPERM ; - annotation =kvm_init_invalid_leaf_owner (owner_id); + annotation =FIELD_PREP (KVM_INVALID_PTE_OWNER_MASK , owner_id) | +FIELD_PREP (KVM_INVALID_PTE_EXTRA_MASK , meta); ret =host_stage2_try (kvm_pgtable_stage2_annotate, &host_mmu.pgt, addr, size, &host_s2_pool, annotation);if (ret)
@@ -570,6 +570,11 @@ int host_stage2_set_owner_locked(phys_addr_t addr, u64 size, u8 owner_id)return 0 ; } +int host_stage2_set_owner_locked (phys_addr_t addr,u64 size,u8 owner_id) +{ +return host_stage2_set_owner_metadata_locked (addr, size, owner_id,0 ); +} +static bool host_stage2_force_pte_cb (u64 addr,u64 end,enum kvm_pgtable_prot prot) {/*
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 19/30 KVM: arm64: Annotate guest donations with handle and gfn in host stage-2
In reply to Will DeaconHandling host kernel faults arising from accesses to donated guest memory will require an rmap-like mechanism to identify the guest mapping of the faulting page.
Extend the page donation logic to encode the guest handle and gfn alongside the owner information in the host stage-2 pte.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +17 -1
@@ -1063,6 +1063,19 @@ static void hyp_poison_page(phys_addr_t phys)hyp_fixmap_unmap (); } +#define KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK GENMASK(15, 0) +#define KVM_HOST_INVALID_PTE_GUEST_GFN_MASK GENMASK(56, 16) +static u64 host_stage2_encode_gfn_meta (struct pkvm_hyp_vm *vm,u64 gfn) +{ +pkvm_handle_t handle = vm->kvm.arch.pkvm.handle; + +WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle)); +WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_GFN_MASK , gfn)); + +return FIELD_PREP (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle) | +FIELD_PREP (KVM_HOST_INVALID_PTE_GUEST_GFN_MASK , gfn); +} +int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm) {u64 ipa =hyp_pfn_to_phys (gfn);
@@ -1105,6 +1118,7 @@ int __pkvm_host_donate_guest(u64 pfn, u64 gfn, struct pkvm_hyp_vcpu *vcpu)struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu);u64 phys =hyp_pfn_to_phys (pfn);u64 ipa =hyp_pfn_to_phys (gfn); +u64 meta;int ret;host_lock_component ();
@@ -1118,7 +1132,9 @@ int __pkvm_host_donate_guest(u64 pfn, u64 gfn, struct pkvm_hyp_vcpu *vcpu)if (ret) goto unlock; -WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); + meta =host_stage2_encode_gfn_meta (vm, gfn); +WARN_ON (host_stage2_set_owner_metadata_locked (phys,PAGE_SIZE , +PKVM_ID_GUEST , meta));WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys,pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), &vcpu->vcpu.arch.pkvm_memcache,0 ));
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 19/30] KVM: arm64: Annotate guest donations with handle and gfn in host stage-2
Hi Will,
Quoted 18 lines · Will Deacon
Handling host kernel faults arising from accesses to donated guest memory will require an rmap-like mechanism to identify the guest mapping of the faulting page.
Extend the page donation logic to encode the guest handle and gfn alongside the owner information in the host stage-2 pte.
Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)arch/arm64/kvm/hyp/nvhe/mem_protect.c +0
-0
arch/arm64/kvm/hyp/nvhe/mem_protect.c static void hyp_poison_page(phys_addr_t phys) · line 1072
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +7
-0@@ -1063,6 +1063,19 @@ static void hyp_poison_page(phys_addr_t phys)hyp_fixmap_unmap (); } +#define KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK GENMASK(15, 0) +#define KVM_HOST_INVALID_PTE_GUEST_GFN_MASK GENMASK(56, 16) +static u64 host_stage2_encode_gfn_meta (struct pkvm_hyp_vm *vm,u64 gfn) +{ +pkvm_handle_t handle = vm->kvm.arch.pkvm.handle; + +WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle));
Instead of (or in addition to) this check, should we also have a compile time check to ensure that the handle fits? We have KVM_MAX_PVMS and HANDLE_OFFSET, albeit HANDLE_OFFSET, so we can calculate whether it fits.
Cheers,
/fuad
Quoted 31 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +10
-2+WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_GFN_MASK , gfn)); + +return FIELD_PREP (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle) | +FIELD_PREP (KVM_HOST_INVALID_PTE_GUEST_GFN_MASK , gfn); +} +int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm) {u64 ipa =hyp_pfn_to_phys (gfn);@@ -1105,6 +1118,7 @@ int __pkvm_host_donate_guest(u64 pfn, u64 gfn, struct pkvm_hyp_vcpu *vcpu)struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu);u64 phys =hyp_pfn_to_phys (pfn);u64 ipa =hyp_pfn_to_phys (gfn); +u64 meta;int ret;host_lock_component ();@@ -1118,7 +1132,9 @@ int __pkvm_host_donate_guest(u64 pfn, u64 gfn, struct pkvm_hyp_vcpu *vcpu)if (ret) goto unlock; -WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); + meta =host_stage2_encode_gfn_meta (vm, gfn); +WARN_ON (host_stage2_set_owner_metadata_locked (phys,PAGE_SIZE , +PKVM_ID_GUEST , meta));WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys,pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), &vcpu->vcpu.arch.pkvm_memcache,0 )); --2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 19/30] KVM: arm64: Annotate guest donations with handle and gfn in host stage-2
Quoted 22 lines · Fuad Tabba
Quoted 21 lines · Will Deacon
Handling host kernel faults arising from accesses to donated guest memory will require an rmap-like mechanism to identify the guest mapping of the faulting page.
Extend the page donation logic to encode the guest handle and gfn alongside the owner information in the host stage-2 pte.
Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)arch/arm64/kvm/hyp/nvhe/mem_protect.c +0
-0@@ -1063,6 +1063,19 @@ static void hyp_poison_page(phys_addr_t phys)hyp_fixmap_unmap (); }
Quoted 12 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c static void hyp_poison_page(phys_addr_t phys) · line 1072
Quoted 7 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +7
-0+#define KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK GENMASK(15, 0) +#define KVM_HOST_INVALID_PTE_GUEST_GFN_MASK GENMASK(56, 16) +static u64 host_stage2_encode_gfn_meta (struct pkvm_hyp_vm *vm,u64 gfn) +{ +pkvm_handle_t handle = vm->kvm.arch.pkvm.handle; + +WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle));Instead of (or in addition to) this check, should we also have a compile time check to ensure that the handle fits? We have
KVM_MAX_PVMSandHANDLE_OFFSET, albeitHANDLE_OFFSET, so we can calculate whether it fits.
The problem there is that HANDLE_OFFSET is hidden inside of pkvm.c and I really didn't want to expose that in a header as we might as well just remove it at that point and allocate from 0.
Maybe we could make pkvm_handle_t a u16 instead and I could assert that at compile type?
Will
Re: [PATCH 19/30] KVM: arm64: Annotate guest donations with handle and gfn in host stage-2
Quoted 37 lines · Will Deacon
Quoted 22 lines · Fuad Tabba
Quoted 21 lines · Will Deacon
Handling host kernel faults arising from accesses to donated guest memory will require an rmap-like mechanism to identify the guest mapping of the faulting page.
Extend the page donation logic to encode the guest handle and gfn alongside the owner information in the host stage-2 pte.
Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)arch/arm64/kvm/hyp/nvhe/mem_protect.c +0
-0@@ -1063,6 +1063,19 @@ static void hyp_poison_page(phys_addr_t phys)hyp_fixmap_unmap (); }Quoted 12 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c static void hyp_poison_page(phys_addr_t phys) · line 1072
Quoted 7 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +7
-0+#define KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK GENMASK(15, 0) +#define KVM_HOST_INVALID_PTE_GUEST_GFN_MASK GENMASK(56, 16) +static u64 host_stage2_encode_gfn_meta (struct pkvm_hyp_vm *vm,u64 gfn) +{ +pkvm_handle_t handle = vm->kvm.arch.pkvm.handle; + +WARN_ON (!FIELD_FIT (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , handle));Instead of (or in addition to) this check, should we also have a compile time check to ensure that the handle fits? We have
KVM_MAX_PVMSandHANDLE_OFFSET, albeitHANDLE_OFFSET, so we can calculate whether it fits.
Quoted 6 lines
The problem there is that
HANDLE_OFFSETis hidden inside ofpkvm.cand I really didn't want to expose that in a header as we might as well just remove it at that point and allocate from 0.Maybe we could make
pkvm_handle_ta u16 instead and I could assert that at compile type?
That works.
Cheers,
/fuad
Quoted 1 line
Will
Will Deacon
maintainerPATCH 20/30 KVM: arm64: Introduce hypercall to force reclaim of a protected page
In reply to Will DeaconIntroduce a new hypercall, __pkvm_force_reclaim_guest_page(), to allow the host to forcefully reclaim a physical page that was previous donated to a protected guest. This results in the page being zeroed and the previous guest mapping being poisoned so that new pages cannot be subsequently donated at the same IPA.
--- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/include/nvhe/memory.h | 6 + arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 1 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 118 +++++++++++++++++- arch/arm64/kvm/hyp/nvhe/pkvm.c | 4 +- 7 files changed, 137 insertions(+), 2 deletions(-)
arch/arm64/include/asm/kvm_asm.h +1 -0
@@ -86,6 +86,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu, + __KVM_HOST_SMCCC_FUNC___pkvm_force_reclaim_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_reclaim_dying_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_start_teardown_vm, __KVM_HOST_SMCCC_FUNC___pkvm_finalize_teardown_vm,
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1 -0
@@ -41,6 +41,7 @@ int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages);int __pkvm_host_share_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_unshare_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu); +int __pkvm_host_force_reclaim_page_guest (phys_addr_t phys);int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm);int __pkvm_host_share_guest (u64 pfn,u64 gfn,u64 nr_pages,struct pkvm_hyp_vcpu *vcpu,enum kvm_pgtable_prot prot);
arch/arm64/kvm/hyp/include/nvhe/memory.h +6 -0
@@ -30,6 +30,12 @@ enum pkvm_page_state {* struct hyp_page. */ PKVM_NOPAGE =BIT (0 ) |BIT (1 ), + +/* +* 'Meta-states' which aren't encoded directly in the PTE's SW bits (or +* the hyp_vmemmap entry for the host) +*/ + PKVM_POISON = BIT(2), };#define PKVM_PAGE_STATE_MASK (BIT(0) | BIT(1))
arch/arm64/kvm/hyp/include/nvhe/pkvm.h +1 -0
@@ -78,6 +78,7 @@ int __pkvm_reclaim_dying_guest_page(pkvm_handle_t handle, u64 gfn);int __pkvm_start_teardown_vm (pkvm_handle_t handle);int __pkvm_finalize_teardown_vm (pkvm_handle_t handle); +struct pkvm_hyp_vm *get_vm_by_handle (pkvm_handle_t handle);struct pkvm_hyp_vcpu *pkvm_load_hyp_vcpu (pkvm_handle_t handle,unsigned int vcpu_idx);void pkvm_put_hyp_vcpu (struct pkvm_hyp_vcpu *hyp_vcpu);
arch/arm64/kvm/hyp/nvhe/hyp-main.c +8 -0
@@ -570,6 +570,13 @@ static void handle___pkvm_init_vcpu(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_init_vcpu (handle, host_vcpu, vcpu_hva); } +static void handle___pkvm_force_reclaim_guest_page (struct kvm_cpu_context *host_ctxt) +{ +DECLARE_REG (phys_addr_t, phys, host_ctxt,1 ); + +cpu_reg (host_ctxt,1 ) =__pkvm_host_force_reclaim_page_guest (phys); +} +static void handle___pkvm_reclaim_dying_guest_page (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (pkvm_handle_t, handle, host_ctxt,1 );
@@ -630,6 +637,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_unreserve_vm),HANDLE_FUNC (__pkvm_init_vm),HANDLE_FUNC (__pkvm_init_vcpu), +HANDLE_FUNC (__pkvm_force_reclaim_guest_page),HANDLE_FUNC (__pkvm_reclaim_dying_guest_page),HANDLE_FUNC (__pkvm_start_teardown_vm), HANDLE_FUNC(__pkvm_finalize_teardown_vm),
arch/arm64/kvm/hyp/nvhe/mem_protect.c +117 -1
144 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/pkvm.c +3 -1
@@ -230,10 +230,12 @@ void pkvm_hyp_vm_table_init(void *tbl)/* * Return the hyp vm structure corresponding to the handle. */ -static struct pkvm_hyp_vm *get_vm_by_handle (pkvm_handle_t handle) +struct pkvm_hyp_vm *get_vm_by_handle (pkvm_handle_t handle) {unsigned int idx =vm_handle_to_idx (handle); +hyp_assert_lock_held (&vm_table_lock); +if (unlikely (idx >=KVM_MAX_PVMS ))return NULL ;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 20/30] KVM: arm64: Introduce hypercall to force reclaim of a protected page
arch/arm64/kvm/hyp/nvhe/mem_protect.c static u64 host_stage2_encode_gfn_meta(struct pkvm_hyp_vm *vm, u64 gfn) · line 1097
Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +8
-0+static int host_stage2_decode_gfn_meta (kvm_pte_t pte,struct pkvm_hyp_vm **vm, +u64 *gfn) +{ +pkvm_handle_t handle; +u64 meta; + +if (kvm_pte_valid (pte)) +return -EINVAL ;
Nit:
I can't think of any cases where we'd end up returning -EINVAL here that isn't indicative of a major problem (e.g taking a stage-2 perm fault we don't expect) given that we've extensively checked the state of the page already. Upgrade to WARN()? It's fatal, but the system is unlikely to make much more progress if we return cleanly anyways, so we might as well make it obvious where things went wrong.
Quoted 14 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +14
-0+if (FIELD_GET (KVM_INVALID_PTE_OWNER_MASK , pte) !=PKVM_ID_GUEST ) +return -EPERM ; + + meta =FIELD_GET (KVM_INVALID_PTE_EXTRA_MASK , pte); + handle =FIELD_GET (KVM_HOST_INVALID_PTE_GUEST_HANDLE_MASK , meta); + *vm =get_vm_by_handle (handle); +if (!*vm) { +/* We probably raced with teardown; try again */ +return -EAGAIN ; + } + + *gfn =FIELD_GET (KVM_HOST_INVALID_PTE_GUEST_GFN_MASK , meta); +return 0 ; +}
Will Deacon
maintainerRe: [PATCH 20/30] KVM: arm64: Introduce hypercall to force reclaim of a protected page
Quoted 11 lines · Quentin Perret
arch/arm64/kvm/hyp/nvhe/mem_protect.c static u64 host_stage2_encode_gfn_meta(struct pkvm_hyp_vm *vm, u64 gfn) · line 1097
Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +8
-0+static int host_stage2_decode_gfn_meta (kvm_pte_t pte,struct pkvm_hyp_vm **vm, +u64 *gfn) +{ +pkvm_handle_t handle; +u64 meta; + +if (kvm_pte_valid (pte)) +return -EINVAL ;Nit:
Quoted 6 lines
I can't think of any cases where we'd end up returning
-EINVALhere that isn't indicative of a major problem (e.g taking a stage-2 perm fault we don't expect) given that we've extensively checked the state of the page already. Upgrade toWARN()? It's fatal, but the system is unlikely to make much more progress if we return cleanly anyways, so we might as well make it obvious where things went wrong.
Good spot. We already confirmed the NOPAGE state, so a WARN() if the pte is valid makes perfect sense to me.
Will
Will Deacon
maintainerPATCH 21/30 KVM: arm64: Reclaim faulting page from pKVM in spurious fault handler
In reply to Will DeaconHost kernel accesses to pages that are inaccessible at stage-2 result in the injection of a translation fault, which is fatal unless an exception table fixup is registered for the faulting PC (e.g. for user access routines). This is undesirable, since a get_user_pages() call could be used to obtain a reference to a donated page and then a subsequent access via a kernel mapping would lead to a panic().
Rework the spurious fault handler so that stage-2 faults injected back into the host result in the target page being forcefully reclaimed when no exception table fixup handler is registered.
--- arch/arm64/include/asm/virt.h | 6 ++++++ arch/arm64/kvm/pkvm.c | 7 +++++++ arch/arm64/mm/fault.c | 15 +++++++++------ 3 files changed, 22 insertions(+), 6 deletions(-)
arch/arm64/include/asm/virt.h +6 -0
@@ -94,6 +94,12 @@ static inline bool is_pkvm_initialized(void)static_branch_likely (&kvm_protected_mode_initialized); } +#ifdef CONFIG_KVM +bool pkvm_reclaim_guest_page (phys_addr_t phys); +#else +static inline bool pkvm_reclaim_guest_page (phys_addr_t phys) {return false; } +#endif +/* Reports the availability of HYP mode */ static inline bool is_hyp_mode_available (void ) {
arch/arm64/kvm/pkvm.c +7 -0
@@ -563,3 +563,10 @@ int pkvm_pgtable_stage2_split(struct kvm_pgtable *pgt, u64 addr, u64 size,WARN_ON_ONCE (1 );return -EINVAL ; } + +bool pkvm_reclaim_guest_page (phys_addr_t phys) +{ +int ret =kvm_call_hyp_nvhe (__pkvm_force_reclaim_guest_page, phys); + +return !ret || ret == -EAGAIN ; +}
arch/arm64/mm/fault.c +9 -6
@@ -289,9 +289,6 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,if (!is_el1_data_abort (esr) || !esr_fsc_is_translation_fault (esr))return false; -if (is_pkvm_stage2_abort (esr)) -return false; -local_irq_save (flags); asmvolatile ("at s1e1r, %0" ::"r" (addr));isb ();
@@ -302,8 +299,12 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,* If we now have a valid translation, treat the translation fault as * spurious. */ -if (!(par &SYS_PAR_EL1_F )) +if (!(par &SYS_PAR_EL1_F )) { +if (is_pkvm_stage2_abort (esr)) +return pkvm_reclaim_guest_page (par &SYS_PAR_EL1_PA ); +return true; + }/* * If we got a different type of fault from the AT instruction,
@@ -389,9 +390,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned long esr,if (!is_el1_instruction_abort (esr) &&fixup_exception (regs, esr))return ; -if (WARN_RATELIMIT (is_spurious_el1_translation_fault (addr, esr, regs), -"Ignoring spurious kernel translation fault at virtual address %016lx\n" , addr)) +if (is_spurious_el1_translation_fault (addr, esr, regs)) { +WARN_RATELIMIT (!is_pkvm_stage2_abort (esr), +"Ignoring spurious kernel translation fault at virtual address %016lx\n" , addr);return ; + }if (is_el1_mte_sync_tag_check_fault (esr)) {do_tag_recovery (addr, esr, regs);
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 22/30 KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
In reply to Will DeaconIf a protected vCPU faults on an IPA which appears to be mapped, query the hypervisor to determine whether or not the faulting pte has been poisoned by a forceful reclaim. If the pte has been poisoned, return -EFAULT back to userspace rather than retrying the instruction forever.
--- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 +++++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 37 +++++++++++++++++++ arch/arm64/kvm/pkvm.c | 9 +++-- 5 files changed, 55 insertions(+), 3 deletions(-)
arch/arm64/include/asm/kvm_asm.h +1 -0
@@ -86,6 +86,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vm, __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu, + __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_in_poison_fault, __KVM_HOST_SMCCC_FUNC___pkvm_force_reclaim_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_reclaim_dying_guest_page, __KVM_HOST_SMCCC_FUNC___pkvm_start_teardown_vm,
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1 -0
@@ -41,6 +41,7 @@ int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages);int __pkvm_host_share_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_unshare_ffa (u64 pfn,u64 nr_pages);int __pkvm_host_donate_guest (u64 pfn,u64 gfn,struct pkvm_hyp_vcpu *vcpu); +int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu);int __pkvm_host_force_reclaim_page_guest (phys_addr_t phys);int __pkvm_host_reclaim_page_guest (u64 gfn,struct pkvm_hyp_vm *vm); int __pkvm_host_share_guest(u64 pfn, u64 gfn, u64 nr_pages, struct pkvm_hyp_vcpu *vcpu,
arch/arm64/kvm/hyp/nvhe/hyp-main.c +10 -0
@@ -570,6 +570,15 @@ static void handle___pkvm_init_vcpu(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_init_vcpu (handle, host_vcpu, vcpu_hva); } +static void handle___pkvm_vcpu_in_poison_fault (struct kvm_cpu_context *host_ctxt) +{ +int ret; +struct pkvm_hyp_vcpu *hyp_vcpu =pkvm_get_loaded_hyp_vcpu (); + + ret = hyp_vcpu ?__pkvm_vcpu_in_poison_fault (hyp_vcpu) : -EINVAL ; +cpu_reg (host_ctxt,1 ) = ret; +} +static void handle___pkvm_force_reclaim_guest_page (struct kvm_cpu_context *host_ctxt) {DECLARE_REG (phys_addr_t, phys, host_ctxt,1 );
@@ -637,6 +646,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_unreserve_vm),HANDLE_FUNC (__pkvm_init_vm),HANDLE_FUNC (__pkvm_init_vcpu), +HANDLE_FUNC (__pkvm_vcpu_in_poison_fault),HANDLE_FUNC (__pkvm_force_reclaim_guest_page),HANDLE_FUNC (__pkvm_reclaim_dying_guest_page), HANDLE_FUNC(__pkvm_start_teardown_vm),
arch/arm64/kvm/hyp/nvhe/mem_protect.c +37 -0
@@ -825,6 +825,43 @@ static int get_valid_guest_pte(struct pkvm_hyp_vm *vm, u64 ipa, kvm_pte_t *ptep,return 0 ; } +int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (hyp_vcpu); +kvm_pte_t pte; +s8 level; +u64 ipa; +int ret; + +switch (kvm_vcpu_trap_get_class (&hyp_vcpu->vcpu)) { +case ESR_ELx_EC_DABT_LOW: +case ESR_ELx_EC_IABT_LOW: +if (kvm_vcpu_trap_is_translation_fault (&hyp_vcpu->vcpu)) +break ; + fallthrough; +default : +return -EINVAL ; + } + + ipa =kvm_vcpu_get_fault_ipa (&hyp_vcpu->vcpu); + ipa |=kvm_vcpu_get_hfar (&hyp_vcpu->vcpu) &GENMASK (11 ,0 ); + +guest_lock_component (vm); + ret =kvm_pgtable_get_leaf (&vm->pgt, ipa, &pte, &level); +if (ret) + goto unlock; + +if (level !=KVM_PGTABLE_LAST_LEVEL ) { + ret = -EINVAL ; + goto unlock; + } + + ret =guest_pte_is_poisoned (pte); +unlock: +guest_unlock_component (vm); +return ret; +} +int __pkvm_host_share_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);
arch/arm64/kvm/pkvm.c +6 -3
@@ -417,10 +417,13 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EINVAL ;/* -* We raced with another vCPU. +* We either raced with another vCPU or the guest PTE +* has been poisoned by an erroneous host access. */ -if (mapping) -return -EAGAIN ; +if (mapping) { + ret =kvm_call_hyp_nvhe (__pkvm_vcpu_in_poison_fault); +return ret ? -EFAULT : -EAGAIN ; + } ret =kvm_call_hyp_nvhe (__pkvm_host_donate_guest, pfn, gfn); }else {
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 22/30] KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
Quoted 10 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/mem_protect.c +10
-0+int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (hyp_vcpu); +kvm_pte_t pte; +s8 level; +u64 ipa; +int ret; + +switch (kvm_vcpu_trap_get_class (&hyp_vcpu->vcpu)) { +case ESR_ELx_EC_DABT_LOW:
arch/arm64/kvm/hyp/nvhe/mem_protect.c static int get_valid_guest_pte(struct pkvm_hyp_vm *vm, u64 ipa, kvm_pte_t *ptep, · line 847
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +10
-0+case ESR_ELx_EC_IABT_LOW: +if (kvm_vcpu_trap_is_translation_fault (&hyp_vcpu->vcpu)) +break ; + fallthrough; +default : +return -EINVAL ; + } + + ipa =kvm_vcpu_get_fault_ipa (&hyp_vcpu->vcpu); + ipa |=kvm_vcpu_get_hfar (&hyp_vcpu->vcpu) &GENMASK (11 ,0 );
Why is all the above needed? Could we simplify by having the host pass the IPA to the hcall?
Quoted 25 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +16
-0+guest_lock_component (vm); + ret =kvm_pgtable_get_leaf (&vm->pgt, ipa, &pte, &level); +if (ret) + goto unlock; + +if (level !=KVM_PGTABLE_LAST_LEVEL ) { + ret = -EINVAL ; + goto unlock; + } + + ret =guest_pte_is_poisoned (pte); +unlock: +guest_unlock_component (vm); +return ret; +} +int __pkvm_host_share_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);arch/arm64/kvm/pkvm.c +0
-0@@ -417,10 +417,13 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EINVAL ;
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 424
Quoted 10 lines
arch/arm64/kvm/pkvm.c +4
-3/* -* We raced with another vCPU. +* We either raced with another vCPU or the guest PTE +* has been poisoned by an erroneous host access. */ -if (mapping) -return -EAGAIN ; +if (mapping) { + ret =kvm_call_hyp_nvhe (__pkvm_vcpu_in_poison_fault);
It's not too bad, but it's a shame we now issue that every time we have such a race (which is frequent-ish). Could we perhaps only issue it if at least one page has been forcefully reclaimed since boot?
Quoted 8 lines
arch/arm64/kvm/pkvm.c +2
-0+return ret ? -EFAULT : -EAGAIN ; + } ret =kvm_call_hyp_nvhe (__pkvm_host_donate_guest, pfn, gfn); }else {Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 22/30] KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
Quoted 16 lines · Quentin Perret
Quoted 15 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/mem_protect.c +15
-0+int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (hyp_vcpu); +kvm_pte_t pte; +s8 level; +u64 ipa; +int ret; + +switch (kvm_vcpu_trap_get_class (&hyp_vcpu->vcpu)) { +case ESR_ELx_EC_DABT_LOW: +case ESR_ELx_EC_IABT_LOW: +if (kvm_vcpu_trap_is_translation_fault (&hyp_vcpu->vcpu)) +break ; + fallthrough; +default :
Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c static int get_valid_guest_pte(struct pkvm_hyp_vm *vm, u64 ipa, kvm_pte_t *ptep, · line 847
Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +5
-0+return -EINVAL ; + } + + ipa =kvm_vcpu_get_fault_ipa (&hyp_vcpu->vcpu); + ipa |=kvm_vcpu_get_hfar (&hyp_vcpu->vcpu) &GENMASK (11 ,0 );Why is all the above needed? Could we simplify by having the host pass the IPA to the hcall?
I was just a little nervous about exposing an oracle here if we take the gfn as an argument as it would provide the host with a pretty easy mechanism to monitor the page access pattern of a guest after the initial donation had occurred.
Quoted 26 lines
Quoted 26 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +16
-0+guest_lock_component (vm); + ret =kvm_pgtable_get_leaf (&vm->pgt, ipa, &pte, &level); +if (ret) + goto unlock; + +if (level !=KVM_PGTABLE_LAST_LEVEL ) { + ret = -EINVAL ; + goto unlock; + } + + ret =guest_pte_is_poisoned (pte); +unlock: +guest_unlock_component (vm); +return ret; +} +int __pkvm_host_share_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);arch/arm64/kvm/pkvm.c +0
-0@@ -417,10 +417,13 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EINVAL ;
Quoted 13 lines
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 424
Quoted 9 lines
arch/arm64/kvm/pkvm.c +4
-3/* -* We raced with another vCPU. +* We either raced with another vCPU or the guest PTE +* has been poisoned by an erroneous host access. */ -if (mapping) -return -EAGAIN ; +if (mapping) { + ret =kvm_call_hyp_nvhe (__pkvm_vcpu_in_poison_fault);It's not too bad, but it's a shame we now issue that every time we have such a race (which is frequent-ish). Could we perhaps only issue it if at least one page has been forcefully reclaimed since boot?
On the plus side, it avoids an unconditional walk from the fault path at EL2 (which is what we have in Android!).
It's a bit fiddly to implement your idea in the host, since the forceful reclaim happens in a really terrible context but I could track it at EL2 and make __pkvm_vcpu_in_poison_fault() return early instead? It's also worth bearing in mind that we've already serialised the concurrent fault and done a GUP by this point, so performance is somewhat of a lost cause...
WDYT?
Will
Re: [PATCH 22/30] KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
Quoted 26 lines · Will Deacon
Quoted 16 lines · Quentin Perret
Quoted 15 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/mem_protect.c +15
-0+int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (hyp_vcpu); +kvm_pte_t pte; +s8 level; +u64 ipa; +int ret; + +switch (kvm_vcpu_trap_get_class (&hyp_vcpu->vcpu)) { +case ESR_ELx_EC_DABT_LOW: +case ESR_ELx_EC_IABT_LOW: +if (kvm_vcpu_trap_is_translation_fault (&hyp_vcpu->vcpu)) +break ; + fallthrough; +default :Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c static int get_valid_guest_pte(struct pkvm_hyp_vm *vm, u64 ipa, kvm_pte_t *ptep, · line 847
Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +5
-0+return -EINVAL ; + } + + ipa =kvm_vcpu_get_fault_ipa (&hyp_vcpu->vcpu); + ipa |=kvm_vcpu_get_hfar (&hyp_vcpu->vcpu) &GENMASK (11 ,0 );Why is all the above needed? Could we simplify by having the host pass the IPA to the hcall?
Quoted 4 lines
I was just a little nervous about exposing an oracle here if we take the gfn as an argument as it would provide the host with a pretty easy mechanism to monitor the page access pattern of a guest after the initial donation had occurred.
Aha, I see what you mean. I guess if we scope that hcall to only discover if a gfn is poisoned we're not exposing too much, but contextualizing the call to the fault also sounds good to me. Perhaps a small comment would help?
Quoted 40 lines
Quoted 26 lines
Quoted 26 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +16
-0+guest_lock_component (vm); + ret =kvm_pgtable_get_leaf (&vm->pgt, ipa, &pte, &level); +if (ret) + goto unlock; + +if (level !=KVM_PGTABLE_LAST_LEVEL ) { + ret = -EINVAL ; + goto unlock; + } + + ret =guest_pte_is_poisoned (pte); +unlock: +guest_unlock_component (vm); +return ret; +} +int __pkvm_host_share_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);arch/arm64/kvm/pkvm.c +0
-0@@ -417,10 +417,13 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EINVAL ;Quoted 13 lines
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 424
Quoted 9 lines
arch/arm64/kvm/pkvm.c +4
-3/* -* We raced with another vCPU. +* We either raced with another vCPU or the guest PTE +* has been poisoned by an erroneous host access. */ -if (mapping) -return -EAGAIN ; +if (mapping) { + ret =kvm_call_hyp_nvhe (__pkvm_vcpu_in_poison_fault);It's not too bad, but it's a shame we now issue that every time we have such a race (which is frequent-ish). Could we perhaps only issue it if at least one page has been forcefully reclaimed since boot?
Quoted 6 lines
On the plus side, it avoids an unconditional walk from the fault path at EL2 (which is what we have in Android!).
It's a bit fiddly to implement your idea in the host, since the forceful reclaim happens in a really terrible context but I could track it at EL2 and make
__pkvm_vcpu_in_poison_fault()return early instead?
I guess EL2 could easily publish something in the host kvm struct as well if we really wanted to, it's pinned as shared with EL2 and accessible from the hyp_vm, which we retrieve in the force reclaim path.
Quoted 4 lines
It's also
worth bearing in mind that we've already serialised the concurrent fault and done a GUP by this point, so performance is somewhat of a lost cause...
That is very true, so happy to keep all these micro-optimization for later.
Thanks,
Quentin
Will Deacon
maintainerRe: [PATCH 22/30] KVM: arm64: Return -EFAULT from VCPU_RUN on access to a poisoned pte
Quoted 32 lines · Quentin Perret
Quoted 26 lines · Will Deacon
Quoted 16 lines · Quentin Perret
Quoted 15 lines · Will Deacon
arch/arm64/kvm/hyp/nvhe/mem_protect.c +15
-0+int __pkvm_vcpu_in_poison_fault (struct pkvm_hyp_vcpu *hyp_vcpu) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (hyp_vcpu); +kvm_pte_t pte; +s8 level; +u64 ipa; +int ret; + +switch (kvm_vcpu_trap_get_class (&hyp_vcpu->vcpu)) { +case ESR_ELx_EC_DABT_LOW: +case ESR_ELx_EC_IABT_LOW: +if (kvm_vcpu_trap_is_translation_fault (&hyp_vcpu->vcpu)) +break ; + fallthrough; +default :Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c static int get_valid_guest_pte(struct pkvm_hyp_vm *vm, u64 ipa, kvm_pte_t *ptep, · line 847
Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +5
-0+return -EINVAL ; + } + + ipa =kvm_vcpu_get_fault_ipa (&hyp_vcpu->vcpu); + ipa |=kvm_vcpu_get_hfar (&hyp_vcpu->vcpu) &GENMASK (11 ,0 );Why is all the above needed? Could we simplify by having the host pass the IPA to the hcall?
Quoted 4 lines
I was just a little nervous about exposing an oracle here if we take the gfn as an argument as it would provide the host with a pretty easy mechanism to monitor the page access pattern of a guest after the initial donation had occurred.
Quoted 4 lines
Aha, I see what you mean. I guess if we scope that hcall to only discover if a gfn is poisoned we're not exposing too much, but contextualizing the call to the fault also sounds good to me. Perhaps a small comment would help?
Good idea, I'll add something to the hypercall.
Quoted 24 lines
Quoted 24 lines
Quoted 7 lines
Quoted 7 lines
arch/arm64/kvm/pkvm.c +0
-0@@ -417,10 +417,13 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,return -EINVAL ;Quoted 13 lines
arch/arm64/kvm/pkvm.c int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size, · line 424
Quoted 9 lines
arch/arm64/kvm/pkvm.c +4
-3/* -* We raced with another vCPU. +* We either raced with another vCPU or the guest PTE +* has been poisoned by an erroneous host access. */ -if (mapping) -return -EAGAIN ; +if (mapping) { + ret =kvm_call_hyp_nvhe (__pkvm_vcpu_in_poison_fault);It's not too bad, but it's a shame we now issue that every time we have such a race (which is frequent-ish). Could we perhaps only issue it if at least one page has been forcefully reclaimed since boot?
On the plus side, it avoids an unconditional walk from the fault path at EL2 (which is what we have in Android!).
Quoted 7 lines
Quoted 3 lines
It's a bit fiddly to implement your idea in the host, since the forceful reclaim happens in a really terrible context but I could track it at EL2 and make
__pkvm_vcpu_in_poison_fault()return early instead?I guess EL2 could easily publish something in the host kvm struct as well if we really wanted to, it's pinned as shared with EL2 and accessible from the
hyp_vm, which we retrieve in the force reclaim path.
Ah yeah, that sounds do-able.
Quoted 7 lines
Quoted 4 lines
It's also
worth bearing in mind that we've already serialised the concurrent fault and done a GUP by this point, so performance is somewhat of a lost cause...That is very true, so happy to keep all these micro-optimization for later.
Sounds good to me. We'll have a bunch of performance work once the functionality is there, so I'll leave this part as-is for now.
Will
Will Deacon
maintainerPATCH 23/30 KVM: arm64: Add hvc handler at EL2 for hypercalls from protected VMs
In reply to Will DeaconAdd a hypercall handler at EL2 for hypercalls originating from protected VMs. For now, this implements only the FEATURES and MEMINFO calls, but subsequent patches will implement the SHARE and UNSHARE functions necessary for virtio.
Unhandled hypercalls (including PSCI) are passed back to the host.
--- arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 1 + arch/arm64/kvm/hyp/nvhe/pkvm.c | 37 ++++++++++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 1 + 3 files changed, 39 insertions(+)
arch/arm64/kvm/hyp/include/nvhe/pkvm.h +1 -0
@@ -88,6 +88,7 @@ struct pkvm_hyp_vm *get_pkvm_hyp_vm(pkvm_handle_t handle);struct pkvm_hyp_vm *get_np_pkvm_hyp_vm (pkvm_handle_t handle);void put_pkvm_hyp_vm (struct pkvm_hyp_vm *hyp_vm); +bool kvm_handle_pvm_hvc64 (struct kvm_vcpu *vcpu,u64 *exit_code);bool kvm_handle_pvm_sysreg (struct kvm_vcpu *vcpu,u64 *exit_code);bool kvm_handle_pvm_restricted (struct kvm_vcpu *vcpu,u64 *exit_code);void kvm_init_pvm_id_regs (struct kvm_vcpu *vcpu);
arch/arm64/kvm/hyp/nvhe/pkvm.c +37 -0
@@ -4,6 +4,8 @@* Author: Fuad Tabba <[email protected]> */ +#include <kvm/arm_hypercalls.h> +#include <linux/kvm_host.h> #include <linux/mm.h>
@@ -934,3 +936,38 @@ int __pkvm_finalize_teardown_vm(pkvm_handle_t handle)hyp_spin_unlock (&vm_table_lock);return err; } +/* +* Handler for protected VM HVC calls. +* +* Returns true if the hypervisor has handled the exit (and control +* should return to the guest) or false if it hasn't (and the handling +* should be performed by the host). +*/ +bool kvm_handle_pvm_hvc64 (struct kvm_vcpu *vcpu,u64 *exit_code) +{ +u64 val[4 ] = {SMCCC_RET_INVALID_PARAMETER }; +bool handled = true; + +switch (smccc_get_function (vcpu)) { +case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID : + val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); +break ; +case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : +if (smccc_get_arg1 (vcpu) || +smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + } + + val[0 ] =PAGE_SIZE ; +break ; +default : +/* Punt everything else back to the host, for now. */ + handled = false; + } + +if (handled) +smccc_set_retval (vcpu, val[0 ], val[1 ], val[2 ], val[3 ]); +return handled; +}
arch/arm64/kvm/hyp/nvhe/switch.c +1 -0
@@ -190,6 +190,7 @@ static const exit_handler_fn hyp_exit_handlers[] = {static const exit_handler_fn pvm_exit_handlers[] = { [0 ... ESR_ELx_EC_MAX] =NULL , + [ESR_ELx_EC_HVC64] = kvm_handle_pvm_hvc64, [ESR_ELx_EC_SYS64] = kvm_handle_pvm_sys64, [ESR_ELx_EC_SVE] = kvm_handle_pvm_restricted, [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 23/30] KVM: arm64: Add hvc handler at EL2 for hypercalls from protected VMs
Quoted 8 lines · Will Deacon
Add a hypercall handler at EL2 for hypercalls originating from protected VMs. For now, this implements only the FEATURES and MEMINFO calls, but subsequent patches will implement the SHARE and UNSHARE functions necessary for virtio.
Unhandled hypercalls (including PSCI) are passed back to the host.
Signed-off-by Will Deacon <[email protected]>
Quoted 85 lines
--- arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 1 + arch/arm64/kvm/hyp/nvhe/pkvm.c | 37 ++++++++++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 1 + 3 files changed, 39 insertions(+)arch/arm64/kvm/hyp/include/nvhe/pkvm.h +1
-0@@ -88,6 +88,7 @@ struct pkvm_hyp_vm *get_pkvm_hyp_vm(pkvm_handle_t handle);struct pkvm_hyp_vm *get_np_pkvm_hyp_vm (pkvm_handle_t handle);void put_pkvm_hyp_vm (struct pkvm_hyp_vm *hyp_vm); +bool kvm_handle_pvm_hvc64 (struct kvm_vcpu *vcpu,u64 *exit_code);bool kvm_handle_pvm_sysreg (struct kvm_vcpu *vcpu,u64 *exit_code);bool kvm_handle_pvm_restricted (struct kvm_vcpu *vcpu,u64 *exit_code);void kvm_init_pvm_id_regs (struct kvm_vcpu *vcpu);arch/arm64/kvm/hyp/nvhe/pkvm.c +37
-0@@ -4,6 +4,8 @@* Author: Fuad Tabba <[email protected]> */ +#include <kvm/arm_hypercalls.h> +#include <linux/kvm_host.h> #include <linux/mm.h> @@ -934,3 +936,38 @@ int __pkvm_finalize_teardown_vm(pkvm_handle_t handle)hyp_spin_unlock (&vm_table_lock);return err; } +/* +* Handler for protected VM HVC calls. +* +* Returns true if the hypervisor has handled the exit (and control +* should return to the guest) or false if it hasn't (and the handling +* should be performed by the host). +*/ +bool kvm_handle_pvm_hvc64 (struct kvm_vcpu *vcpu,u64 *exit_code) +{ +u64 val[4 ] = {SMCCC_RET_INVALID_PARAMETER }; +bool handled = true; + +switch (smccc_get_function (vcpu)) { +case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID : + val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); +break ; +case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : +if (smccc_get_arg1 (vcpu) || +smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + } + + val[0 ] =PAGE_SIZE ; +break ; +default : +/* Punt everything else back to the host, for now. */ + handled = false; + } + +if (handled) +smccc_set_retval (vcpu, val[0 ], val[1 ], val[2 ], val[3 ]); +return handled; +}arch/arm64/kvm/hyp/nvhe/switch.c +1
-0@@ -190,6 +190,7 @@ static const exit_handler_fn hyp_exit_handlers[] = {static const exit_handler_fn pvm_exit_handlers[] = { [0 ... ESR_ELx_EC_MAX] =NULL , + [ESR_ELx_EC_HVC64] = kvm_handle_pvm_hvc64, [ESR_ELx_EC_SYS64] = kvm_handle_pvm_sys64, [ESR_ELx_EC_SVE] = kvm_handle_pvm_restricted, [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 24/30 KVM: arm64: Implement the MEM_SHARE hypercall for protected VMs
In reply to Will DeaconImplement the ARM_SMCCC_KVM_FUNC_MEM_SHARE hypercall to allow protected VMs to share memory (e.g. the swiotlb bounce buffers) back to the host.
--- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/nvhe/mem_protect.c | 32 ++++++++++ arch/arm64/kvm/hyp/nvhe/pkvm.c | 61 +++++++++++++++++++ 3 files changed, 94 insertions(+)
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1 -0
@@ -35,6 +35,7 @@ extern unsigned long hyp_nr_cpus;int __pkvm_prot_finalize (void );int __pkvm_host_share_hyp (u64 pfn); +int __pkvm_guest_share_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn);int __pkvm_host_unshare_hyp (u64 pfn);int __pkvm_host_donate_hyp (u64 pfn,u64 nr_pages);int __pkvm_hyp_donate_host (u64 pfn,u64 nr_pages);
arch/arm64/kvm/hyp/nvhe/mem_protect.c +32 -0
@@ -888,6 +888,38 @@ int __pkvm_host_share_hyp(u64 pfn)return ret; } +int __pkvm_guest_share_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu); +u64 phys, ipa =hyp_pfn_to_phys (gfn); +kvm_pte_t pte; +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =get_valid_guest_pte (vm, ipa, &pte, &phys); +if (ret) + goto unlock; + + ret = -EPERM ; +if (pkvm_getstate (kvm_pgtable_stage2_pte_prot (pte)) !=PKVM_PAGE_OWNED ) + goto unlock; +if (__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_NOPAGE )) + goto unlock; + + ret =0 ; +WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys, +pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_SHARED_OWNED ), + &vcpu->vcpu.arch.pkvm_memcache,0 )); +WARN_ON (__host_set_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_SHARED_BORROWED )); +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_unshare_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);
arch/arm64/kvm/hyp/nvhe/pkvm.c +61 -0
79 lines not on this page · raw message
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 24/30] KVM: arm64: Implement the MEM_SHARE hypercall for protected VMs
[...]
Quoted 9 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c +1
-0@@ -952,6 +1004,7 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID : val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_SHARE );break ;case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : if (smccc_get_arg1(vcpu) ||
arch/arm64/kvm/hyp/nvhe/pkvm.c bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) · line 1022
Quoted 8 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c +5
-0@@ -962,6 +1015,14 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) val[0 ] =PAGE_SIZE ;break ; +case ARM_SMCCC_VENDOR_HYP_KVM_MEM_SHARE_FUNC_ID : +if (smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + }
I wonder if that shouldn't go into pkvm_memshare_call(): that function has the knowledge of the argument content since we pass vcpu.
Otherwise:
Quoted 9 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c +3
-0+ + handled =pkvm_memshare_call (val, vcpu, exit_code); +break ;default :/* Punt everything else back to the host, for now. */ handled = false;Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 24/30] KVM: arm64: Implement the MEM_SHARE hypercall for protected VMs
Quoted 12 lines · Vincent Donnefort
[...]
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c +1
-0@@ -952,6 +1004,7 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID : val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_SHARE );break ;case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : if (smccc_get_arg1(vcpu) ||@@ -962,6 +1015,14 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) · line 1022
Quoted 7 lines
arch/arm64/kvm/hyp/nvhe/pkvm.c +5
-0val[0 ] =PAGE_SIZE ;break ; +case ARM_SMCCC_VENDOR_HYP_KVM_MEM_SHARE_FUNC_ID : +if (smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + }I wonder if that shouldn't go into
pkvm_memshare_call(): that function has the knowledge of the argument content since we pass vcpu.
I decided to keep all the zero-arg checking in the top-level switch so that people adding new hypercalls are encouraged to add those checks as well :p
Quoted 3 lines
Otherwise:
Reviewed-by Vincent Donnefort <[email protected]>
Thanks,
Will
Will Deacon
maintainerPATCH 25/30 KVM: arm64: Implement the MEM_UNSHARE hypercall for protected VMs
In reply to Will DeaconImplement the ARM_SMCCC_KVM_FUNC_MEM_UNSHARE hypercall to allow protected VMs to unshare memory that was previously shared with the host using the ARM_SMCCC_KVM_FUNC_MEM_SHARE hypercall.
--- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/nvhe/mem_protect.c | 32 +++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/pkvm.c | 22 +++++++++++++ 3 files changed, 55 insertions(+)
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1 -0
@@ -36,6 +36,7 @@ extern unsigned long hyp_nr_cpus;int __pkvm_prot_finalize (void );int __pkvm_host_share_hyp (u64 pfn);int __pkvm_guest_share_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn); +int __pkvm_guest_unshare_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn);int __pkvm_host_unshare_hyp (u64 pfn);int __pkvm_host_donate_hyp (u64 pfn,u64 nr_pages);int __pkvm_hyp_donate_host (u64 pfn,u64 nr_pages);
arch/arm64/kvm/hyp/nvhe/mem_protect.c +32 -0
@@ -920,6 +920,38 @@ int __pkvm_guest_share_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn)return ret; } +int __pkvm_guest_unshare_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu); +u64 phys, ipa =hyp_pfn_to_phys (gfn); +kvm_pte_t pte; +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =get_valid_guest_pte (vm, ipa, &pte, &phys); +if (ret) + goto unlock; + + ret = -EPERM ; +if (pkvm_getstate (kvm_pgtable_stage2_pte_prot (pte)) !=PKVM_PAGE_SHARED_OWNED ) + goto unlock; +if (__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_SHARED_BORROWED )) + goto unlock; + + ret =0 ; +WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); +WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys, +pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), + &vcpu->vcpu.arch.pkvm_memcache,0 )); +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_unshare_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);
arch/arm64/kvm/hyp/nvhe/pkvm.c +22 -0
@@ -988,6 +988,19 @@ static bool pkvm_memshare_call(u64 *ret, struct kvm_vcpu *vcpu, u64 *exit_code)return false; } +static void pkvm_memunshare_call (u64 *ret,struct kvm_vcpu *vcpu) +{ +struct pkvm_hyp_vcpu *hyp_vcpu; +u64 ipa =smccc_get_arg1 (vcpu); + +if (!PAGE_ALIGNED (ipa)) +return ; + + hyp_vcpu = container_of(vcpu, struct pkvm_hyp_vcpu, vcpu); +if (!__pkvm_guest_unshare_host (hyp_vcpu,hyp_phys_to_pfn (ipa))) + ret[0 ] =SMCCC_RET_SUCCESS ; +} +/* * Handler for protected VM HVC calls. *
@@ -1005,6 +1018,7 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_SHARE ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_UNSHARE );break ;case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : if (smccc_get_arg1(vcpu) ||
@@ -1023,6 +1037,14 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) handled =pkvm_memshare_call (val, vcpu, exit_code);break ; +case ARM_SMCCC_VENDOR_HYP_KVM_MEM_UNSHARE_FUNC_ID : +if (smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + } + +pkvm_memunshare_call (val, vcpu); +break ;default :/* Punt everything else back to the host, for now. */ handled = false;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 25/30] KVM: arm64: Implement the MEM_UNSHARE hypercall for protected VMs
Quoted 5 lines · Will Deacon
Implement the
ARM_SMCCC_KVM_FUNC_MEM_UNSHAREhypercall to allow protected VMs to unshare memory that was previously shared with the host using theARM_SMCCC_KVM_FUNC_MEM_SHAREhypercall.Signed-off-by Will Deacon <[email protected]>
Quoted 111 lines
--- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 + arch/arm64/kvm/hyp/nvhe/mem_protect.c | 32 +++++++++++++++++++ arch/arm64/kvm/hyp/nvhe/pkvm.c | 22 +++++++++++++ 3 files changed, 55 insertions(+)arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +1
-0@@ -36,6 +36,7 @@ extern unsigned long hyp_nr_cpus;int __pkvm_prot_finalize (void );int __pkvm_host_share_hyp (u64 pfn);int __pkvm_guest_share_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn); +int __pkvm_guest_unshare_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn);int __pkvm_host_unshare_hyp (u64 pfn);int __pkvm_host_donate_hyp (u64 pfn,u64 nr_pages);int __pkvm_hyp_donate_host (u64 pfn,u64 nr_pages);arch/arm64/kvm/hyp/nvhe/mem_protect.c +32
-0@@ -920,6 +920,38 @@ int __pkvm_guest_share_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn)return ret; } +int __pkvm_guest_unshare_host (struct pkvm_hyp_vcpu *vcpu,u64 gfn) +{ +struct pkvm_hyp_vm *vm =pkvm_hyp_vcpu_to_hyp_vm (vcpu); +u64 phys, ipa =hyp_pfn_to_phys (gfn); +kvm_pte_t pte; +int ret; + +host_lock_component (); +guest_lock_component (vm); + + ret =get_valid_guest_pte (vm, ipa, &pte, &phys); +if (ret) + goto unlock; + + ret = -EPERM ; +if (pkvm_getstate (kvm_pgtable_stage2_pte_prot (pte)) !=PKVM_PAGE_SHARED_OWNED ) + goto unlock; +if (__host_check_page_state_range (phys,PAGE_SIZE ,PKVM_PAGE_SHARED_BORROWED )) + goto unlock; + + ret =0 ; +WARN_ON (host_stage2_set_owner_locked (phys,PAGE_SIZE ,PKVM_ID_GUEST )); +WARN_ON (kvm_pgtable_stage2_map (&vm->pgt, ipa,PAGE_SIZE , phys, +pkvm_mkstate (KVM_PGTABLE_PROT_RWX ,PKVM_PAGE_OWNED ), + &vcpu->vcpu.arch.pkvm_memcache,0 )); +unlock: +guest_unlock_component (vm); +host_unlock_component (); + +return ret; +} +int __pkvm_host_unshare_hyp (u64 pfn) {u64 phys =hyp_pfn_to_phys (pfn);arch/arm64/kvm/hyp/nvhe/pkvm.c +22
-0@@ -988,6 +988,19 @@ static bool pkvm_memshare_call(u64 *ret, struct kvm_vcpu *vcpu, u64 *exit_code)return false; } +static void pkvm_memunshare_call (u64 *ret,struct kvm_vcpu *vcpu) +{ +struct pkvm_hyp_vcpu *hyp_vcpu; +u64 ipa =smccc_get_arg1 (vcpu); + +if (!PAGE_ALIGNED (ipa)) +return ; + + hyp_vcpu = container_of(vcpu, struct pkvm_hyp_vcpu, vcpu); +if (!__pkvm_guest_unshare_host (hyp_vcpu,hyp_phys_to_pfn (ipa))) + ret[0 ] =SMCCC_RET_SUCCESS ; +} +/* * Handler for protected VM HVC calls. * @@ -1005,6 +1018,7 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) val[0 ] =BIT (ARM_SMCCC_KVM_FUNC_FEATURES ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_HYP_MEMINFO ); val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_SHARE ); + val[0 ] |=BIT (ARM_SMCCC_KVM_FUNC_MEM_UNSHARE );break ;case ARM_SMCCC_VENDOR_HYP_KVM_HYP_MEMINFO_FUNC_ID : if (smccc_get_arg1(vcpu) ||@@ -1023,6 +1037,14 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) handled =pkvm_memshare_call (val, vcpu, exit_code);break ; +case ARM_SMCCC_VENDOR_HYP_KVM_MEM_UNSHARE_FUNC_ID : +if (smccc_get_arg2 (vcpu) || +smccc_get_arg3 (vcpu)) { +break ; + } + +pkvm_memunshare_call (val, vcpu); +break ;default :/* Punt everything else back to the host, for now. */ handled = false;Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 26/30 KVM: arm64: Allow userspace to create protected VMs when pKVM is enabled
In reply to Will DeaconIntroduce a new VM type for KVM/arm64 to allow userspace to request the creation of a "protected VM" when the host has booted with pKVM enabled.
For now, this depends on CONFIG_EXPERT and results in a taint on first use as many aspects of a protected VM are not yet protected!
--- arch/arm64/include/asm/kvm_pkvm.h | 2 +- arch/arm64/kvm/Kconfig | 10 ++++++++++ arch/arm64/kvm/arm.c | 8 +++++++- arch/arm64/kvm/mmu.c | 3 --- arch/arm64/kvm/pkvm.c | 11 ++++++++++- include/uapi/linux/kvm.h | 5 +++++ 6 files changed, 33 insertions(+), 6 deletions(-)
arch/arm64/include/asm/kvm_pkvm.h +1 -1
@@ -16,7 +16,7 @@#define HYP_MEMBLOCK_REGIONS 128 -int pkvm_init_host_vm (struct kvm *kvm); +int pkvm_init_host_vm (struct kvm *kvm,unsigned long type);int pkvm_create_hyp_vm (struct kvm *kvm);bool pkvm_hyp_vm_is_created (struct kvm *kvm);void pkvm_destroy_hyp_vm (struct kvm *kvm);
arch/arm64/kvm/Kconfig +10 -0
@@ -83,4 +83,14 @@ config PTDUMP_STAGE2_DEBUGFS If in doubt, say N. +config PROTECTED_VM_UAPI + bool "Expose protected VMs to userspace (experimental)" + depends on KVM && EXPERT + help + Say Y here to enable experimental (i.e. in development) + support for creating protected virtual machines using KVM's + KVM_CREATE_VM ioctl() when booted with pKVM enabled. + + Unless you are a KVM developer, say N. + endif # VIRTUALIZATION
arch/arm64/kvm/arm.c +7 -1
@@ -157,6 +157,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) {int ret; +if (type & ~KVM_VM_TYPE_ARM_MASK ) +return -EINVAL ; +mutex_init (&kvm->arch.config_lock);#ifdef CONFIG_LOCKDEP
@@ -188,9 +191,12 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)* If any failures occur after this is successful, make sure to * call __pkvm_unreserve_vm to unreserve the VM in hyp. */ - ret =pkvm_init_host_vm (kvm); + ret =pkvm_init_host_vm (kvm, type);if (ret) goto err_free_cpumask; + }else if (type &KVM_VM_TYPE_ARM_PROTECTED ) { + ret = -EINVAL ; + goto err_free_cpumask; }kvm_vgic_early_init (kvm);
arch/arm64/kvm/mmu.c +0 -3
@@ -881,9 +881,6 @@ static int kvm_init_ipa_range(struct kvm_s2_mmu *mmu, unsigned long type)u64 mmfr0, mmfr1;u32 phys_shift; -if (type & ~KVM_VM_TYPE_ARM_IPA_SIZE_MASK ) -return -EINVAL ; - phys_shift =KVM_VM_TYPE_ARM_IPA_SIZE (type);if (is_protected_kvm_enabled ()) { phys_shift = kvm_ipa_limit;
arch/arm64/kvm/pkvm.c +10 -1
@@ -219,9 +219,13 @@ void pkvm_destroy_hyp_vm(struct kvm *kvm)mutex_unlock (&kvm->arch.config_lock); } -int pkvm_init_host_vm (struct kvm *kvm) +int pkvm_init_host_vm (struct kvm *kvm,unsigned long type) {int ret; +bool protected = type &KVM_VM_TYPE_ARM_PROTECTED ; + +if (protected && !IS_ENABLED (CONFIG_PROTECTED_VM_UAPI )) +return -EINVAL ;if (pkvm_hyp_vm_is_created (kvm))return -EINVAL ;
@@ -236,6 +240,11 @@ int pkvm_init_host_vm(struct kvm *kvm)return ret; kvm->arch.pkvm.handle = ret; + kvm->arch.pkvm.is_protected = protected; +if (protected) { +pr_warn_once ("kvm: protected VMs are for development only, tainting kernel\n" ); +add_taint (TAINT_USER ,LOCKDEP_STILL_OK ); + }return 0 ; }
include/uapi/linux/kvm.h +5 -0
@@ -689,6 +689,11 @@ struct kvm_enable_cap {#define KVM_VM_TYPE_ARM_IPA_SIZE_MASK 0xffULL#define KVM_VM_TYPE_ARM_IPA_SIZE (x) \ ((x) & KVM_VM_TYPE_ARM_IPA_SIZE_MASK) + +#define KVM_VM_TYPE_ARM_PROTECTED (1UL << 31) +#define KVM_VM_TYPE_ARM_MASK (KVM_VM_TYPE_ARM_IPA_SIZE_MASK | \ + KVM_VM_TYPE_ARM_PROTECTED) +/* * ioctls for /dev/kvm fds: */
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 27/30 KVM: arm64: Add some initial documentation for pKVM
In reply to Will DeaconAdd some initial documentation for pKVM to help people understand what is supported, the limitations of protected VMs when compared to non-protected VMs and also what is left to do.
--- .../admin-guide/kernel-parameters.txt | 4 +- Documentation/virt/kvm/arm/index.rst | 1 + Documentation/virt/kvm/arm/pkvm.rst | 101 ++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 Documentation/virt/kvm/arm/pkvm.rst
Documentation/admin-guide/kernel-parameters.txt +2 -2
@@ -3141,8 +3141,8 @@ Kernel parameters for the host. To force nVHE on VHE hardware, add "arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" to the command-line. - "nested" is experimental and should be used with - extreme caution. + "nested" and "protected" are experimental and should be + used with extreme caution. kvm-arm.vgic_v3_group0_trap= [KVM,ARM,EARLY] Trap guest accesses to GICv3 group-0
Documentation/virt/kvm/arm/index.rst +1 -0
@@ -10,6 +10,7 @@ ARM fw-pseudo-registers hyp-abi hypercalls + pkvm pvtime ptp_kvm vcpu-features
Documentation/virt/kvm/arm/pkvm.rst new file +101 -0
101 lines not on this page · raw message
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Re: [PATCH 27/30] KVM: arm64: Add some initial documentation for pKVM
Quoted 90 lines · Will Deacon
Add some initial documentation for pKVM to help people understand what is supported, the limitations of protected VMs when compared to non-protected VMs and also what is left to do.
Signed-off-by Will Deacon <[email protected]>--- .../admin-guide/kernel-parameters.txt | 4 +- Documentation/virt/kvm/arm/index.rst | 1 + Documentation/virt/kvm/arm/pkvm.rst | 101 ++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 Documentation/virt/kvm/arm/pkvm.rstDocumentation/admin-guide/kernel-parameters.txt +2
-2@@ -3141,8 +3141,8 @@ Kernel parameters for the host. To force nVHE on VHE hardware, add "arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" to the command-line. - "nested" is experimental and should be used with - extreme caution. + "nested" and "protected" are experimental and should be + used with extreme caution. kvm-arm.vgic_v3_group0_trap= [KVM,ARM,EARLY] Trap guest accesses to GICv3 group-0Documentation/virt/kvm/arm/index.rst +1
-0@@ -10,6 +10,7 @@ ARM fw-pseudo-registers hyp-abi hypercalls + pkvm pvtime ptp_kvm vcpu-featuresDocumentation/virt/kvm/arm/pkvm.rst new file +45
-0@@ -0,0 +1,101 @@ +.. SPDX-License-Identifier: GPL-2.0 + +==================== +Protected KVM (pKVM) +==================== + +**NOTE**: pKVM is currently an experimental, development feature and +subject to breaking changes as new isolation features are implemented. +Please reach out to the developers at [email protected] if you have +any questions. + +Overview +======== + +Booting a host kernel with '``kvm-arm.mode=protected``' enables +"Protected KVM" (pKVM). During boot, pKVM installs a stage-2 identity +map page-table for the host and uses it to isolate the hypervisor +running at EL2 from the rest of the host running at EL1/0. + +If ``CONFIG_PROTECTED_VM_UAPI=y``, pKVM permits creation of protected +virtual machines (pVMs) by passing the ``KVM_VM_TYPE_ARM_PROTECTED`` +machine type identifier to the ``KVM_CREATE_VM`` ioctl(). The hypervisor +isolates pVMs from the host by unmapping pages from the stage-2 identity +map as they are accessed by a pVM. Hypercalls are provided for a pVM to +share specific regions of its IPA space back with the host, allowing +for communication with the VMM. See hypercalls.rst for more details. + +Isolation mechanisms +==================== + +pKVM relies on a number of mechanisms to isolate PVMs from the host: + +CPU memory isolation +-------------------- + +Status: Isolation of anonymous memory and metadata pages. + +Metadata pages (e.g. page-table pages and '``struct kvm_vcpu``' pages) +are donated from the host to the hypervisor during pVM creation and +are consequently unmapped from the stage-2 identity map until the pVM is +destroyed. + +Similarly to regular KVM, pages are lazily mapped into the guest in +response to stage-2 page faults handled by the host. However, when +running a pVM, these pages are first pinned and then unmapped from the
Documentation/virt/kvm/arm/pkvm.rst line 55
Quoted 10 lines
Documentation/virt/kvm/arm/pkvm.rst new file +10
-0+stage-2 identity map as part of the donation procedure. This gives rise +to some user-visible differences when compared to non-protected VMs, +largely due to the lack of MMU notifiers: + +* Memslots cannot be moved or deleted once the pVM has started running. +* Read-only memslots and dirty logging are not supported. +* With the exception of swap, file-backed pages cannot be mapped into a + pVM. +* Donated pages are accounted against ``RLIMIT_MLOCK`` and so the VMM + must have a sufficient resource limit or be granted ``CAP_IPC_LOCK``.
Perhaps worth to add that there's no runtime reclaim either so the accounting will only grow until the VM is destroyed?
Quoted 49 lines
Documentation/virt/kvm/arm/pkvm.rst new file +46
-0+* Changes to the VMM address space (e.g. a ``MAP_FIXED`` mmap() over a + mapping associated with a memslot) are not reflected in the guest and + may lead to loss of coherency. +* Accessing pVM memory that has not been shared back will result in the + delivery of a SIGSEGV. +* If a system call accesses pVM memory that has not been shared back + then it will either return ``-EFAULT`` or forcefully reclaim the + memory pages. Reclaimed memory is zeroed by the hypervisor and a + subsequent attempt to access it in the pVM will return ``-EFAULT`` + from the ``VCPU_RUN`` ioctl(). + +CPU state isolation +------------------- + +Status: **Unimplemented.** + +DMA isolation using an IOMMU +---------------------------- + +Status: **Unimplemented.** + +Proxying of Trustzone services +------------------------------ + +Status: FF-A and PSCI calls from the host are proxied by the pKVM +hypervisor. + +The FF-A proxy ensures that the host cannot share pVM or hypervisor +memory with Trustzone as part of a "confused deputy" attack. + +The PSCI proxy ensures that CPUs always have the stage-2 identity map +installed when they are executing in the host. + +Protected VM firmware (pvmfw) +----------------------------- + +Status: **Unimplemented.** + +Resources +========= + +Quentin Perret's KVM Forum 2022 talk entitled "Protected KVM on arm64: A +technical deep dive" remains a good resource for learning more about +pKVM, despite some of the details having changed in the meantime: + +https://www.youtube.com/watch?v=9npebeVFbFwSignature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerRe: [PATCH 27/30] KVM: arm64: Add some initial documentation for pKVM
Quoted 19 lines · Vincent Donnefort
Quoted 18 lines · Will Deacon
Documentation/virt/kvm/arm/pkvm.rst new file +18
-0+CPU memory isolation +-------------------- + +Status: Isolation of anonymous memory and metadata pages. + +Metadata pages (e.g. page-table pages and '``struct kvm_vcpu``' pages) +are donated from the host to the hypervisor during pVM creation and +are consequently unmapped from the stage-2 identity map until the pVM is +destroyed. + +Similarly to regular KVM, pages are lazily mapped into the guest in +response to stage-2 page faults handled by the host. However, when +running a pVM, these pages are first pinned and then unmapped from the +stage-2 identity map as part of the donation procedure. This gives rise +to some user-visible differences when compared to non-protected VMs, +largely due to the lack of MMU notifiers: + +* Memslots cannot be moved or deleted once the pVM has started running.
Quoted 8 lines
Documentation/virt/kvm/arm/pkvm.rst line 55
Quoted 5 lines
Documentation/virt/kvm/arm/pkvm.rst new file +5
-0+* Read-only memslots and dirty logging are not supported. +* With the exception of swap, file-backed pages cannot be mapped into a + pVM. +* Donated pages are accounted against ``RLIMIT_MLOCK`` and so the VMM + must have a sufficient resource limit or be granted ``CAP_IPC_LOCK``.Perhaps worth to add that there's no runtime reclaim either so the accounting will only grow until the VM is destroyed?
Good idea, I'll extend this to cover that. We just need to remember to remove it when we add support for the relinquish hypercall.
Cheers,
Will
Will Deacon
maintainerPATCH 28/30 KVM: arm64: Extend pKVM page ownership selftests to cover guest donation
In reply to Will DeaconExtend the pKVM page ownership selftests to donate and reclaim a page to/from a guest.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +22 -0
@@ -1712,6 +1712,7 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , hyp_pin_shared_mem, virt, virt + size);assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot);assert_transition_res (-ENOENT , __pkvm_host_unshare_guest, gfn,1 , vm); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu); selftest_state.host =PKVM_PAGE_OWNED ; selftest_state.hyp =PKVM_NOPAGE ;
@@ -1731,6 +1732,7 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 );assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot);assert_transition_res (-ENOENT , __pkvm_host_unshare_guest, gfn,1 , vm); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu);assert_transition_res (0 , hyp_pin_shared_mem, virt, virt + size);assert_transition_res (0 , hyp_pin_shared_mem, virt, virt + size);
@@ -1743,6 +1745,7 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 );assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot);assert_transition_res (-ENOENT , __pkvm_host_unshare_guest, gfn,1 , vm); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu);hyp_unpin_shared_mem (virt, virt + size);assert_page_state ();
@@ -1762,6 +1765,7 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 );assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot);assert_transition_res (-ENOENT , __pkvm_host_unshare_guest, gfn,1 , vm); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu);assert_transition_res (-EPERM , hyp_pin_shared_mem, virt, virt + size); selftest_state.host =PKVM_PAGE_OWNED ;
@@ -1778,6 +1782,7 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , __pkvm_host_share_hyp, pfn);assert_transition_res (-EPERM , __pkvm_host_unshare_hyp, pfn);assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 ); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu);assert_transition_res (-EPERM , hyp_pin_shared_mem, virt, virt + size); selftest_state.guest[1 ] =PKVM_PAGE_SHARED_BORROWED ;
@@ -1791,6 +1796,23 @@ void pkvm_ownership_selftest(void *base) selftest_state.host =PKVM_PAGE_OWNED ;assert_transition_res (0 , __pkvm_host_unshare_guest, gfn +1 ,1 , vm); + selftest_state.host =PKVM_NOPAGE ; + selftest_state.guest[0 ] =PKVM_PAGE_OWNED ; +assert_transition_res (0 , __pkvm_host_donate_guest, pfn, gfn, vcpu); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn +1 , vcpu); +assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot); +assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn +1 ,1 , vcpu, prot); +assert_transition_res (-EPERM , __pkvm_host_share_ffa, pfn,1 ); +assert_transition_res (-EPERM , __pkvm_host_donate_hyp, pfn,1 ); +assert_transition_res (-EPERM , __pkvm_host_share_hyp, pfn); +assert_transition_res (-EPERM , __pkvm_host_unshare_hyp, pfn); +assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 ); + + selftest_state.host =PKVM_PAGE_OWNED ; + selftest_state.guest[0 ] =PKVM_NOPAGE ; +assert_transition_res (0 , __pkvm_host_reclaim_page_guest, gfn, vm); + selftest_state.host =PKVM_NOPAGE ; selftest_state.hyp =PKVM_PAGE_OWNED ;assert_transition_res (0 , __pkvm_host_donate_hyp, pfn,1 );
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 29/30 KVM: arm64: Register 'selftest_vm' in the VM table
In reply to Will DeaconIn preparation for extending the pKVM page ownership selftests to cover forceful reclaim of donated pages, rework the creation of the 'selftest_vm' so that it is registered in the VM table while the tests are running.
--- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 + arch/arm64/kvm/hyp/nvhe/mem_protect.c | 53 ++++--------------- arch/arm64/kvm/hyp/nvhe/pkvm.c | 49 +++++++++++++++++ 3 files changed, 61 insertions(+), 43 deletions(-)
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +2 -0
@@ -77,6 +77,8 @@ static __always_inline void __load_host_stage2(void)#ifdef CONFIG_NVHE_EL2_DEBUG void pkvm_ownership_selftest (void *base); +struct pkvm_hyp_vcpu *init_selftest_vm (void *virt); +void teardown_selftest_vm (void );#else static inline void pkvm_ownership_selftest (void *base) { }#endif
arch/arm64/kvm/hyp/nvhe/mem_protect.c +10 -43
93 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/pkvm.c +49 -0
55 lines not on this page · raw message
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Will Deacon
maintainerPATCH 30/30 KVM: arm64: Extend pKVM page ownership selftests to cover forced reclaim
In reply to Will DeaconExtend the pKVM page ownership selftests to forcefully reclaim a donated page and check that it cannot be re-donated at the same IPA.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +14 -2
@@ -1776,8 +1776,20 @@ void pkvm_ownership_selftest(void *base)assert_transition_res (-EPERM , __pkvm_hyp_donate_host, pfn,1 ); selftest_state.host =PKVM_PAGE_OWNED ; - selftest_state.guest[0 ] =PKVM_NOPAGE ; -assert_transition_res (0 , __pkvm_host_reclaim_page_guest, gfn, vm); + selftest_state.guest[0 ] =PKVM_POISON ; +assert_transition_res (0 , __pkvm_host_force_reclaim_page_guest, phys); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu); +assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot); + + selftest_state.host =PKVM_NOPAGE ; + selftest_state.guest[1 ] =PKVM_PAGE_OWNED ; +assert_transition_res (0 , __pkvm_host_donate_guest, pfn, gfn +1 , vcpu); + + selftest_state.host =PKVM_PAGE_OWNED ; + selftest_state.guest[1 ] =PKVM_NOPAGE ; +assert_transition_res (0 , __pkvm_host_reclaim_page_guest, gfn +1 , vm); +assert_transition_res (-EPERM , __pkvm_host_donate_guest, pfn, gfn, vcpu); +assert_transition_res (-EPERM , __pkvm_host_share_guest, pfn, gfn,1 , vcpu, prot); selftest_state.host =PKVM_NOPAGE ; selftest_state.hyp =PKVM_PAGE_OWNED ;
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
Hi Will,
Quoted 95 lines · Will Deacon
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for
guest_memfd()but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while theguest_memfd()work continues to evolve.Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer
Kconfigoption to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.More information about what is and what isn't implemented is described in the
pkvm.rstdocumentation added by this series. The intention is to update this file as we introduce additional protection features and ultimately to remove the taint.The series is loosely structured as follows:
Patch 1: A dependent pgtable fix that I sent out previously
Patches 2-8: Cleanups/fixes to the existing code to prepare for pVMs
Patches 9-14: Support for memory donation and reclaim Patches 15-22: Handling of bad host accesses to protected memory Patches 23-25: Support for SHARE and UNSHARE guest hypercalls Patches 26-27: UAPI and developer documentation
Patches 28-30: Selftest additions for new page ownership transitionsPatches are based on v6.19-rc4 and are also available at [3].
All feedback welcome.
Cheers,
Will
[1] https://lore.kernel.org/all/[email protected]/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/log/?h=pkvm
[3] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=kvm/protected-memoryCc Marc Zyngier <[email protected]>Cc Oliver Upton <[email protected]>Cc Joey Gouly <[email protected]>Cc Suzuki K Poulose <[email protected]>Cc Zenghui Yu <[email protected]>Cc Catalin Marinas <[email protected]>Cc Quentin Perret <[email protected]>Cc Fuad Tabba <[email protected]>Cc Vincent Donnefort <[email protected]>Cc Mostafa Saleh <[email protected]>--->8
Fuad Tabba (1):
KVM: arm64: Expose self-hosted debug regs as RAZ/WI for protected
guestsQuentin Perret (2):
KVM: arm64: Refactorenter_exception64()
KVM: arm64: Inject SIGSEGV on illegal accessesWill Deacon (27):
KVM: arm64: InvertKVM_PGTABLE_WALK_HANDLE_FAULTto fix pKVM walkers
KVM: arm64: Remove redundant 'pgt' pointer checks from MMU notifiers
KVM: arm64: Rename__pkvm_pgtable_stage2_unmap()
KVM: arm64: Don't advertise unsupported features for protected guests
KVM: arm64: Remove pointlessis_protected_kvm_enabled()checks from
hyp
KVM: arm64: Ignore MMU notifier callbacks for protected VMs
KVM: arm64: Prevent unsupported memslot operations on protected VMs
KVM: arm64: Split teardown hypercall into two phases
KVM: arm64: Introduce__pkvm_host_donate_guest()
KVM: arm64: Hook up donation hypercall topkvm_pgtable_stage2_map()
KVM: arm64: Handle aborts from protected VMs
KVM: arm64: Introduce__pkvm_reclaim_dying_guest_page()
KVM: arm64: Hook up reclaim hypercall topkvm_pgtable_stage2_destroy()
KVM: arm64: Generalisekvm_pgtable_stage2_set_owner()
KVM: arm64: Introducehost_stage2_set_owner_metadata_locked()
KVM: arm64: Annotate guest donations with handle and gfn in hoststage-2
Quoted 14 lines
KVM: arm64: Introduce hypercall to force reclaim of a protected page
KVM: arm64: Reclaim faulting page from pKVM in spurious fault handler
KVM: arm64: Return-EFAULTfromVCPU_RUNon access to a poisoned pte
KVM: arm64: Add hvc handler at EL2 for hypercalls from protected VMs
KVM: arm64: Implement theMEM_SHAREhypercall for protected VMs
KVM: arm64: Implement theMEM_UNSHAREhypercall for protected VMs
KVM: arm64: Allow userspace to create protected VMs when pKVM is
enabled
KVM: arm64: Add some initial documentation for pKVM
KVM: arm64: Extend pKVM page ownership selftests to cover guest
donation
KVM: arm64: Register 'selftest_vm' in the VM table
KVM: arm64: Extend pKVM page ownership selftests to cover forced
reclaim
I tested the patches on Lenovo ideacenter Mini X Gen 10 Snapdragon with nvhe as for some reason I am facing issues with hvhe on this HW even with upstream.
I can boot ToT Linux kernel as both protected and non-protected VMs using the kvmtool version in here.
For reference, I am using:
- Config: defconfig + CONFIG_PROTECTED_VM_UAPI + CONFIG_ARM_PKVM_GUEST
- Run: ./lkvm-static run --irqchip gicv3 -k Image -d rootfs.ext2 --force-pci -c 1 --debug (--protected)
For the whole series:
Thanks,
Mostafa
Quoted 32 lines
.../admin-guide/kernel-parameters.txt | 4 +- Documentation/virt/kvm/arm/index.rst | 1 + Documentation/virt/kvm/arm/pkvm.rst | 101 ++++ arch/arm64/include/asm/kvm_asm.h | 7 +- arch/arm64/include/asm/kvm_emulate.h | 5 + arch/arm64/include/asm/kvm_host.h | 7 + arch/arm64/include/asm/kvm_pgtable.h | 27 +- arch/arm64/include/asm/kvm_pkvm.h | 4 +- arch/arm64/include/asm/virt.h | 6 + arch/arm64/kvm/Kconfig | 10 + arch/arm64/kvm/arm.c | 8 +- arch/arm64/kvm/hyp/exception.c | 100 ++-- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 9 + arch/arm64/kvm/hyp/include/nvhe/memory.h | 6 + arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 7 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 95 ++-- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 500 ++++++++++++++++-- arch/arm64/kvm/hyp/nvhe/pkvm.c | 223 +++++++- arch/arm64/kvm/hyp/nvhe/switch.c | 1 + arch/arm64/kvm/hyp/nvhe/sys_regs.c | 8 + arch/arm64/kvm/hyp/pgtable.c | 22 +- arch/arm64/kvm/mmu.c | 122 ++++- arch/arm64/kvm/pkvm.c | 149 +++++- arch/arm64/mm/fault.c | 31 +- include/uapi/linux/kvm.h | 5 + 25 files changed, 1249 insertions(+), 209 deletions(-) create mode 100644 Documentation/virt/kvm/arm/pkvm.rstSignature · 1 line
2.52.0.351.gbe84eed79e-goog
Hi Will,
Quoted 16 lines · Will Deacon
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for
guest_memfd()but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while theguest_memfd()work continues to evolve.
Quoted 12 lines
Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer
Kconfigoption to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.
Good to see Protected VM support in upstream w/ pKVM.
We (Qualcomm) have been trying to resume Gunyah upstreaming [1] efforts for some time but the path to re-use guest_memfd is not straight forward as guest_memfd is tightly coupled with KVM. While the efforts to use it for pKVM is pending and refactoring to make it use outside KVM is not happening anytime soon, we plan to send Gunyah series similar to how this series is dealt with pages lent/donated to the Guest. Please let us know if you have any suggestions/comments for us.
[1]
https://lore.kernel.org/all/[email protected]/
Thanks,
Pavan
Will Deacon
maintainerQuoted 34 lines · Pavan Kondeti
Hi Will,
Quoted 16 lines · Will Deacon
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for
guest_memfd()but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while theguest_memfd()work continues to evolve.Quoted 12 lines
Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer
Kconfigoption to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.Good to see Protected VM support in upstream w/ pKVM.
Quoted 7 lines
We (Qualcomm) have been trying to resume Gunyah upstreaming [1] efforts for some time but the path to re-use
guest_memfdis not straight forward asguest_memfdis tightly coupled with KVM. While the efforts to use it for pKVM is pending and refactoring to make it use outside KVM is not happening anytime soon, we plan to send Gunyah series similar to how this series is dealt with pages lent/donated to the Guest. Please let us know if you have any suggestions/comments for us.
The major problem I see with this is that the host/hyp interface for handling stage-2 faults is internal to pKVM. The exception is injected back into the host using a funky ESR encoding and the hypercall used to forcefully reclaim the page is not ABI. I have no appetite for standardising these mechanisms (the flexibility is one of pKVM's big advantages) but I also do not want to complicate EL1 fault handling path with hypervisor-specific crap that we have to maintain forever.
Will
Quoted 43 lines · Will Deacon
Quoted 34 lines · Pavan Kondeti
Hi Will,
Quoted 16 lines · Will Deacon
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for
guest_memfd()but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while theguest_memfd()work continues to evolve.Quoted 12 lines
Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer
Kconfigoption to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.Good to see Protected VM support in upstream w/ pKVM.
Quoted 7 lines
We (Qualcomm) have been trying to resume Gunyah upstreaming [1] efforts for some time but the path to re-use
guest_memfdis not straight forward asguest_memfdis tightly coupled with KVM. While the efforts to use it for pKVM is pending and refactoring to make it use outside KVM is not happening anytime soon, we plan to send Gunyah series similar to how this series is dealt with pages lent/donated to the Guest. Please let us know if you have any suggestions/comments for us.
Quoted 7 lines
The major problem I see with this is that the host/hyp interface for handling stage-2 faults is internal to pKVM. The exception is injected back into the host using a funky ESR encoding and the hypercall used to forcefully reclaim the page is not ABI. I have no appetite for standardising these mechanisms (the flexibility is one of pKVM's big advantages) but I also do not want to complicate EL1 fault handling path with hypervisor-specific crap that we have to maintain forever.
Thanks Will for the feedback. Agree that we don't want to sprinkle Gunyah specific checks in fault handling code. Do we need to handle anything apart from
(a) user space access a memory that is lent to the guest. Gunyah will inject Synchronous External Abort and the offending process will be killed.
(b) For kernel access, we need to unmap the memory at S1 while lending it to the guest. Earlier, Elliot attempted this with [1]. I am thinking we can leverage from Direct map removal work done for guest_memfd w/o really using it :-) . I am hoping [2] can be made available for Gunyah module as well.
For the (b) problem above, pKVM takes a different route in upstream i.e pkvm_force_reclaim_guest_page(). I believe this avoids kernel panic at the expense of memory corruption in the guest. correct?
Thanks,
Pavan
[1]
https://lore.kernel.org/all/[email protected]/
[2]
https://lore.kernel.org/all/[email protected]/
Quoted 65 lines · Pavan Kondeti
Quoted 43 lines · Will Deacon
Quoted 34 lines · Pavan Kondeti
Hi Will,
Quoted 16 lines · Will Deacon
Hi folks,
Although pKVM has been shipping in Android kernels for a while now, protected guest (pVM) support has been somewhat languishing upstream. This has partly been because we've been waiting for
guest_memfd()but also because it hasn't been clear how to expose pVMs to userspace (which is necessary for testing) without getting everything in place beforehand. This has led to frustration on both sides of the fence [1] and so this patch series attempts to get things moving again by exposing pVM features in an incremental fashion based on top of anonymous memory, which is what we have been using in Android. The big difference between this series and the Android implementation is the graceful handling of host stage-2 faults arising from accesses made using kernel mappings. The hope is that this will unblock pKVM upstreaming efforts while theguest_memfd()work continues to evolve.Quoted 12 lines
Specifically, this patch series implements support for protected guest memory with pKVM, where pages are unmapped from the host as they are faulted into the guest and can be shared back from the guest using pKVM hypercalls. Protected guests are created using a new machine type identifier and can be booted to a shell using the kvmtool patches available at [2], which finally means that we are able to test the pVM logic in pKVM. Since this is an incremental step towards full isolation from the host (for example, the CPU register state and DMA accesses are not yet isolated), creating a pVM requires a developer
Kconfigoption to be enabled in addition to booting with 'kvm-arm.mode=protected' and results in a kernel taint.Good to see Protected VM support in upstream w/ pKVM.
Quoted 7 lines
We (Qualcomm) have been trying to resume Gunyah upstreaming [1] efforts for some time but the path to re-use
guest_memfdis not straight forward asguest_memfdis tightly coupled with KVM. While the efforts to use it for pKVM is pending and refactoring to make it use outside KVM is not happening anytime soon, we plan to send Gunyah series similar to how this series is dealt with pages lent/donated to the Guest. Please let us know if you have any suggestions/comments for us.Quoted 7 lines
The major problem I see with this is that the host/hyp interface for handling stage-2 faults is internal to pKVM. The exception is injected back into the host using a funky ESR encoding and the hypercall used to forcefully reclaim the page is not ABI. I have no appetite for standardising these mechanisms (the flexibility is one of pKVM's big advantages) but I also do not want to complicate EL1 fault handling path with hypervisor-specific crap that we have to maintain forever.
Thanks Will for the feedback. Agree that we don't want to sprinkle Gunyah specific checks in fault handling code. Do we need to handle anything apart from
(a) user space access a memory that is lent to the guest. Gunyah will inject Synchronous External Abort and the offending process will be killed.
(b) For kernel access, we need to unmap the memory at S1 while lending it to the guest. Earlier, Elliot attempted this with [1]. I am thinking we can leverage from Direct map removal work done for
guest_memfdw/o really using it :-) . I am hoping [2] can be made available for Gunyah module as well.
Quoted 3 lines
For the (b) problem above, pKVM takes a different route in upstream i.e
pkvm_force_reclaim_guest_page(). I believe this avoids kernel panic at the expense of memory corruption in the guest. correct?
sorry, not memory corruption but returning -EFAULT to VCPU_RUN ioctl which probably make VMM to kill the VM.
Thanks,
Pavan