LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Bharata B Rao <bharata@linux.ibm.com>
To: Paul Mackerras <paulus@ozlabs.org>
Cc: linuxram@us.ibm.com, cclaudio@linux.ibm.com,
	kvm-ppc@vger.kernel.org, linux-mm@kvack.org, jglisse@redhat.com,
	aneesh.kumar@linux.vnet.ibm.com, paulus@au1.ibm.com,
	sukadev@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
	hch@lst.de
Subject: Re: [PATCH v10 6/8] KVM: PPC: Support reset of secure guest
Date: Wed, 13 Nov 2019 20:59:08 +0530
Message-ID: <20191113152908.GI21634@in.ibm.com> (raw)
In-Reply-To: <20191112053434.GA10885@oak.ozlabs.ibm.com>

On Tue, Nov 12, 2019 at 04:34:34PM +1100, Paul Mackerras wrote:
> On Mon, Nov 04, 2019 at 09:47:58AM +0530, Bharata B Rao wrote:
> [snip]
> > @@ -5442,6 +5471,64 @@ static int kvmhv_store_to_eaddr(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr,
> >  	return rc;
> >  }
> >  
> > +/*
> > + *  IOCTL handler to turn off secure mode of guest
> > + *
> > + * - Issue ucall to terminate the guest on the UV side
> > + * - Unpin the VPA pages (Enables these pages to be migrated back
> > + *   when VM becomes secure again)
> > + * - Recreate partition table as the guest is transitioning back to
> > + *   normal mode
> > + * - Release all device pages
> > + */
> > +static int kvmhv_svm_off(struct kvm *kvm)
> > +{
> > +	struct kvm_vcpu *vcpu;
> > +	int srcu_idx;
> > +	int ret = 0;
> > +	int i;
> > +
> > +	if (!(kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START))
> > +		return ret;
> > +
> 
> A further comment on this code: it should check that no vcpus are
> running and fail if any are running, and it should prevent any vcpus
> from running until the function is finished, using code like that in
> kvmhv_configure_mmu().  That is, it should do something like this:
> 
> 	mutex_lock(&kvm->arch.mmu_setup_lock);
> 	mmu_was_ready = kvm->arch.mmu_ready;
> 	if (kvm->arch.mmu_ready) {
> 		kvm->arch.mmu_ready = 0;
> 		/* order mmu_ready vs. vcpus_running */
> 		smp_mb();
> 		if (atomic_read(&kvm->arch.vcpus_running)) {
> 			kvm->arch.mmu_ready = 1;
> 			ret = -EBUSY;
> 			goto out_unlock;
> 		}
> 	}
> 
> and then after clearing kvm->arch.secure_guest below:
> 
> > +	srcu_idx = srcu_read_lock(&kvm->srcu);
> > +	for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) {
> > +		struct kvm_memory_slot *memslot;
> > +		struct kvm_memslots *slots = __kvm_memslots(kvm, i);
> > +
> > +		if (!slots)
> > +			continue;
> > +
> > +		kvm_for_each_memslot(memslot, slots) {
> > +			kvmppc_uvmem_drop_pages(memslot, kvm, true);
> > +			uv_unregister_mem_slot(kvm->arch.lpid, memslot->id);
> > +		}
> > +	}
> > +	srcu_read_unlock(&kvm->srcu, srcu_idx);
> > +
> > +	ret = uv_svm_terminate(kvm->arch.lpid);
> > +	if (ret != U_SUCCESS) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	kvm_for_each_vcpu(i, vcpu, kvm) {
> > +		spin_lock(&vcpu->arch.vpa_update_lock);
> > +		unpin_vpa_reset(kvm, &vcpu->arch.dtl);
> > +		unpin_vpa_reset(kvm, &vcpu->arch.slb_shadow);
> > +		unpin_vpa_reset(kvm, &vcpu->arch.vpa);
> > +		spin_unlock(&vcpu->arch.vpa_update_lock);
> > +	}
> > +
> > +	ret = kvmppc_reinit_partition_table(kvm);
> > +	if (ret)
> > +		goto out;
> > +
> > +	kvm->arch.secure_guest = 0;
> 
> you need to do:
> 
> 	kvm->arch.mmu_ready = mmu_was_ready;
>  out_unlock:
> 	mutex_unlock(&kvm->arch.mmu_setup_lock);
> 
> > +out:
> > +	return ret;
> > +}
> > +
> 
> With that extra check in place, it should be safe to unpin the vpas if
> there is a good reason to do so.  ("Userspace has some bug that we
> haven't found" isn't a good reason to do so.)

