Paul Mackerras [paulus@xxxxxxxxxx] 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@xxxxxxxxxxxxx>
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@xxxxxxxxxxxxx>
> > Signed-off-by: Bharata B Rao <bharata@xxxxxxxxxxxxx>
>
> 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
