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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.03.2022 15:39, Tamas K Lengyel wrote:
> On Mon, Mar 21, 2022 at 8:16 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 17.03.2022 16:57, Tamas K Lengyel wrote:
>>> @@ -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.

It used to be a thing long ago; it may not work right now for no-one
testing it.

> 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?

I don't think such a field is needed, at least not right away, as
long as the respectively other vendor's fields are left zero when
storing the data. These fields being zero matches current behavior
of not communicating the values at all. A separate flag might be
needed if the receiving side would want to "emulate" settings from
incoming values from the respectively other vendor. Yet even then
only one of the two sets of fields could potentially be non-zero
(both being non-zero is an error imo); both fields being zero
would be compatible both ways. Hence it would be possible to
determine the source vendor without an extra field even then, I
would think.

A separate flag would of course be needed if we meant to overlay
the vendors' data in a union. But as per my earlier reply I think
we're better off not using a union in this case.

Jan