RE: [PATCH] x86/hvm: Include interruptibility state in hvm_hw_cpu

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>
> Sent: Monday, March 14, 2022 8:14 PM
> 
> On Mon, Mar 14, 2022 at 3:22 AM Tian, Kevin <kevin.tian@xxxxxxxxx> wrote:
> >
> > > From: Lengyel, Tamas <tamas.lengyel@xxxxxxxxx>
> > > Sent: Friday, March 11, 2022 2:45 AM
> > >
> > > During VM fork resetting a failed vmentry has been observed when the
> reset
> > > is performed immediately after a STI instruction executed. This is due to
> > > the guest interruptibility state in the VMCS being modified by STI but the
> > > subsequent reset removes the IF bit from FLAGS, causing the failed
> vmentry.
> >
> > I didn't get the rationale here. Before this patch the interruptibility 
> > state is
> > not saved/restored thus I suppose after reset it will be cleared thus 
> > aligned
> > with RFLAGS.IF=0. Can you elaborate a bit how exactly above problem is
> > caused?
> 
> The problem is that the interruptibility state is not cleared and thus
> isn't aligned with RFLAGS.IF=0 after RFLAGS is reset. They go out of
> sync leading to the failed vmentry. The interruptibility state needs
> to be included in the hvm hw cpu struct for it to get re-aligned
> during reset to avoid the failed vmentry.

I'm still confused here. The interruptibility state should have bit 0 as 1
after a STI instruction is executed (RFLAGS.IF=1). Saving/restoring it
still doesn't match RFLAGS.IF=0 after vm fork reset. So I didn't understand
how this patch actually fixes the problem.

Also if there is a real problem shouldn't we just reset the interruptbility
state to match RFLAGS.IF=0?

> 
> >
> > >
> > > Include the interruptibility state information in the public hvm_hw_cpu
> struct
> > > so that the CPU can be safely saved/restored.
> > >
> > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
> > > ---
> > >  xen/arch/x86/hvm/hvm.c                 |  9 +++++----
> > >  xen/arch/x86/hvm/vmx/vmx.c             |  4 ++++
> > >  xen/arch/x86/include/asm/hvm/hvm.h     | 26
> >
> > Why is this change only applied to vmx instead of svm?
> 
> VM forking is implemented only for vmx, thus this change is only
> relevant where a VM would be immediately reset after a STI

but the ops is generic and SVM already has the related callbacks.

> instruction. Normal VM save/restore/migration doesn't attempt to
> capture a VM state immediately after STI thus it's not relevant for
> SVM.
> 

Can you elaborate why save/restore/migration won't happen
right after STI while it does for vm fork?

Thanks
Kevin