| Message ID | 1590892071-25549-4-git-send-email-linuxram@us.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Migrate non-migrated pages of a SVM. | expand |
On Sat, May 30, 2020 at 07:27:50PM -0700, Ram Pai wrote: > H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly > called H_SVM_PAGE_IN for all secure pages. I don't think that is quite true. HV doesn't assume anything about secure pages by itself. > These GFNs continue to be > normal GFNs associated with normal PFNs; when infact, these GFNs should > have been secure GFNs associated with device PFNs. Transition to secure state is driven by SVM/UV and HV just responds to hcalls by issuing appropriate uvcalls. SVM/UV is in the best position to determine the required pages that need to be moved into secure side. HV just responds to it and tracks such pages as device private pages. If SVM/UV doesn't get in all the pages to secure side by the time of H_SVM_INIT_DONE, the remaining pages are just normal (shared or otherwise) pages as far as HV is concerned. Why should HV assume that SVM/UV didn't ask for a few pages and hence push those pages during H_SVM_INIT_DONE? I think UV should drive the movement of pages into secure side both of boot-time SVM memory and hot-plugged memory. HV does memslot registration uvcall when new memory is plugged in, UV should explicitly get the required pages in at that time instead of expecting HV to drive the same. > +static int uv_migrate_mem_slot(struct kvm *kvm, > + const struct kvm_memory_slot *memslot) > +{ > + unsigned long gfn = memslot->base_gfn; > + unsigned long end; > + bool downgrade = false; > + struct vm_area_struct *vma; > + int i, ret = 0; > + unsigned long start = gfn_to_hva(kvm, gfn); > + > + if (kvm_is_error_hva(start)) > + return H_STATE; > + > + end = start + (memslot->npages << PAGE_SHIFT); > + > + down_write(&kvm->mm->mmap_sem); > + > + mutex_lock(&kvm->arch.uvmem_lock); > + vma = find_vma_intersection(kvm->mm, start, end); > + if (!vma || vma->vm_start > start || vma->vm_end < end) { > + ret = H_STATE; > + goto out_unlock; > + } > + > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > + MADV_UNMERGEABLE, &vma->vm_flags); > + downgrade_write(&kvm->mm->mmap_sem); > + downgrade = true; > + if (ret) { > + ret = H_STATE; > + goto out_unlock; > + } > + > + for (i = 0; i < memslot->npages; i++, ++gfn) { > + /* skip paged-in pages and shared pages */ > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) || > + kvmppc_gfn_is_uvmem_shared(gfn, kvm)) > + continue; > + > + start = gfn_to_hva(kvm, gfn); > + end = start + (1UL << PAGE_SHIFT); > + ret = kvmppc_svm_migrate_page(vma, start, end, > + (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false); > + > + if (ret) > + goto out_unlock; > + } Is there a guarantee that the vma you got for the start address remains valid for all the addresses till end in a memslot? If not, you should re-get the vma for the current address in each iteration I suppose. Regards, Bharata.
On Mon, Jun 01, 2020 at 05:25:18PM +0530, Bharata B Rao wrote: > On Sat, May 30, 2020 at 07:27:50PM -0700, Ram Pai wrote: > > H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly > > called H_SVM_PAGE_IN for all secure pages. > > I don't think that is quite true. HV doesn't assume anything about > secure pages by itself. Yes. Currently, it does not assume anything about secure pages. But I am proposing that it should consider all pages (except the shared pages) as secure pages, when H_SVM_INIT_DONE is called. In other words, HV should treat all pages; except shared pages, as secure pages once H_SVM_INIT_DONE is called. And this includes pages added subsequently through memory hotplug. Yes. the Ultravisor can explicitly request the HV to move the pages individually. But that will slow down the transition too significantly. It takes above 20min to transition them, for a SVM of size 100G. With this proposed enhancement, the switch completes in a few seconds. > > > These GFNs continue to be > > normal GFNs associated with normal PFNs; when infact, these GFNs should > > have been secure GFNs associated with device PFNs. > > Transition to secure state is driven by SVM/UV and HV just responds to > hcalls by issuing appropriate uvcalls. SVM/UV is in the best position to > determine the required pages that need to be moved into secure side. > HV just responds to it and tracks such pages as device private pages. > > If SVM/UV doesn't get in all the pages to secure side by the time > of H_SVM_INIT_DONE, the remaining pages are just normal (shared or > otherwise) pages as far as HV is concerned. Why should HV assume that > SVM/UV didn't ask for a few pages and hence push those pages during > H_SVM_INIT_DONE? By definition, SVM is a VM backed by secure pages. Hence all pages(except shared) must turn secure when a VM switches to SVM. UV is interested in only a certain pages for the VM, which it will request explicitly through H_SVM_PAGE_IN. All other pages, need not be paged-in through UV_PAGE_IN. They just need to be switched to device-pages. > > I think UV should drive the movement of pages into secure side both > of boot-time SVM memory and hot-plugged memory. HV does memslot > registration uvcall when new memory is plugged in, UV should explicitly > get the required pages in at that time instead of expecting HV to drive > the same. > > > +static int uv_migrate_mem_slot(struct kvm *kvm, > > + const struct kvm_memory_slot *memslot) > > +{ > > + unsigned long gfn = memslot->base_gfn; > > + unsigned long end; > > + bool downgrade = false; > > + struct vm_area_struct *vma; > > + int i, ret = 0; > > + unsigned long start = gfn_to_hva(kvm, gfn); > > + > > + if (kvm_is_error_hva(start)) > > + return H_STATE; > > + > > + end = start + (memslot->npages << PAGE_SHIFT); > > + > > + down_write(&kvm->mm->mmap_sem); > > + > > + mutex_lock(&kvm->arch.uvmem_lock); > > + vma = find_vma_intersection(kvm->mm, start, end); > > + if (!vma || vma->vm_start > start || vma->vm_end < end) { > > + ret = H_STATE; > > + goto out_unlock; > > + } > > + > > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > > + MADV_UNMERGEABLE, &vma->vm_flags); > > + downgrade_write(&kvm->mm->mmap_sem); > > + downgrade = true; > > + if (ret) { > > + ret = H_STATE; > > + goto out_unlock; > > + } > > + > > + for (i = 0; i < memslot->npages; i++, ++gfn) { > > + /* skip paged-in pages and shared pages */ > > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) || > > + kvmppc_gfn_is_uvmem_shared(gfn, kvm)) > > + continue; > > + > > + start = gfn_to_hva(kvm, gfn); > > + end = start + (1UL << PAGE_SHIFT); > > + ret = kvmppc_svm_migrate_page(vma, start, end, > > + (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false); > > + > > + if (ret) > > + goto out_unlock; > > + } > > Is there a guarantee that the vma you got for the start address remains > valid for all the addresses till end in a memslot? If not, you should > re-get the vma for the current address in each iteration I suppose. mm->mmap_sem is the semaphore that guards the vma. right? If that semaphore is held, can the vma change? Thanks for your comments, RP
On Mon, Jun 01, 2020 at 12:05:35PM -0700, Ram Pai wrote: > On Mon, Jun 01, 2020 at 05:25:18PM +0530, Bharata B Rao wrote: > > On Sat, May 30, 2020 at 07:27:50PM -0700, Ram Pai wrote: > > > H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly > > > called H_SVM_PAGE_IN for all secure pages. > > > > I don't think that is quite true. HV doesn't assume anything about > > secure pages by itself. > > Yes. Currently, it does not assume anything about secure pages. But I am > proposing that it should consider all pages (except the shared pages) as > secure pages, when H_SVM_INIT_DONE is called. Ok, then may be also add the proposed changes to H_SVM_INIT_DONE documentation. > > In other words, HV should treat all pages; except shared pages, as > secure pages once H_SVM_INIT_DONE is called. And this includes pages > added subsequently through memory hotplug. So after H_SVM_INIT_DONE, if HV touches a secure page for any reason and gets encrypted contents via page-out, HV drops the device pfn at that time. So what state we would be in that case? We have completed H_SVM_INIT_DONE, but still have a normal (but encrypted) page in HV? > > Yes. the Ultravisor can explicitly request the HV to move the pages > individually. But that will slow down the transition too significantly. > It takes above 20min to transition them, for a SVM of size 100G. > > With this proposed enhancement, the switch completes in a few seconds. I think, many pages during initial switch and most pages for hotplugged memory are zero pages, for which we don't anyway issue UV page-in calls. So the 20min saving you are observing is purely due to hcall overhead? How about extending H_SVM_PAGE_IN interface or a new hcall to request multiple pages in one request? Also, how about requesting for bigger page sizes (2M)? Ralph Campbell had patches that added THP support for migrate_vma_* calls. > > > > > > These GFNs continue to be > > > normal GFNs associated with normal PFNs; when infact, these GFNs should > > > have been secure GFNs associated with device PFNs. > > > > Transition to secure state is driven by SVM/UV and HV just responds to > > hcalls by issuing appropriate uvcalls. SVM/UV is in the best position to > > determine the required pages that need to be moved into secure side. > > HV just responds to it and tracks such pages as device private pages. > > > > If SVM/UV doesn't get in all the pages to secure side by the time > > of H_SVM_INIT_DONE, the remaining pages are just normal (shared or > > otherwise) pages as far as HV is concerned. Why should HV assume that > > SVM/UV didn't ask for a few pages and hence push those pages during > > H_SVM_INIT_DONE? > > By definition, SVM is a VM backed by secure pages. > Hence all pages(except shared) must turn secure when a VM switches to SVM. > > UV is interested in only a certain pages for the VM, which it will > request explicitly through H_SVM_PAGE_IN. All other pages, need not > be paged-in through UV_PAGE_IN. They just need to be switched to > device-pages. > > > > > I think UV should drive the movement of pages into secure side both > > of boot-time SVM memory and hot-plugged memory. HV does memslot > > registration uvcall when new memory is plugged in, UV should explicitly > > get the required pages in at that time instead of expecting HV to drive > > the same. > > > > > +static int uv_migrate_mem_slot(struct kvm *kvm, > > > + const struct kvm_memory_slot *memslot) > > > +{ > > > + unsigned long gfn = memslot->base_gfn; > > > + unsigned long end; > > > + bool downgrade = false; > > > + struct vm_area_struct *vma; > > > + int i, ret = 0; > > > + unsigned long start = gfn_to_hva(kvm, gfn); > > > + > > > + if (kvm_is_error_hva(start)) > > > + return H_STATE; > > > + > > > + end = start + (memslot->npages << PAGE_SHIFT); > > > + > > > + down_write(&kvm->mm->mmap_sem); > > > + > > > + mutex_lock(&kvm->arch.uvmem_lock); > > > + vma = find_vma_intersection(kvm->mm, start, end); > > > + if (!vma || vma->vm_start > start || vma->vm_end < end) { > > > + ret = H_STATE; > > > + goto out_unlock; > > > + } > > > + > > > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > > > + MADV_UNMERGEABLE, &vma->vm_flags); > > > + downgrade_write(&kvm->mm->mmap_sem); > > > + downgrade = true; > > > + if (ret) { > > > + ret = H_STATE; > > > + goto out_unlock; > > > + } > > > + > > > + for (i = 0; i < memslot->npages; i++, ++gfn) { > > > + /* skip paged-in pages and shared pages */ > > > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) || > > > + kvmppc_gfn_is_uvmem_shared(gfn, kvm)) > > > + continue; > > > + > > > + start = gfn_to_hva(kvm, gfn); > > > + end = start + (1UL << PAGE_SHIFT); > > > + ret = kvmppc_svm_migrate_page(vma, start, end, > > > + (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false); > > > + > > > + if (ret) > > > + goto out_unlock; > > > + } > > > > Is there a guarantee that the vma you got for the start address remains > > valid for all the addresses till end in a memslot? If not, you should > > re-get the vma for the current address in each iteration I suppose. > > > mm->mmap_sem is the semaphore that guards the vma. right? If that > semaphore is held, can the vma change? I am not sure if the vma you obtained would span the entire address range in the memslot. Regards, Bharata.
On Tue, Jun 02, 2020 at 03:36:39PM +0530, Bharata B Rao wrote: > On Mon, Jun 01, 2020 at 12:05:35PM -0700, Ram Pai wrote: > > On Mon, Jun 01, 2020 at 05:25:18PM +0530, Bharata B Rao wrote: > > > On Sat, May 30, 2020 at 07:27:50PM -0700, Ram Pai wrote: > > > > H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly > > > > called H_SVM_PAGE_IN for all secure pages. > > > > > > I don't think that is quite true. HV doesn't assume anything about > > > secure pages by itself. > > > > Yes. Currently, it does not assume anything about secure pages. But I am > > proposing that it should consider all pages (except the shared pages) as > > secure pages, when H_SVM_INIT_DONE is called. > > Ok, then may be also add the proposed changes to H_SVM_INIT_DONE > documentation. ok. > > > > > In other words, HV should treat all pages; except shared pages, as > > secure pages once H_SVM_INIT_DONE is called. And this includes pages > > added subsequently through memory hotplug. > > So after H_SVM_INIT_DONE, if HV touches a secure page for any > reason and gets encrypted contents via page-out, HV drops the > device pfn at that time. So what state we would be in that case? We > have completed H_SVM_INIT_DONE, but still have a normal (but encrypted) > page in HV? Good point. The corresponding GFN will continue to be a secure GFN. Just that its backing PFN is not a device-PFN, but a memory-PFN. Also that backing memory-PFN contains encrypted content. I will clarify this in the patch; about secure-GFN state. > > > > > Yes. the Ultravisor can explicitly request the HV to move the pages > > individually. But that will slow down the transition too significantly. > > It takes above 20min to transition them, for a SVM of size 100G. > > > > With this proposed enhancement, the switch completes in a few seconds. > > I think, many pages during initial switch and most pages for hotplugged > memory are zero pages, for which we don't anyway issue UV page-in calls. > So the 20min saving you are observing is purely due to hcall overhead? Apparently, that seems to be the case. > > How about extending H_SVM_PAGE_IN interface or a new hcall to request > multiple pages in one request? > > Also, how about requesting for bigger page sizes (2M)? Ralph Campbell > had patches that added THP support for migrate_vma_* calls. yes. that should give further boost. I think the API does not stop us from using that feature. Its the support on the Ultravisor side. Hopefully we will have contributions to the ultravisor once it is opensourced. > > > > > > > > > > These GFNs continue to be > > > > normal GFNs associated with normal PFNs; when infact, these GFNs should > > > > have been secure GFNs associated with device PFNs. > > > > > > Transition to secure state is driven by SVM/UV and HV just responds to > > > hcalls by issuing appropriate uvcalls. SVM/UV is in the best position to > > > determine the required pages that need to be moved into secure side. > > > HV just responds to it and tracks such pages as device private pages. > > > > > > If SVM/UV doesn't get in all the pages to secure side by the time > > > of H_SVM_INIT_DONE, the remaining pages are just normal (shared or > > > otherwise) pages as far as HV is concerned. Why should HV assume that > > > SVM/UV didn't ask for a few pages and hence push those pages during > > > H_SVM_INIT_DONE? > > > > By definition, SVM is a VM backed by secure pages. > > Hence all pages(except shared) must turn secure when a VM switches to SVM. > > > > UV is interested in only a certain pages for the VM, which it will > > request explicitly through H_SVM_PAGE_IN. All other pages, need not > > be paged-in through UV_PAGE_IN. They just need to be switched to > > device-pages. > > > > > > > > I think UV should drive the movement of pages into secure side both > > > of boot-time SVM memory and hot-plugged memory. HV does memslot > > > registration uvcall when new memory is plugged in, UV should explicitly > > > get the required pages in at that time instead of expecting HV to drive > > > the same. > > > > > > > +static int uv_migrate_mem_slot(struct kvm *kvm, > > > > + const struct kvm_memory_slot *memslot) > > > > +{ > > > > + unsigned long gfn = memslot->base_gfn; > > > > + unsigned long end; > > > > + bool downgrade = false; > > > > + struct vm_area_struct *vma; > > > > + int i, ret = 0; > > > > + unsigned long start = gfn_to_hva(kvm, gfn); > > > > + > > > > + if (kvm_is_error_hva(start)) > > > > + return H_STATE; > > > > + > > > > + end = start + (memslot->npages << PAGE_SHIFT); > > > > + > > > > + down_write(&kvm->mm->mmap_sem); > > > > + > > > > + mutex_lock(&kvm->arch.uvmem_lock); > > > > + vma = find_vma_intersection(kvm->mm, start, end); > > > > + if (!vma || vma->vm_start > start || vma->vm_end < end) { > > > > + ret = H_STATE; > > > > + goto out_unlock; > > > > + } > > > > + > > > > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > > > > + MADV_UNMERGEABLE, &vma->vm_flags); > > > > + downgrade_write(&kvm->mm->mmap_sem); > > > > + downgrade = true; > > > > + if (ret) { > > > > + ret = H_STATE; > > > > + goto out_unlock; > > > > + } > > > > + > > > > + for (i = 0; i < memslot->npages; i++, ++gfn) { > > > > + /* skip paged-in pages and shared pages */ > > > > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) || > > > > + kvmppc_gfn_is_uvmem_shared(gfn, kvm)) > > > > + continue; > > > > + > > > > + start = gfn_to_hva(kvm, gfn); > > > > + end = start + (1UL << PAGE_SHIFT); > > > > + ret = kvmppc_svm_migrate_page(vma, start, end, > > > > + (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false); > > > > + > > > > + if (ret) > > > > + goto out_unlock; > > > > + } > > > > > > Is there a guarantee that the vma you got for the start address remains > > > valid for all the addresses till end in a memslot? If not, you should > > > re-get the vma for the current address in each iteration I suppose. > > > > > > mm->mmap_sem is the semaphore that guards the vma. right? If that > > semaphore is held, can the vma change? > > I am not sure if the vma you obtained would span the entire address range > in the memslot. will check. Thanks, RP
On Wed, Jun 03, 2020 at 04:10:25PM -0700, Ram Pai wrote: > On Tue, Jun 02, 2020 at 03:36:39PM +0530, Bharata B Rao wrote: > > On Mon, Jun 01, 2020 at 12:05:35PM -0700, Ram Pai wrote: > > > On Mon, Jun 01, 2020 at 05:25:18PM +0530, Bharata B Rao wrote: > > > > On Sat, May 30, 2020 at 07:27:50PM -0700, Ram Pai wrote: > > > > > H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly > > > > > called H_SVM_PAGE_IN for all secure pages. > > > > > > > > I don't think that is quite true. HV doesn't assume anything about > > > > secure pages by itself. > > > > > > Yes. Currently, it does not assume anything about secure pages. But I am > > > proposing that it should consider all pages (except the shared pages) as > > > secure pages, when H_SVM_INIT_DONE is called. > > > > Ok, then may be also add the proposed changes to H_SVM_INIT_DONE > > documentation. > > ok. > > > > > > > > > In other words, HV should treat all pages; except shared pages, as > > > secure pages once H_SVM_INIT_DONE is called. And this includes pages > > > added subsequently through memory hotplug. > > > > So after H_SVM_INIT_DONE, if HV touches a secure page for any > > reason and gets encrypted contents via page-out, HV drops the > > device pfn at that time. So what state we would be in that case? We > > have completed H_SVM_INIT_DONE, but still have a normal (but encrypted) > > page in HV? > > Good point. > > The corresponding GFN will continue to be a secure GFN. Just that its > backing PFN is not a device-PFN, but a memory-PFN. Also that backing > memory-PFN contains encrypted content. > > I will clarify this in the patch; about secure-GFN state. I feel all this is complicating the states in HV and is avoidable if UV just issued page-in calls during memslot registration uvcall. Regards, Bharata.
diff --git a/Documentation/powerpc/ultravisor.rst b/Documentation/powerpc/ultravisor.rst index 363736d..3bc8957 100644 --- a/Documentation/powerpc/ultravisor.rst +++ b/Documentation/powerpc/ultravisor.rst @@ -933,6 +933,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 ~~~~~~~~~~~ diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index 2ef1e03..36dda1d 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -318,14 +318,149 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) return ret; } +static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm); + +/* + * Alloc a PFN from private device memory pool. If @pagein is true, + * copy page from normal memory to secure memory using UV_PAGE_IN uvcall. + */ +static int kvmppc_svm_migrate_page(struct vm_area_struct *vma, + unsigned long start, + unsigned long end, unsigned long gpa, struct kvm *kvm, + unsigned long page_shift, + bool pagein) +{ + unsigned long src_pfn, dst_pfn = 0; + struct migrate_vma mig; + struct page *dpage; + struct page *spage; + unsigned long pfn; + int ret = 0; + + memset(&mig, 0, sizeof(mig)); + mig.vma = vma; + mig.start = start; + mig.end = end; + mig.src = &src_pfn; + mig.dst = &dst_pfn; + + ret = migrate_vma_setup(&mig); + if (ret) + return ret; + + if (!(*mig.src & MIGRATE_PFN_MIGRATE)) { + ret = -1; + goto out_finalize; + } + + dpage = kvmppc_uvmem_get_page(gpa, kvm); + if (!dpage) { + ret = -1; + goto out_finalize; + } + + 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) + goto out_finalize; + } + } + + *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED; + migrate_vma_pages(&mig); +out_finalize: + migrate_vma_finalize(&mig); + return ret; +} + +static int uv_migrate_mem_slot(struct kvm *kvm, + const struct kvm_memory_slot *memslot) +{ + unsigned long gfn = memslot->base_gfn; + unsigned long end; + bool downgrade = false; + struct vm_area_struct *vma; + int i, ret = 0; + unsigned long start = gfn_to_hva(kvm, gfn); + + if (kvm_is_error_hva(start)) + return H_STATE; + + end = start + (memslot->npages << PAGE_SHIFT); + + down_write(&kvm->mm->mmap_sem); + + mutex_lock(&kvm->arch.uvmem_lock); + vma = find_vma_intersection(kvm->mm, start, end); + if (!vma || vma->vm_start > start || vma->vm_end < end) { + ret = H_STATE; + goto out_unlock; + } + + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, + MADV_UNMERGEABLE, &vma->vm_flags); + downgrade_write(&kvm->mm->mmap_sem); + downgrade = true; + if (ret) { + ret = H_STATE; + goto out_unlock; + } + + for (i = 0; i < memslot->npages; i++, ++gfn) { + /* skip paged-in pages and shared pages */ + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) || + kvmppc_gfn_is_uvmem_shared(gfn, kvm)) + continue; + + start = gfn_to_hva(kvm, gfn); + end = start + (1UL << PAGE_SHIFT); + ret = kvmppc_svm_migrate_page(vma, start, end, + (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false); + + if (ret) + goto out_unlock; + } + +out_unlock: + mutex_unlock(&kvm->arch.uvmem_lock); + if (downgrade) + up_read(&kvm->mm->mmap_sem); + else + up_write(&kvm->mm->mmap_sem); + return ret; +} + unsigned long kvmppc_h_svm_init_done(struct kvm *kvm) { + struct kvm_memslots *slots; + struct kvm_memory_slot *memslot; + int srcu_idx; + long ret = H_SUCCESS; + if (!(kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START)) return H_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 = uv_migrate_mem_slot(kvm, memslot); + if (ret) { + ret = H_STATE; + goto out; + } + } + kvm->arch.secure_guest |= KVMPPC_SECURE_INIT_DONE; pr_info("LPID %d went secure\n", kvm->arch.lpid); - return H_SUCCESS; + +out: + srcu_read_unlock(&kvm->srcu, srcu_idx); + return ret; } /* @@ -459,68 +594,6 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm) } /* - * Alloc a PFN from private device memory pool and copy page from normal - * memory to secure memory using UV_PAGE_IN uvcall. - */ -static int kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start, - unsigned long end, unsigned long gpa, struct kvm *kvm, - unsigned long page_shift, bool *downgrade) -{ - unsigned long src_pfn, dst_pfn = 0; - struct migrate_vma mig; - struct page *spage; - unsigned long pfn; - struct page *dpage; - int ret = 0; - - memset(&mig, 0, sizeof(mig)); - mig.vma = vma; - mig.start = start; - mig.end = end; - mig.src = &src_pfn; - mig.dst = &dst_pfn; - - /* - * We come here with mmap_sem write lock held just for - * ksm_madvise(), otherwise we only need read mmap_sem. - * Hence downgrade to read lock once ksm_madvise() is done. - */ - ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, - MADV_UNMERGEABLE, &vma->vm_flags); - downgrade_write(&kvm->mm->mmap_sem); - *downgrade = true; - if (ret) - return ret; - - ret = migrate_vma_setup(&mig); - if (ret) - return ret; - - if (!(*mig.src & MIGRATE_PFN_MIGRATE)) { - ret = -1; - goto out_finalize; - } - - dpage = kvmppc_uvmem_get_page(gpa, kvm); - if (!dpage) { - ret = -1; - goto out_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); - - *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED; - migrate_vma_pages(&mig); -out_finalize: - migrate_vma_finalize(&mig); - return ret; -} - -/* * Shares the page with HV, thus making it a normal page. * * - If the page is already secure, then provision a new page and share @@ -623,11 +696,23 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa, if (!vma || vma->vm_start > start || vma->vm_end < end) goto out_unlock; - if (!kvmppc_svm_page_in(vma, start, end, gpa, kvm, page_shift, - &downgrade)) { - kvmppc_gfn_uvmem_shared(gfn, kvm, false); - ret = H_SUCCESS; + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, + MADV_UNMERGEABLE, &vma->vm_flags); + downgrade_write(&kvm->mm->mmap_sem); + downgrade = true; + if (ret) { + ret = H_PARAMETER; + goto out_unlock; } + + ret = H_PARAMETER; + if (kvmppc_svm_migrate_page(vma, start, end, gpa, kvm, page_shift, + true)) + goto out_unlock; + + kvmppc_gfn_uvmem_shared(gfn, kvm, false); + ret = H_SUCCESS; + out_unlock: mutex_unlock(&kvm->arch.uvmem_lock); out:
H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly called H_SVM_PAGE_IN for all secure pages. These GFNs continue to be normal GFNs associated with normal PFNs; when infact, these GFNs should have been secure GFNs associated with device PFNs. Move all the PFN associated with the SVM's GFNs to device PFNs, in H_SVM_INIT_DONE. Skip the GFNs that are already Paged-in or Shared through H_SVM_PAGE_IN. Cc: Paul Mackerras <paulus@ozlabs.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Bharata B Rao <bharata@linux.ibm.com> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> Cc: Laurent Dufour <ldufour@linux.ibm.com> Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Claudio Carvalho <cclaudio@linux.ibm.com> Cc: kvm-ppc@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Ram Pai <linuxram@us.ibm.com> --- Documentation/powerpc/ultravisor.rst | 2 + arch/powerpc/kvm/book3s_hv_uvmem.c | 219 ++++++++++++++++++++++++----------- 2 files changed, 154 insertions(+), 67 deletions(-)