QEMU indeed does set_one_reg to reset the VPAs but that only marks
the VPA update as pending. The actual unpinning happens when vcpu
gets to run after reset at which time the VPAs are updated after
any unpinning (if required)

When secure guest reboots, vpu 0 gets to run and does unpin its
VPA pages and then proceeds with switching to secure. Here UV
tries to page-in all the guest pages, including the still pinned
VPA pages corresponding to other vcpus which haven't had a chance
to run till now. They are all still pinned and hence page-in fails.

To prevent this, we have to explicitly unpin the VPA pages during
this svm off ioctl. This will ensure that SMP secure guest is able
to reboot correctly.

So I will incorporate the code chunk you have shown above to fail
if any vcpu is running and prevent any vcpu from running when
we unpin VPAs from this ioctl.

Regards,
Bharata.


  reply index

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04  4:17 [PATCH v10 0/8] KVM: PPC: Driver to manage pages " Bharata B Rao
2019-11-04  4:17 ` [PATCH v10 1/8] mm: ksm: Export ksm_madvise() Bharata B Rao
2019-11-06  4:33   ` Paul Mackerras
2019-11-06  6:45     ` Bharata B Rao
2019-11-07  5:45       ` Paul Mackerras
2019-11-15 14:10         ` Bharata B Rao
2019-11-04  4:17 ` [PATCH v10 2/8] KVM: PPC: Support for running secure guests Bharata B Rao
2019-11-06  4:34   ` Paul Mackerras
2019-11-04  4:17 ` [PATCH v10 3/8] KVM: PPC: Shared pages support for " Bharata B Rao
2019-11-06  4:52   ` Paul Mackerras
2019-11-06  8:22     ` Bharata B Rao
2019-11-06  8:29       ` Bharata B Rao
2019-11-04  4:17 ` [PATCH v10 4/8] KVM: PPC: Radix changes for secure guest Bharata B Rao
2019-11-06  5:58   ` Paul Mackerras
2019-11-06  8:36     ` Bharata B Rao
2019-11-04  4:17 ` [PATCH v10 5/8] KVM: PPC: Handle memory plug/unplug to secure VM Bharata B Rao
2019-11-11  4:25   ` Paul Mackerras
2019-11-04  4:17 ` [PATCH v10 6/8] KVM: PPC: Support reset of secure guest Bharata B Rao
2019-11-11  5:28   ` Paul Mackerras
2019-11-11  6:55     ` Bharata B Rao
2019-11-12  5:34   ` Paul Mackerras
2019-11-13 15:29     ` Bharata B Rao [this message]
2019-11-14  5:07       ` Paul Mackerras
2019-11-04  4:17 ` [PATCH v10 7/8] KVM: PPC: Implement H_SVM_INIT_ABORT hcall Bharata B Rao
2019-11-11  4:19   ` Paul Mackerras
2019-11-12  1:01     ` Ram Pai
2019-11-12  5:38       ` Paul Mackerras
2019-11-12  7:52         ` Ram Pai
2019-11-12 11:32           ` Paul Mackerras
2019-11-12 14:45             ` Ram Pai
2019-11-13  0:14               ` Paul Mackerras
2019-11-13  6:32                 ` Ram Pai
2019-11-13 21:18                   ` Paul Mackerras
2019-11-13 21:50                     ` Ram Pai
2019-11-14  5:08                       ` Paul Mackerras
2019-11-14  7:02                         ` Ram Pai
2019-11-04  4:18 ` [PATCH v10 8/8] KVM: PPC: Ultravisor: Add PPC_UV config option Bharata B Rao
2019-11-06  4:30 ` [PATCH v10 0/8] KVM: PPC: Driver to manage pages of secure guest Paul Mackerras
2019-11-06  6:20   ` Bharata B Rao

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191113152908.GI21634@in.ibm.com \
    --to=bharata@linux.ibm.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=cclaudio@linux.ibm.com \
    --cc=hch@lst.de \
    --cc=jglisse@redhat.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=linuxram@us.ibm.com \
    --cc=paulus@au1.ibm.com \
    --cc=paulus@ozlabs.org \
    --cc=sukadev@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git