On Tue, Nov 12, 2019 at 04:38:36PM +1100, Paul Mackerras wrote:
On Mon, Nov 11, 2019 at 05:01:58PM -0800, Ram Pai wrote:
quoted
On Mon, Nov 11, 2019 at 03:19:24PM +1100, Paul Mackerras wrote:
quoted
On Mon, Nov 04, 2019 at 09:47:59AM +0530, Bharata B Rao wrote:
quoted
From: Sukadev Bhattiprolu <redacted>
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 SVM.
In case of H_SVM_INIT_ABORT, we should page-out all the pages back to
HV (i.e., we should not skip the page-out). Otherwise the VM's pages,
possibly including its text/data would be stuck in secure memory.
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.
...skip...
quoted
If the ultravisor cleans up the SVM's state on its side and then informs
the Hypervisor to abort the SVM, the hypervisor will not be able to
cleanly terminate the VM. Because to terminate the SVM, the hypervisor
still needs the services of the Ultravisor. For example: to get the
pages back into the hypervisor if needed. Another example is, the
hypervisor can call UV_SVM_TERMINATE. Regardless of which ucall
gets called, the ultravisor has to hold on to enough state of the
SVM to service that request.
OK, that's a good reason. That should be explained in the commit
message.
quoted
The current design assumes that the hypervisor explicitly informs the
ultravisor, that it is done with the SVM, through the UV_SVM_TERMINATE
ucall. Till that point the Ultravisor must to be ready to service any
ucalls made by the hypervisor on the SVM's behalf.
I see that UV_SVM_TERMINATE is done when the VM is being destroyed (at
which point kvm->arch.secure_guest doesn't matter any more), and in
kvmhv_svm_off(), where kvm->arch.secure_guest gets cleared
explicitly. Hence I don't see any need for clearing it in the
assembly code on the next secure guest entry. I think the change to
book3s_hv_rmhandlers.S can just be dropped.
There is subtle problem removing that code from the assembly.
If the H_SVM_INIT_ABORT hcall returns to the ultravisor without clearing
kvm->arch.secure_guest, the hypervisor will continue to think that the
VM is a secure VM. However the primary reason the H_SVM_INIT_ABORT
hcall was invoked, was to inform the Hypervisor that it should no longer
consider the VM as a Secure VM. So there is a inconsistency there.
This is fine, as long as the VM does not invoke any hcall or does not
receive any hypervisor-exceptions. The moment either of those happen,
the control goes into the hypervisor, the hypervisor services
the exception/hcall and while returning, it will see that the
kvm->arch.secure_guest flag is set and **incorrectly** return
to the ultravisor through a UV_RETURN ucall. Ultravisor will
not know what to do with it, because it does not consider that
VM as a Secure VM. Bad things happen.
( Sidenote: when H_SVM_INIT_ABORT hcalls returns from the hypervisor,
the ultravisor cleans up its internal state corresponding of that
aborted-SVM and returns back to the caller with MSR[S]=0 )
RP