The time to switch a VM to Secure-VM, increases by the size of the VM.
A 100GB VM takes about 7minutes. This is unacceptable. This linear
increase is caused by a suboptimal behavior by the Ultravisor and the
Hypervisor. The Ultravisor unnecessarily migrates all the GFN of the
VM from normal-memory to secure-memory. It has to just migrate the
necessary and sufficient GFNs.
However when the optimization is incorporated in the Ultravisor, the
Hypervisor starts misbehaving. The Hypervisor has a inbuilt assumption
that the Ultravisor will explicitly request to migrate, each and every
GFN of the VM. If only necessary and sufficient GFNs are requested for
migration, the Hypervisor continues to manage the remaining GFNs as
normal GFNs. This leads to memory corruption; manifested
consistently when the SVM reboots.
The same is true, when a memory slot is hotplugged into a SVM. The
Hypervisor expects the ultravisor to request migration of all GFNs to
secure-GFN. But the hypervisor cannot handle any H_SVM_PAGE_IN
requests from the Ultravisor, done in the context of
UV_REGISTER_MEM_SLOT ucall. This problem manifests as random errors
in the SVM, when a memory-slot is hotplugged.
This patch series automatically migrates the non-migrated pages of a
SVM, and thus solves the problem.
Testing: Passed rigorous testing using various sized SVMs.
Changelog:
v5: . This patch series includes Laurent's fix for memory hotplug/unplug
. drop pages first and then delete the memslot. Otherwise
the memslot does not get cleanly deleted, causing
problems during reboot.
. recreatable through the following set of commands
. device_add pc-dimm,id=dimm1,memdev=mem1
. device_del dimm1
. device_add pc-dimm,id=dimm1,memdev=mem1
Further incorporates comments from Bharata:
. fix for off-by-one while disabling migration.
. code-reorganized to maximize sharing in init_start path
and in memory-hotplug path
. locking adjustments in mass-page migration during H_SVM_INIT_DONE.
. improved recovery on error paths.
. additional comments in the code for better understanding.
. removed the retry-on-migration-failure code.
. re-added the initial patch that adjust some prototype to overcome
a git problem, where it messes up the code context. Had
accidently dropped the patch in the last version.
v4: . Incorported Bharata's comments:
- Optimization -- replace write mmap semaphore with read mmap semphore.
- disable page-merge during memory hotplug.
- rearranged the patches. consolidated the page-migration-retry logic
in a single patch.
v3: . Optimized the page-migration retry-logic.
. Relax and relinquish the cpu regularly while bulk migrating
the non-migrated pages. This issue was causing soft-lockups.
Fixed it.
. Added a new patch, to retry page-migration a couple of times
before returning H_BUSY in H_SVM_PAGE_IN. This issue was
seen a few times in a 24hour continuous reboot test of the SVMs.
v2: . fixed a bug observed by Laurent. The state of the GFN's associated
with Secure-VMs were not reset during memslot flush.
. Re-organized the code, for easier review.
. Better description of the patch series.
v1: . fixed a bug observed by Bharata. Pages that where paged-in and later
paged-out must also be skipped from migration during H_SVM_INIT_DONE.
Laurent Dufour (3):
KVM: PPC: Book3S HV: migrate hot plugged memory
KVM: PPC: Book3S HV: move kvmppc_svm_page_out up
KVM: PPC: Book3S HV: rework secure mem slot dropping
Ram Pai (4):
KVM: PPC: Book3S HV: Fix function definition in book3s_hv_uvmem.c
KVM: PPC: Book3S HV: Disable page merging in H_SVM_INIT_START
KVM: PPC: Book3S HV: track the state GFNs associated with secure VMs
KVM: PPC: Book3S HV: in H_SVM_INIT_DONE, migrate remaining normal-GFNs
to secure-GFNs.
Documentation/powerpc/ultravisor.rst | 3 +
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 16 +
arch/powerpc/kvm/book3s_hv.c | 10 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 690 +++++++++++++++++++++-------
4 files changed, 548 insertions(+), 171 deletions(-)
--
1.8.3.1
Page-merging of pages in memory-slots associated with a Secure VM,
is disabled in H_SVM_PAGE_IN handler.
This operation should have been done the much earlier; the moment the VM
is initiated for secure-transition. Delaying this operation, increases
the probability for those pages to acquire new references , making it
impossible to migrate those pages in H_SVM_PAGE_IN handler.
Disable page-migration in H_SVM_INIT_START handling.
Reviewed-by: Bharata B Rao <redacted>
Signed-off-by: Ram Pai <redacted>
---
Documentation/powerpc/ultravisor.rst | 1 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 123 +++++++++++++++++++++++++----------
2 files changed, 89 insertions(+), 35 deletions(-)
@@ -895,6 +895,7 @@ Return values One of the following values:* H_SUCCESS on success.+* H_STATE if the VM is not in a position to switch to secure. Description ~~~~~~~~~~~
During the life of SVM, its GFNs transition through normal, secure and
shared states. Since the kernel does not track GFNs that are shared, it
is not possible to disambiguate a shared GFN from a GFN whose PFN has
not yet been migrated to a secure-PFN. Also it is not possible to
disambiguate a secure-GFN from a GFN whose GFN has been pagedout from
the ultravisor.
The ability to identify the state of a GFN is needed to skip migration
of its PFN to secure-PFN during ESM transition.
The code is re-organized to track the states of a GFN as explained
below.
************************************************************************
1. States of a GFN
---------------
The GFN can be in one of the following states.
(a) Secure - The GFN is secure. The GFN is associated with
a Secure VM, the contents of the GFN is not accessible
to the Hypervisor. This GFN can be backed by a secure-PFN,
or can be backed by a normal-PFN with contents encrypted.
The former is true when the GFN is paged-in into the
ultravisor. The latter is true when the GFN is paged-out
of the ultravisor.
(b) Shared - The GFN is shared. The GFN is associated with a
a secure VM. The contents of the GFN is accessible to
Hypervisor. This GFN is backed by a normal-PFN and its
content is un-encrypted.
(c) Normal - The GFN is a normal. The GFN is associated with
a normal VM. The contents of the GFN is accesible to
the Hypervisor. Its content is never encrypted.
2. States of a VM.
---------------
(a) Normal VM: A VM whose contents are always accessible to
the hypervisor. All its GFNs are normal-GFNs.
(b) Secure VM: A VM whose contents are not accessible to the
hypervisor without the VM's consent. Its GFNs are
either Shared-GFN or Secure-GFNs.
(c) Transient VM: A Normal VM that is transitioning to secure VM.
The transition starts on successful return of
H_SVM_INIT_START, and ends on successful return
of H_SVM_INIT_DONE. This transient VM, can have GFNs
in any of the three states; i.e Secure-GFN, Shared-GFN,
and Normal-GFN. The VM never executes in this state
in supervisor-mode.
3. Memory slot State.
------------------
The state of a memory slot mirrors the state of the
VM the memory slot is associated with.
4. VM State transition.
--------------------
A VM always starts in Normal Mode.
H_SVM_INIT_START moves the VM into transient state. During this
time the Ultravisor may request some of its GFNs to be shared or
secured. So its GFNs can be in one of the three GFN states.
H_SVM_INIT_DONE moves the VM entirely from transient state to
secure-state. At this point any left-over normal-GFNs are
transitioned to Secure-GFN.
H_SVM_INIT_ABORT moves the transient VM back to normal VM.
All its GFNs are moved to Normal-GFNs.
UV_TERMINATE transitions the secure-VM back to normal-VM. All
the secure-GFN and shared-GFNs are tranistioned to normal-GFN
Note: The contents of the normal-GFN is undefined at this point.
5. GFN state implementation:
-------------------------
Secure GFN is associated with a secure-PFN; also called uvmem_pfn,
when the GFN is paged-in. Its pfn[] has KVMPPC_GFN_UVMEM_PFN flag
set, and contains the value of the secure-PFN.
It is associated with a normal-PFN; also called mem_pfn, when
the GFN is pagedout. Its pfn[] has KVMPPC_GFN_MEM_PFN flag set.
The value of the normal-PFN is not tracked.
Shared GFN is associated with a normal-PFN. Its pfn[] has
KVMPPC_UVMEM_SHARED_PFN flag set. The value of the normal-PFN
is not tracked.
Normal GFN is associated with normal-PFN. Its pfn[] has
no flag set. The value of the normal-PFN is not tracked.
6. Life cycle of a GFN
--------------------
--------------------------------------------------------------
| | Share | Unshare | SVM |H_SVM_INIT_DONE|
| |operation |operation | abort/ | |
| | | | terminate | |
-------------------------------------------------------------
| | | | | |
| Secure | Shared | Secure |Normal |Secure |
| | | | | |
| Shared | Shared | Secure |Normal |Shared |
| | | | | |
| Normal | Shared | Secure |Normal |Secure |
--------------------------------------------------------------
7. Life cycle of a VM
--------------------
--------------------------------------------------------------------
| | start | H_SVM_ |H_SVM_ |H_SVM_ |UV_SVM_ |
| | VM |INIT_START|INIT_DONE|INIT_ABORT |TERMINATE |
| | | | | | |
--------- ----------------------------------------------------------
| | | | | | |
| Normal | Normal | Transient|Error |Error |Normal |
| | | | | | |
| Secure | Error | Error |Error |Error |Normal |
| | | | | | |
|Transient| N/A | Error |Secure |Normal |Normal |
--------------------------------------------------------------------
************************************************************************
Cc: Paul Mackerras <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <redacted>
Cc: Aneesh Kumar K.V <redacted>
Cc: Sukadev Bhattiprolu <redacted>
Cc: Laurent Dufour <redacted>
Cc: Thiago Jung Bauermann <redacted>
Cc: David Gibson <redacted>
Cc: Claudio Carvalho <redacted>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Bharata B Rao <redacted>
Reviewed-by: Thiago Jung Bauermann <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 191 +++++++++++++++++++++++++++++++++----
1 file changed, 172 insertions(+), 19 deletions(-)
@@ -172,24 +292,41 @@ static void kvmppc_uvmem_pfn_insert(unsigned long gfn, unsigned long uvmem_pfn,if(gfn>=p->base_pfn&&gfn<p->base_pfn+p->nr_pfns){unsignedlongindex=gfn-p->base_pfn;-p->pfns[index]=uvmem_pfn|KVMPPC_UVMEM_PFN;+if(flag==KVMPPC_GFN_UVMEM_PFN)+p->pfns[index]=uvmem_pfn|flag;+else+p->pfns[index]=flag;return;}}}-staticvoidkvmppc_uvmem_pfn_remove(unsignedlonggfn,structkvm*kvm)+/* mark the GFN as secure-GFN associated with @uvmem pfn device-PFN. */+staticvoidkvmppc_gfn_secure_uvmem_pfn(unsignedlonggfn,+unsignedlonguvmem_pfn,structkvm*kvm){-structkvmppc_uvmem_slot*p;+kvmppc_mark_gfn(gfn,kvm,KVMPPC_GFN_UVMEM_PFN,uvmem_pfn);+}-list_for_each_entry(p,&kvm->arch.uvmem_pfns,list){-if(gfn>=p->base_pfn&&gfn<p->base_pfn+p->nr_pfns){-p->pfns[gfn-p->base_pfn]=0;-return;-}-}+/* mark the GFN as secure-GFN associated with a memory-PFN. */+staticvoidkvmppc_gfn_secure_mem_pfn(unsignedlonggfn,structkvm*kvm)+{+kvmppc_mark_gfn(gfn,kvm,KVMPPC_GFN_MEM_PFN,0);}+/* mark the GFN as a shared GFN. */+staticvoidkvmppc_gfn_shared(unsignedlonggfn,structkvm*kvm)+{+kvmppc_mark_gfn(gfn,kvm,KVMPPC_GFN_SHARED,0);+}++/* mark the GFN as a non-existent GFN. */+staticvoidkvmppc_gfn_remove(unsignedlonggfn,structkvm*kvm)+{+kvmppc_mark_gfn(gfn,kvm,0,0);+}++/* return true, if the GFN is a secure-GFN backed by a secure-PFN */staticboolkvmppc_gfn_is_uvmem_pfn(unsignedlonggfn,structkvm*kvm,unsignedlong*uvmem_pfn){
@@ -525,6 +664,11 @@ static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa,uvmem_page=pfn_to_page(uvmem_pfn);pvt=uvmem_page->zone_device_data;pvt->skip_page_out=true;+/*+*donotdroptheGFN.ItisavalidGFN+*thatistransitionedtoasharedGFN.+*/+pvt->remove_gfn=false;}retry:
@@ -538,12 +682,16 @@ static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa,uvmem_page=pfn_to_page(uvmem_pfn);pvt=uvmem_page->zone_device_data;pvt->skip_page_out=true;+pvt->remove_gfn=false;/* it continues to be a valid GFN */kvm_release_pfn_clean(pfn);gotoretry;}-if(!uv_page_in(kvm->arch.lpid,pfn<<page_shift,gpa,0,page_shift))+if(!uv_page_in(kvm->arch.lpid,pfn<<page_shift,gpa,0,+page_shift)){+kvmppc_gfn_shared(gfn,kvm);ret=H_SUCCESS;+}kvm_release_pfn_clean(pfn);mutex_unlock(&kvm->arch.uvmem_lock);out:
@@ -599,6 +747,7 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,if(!kvmppc_svm_page_in(vma,start,end,gpa,kvm,page_shift))ret=H_SUCCESS;+out_unlock:mutex_unlock(&kvm->arch.uvmem_lock);out:
The Ultravisor is expected to explicitly call H_SVM_PAGE_IN for all the
pages of the SVM before calling H_SVM_INIT_DONE. This causes a huge
delay in tranistioning the VM to SVM. The Ultravisor is only interested
in the pages that contain the kernel, initrd and other important data
structures. The rest contain throw-away content.
However if not all pages are requested by the Ultravisor, the Hypervisor
continues to consider the GFNs corresponding to the non-requested pages
as normal GFNs. This can lead to data-corruption and undefined behavior.
In H_SVM_INIT_DONE handler, move all the PFNs associated with the SVM's
GFNs to secure-PFNs. Skip the GFNs that are already Paged-in or Shared
or Paged-in followed by a Paged-out.
Cc: Paul Mackerras <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <redacted>
Cc: Aneesh Kumar K.V <redacted>
Cc: Sukadev Bhattiprolu <redacted>
Cc: Laurent Dufour <redacted>
Cc: Thiago Jung Bauermann <redacted>
Cc: David Gibson <redacted>
Cc: Claudio Carvalho <redacted>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Ram Pai <redacted>
---
Documentation/powerpc/ultravisor.rst | 2 +
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 2 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 136 +++++++++++++++++++++++++---
3 files changed, 127 insertions(+), 13 deletions(-)
@@ -934,6 +934,8 @@ Return values* H_UNSUPPORTED if called from the wrong context (e.g. from an SVM or before an H_SVM_INIT_START hypercall).+* H_STATE if the hypervisor could not successfully+ transition the VM to Secure VM. Description ~~~~~~~~~~~
@@ -462,12 +498,40 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)unsignedlongkvmppc_h_svm_init_done(structkvm*kvm){+structkvm_memslots*slots;+structkvm_memory_slot*memslot;+intsrcu_idx;+longret=H_SUCCESS;+if(!(kvm->arch.secure_guest&KVMPPC_SECURE_INIT_START))returnH_UNSUPPORTED;+/* migrate any unmoved normal pfn to device pfns*/+srcu_idx=srcu_read_lock(&kvm->srcu);+slots=kvm_memslots(kvm);+kvm_for_each_memslot(memslot,slots){+ret=kvmppc_uv_migrate_mem_slot(kvm,memslot);+if(ret){+/*+*Thepageswillremaintransitioned.+*Itsthecallersresponsibilityto+*terminatetheVM,whichwillundo+*allstateoftheVM.Tillthen+*thisVMisinaerroneousstate.+*ItsKVMPPC_SECURE_INIT_DONEwill+*remainunset.+*/+ret=H_STATE;+gotoout;+}+}+kvm->arch.secure_guest|=KVMPPC_SECURE_INIT_DONE;pr_info("LPID %d went secure\n",kvm->arch.lpid);-returnH_SUCCESS;++out:+srcu_read_unlock(&kvm->srcu,srcu_idx);+returnret;}/*
@@ -624,11 +690,16 @@ static int kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,gotoout_finalize;}-pfn=*mig.src>>MIGRATE_PFN_SHIFT;-spage=migrate_pfn_to_page(*mig.src);-if(spage)-uv_page_in(kvm->arch.lpid,pfn<<page_shift,gpa,0,-page_shift);+if(pagein){+pfn=*mig.src>>MIGRATE_PFN_SHIFT;+spage=migrate_pfn_to_page(*mig.src);+if(spage){+ret=uv_page_in(kvm->arch.lpid,pfn<<page_shift,+gpa,0,page_shift);+if(ret)+gotoout_finalize;+}+}*mig.dst=migrate_pfn(page_to_pfn(dpage))|MIGRATE_PFN_LOCKED;migrate_vma_pages(&mig);
@@ -637,6 +708,42 @@ static int kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,returnret;}+intkvmppc_uv_migrate_mem_slot(structkvm*kvm,+conststructkvm_memory_slot*memslot)+{+unsignedlonggfn=memslot->base_gfn;+structvm_area_struct*vma;+unsignedlongstart,end;+intret=0;++mmap_read_lock(kvm->mm);+mutex_lock(&kvm->arch.uvmem_lock);+while(kvmppc_next_nontransitioned_gfn(memslot,kvm,&gfn)){+ret=H_STATE;+start=gfn_to_hva(kvm,gfn);+if(kvm_is_error_hva(start))+break;++end=start+(1UL<<PAGE_SHIFT);+vma=find_vma_intersection(kvm->mm,start,end);+if(!vma||vma->vm_start>start||vma->vm_end<end)+break;++ret=kvmppc_svm_page_in(vma,start,end,+(gfn<<PAGE_SHIFT),kvm,PAGE_SHIFT,false);+if(ret){+ret=H_STATE;+break;+}++/* relinquish the cpu if needed */+cond_resched();+}+mutex_unlock(&kvm->arch.uvmem_lock);+mmap_read_unlock(kvm->mm);+returnret;+}+/**SharesthepagewithHV,thusmakingitanormalpage.*
@@ -745,8 +852,11 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,if(!vma||vma->vm_start>start||vma->vm_end<end)gotoout_unlock;-if(!kvmppc_svm_page_in(vma,start,end,gpa,kvm,page_shift))-ret=H_SUCCESS;+if(kvmppc_svm_page_in(vma,start,end,gpa,kvm,page_shift,+true))+gotoout_unlock;++ret=H_SUCCESS;out_unlock:mutex_unlock(&kvm->arch.uvmem_lock);
From: Laurent Dufour <redacted>
When a memory slot is hot plugged to a SVM, PFNs associated with the
GFNs in that slot must be migrated to the secure-PFNs, aka device-PFNs.
Call kvmppc_uv_migrate_mem_slot() to accomplish this.
Disable page-merge for all pages in the memory slot.
Signed-off-by: Ram Pai <redacted>
[rearranged the code, and modified the commit log]
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 14 ++++++++++++++
arch/powerpc/kvm/book3s_hv.c | 10 ++--------
arch/powerpc/kvm/book3s_hv_uvmem.c | 23 +++++++++++++++++++----
3 files changed, 35 insertions(+), 12 deletions(-)
From: Laurent Dufour <redacted>
kvmppc_svm_page_out() will need to be called by kvmppc_uvmem_drop_pages()
so move it upper in this file.
Furthermore it will be interesting to call this function when already
holding the kvm->arch.uvmem_lock, so prefix the original function with __
and remove the locking in it, and introduce a wrapper which call that
function with the lock held.
There is no functional change.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 166 ++++++++++++++++++++-----------------
1 file changed, 90 insertions(+), 76 deletions(-)
@@ -535,6 +535,96 @@ unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)}/*+*ProvisionanewpageonHVsideandcopyoverthecontents+*fromsecurememoryusingUV_PAGE_OUTuvcall.+*Callermustheldkvm->arch.uvmem_lock.+*/+staticint__kvmppc_svm_page_out(structvm_area_struct*vma,+unsignedlongstart,+unsignedlongend,unsignedlongpage_shift,+structkvm*kvm,unsignedlonggpa)+{+unsignedlongsrc_pfn,dst_pfn=0;+structmigrate_vmamig;+structpage*dpage,*spage;+structkvmppc_uvmem_page_pvt*pvt;+unsignedlongpfn;+intret=U_SUCCESS;++memset(&mig,0,sizeof(mig));+mig.vma=vma;+mig.start=start;+mig.end=end;+mig.src=&src_pfn;+mig.dst=&dst_pfn;+mig.src_owner=&kvmppc_uvmem_pgmap;++/* The requested page is already paged-out, nothing to do */+if(!kvmppc_gfn_is_uvmem_pfn(gpa>>page_shift,kvm,NULL))+returnret;++ret=migrate_vma_setup(&mig);+if(ret)+return-1;++spage=migrate_pfn_to_page(*mig.src);+if(!spage||!(*mig.src&MIGRATE_PFN_MIGRATE))+gotoout_finalize;++if(!is_zone_device_page(spage))+gotoout_finalize;++dpage=alloc_page_vma(GFP_HIGHUSER,vma,start);+if(!dpage){+ret=-1;+gotoout_finalize;+}++lock_page(dpage);+pvt=spage->zone_device_data;+pfn=page_to_pfn(dpage);++/*+*Thisfunctionisusedintwocases:+*-WhenHVtouchesasecurepage,forwhichwedoUV_PAGE_OUT+*-Whenasecurepageisconvertedtosharedpage,we*get*+*thepagetoessentiallyunmapthedevicepage.Inthis+*caseweskippage-out.+*/+if(!pvt->skip_page_out)+ret=uv_page_out(kvm->arch.lpid,pfn<<page_shift,+gpa,0,page_shift);++if(ret==U_SUCCESS)+*mig.dst=migrate_pfn(pfn)|MIGRATE_PFN_LOCKED;+else{+unlock_page(dpage);+__free_page(dpage);+gotoout_finalize;+}++migrate_vma_pages(&mig);++out_finalize:+migrate_vma_finalize(&mig);+returnret;+}++staticinlineintkvmppc_svm_page_out(structvm_area_struct*vma,+unsignedlongstart,unsignedlongend,+unsignedlongpage_shift,+structkvm*kvm,unsignedlonggpa)+{+intret;++mutex_lock(&kvm->arch.uvmem_lock);+ret=__kvmppc_svm_page_out(vma,start,end,page_shift,kvm,gpa);+mutex_unlock(&kvm->arch.uvmem_lock);++returnret;+}++/**Dropdevicepagesthatwemaintainforthesecureguest**WefirstmarkthepagestobeskippedfromUV_PAGE_OUTwhenthere
@@ -866,82 +956,6 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,returnret;}-/*-*ProvisionanewpageonHVsideandcopyoverthecontents-*fromsecurememoryusingUV_PAGE_OUTuvcall.-*/-staticintkvmppc_svm_page_out(structvm_area_struct*vma,-unsignedlongstart,-unsignedlongend,unsignedlongpage_shift,-structkvm*kvm,unsignedlonggpa)-{-unsignedlongsrc_pfn,dst_pfn=0;-structmigrate_vmamig;-structpage*dpage,*spage;-structkvmppc_uvmem_page_pvt*pvt;-unsignedlongpfn;-intret=U_SUCCESS;--memset(&mig,0,sizeof(mig));-mig.vma=vma;-mig.start=start;-mig.end=end;-mig.src=&src_pfn;-mig.dst=&dst_pfn;-mig.src_owner=&kvmppc_uvmem_pgmap;--mutex_lock(&kvm->arch.uvmem_lock);-/* The requested page is already paged-out, nothing to do */-if(!kvmppc_gfn_is_uvmem_pfn(gpa>>page_shift,kvm,NULL))-gotoout;--ret=migrate_vma_setup(&mig);-if(ret)-gotoout;--spage=migrate_pfn_to_page(*mig.src);-if(!spage||!(*mig.src&MIGRATE_PFN_MIGRATE))-gotoout_finalize;--if(!is_zone_device_page(spage))-gotoout_finalize;--dpage=alloc_page_vma(GFP_HIGHUSER,vma,start);-if(!dpage){-ret=-1;-gotoout_finalize;-}--lock_page(dpage);-pvt=spage->zone_device_data;-pfn=page_to_pfn(dpage);--/*-*Thisfunctionisusedintwocases:-*-WhenHVtouchesasecurepage,forwhichwedoUV_PAGE_OUT-*-Whenasecurepageisconvertedtosharedpage,we*get*-*thepagetoessentiallyunmapthedevicepage.Inthis-*caseweskippage-out.-*/-if(!pvt->skip_page_out)-ret=uv_page_out(kvm->arch.lpid,pfn<<page_shift,-gpa,0,page_shift);--if(ret==U_SUCCESS)-*mig.dst=migrate_pfn(pfn)|MIGRATE_PFN_LOCKED;-else{-unlock_page(dpage);-__free_page(dpage);-gotoout_finalize;-}--migrate_vma_pages(&mig);-out_finalize:-migrate_vma_finalize(&mig);-out:-mutex_unlock(&kvm->arch.uvmem_lock);-returnret;-}/**FaulthandlercallbackthatgetscalledwhenHVtouchesanypagethat
From: Laurent Dufour <redacted>
When a secure memslot is dropped, all the pages backed in the secure
device (aka really backed by secure memory by the Ultravisor)
should be paged out to a normal page. Previously, this was
achieved by triggering the page fault mechanism which is calling
kvmppc_svm_page_out() on each pages.
This can't work when hot unplugging a memory slot because the memory
slot is flagged as invalid and gfn_to_pfn() is then not trying to access
the page, so the page fault mechanism is not triggered.
Since the final goal is to make a call to kvmppc_svm_page_out() it seems
simpler to call directly instead of triggering such a mechanism. This
way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
memslot.
Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
the call to __kvmppc_svm_page_out() is made. As
__kvmppc_svm_page_out needs the vma pointer to migrate the pages,
the VMA is fetched in a lazy way, to not trigger find_vma() all
the time. In addition, the mmap_sem is held in read mode during
that time, not in write mode since the virual memory layout is not
impacted, and kvm->arch.uvmem_lock prevents concurrent operation
on the secure device.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
[modified the changelog description]
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 54 ++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 17 deletions(-)
@@ -632,35 +632,55 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,*faultonthem,dofaulttimemigrationtoreplacethedevicePTEsin*QEMUpagetablewithnormalPTEsfromnewlyallocatedpages.*/-voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*free,+voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*slot,structkvm*kvm,boolskip_page_out){inti;structkvmppc_uvmem_page_pvt*pvt;-unsignedlongpfn,uvmem_pfn;-unsignedlonggfn=free->base_gfn;+structpage*uvmem_page;+structvm_area_struct*vma=NULL;+unsignedlonguvmem_pfn,gfn;+unsignedlongaddr,end;++mmap_read_lock(kvm->mm);++addr=slot->userspace_addr;+end=addr+(slot->npages*PAGE_SIZE);-for(i=free->npages;i;--i,++gfn){-structpage*uvmem_page;+gfn=slot->base_gfn;+for(i=slot->npages;i;--i,++gfn,addr+=PAGE_SIZE){++/* Fetch the VMA if addr is not in the latest fetched one */+if(!vma||(addr<vma->vm_start||addr>=vma->vm_end)){+vma=find_vma_intersection(kvm->mm,addr,end);+if(!vma||+vma->vm_start>addr||vma->vm_end<end){+pr_err("Can't find VMA for gfn:0x%lx\n",gfn);+break;+}+}mutex_lock(&kvm->arch.uvmem_lock);-if(!kvmppc_gfn_is_uvmem_pfn(gfn,kvm,&uvmem_pfn)){++if(kvmppc_gfn_is_uvmem_pfn(gfn,kvm,&uvmem_pfn)){+uvmem_page=pfn_to_page(uvmem_pfn);+pvt=uvmem_page->zone_device_data;+pvt->skip_page_out=skip_page_out;+pvt->remove_gfn=true;++if(__kvmppc_svm_page_out(vma,addr,addr+PAGE_SIZE,+PAGE_SHIFT,kvm,pvt->gpa))+pr_err("Can't page out gpa:0x%lx addr:0x%lx\n",+pvt->gpa,addr);+}else{+/* Remove the shared flag if any */kvmppc_gfn_remove(gfn,kvm);-mutex_unlock(&kvm->arch.uvmem_lock);-continue;}-uvmem_page=pfn_to_page(uvmem_pfn);-pvt=uvmem_page->zone_device_data;-pvt->skip_page_out=skip_page_out;-pvt->remove_gfn=true;mutex_unlock(&kvm->arch.uvmem_lock);--pfn=gfn_to_pfn(kvm,gfn);-if(is_error_noslot_pfn(pfn))-continue;-kvm_release_pfn_clean(pfn);}++mmap_read_unlock(kvm->mm);}unsignedlongkvmppc_h_svm_init_abort(structkvm*kvm)
From: Bharata B Rao <hidden> Date: 2020-07-24 03:06:46
On Thu, Jul 23, 2020 at 01:07:24PM -0700, Ram Pai wrote:
quoted hunk
From: Laurent Dufour <redacted>
When a secure memslot is dropped, all the pages backed in the secure
device (aka really backed by secure memory by the Ultravisor)
should be paged out to a normal page. Previously, this was
achieved by triggering the page fault mechanism which is calling
kvmppc_svm_page_out() on each pages.
This can't work when hot unplugging a memory slot because the memory
slot is flagged as invalid and gfn_to_pfn() is then not trying to access
the page, so the page fault mechanism is not triggered.
Since the final goal is to make a call to kvmppc_svm_page_out() it seems
simpler to call directly instead of triggering such a mechanism. This
way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
memslot.
Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
the call to __kvmppc_svm_page_out() is made. As
__kvmppc_svm_page_out needs the vma pointer to migrate the pages,
the VMA is fetched in a lazy way, to not trigger find_vma() all
the time. In addition, the mmap_sem is held in read mode during
that time, not in write mode since the virual memory layout is not
impacted, and kvm->arch.uvmem_lock prevents concurrent operation
on the secure device.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
[modified the changelog description]
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 54 ++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 17 deletions(-)
@@ -632,35 +632,55 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,*faultonthem,dofaulttimemigrationtoreplacethedevicePTEsin*QEMUpagetablewithnormalPTEsfromnewlyallocatedpages.*/-voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*free,+voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*slot,structkvm*kvm,boolskip_page_out){inti;structkvmppc_uvmem_page_pvt*pvt;-unsignedlongpfn,uvmem_pfn;-unsignedlonggfn=free->base_gfn;+structpage*uvmem_page;+structvm_area_struct*vma=NULL;+unsignedlonguvmem_pfn,gfn;+unsignedlongaddr,end;++mmap_read_lock(kvm->mm);++addr=slot->userspace_addr;+end=addr+(slot->npages*PAGE_SIZE);-for(i=free->npages;i;--i,++gfn){-structpage*uvmem_page;+gfn=slot->base_gfn;+for(i=slot->npages;i;--i,++gfn,addr+=PAGE_SIZE){++/* Fetch the VMA if addr is not in the latest fetched one */+if(!vma||(addr<vma->vm_start||addr>=vma->vm_end)){+vma=find_vma_intersection(kvm->mm,addr,end);+if(!vma||+vma->vm_start>addr||vma->vm_end<end){+pr_err("Can't find VMA for gfn:0x%lx\n",gfn);+break;+}
There is a potential issue with the boundary condition check here
which I discussed with Laurent yesterday. Guess he hasn't gotten around
to look at it yet.
Regards,
Bharata.
From: Bharata B Rao <hidden> Date: 2020-07-24 04:29:58
On Thu, Jul 23, 2020 at 01:07:21PM -0700, Ram Pai wrote:
quoted hunk
The Ultravisor is expected to explicitly call H_SVM_PAGE_IN for all the
pages of the SVM before calling H_SVM_INIT_DONE. This causes a huge
delay in tranistioning the VM to SVM. The Ultravisor is only interested
in the pages that contain the kernel, initrd and other important data
structures. The rest contain throw-away content.
However if not all pages are requested by the Ultravisor, the Hypervisor
continues to consider the GFNs corresponding to the non-requested pages
as normal GFNs. This can lead to data-corruption and undefined behavior.
In H_SVM_INIT_DONE handler, move all the PFNs associated with the SVM's
GFNs to secure-PFNs. Skip the GFNs that are already Paged-in or Shared
or Paged-in followed by a Paged-out.
Cc: Paul Mackerras <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <redacted>
Cc: Aneesh Kumar K.V <redacted>
Cc: Sukadev Bhattiprolu <redacted>
Cc: Laurent Dufour <redacted>
Cc: Thiago Jung Bauermann <redacted>
Cc: David Gibson <redacted>
Cc: Claudio Carvalho <redacted>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Ram Pai <redacted>
---
Documentation/powerpc/ultravisor.rst | 2 +
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 2 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 136 +++++++++++++++++++++++++---
3 files changed, 127 insertions(+), 13 deletions(-)
@@ -934,6 +934,8 @@ Return values* H_UNSUPPORTED if called from the wrong context (e.g. from an SVM or before an H_SVM_INIT_START hypercall).+* H_STATE if the hypervisor could not successfully+ transition the VM to Secure VM. Description ~~~~~~~~~~~
@@ -23,6 +23,8 @@ unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,unsignedlongkvmppc_h_svm_init_abort(structkvm*kvm);voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*free,structkvm*kvm,boolskip_page_out);+intkvmppc_uv_migrate_mem_slot(structkvm*kvm,+conststructkvm_memory_slot*memslot);
I still don't see why this be a global function. You should be able
to move around a few functions in book3s_hv_uvmem.c up/down and
satisfy the calling order dependencies.
Otherwise, Reviewed-by: Bharata B Rao [off-list ref]
On Thu, Jul 23, 2020 at 01:07:24PM -0700, Ram Pai wrote:
quoted
From: Laurent Dufour <redacted>
When a secure memslot is dropped, all the pages backed in the secure
device (aka really backed by secure memory by the Ultravisor)
should be paged out to a normal page. Previously, this was
achieved by triggering the page fault mechanism which is calling
kvmppc_svm_page_out() on each pages.
This can't work when hot unplugging a memory slot because the memory
slot is flagged as invalid and gfn_to_pfn() is then not trying to access
the page, so the page fault mechanism is not triggered.
Since the final goal is to make a call to kvmppc_svm_page_out() it seems
simpler to call directly instead of triggering such a mechanism. This
way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
memslot.
Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
the call to __kvmppc_svm_page_out() is made. As
__kvmppc_svm_page_out needs the vma pointer to migrate the pages,
the VMA is fetched in a lazy way, to not trigger find_vma() all
the time. In addition, the mmap_sem is held in read mode during
that time, not in write mode since the virual memory layout is not
impacted, and kvm->arch.uvmem_lock prevents concurrent operation
on the secure device.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
[modified the changelog description]
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 54 ++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 17 deletions(-)
@@ -632,35 +632,55 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,*faultonthem,dofaulttimemigrationtoreplacethedevicePTEsin*QEMUpagetablewithnormalPTEsfromnewlyallocatedpages.*/-voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*free,+voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*slot,structkvm*kvm,boolskip_page_out){inti;structkvmppc_uvmem_page_pvt*pvt;-unsignedlongpfn,uvmem_pfn;-unsignedlonggfn=free->base_gfn;+structpage*uvmem_page;+structvm_area_struct*vma=NULL;+unsignedlonguvmem_pfn,gfn;+unsignedlongaddr,end;++mmap_read_lock(kvm->mm);++addr=slot->userspace_addr;+end=addr+(slot->npages*PAGE_SIZE);-for(i=free->npages;i;--i,++gfn){-structpage*uvmem_page;+gfn=slot->base_gfn;+for(i=slot->npages;i;--i,++gfn,addr+=PAGE_SIZE){++/* Fetch the VMA if addr is not in the latest fetched one */+if(!vma||(addr<vma->vm_start||addr>=vma->vm_end)){+vma=find_vma_intersection(kvm->mm,addr,end);+if(!vma||+vma->vm_start>addr||vma->vm_end<end){+pr_err("Can't find VMA for gfn:0x%lx\n",gfn);+break;+}
There is a potential issue with the boundary condition check here
which I discussed with Laurent yesterday. Guess he hasn't gotten around
to look at it yet.
When a secure memslot is dropped, all the pages backed in the secure
device (aka really backed by secure memory by the Ultravisor)
should be paged out to a normal page. Previously, this was
achieved by triggering the page fault mechanism which is calling
kvmppc_svm_page_out() on each pages.
This can't work when hot unplugging a memory slot because the memory
slot is flagged as invalid and gfn_to_pfn() is then not trying to access
the page, so the page fault mechanism is not triggered.
Since the final goal is to make a call to kvmppc_svm_page_out() it seems
simpler to call directly instead of triggering such a mechanism. This
way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
memslot.
Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
the call to __kvmppc_svm_page_out() is made. As
__kvmppc_svm_page_out needs the vma pointer to migrate the pages,
the VMA is fetched in a lazy way, to not trigger find_vma() all
the time. In addition, the mmap_sem is held in read mode during
that time, not in write mode since the virual memory layout is not
impacted, and kvm->arch.uvmem_lock prevents concurrent operation
on the secure device.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
[modified the changelog description]
Signed-off-by: Laurent Dufour <redacted>
[modified check on the VMA in kvmppc_uvmem_drop_pages]
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 53 ++++++++++++++++++++----------
1 file changed, 36 insertions(+), 17 deletions(-)
@@ -632,35 +632,54 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,*faultonthem,dofaulttimemigrationtoreplacethedevicePTEsin*QEMUpagetablewithnormalPTEsfromnewlyallocatedpages.*/-voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*free,+voidkvmppc_uvmem_drop_pages(conststructkvm_memory_slot*slot,structkvm*kvm,boolskip_page_out){inti;structkvmppc_uvmem_page_pvt*pvt;-unsignedlongpfn,uvmem_pfn;-unsignedlonggfn=free->base_gfn;+structpage*uvmem_page;+structvm_area_struct*vma=NULL;+unsignedlonguvmem_pfn,gfn;+unsignedlongaddr,end;++mmap_read_lock(kvm->mm);++addr=slot->userspace_addr;+end=addr+(slot->npages*PAGE_SIZE);-for(i=free->npages;i;--i,++gfn){-structpage*uvmem_page;+gfn=slot->base_gfn;+for(i=slot->npages;i;--i,++gfn,addr+=PAGE_SIZE){++/* Fetch the VMA if addr is not in the latest fetched one */+if(!vma||addr>=vma->vm_end){+vma=find_vma_intersection(kvm->mm,addr,addr+1);+if(!vma){+pr_err("Can't find VMA for gfn:0x%lx\n",gfn);+break;+}+}mutex_lock(&kvm->arch.uvmem_lock);-if(!kvmppc_gfn_is_uvmem_pfn(gfn,kvm,&uvmem_pfn)){++if(kvmppc_gfn_is_uvmem_pfn(gfn,kvm,&uvmem_pfn)){+uvmem_page=pfn_to_page(uvmem_pfn);+pvt=uvmem_page->zone_device_data;+pvt->skip_page_out=skip_page_out;+pvt->remove_gfn=true;++if(__kvmppc_svm_page_out(vma,addr,addr+PAGE_SIZE,+PAGE_SHIFT,kvm,pvt->gpa))+pr_err("Can't page out gpa:0x%lx addr:0x%lx\n",+pvt->gpa,addr);+}else{+/* Remove the shared flag if any */kvmppc_gfn_remove(gfn,kvm);-mutex_unlock(&kvm->arch.uvmem_lock);-continue;}-uvmem_page=pfn_to_page(uvmem_pfn);-pvt=uvmem_page->zone_device_data;-pvt->skip_page_out=skip_page_out;-pvt->remove_gfn=true;mutex_unlock(&kvm->arch.uvmem_lock);--pfn=gfn_to_pfn(kvm,gfn);-if(is_error_noslot_pfn(pfn))-continue;-kvm_release_pfn_clean(pfn);}++mmap_read_unlock(kvm->mm);}unsignedlongkvmppc_h_svm_init_abort(structkvm*kvm)
From: Bharata B Rao <hidden> Date: 2020-07-27 03:51:39
On Fri, Jul 24, 2020 at 10:35:27AM +0200, Laurent Dufour wrote:
When a secure memslot is dropped, all the pages backed in the secure
device (aka really backed by secure memory by the Ultravisor)
should be paged out to a normal page. Previously, this was
achieved by triggering the page fault mechanism which is calling
kvmppc_svm_page_out() on each pages.
This can't work when hot unplugging a memory slot because the memory
slot is flagged as invalid and gfn_to_pfn() is then not trying to access
the page, so the page fault mechanism is not triggered.
Since the final goal is to make a call to kvmppc_svm_page_out() it seems
simpler to call directly instead of triggering such a mechanism. This
way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
memslot.
Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
the call to __kvmppc_svm_page_out() is made. As
__kvmppc_svm_page_out needs the vma pointer to migrate the pages,
the VMA is fetched in a lazy way, to not trigger find_vma() all
the time. In addition, the mmap_sem is held in read mode during
that time, not in write mode since the virual memory layout is not
impacted, and kvm->arch.uvmem_lock prevents concurrent operation
on the secure device.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
[modified the changelog description]
Signed-off-by: Laurent Dufour <redacted>
[modified check on the VMA in kvmppc_uvmem_drop_pages]
Reviewed-by: Bharata B Rao <redacted>
Regards,
Bharata.
From: Bharata B Rao <hidden> Date: 2020-07-27 03:53:23
On Thu, Jul 23, 2020 at 01:07:23PM -0700, Ram Pai wrote:
From: Laurent Dufour <redacted>
kvmppc_svm_page_out() will need to be called by kvmppc_uvmem_drop_pages()
so move it upper in this file.
Furthermore it will be interesting to call this function when already
holding the kvm->arch.uvmem_lock, so prefix the original function with __
and remove the locking in it, and introduce a wrapper which call that
function with the lock held.
There is no functional change.
Cc: Ram Pai <redacted>
Cc: Bharata B Rao <redacted>
Cc: Paul Mackerras <redacted>
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Laurent Dufour <redacted>
Reviewed-by: Bharata B Rao <redacted>
Regards,
Bharata.
From: Bharata B Rao <hidden> Date: 2020-07-27 03:57:30
On Thu, Jul 23, 2020 at 01:07:22PM -0700, Ram Pai wrote:
quoted hunk
From: Laurent Dufour <redacted>
When a memory slot is hot plugged to a SVM, PFNs associated with the
GFNs in that slot must be migrated to the secure-PFNs, aka device-PFNs.
Call kvmppc_uv_migrate_mem_slot() to accomplish this.
Disable page-merge for all pages in the memory slot.
Signed-off-by: Ram Pai <redacted>
[rearranged the code, and modified the commit log]
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 14 ++++++++++++++
arch/powerpc/kvm/book3s_hv.c | 10 ++--------
arch/powerpc/kvm/book3s_hv_uvmem.c | 23 +++++++++++++++++++----
3 files changed, 35 insertions(+), 12 deletions(-)
Only concern is that kvmppc_uvmem_memslot_create() can fail due
to multiple reasons but we ignore them and go ahead with memory
hotplug.
May be this hasn't been observed in reality but if we can note this
as a TODO in the comments to dig further and explore the possibility
of recovering from here, then
Reviewed-by: Bharata B Rao <redacted>
Regards,
Bharata.