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:
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 (1):
KVM: PPC: Book3S HV: migrate hot plugged memory
Ram Pai (4):
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.
KVM: PPC: Book3S HV: retry page migration before erroring-out
Documentation/powerpc/ultravisor.rst | 4 +
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 12 +
arch/powerpc/kvm/book3s_hv.c | 10 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 508 ++++++++++++++++++++++++----
4 files changed, 457 insertions(+), 77 deletions(-)
--
1.8.3.1
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: Thiago Jung Bauermann <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 187 +++++++++++++++++++++++++++++++++----
1 file changed, 168 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){
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 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.
Disable page-migration in H_SVM_INIT_START handling.
Signed-off-by: Ram Pai <redacted>
---
Documentation/powerpc/ultravisor.rst | 1 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 98 +++++++++++++++++++++++++++---------
2 files changed, 76 insertions(+), 23 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 ~~~~~~~~~~~
@@ -232,11 +291,18 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)returnH_AUTHORITY;srcu_idx=srcu_read_lock(&kvm->srcu);++/* disable page-merging for all memslot */+ret=kvmppc_disable_page_merge(kvm);+if(ret)+gotoout;++/* register the memslot */slots=kvm_memslots(kvm);kvm_for_each_memslot(memslot,slots){if(kvmppc_uvmem_slot_init(kvm,memslot)){ret=H_PARAMETER;-gotoout;+break;}ret=uv_register_mem_slot(kvm->arch.lpid,memslot->base_gfn<<PAGE_SHIFT,
@@ -245,9 +311,12 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)if(ret<0){kvmppc_uvmem_slot_free(kvm,memslot);ret=H_PARAMETER;-gotoout;+break;}}++if(ret)+kvmppc_enable_page_merge(kvm);out:srcu_read_unlock(&kvm->srcu,srcu_idx);returnret;
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 | 135 +++++++++++++++++++++++++---
3 files changed, 125 insertions(+), 14 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 ~~~~~~~~~~~
@@ -461,12 +499,31 @@ 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){+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;}/*
The page requested for page-in; sometimes, can have transient
references, and hence cannot migrate immediately. Retry a few times
before returning error.
The same is true for non-migrated pages that are migrated in
H_SVM_INIT_DONE hanlder. Retry a few times before returning error.
H_SVM_PAGE_IN interface is enhanced to return H_BUSY if the page is
not in a migratable state.
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 | 1 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 106 ++++++++++++++++++++++++-----------
2 files changed, 74 insertions(+), 33 deletions(-)
@@ -1035,6 +1035,7 @@ Return values* H_PARAMETER if ``guest_pa`` is invalid.* H_P2 if ``flags`` is invalid.* H_P3 if ``order`` of page is invalid.+* H_BUSY if ``page`` is not in a state to pagein Description ~~~~~~~~~~~
@@ -812,7 +842,7 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,structvm_area_struct*vma;intsrcu_idx;unsignedlonggfn=gpa>>page_shift;-intret;+intret,repeat_count=REPEAT_COUNT;if(!(kvm->arch.secure_guest&KVMPPC_SECURE_INIT_START))returnH_UNSUPPORTED;
@@ -826,34 +856,44 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,if(flags&H_PAGE_IN_SHARED)returnkvmppc_share_page(kvm,gpa,page_shift);-ret=H_PARAMETER;srcu_idx=srcu_read_lock(&kvm->srcu);-mmap_read_lock(kvm->mm);-start=gfn_to_hva(kvm,gfn);-if(kvm_is_error_hva(start))-gotoout;--mutex_lock(&kvm->arch.uvmem_lock);/* Fail the page-in request of an already paged-in page */-if(kvmppc_gfn_is_uvmem_pfn(gfn,kvm,NULL))-gotoout_unlock;+mutex_lock(&kvm->arch.uvmem_lock);+ret=kvmppc_gfn_is_uvmem_pfn(gfn,kvm,NULL);+mutex_unlock(&kvm->arch.uvmem_lock);+if(ret){+srcu_read_unlock(&kvm->srcu,srcu_idx);+returnH_PARAMETER;+}-end=start+(1UL<<page_shift);-vma=find_vma_intersection(kvm->mm,start,end);-if(!vma||vma->vm_start>start||vma->vm_end<end)-gotoout_unlock;+do{+ret=H_PARAMETER;+mmap_read_lock(kvm->mm);-if(kvmppc_svm_migrate_page(vma,start,end,gpa,kvm,page_shift,-true))-gotoout_unlock;+start=gfn_to_hva(kvm,gfn);+if(kvm_is_error_hva(start)){+mmap_read_unlock(kvm->mm);+break;+}-ret=H_SUCCESS;+end=start+(1UL<<page_shift);+vma=find_vma_intersection(kvm->mm,start,end);+if(!vma||vma->vm_start>start||vma->vm_end<end){+mmap_read_unlock(kvm->mm);+break;+}++mutex_lock(&kvm->arch.uvmem_lock);+ret=kvmppc_svm_migrate_page(vma,start,end,gpa,kvm,page_shift,true);+mutex_unlock(&kvm->arch.uvmem_lock);++mmap_read_unlock(kvm->mm);+}while(ret==-2&&repeat_count--);++if(ret==-2)+ret=H_BUSY;-out_unlock:-mutex_unlock(&kvm->arch.uvmem_lock);-out:-mmap_read_unlock(kvm->mm);srcu_read_unlock(&kvm->srcu,srcu_idx);returnret;}
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 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 | 10 ++++++++++
arch/powerpc/kvm/book3s_hv.c | 10 ++--------
arch/powerpc/kvm/book3s_hv_uvmem.c | 22 ++++++++++++++++++++++
3 files changed, 34 insertions(+), 8 deletions(-)
From: Bharata B Rao <hidden> Date: 2020-07-22 08:54:47
On Fri, Jul 17, 2020 at 01:00:23AM -0700, Ram Pai wrote:
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 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.
Disable page-migration in H_SVM_INIT_START handling.
Signed-off-by: Ram Pai <redacted>
Reviewed-by: Bharata B Rao <redacted>
with a few observations below...
@@ -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 ~~~~~~~~~~~
@@ -232,11 +291,18 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) return H_AUTHORITY; srcu_idx = srcu_read_lock(&kvm->srcu);++ /* disable page-merging for all memslot */+ ret = kvmppc_disable_page_merge(kvm);+ if (ret)+ goto out;++ /* register the memslot */ slots = kvm_memslots(kvm); kvm_for_each_memslot(memslot, slots) {
... you are walking thro' the same set of slots here anyway. I think
it makes sense to issue merge advices from here itself. That will
help you to share code with kvmppc_memslot_create() in 5/5.
All the below 3 calls are common to both the code paths, I think
they can be carved out into a separate function if you prefer.
kvmppc_uvmem_slot_init
kvmppc_memslot_page_merge
uv_register_mem_slot
Regards,
Bharata.
From: Bharata B Rao <hidden> Date: 2020-07-22 10:03:32
On Fri, Jul 17, 2020 at 01:00:27AM -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 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 | 10 ++++++++++
arch/powerpc/kvm/book3s_hv.c | 10 ++--------
arch/powerpc/kvm/book3s_hv_uvmem.c | 22 ++++++++++++++++++++++
3 files changed, 34 insertions(+), 8 deletions(-)
@@ -1089,6 +1089,28 @@ int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)return(ret==U_SUCCESS)?RESUME_GUEST:-EFAULT;}+voidkvmppc_memslot_create(structkvm*kvm,conststructkvm_memory_slot*new)+{+if(kvmppc_uvmem_slot_init(kvm,new))+return;++if(kvmppc_memslot_page_merge(kvm,new,false))+return;++if(uv_register_mem_slot(kvm->arch.lpid,new->base_gfn<<PAGE_SHIFT,+new->npages*PAGE_SIZE,0,new->id))+return;++kvmppc_uv_migrate_mem_slot(kvm,new);
Quite a few things can return failure here including
kvmppc_uv_migrate_mem_slot() and we are ignoring all of those.
I am wondering if this should be called from prepare_memory_region callback
instead of commit_memory_region. In the prepare phase, we have a way
to back out in case of error. Can you check if moving this call to
prepare callback is feasible?
In the other case in 1/5, the code issues ksm unmerge request on error,
but not here.
Also check if the code for 1st three calls can be shared with similar
code in 1/5.
Regards,
Bharata.
From: Bharata B Rao <hidden> Date: 2020-07-23 04:50:46
On Fri, Jul 17, 2020 at 01:00:24AM -0700, Ram Pai wrote:
quoted hunk
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: Thiago Jung Bauermann <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 187 +++++++++++++++++++++++++++++++++----
1 file changed, 168 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){
@@ -524,6 +663,7 @@ 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;}retry:
@@ -537,12 +677,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;
This is the case of making an already secure page as shared page.
A comment here as to why remove_gfn is set to false here will help.
Also isn't it by default false? Is there a situation where it starts
out by default false, becomes true later and you are required to
explicitly mark it false here?
Otherwise, Reviewed-by: Bharata B Rao [off-list ref]
Regards,
Bharata.
This is the case of making an already secure page as shared page.
A comment here as to why remove_gfn is set to false here will help.
Also isn't it by default false? Is there a situation where it starts
out by default false, becomes true later and you are required to
explicitly mark it false here?
It is by default false. And will be true when the GFN is
released/invalidated through kvmppc_uvmem_drop_pages().
It is marked false explicitly here, just to be safe, and protect
against any implicit changes.
Otherwise, Reviewed-by: Bharata B Rao [off-list ref]
What is the guarantee that the gfn is in the same earlier state when you do
do migration here?
Are you worried about the case, where someother thread will sneak-in and
migrate the GFN, and this migration request will become a duplicate one?
That is theortically possible, though practically improbable. This
transition is attempted only when there is one vcpu active in the VM.
However, may be, we should not bake-in that assumption in this code.
Will remove that assumption.
RP
I am dropping this patch based on our conversation, where we agreed, we
need to rootcause the migration failure.
On Thu, Jul 23, 2020 at 11:43:44AM +0530, Bharata B Rao wrote:
On Fri, Jul 17, 2020 at 01:00:26AM -0700, Ram Pai wrote:
quoted
@@ -812,7 +842,7 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa, struct vm_area_struct *vma; int srcu_idx; unsigned long gfn = gpa >> page_shift;- int ret;+ int ret, repeat_count = REPEAT_COUNT; if (!(kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START)) return H_UNSUPPORTED;
@@ -826,34 +856,44 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa, if (flags & H_PAGE_IN_SHARED) return kvmppc_share_page(kvm, gpa, page_shift);- ret = H_PARAMETER; srcu_idx = srcu_read_lock(&kvm->srcu);- mmap_read_lock(kvm->mm);- start = gfn_to_hva(kvm, gfn);- if (kvm_is_error_hva(start))- goto out;-- mutex_lock(&kvm->arch.uvmem_lock); /* Fail the page-in request of an already paged-in page */- if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL))- goto out_unlock;+ mutex_lock(&kvm->arch.uvmem_lock);+ ret = kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL);+ mutex_unlock(&kvm->arch.uvmem_lock);
Same comment as for the prev patch. I don't think you can release
the lock here.