[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] x86/vmx: save guest non-register state in hvm_hw_cpu



On Mon, Mar 21, 2022 at 8:16 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 17.03.2022 16:57, Tamas K Lengyel wrote:
> > During VM forking and resetting a failed vmentry has been observed due
> > to the guest non-register state going out-of-sync with the guest register
> > state. For example, a VM fork reset right after a STI instruction can 
> > trigger
> > the failed entry. This is due to the guest non-register state not being 
> > saved
> > from the parent VM, thus the reset operation only copies the register state.
> >
> > Fix this by including the guest non-register state in hvm_hw_cpu so that 
> > when
> > its copied from the parent VM the vCPU state remains in sync.
> >
> > SVM is not currently wired-in as VM forking is VMX only and saving 
> > non-register
> > state during normal save/restore/migration operation hasn't been needed.
>
> Given that it was pointed out that e.g. STI- and MOV-SS-shadow aren't
> VMX specific and also aren't impossible to hit with ordinary save /
> restore / migrate, I'm not convinced of this argumentation. But of
> course fixing VMX alone is better than nothing. However, ...
>
> > @@ -166,6 +167,11 @@ struct hvm_hw_cpu {
> >  #define XEN_X86_FPU_INITIALISED         (1U<<_XEN_X86_FPU_INITIALISED)
> >      uint32_t flags;
> >      uint32_t pad0;
> > +
> > +    /* non-register state */
> > +    uint32_t activity_state;
> > +    uint32_t interruptibility_state;
> > +    uint64_t pending_dbg;
> >  };
>
> ... these fields now represent vendor state in a supposedly vendor
> independent structure. Besides my wish to see this represented in
> field naming (thus at least making provisions for SVM to gain
> similar support; perhaps easiest would be to include these in a
> sub-structure with a field name of "vmx"), I wonder in how far cross-
> vendor migration was taken into consideration. As long as the fields
> are zero / ignored, things wouldn't be worse than before your
> change, but I think it wants spelling out that the SVM counterpart(s)
> may not be added by way of making a VMX/SVM union.

I wasn't aware cross-vendor migration is even a thing. But adding a
vmx sub-structure seems to me a workable route, we would perhaps just
need an extra field that specifies where the fields were taken
(vmx/svm) and ignore them if the place where the restore is taking
place doesn't match where the save happened. That would be equivalent
to how migration works today. Thoughts?

Tamas



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.