Re: [Xen-devel] [PATCH v2 1/2] SVM: re-work VMCB sync-ing

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/04/18 12:37, Jan Beulich wrote:
> While the main problem to be addressed here is the issue of what so far
> was named "vmcb_in_sync" starting out with the wrong value (should have
> been true instead of false, to prevent performing a VMSAVE without ever
> having VMLOADed the vCPU's state), go a step further and make the
> sync-ed state a tristate: CPU and memory may be in sync or an update
> may be required in either direction. Rename the field and introduce an
> enum. Callers of svm_sync_vmcb() now indicate the intended new state
> (with a slight "anomaly" when requesting VMLOAD: we could store
> vmcb_needs_vmsave in those cases as the callers request, but the VMCB
> really is in sync at that point, and hence there's no need to VMSAVE in
> case we don't make it out to guest context), and all syncing goes
> through that function.
>
> With that, there's no need to VMLOAD the state perhaps multiple times;
> all that's needed is loading it once before VM entry.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Also handle VMLOAD in svm_sync_vmcb(). Add comment to enum
>     vmcb_sync_state.

-1 from me.  This is even more confusing to use than v1.

It is not obvious at all that using svm_sync_vmcb(v, vmcb_needs_vmsave);
means "vmload", and its actively wrong that the state doesn't remain
in-sync.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel