Re: [PATCH V3 2/2] KVM: PPC: Implement H_SVM_INIT_ABORT hcall

[Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>]

Paul Mackerras [paulus@ozlabs.org] wrote:
> On Sat, Dec 14, 2019 at 06:12:08PM -0800, Sukadev Bhattiprolu wrote:
> > 
> > Implement the H_SVM_INIT_ABORT hcall which the Ultravisor can use to
> > abort an SVM after it has issued the H_SVM_INIT_START and before the
> > H_SVM_INIT_DONE hcalls. This hcall could be used when Ultravisor
> > encounters security violations or other errors when starting an SVM.
> > 
> > Note that this hcall is different from UV_SVM_TERMINATE ucall which
> > is used by HV to terminate/cleanup an VM that has becore secure.
> > 
> > The H_SVM_INIT_ABORT should basically undo operations that were done
> > since the H_SVM_INIT_START hcall - i.e page-out all the VM pages back
> > to normal memory, and terminate the SVM.
> > 
> > (If we do not bring the pages back to normal memory, the text/data
> > of the VM would be stuck in secure memory and since the SVM did not
> > go secure, its MSR_S bit will be clear and the VM wont be able to
> > access its pages even to do a clean exit).
> > 
> > Based on patches and discussion with Paul Mackerras, Ram Pai and
> > Bharata Rao.
> > 
> > Signed-off-by: Ram Pai <linuxram@linux.ibm.com>
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> 
> Minor comment below, but not a showstopper.  Also, as Bharata noted
> you need to hold the srcu lock for reading.

Yes, I fixed that.

> 
> > +	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, false);
> > +	}
> 
> Since we use the default KVM_ADDRESS_SPACE_NUM, which is 1, this code
> isn't wrong but it is more verbose than it needs to be.  It could be
> 
> 	kvm_for_each_memslot(kvm_memslots(kvm), slots)
> 		kvmppc_uvmem_drop_pages(memslot, kvm, false);

and simplified this.

Thanks.

Sukadev