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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan