Re: [PATCH v3 8/8] KVM: nSVM: implement ondemand allocation of the nested state

From: Maxim Levitsky
Date: Sun Sep 13 2020 - 15:57:52 EST


On Sat, 2020-09-12 at 18:32 +0200, Paolo Bonzini wrote:
> On 27/08/20 19:11, Maxim Levitsky wrote:
> > + hsave_page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> > + if (!hsave_page)
> > + goto error;
> > +
>
> I think an error here should be just an internal error userspace exit,
> or a -ENOMEM from KVM_RUN; not a #GP in the guest[1]. However, that's
> difficult to plug into KVM. Can you instead allocate nested state if
> KVM_SET_CPUID2 sets the SVM bit? Returning -ENOMEM from KVM_SET_CPUID2
> is more likely to be something that userspace copes with.

This would be a bit sad thing to do, as it would allocate nested state for each
guest where it is enabled in CPUID, which is IMHO already the default or soon to be.

Currently nested KVM is nice to only enable EFER.SVME when a vm is created (in svm_hardware_enable)
which means that with this on demand nested state allocation, we only use nested state
for guests that actually use nested as opposed to be merely enabled.

About injecting the #GP,
I probably can make the guest triple fault instead if we consider this to be better solution.
in terms of not allowing to continue guest execution.
I somehow thought that guest will already be killed on emulation failure,
but now I do see that kvm_emulate_wrmsr and that it injects a #GP

What do you think?

>
> I queued patches 1-5, and 7 for 5.9-rc.

Thanks!

Best regards,
Maxim Levitsky

>
> Paolo
>
> [1] Though in practice an order 0 allocation will never fail
>