Re: [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the nested state

[Maxim Levitsky <mlevitsk@redhat.com>]

On Mon, 2021-05-03 at 16:00 +0200, Paolo Bonzini wrote:
> On 03/05/21 14:54, Maxim Levitsky wrote:
> > While after a reset the GIF value is already 1,
> > it doesn't have to have this value if the nested state
> > is loaded later.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >   arch/x86/kvm/svm/nested.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 32400cba608d..12a12ae940fa 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -1314,6 +1314,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >   	else
> >   		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
> >   
> > +	/* Force L1's GIF to true */
> > +	svm_set_gif(svm, true);
> > +
> >   	svm->nested.nested_run_pending =
> >   		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
> >   
> > 
> 
> Hmm, not sure about this one.  It is possible in principle to do CLGI in 
> L2 with the intercept disabled.

I need to think about this a bit more. 
In theory we have L0 GIF, the L1 GIF and the L2 GIF.
L0 GIF is always KVM's, so no problem.
L1 GIF can be toggled with L1 executing clgi/stgi, and it will be either stored in 
vmcb.int_ctl (vmcb01 or vmcb02) or in hflags depending if vGIF is enabled.
(the L1 owned bits are copied in nested_vmcb02_prepare_control)

For L2 we never advertise virtual gif and we don't let it set V_GIF_ENABLE_MASK
in int_ctl, so it either intercepts clgi/stgi and does its own businesses with it
or it doesn't intercept it in which case L2 indeed just modifies L1 GIF.


> 
> You need to use
> 
> svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));


Assuming that the above is correct, then indeed, this should be done,
so I'll send a patch for this.
Thanks a lot!!

Best regards,
	Maxim Levitsky



> 
> instead.
> 
> Paolo
>